[Sync] Remove backend unrecoverable error handler

It was being used to allowed passing ownership of an error handler, but passing
a weak handle works just as well.

BUG=511235

Review URL: https://codereview.chromium.org/1275743002

Cr-Commit-Position: refs/heads/master@{#342384}

chrome/browser/sync/glue/sync_backend_host.h

@@ -20,7 +20,6 @@
 #include "sync/internal_api/public/sync_context_proxy.h"
 #include "sync/internal_api/public/sync_manager.h"
 #include "sync/internal_api/public/sync_manager_factory.h"
-#include "sync/internal_api/public/util/unrecoverable_error_handler.h"
 #include "sync/internal_api/public/util/weak_handle.h"
 
 class GURL;
@@ -32,6 +31,7 @@ class MessageLoop;
 namespace syncer {
 class NetworkResources;
 class SyncManagerFactory;
+class UnrecoverableErrorHandler;
 }
 
 namespace sync_driver {
@@ -57,8 +57,6 @@ class SyncBackendHost : public sync_driver::BackendDataTypeConfigurer {
   // Optionally deletes the "Sync Data" folder during init in order to make
   // sure we're starting fresh.
   //
-  // Note: |unrecoverable_error_handler| may be invoked from any thread.
-  //
   // |saved_nigori_state| is optional nigori state to restore from a previous
   // backend instance. May be null.
   virtual void Initialize(
@@ -69,7 +67,8 @@ class SyncBackendHost : public sync_driver::BackendDataTypeConfigurer {
       const syncer::SyncCredentials& credentials,
       bool delete_sync_data_folder,
       scoped_ptr<syncer::SyncManagerFactory> sync_manager_factory,
-      scoped_ptr<syncer::UnrecoverableErrorHandler> unrecoverable_error_handler,
+      const syncer::WeakHandle<syncer::UnrecoverableErrorHandler>&
+          unrecoverable_error_handler,
       const base::Closure& report_unrecoverable_error_function,
       syncer::NetworkResources* network_resources,
       scoped_ptr<syncer::SyncEncryptionHandler::NigoriState>

chrome/browser/sync/glue/sync_backend_host_core.cc

@@ -69,7 +69,8 @@ DoInitializeOptions::DoInitializeOptions(
     const std::string& restored_key_for_bootstrapping,
     const std::string& restored_keystore_key_for_bootstrapping,
     scoped_ptr<syncer::InternalComponentsFactory> internal_components_factory,
-    scoped_ptr<syncer::UnrecoverableErrorHandler> unrecoverable_error_handler,
+    const syncer::WeakHandle<syncer::UnrecoverableErrorHandler>&
+        unrecoverable_error_handler,
     const base::Closure& report_unrecoverable_error_function,
     scoped_ptr<syncer::SyncEncryptionHandler::NigoriState> saved_nigori_state,
     syncer::PassphraseTransitionClearDataOption clear_data_option,
@@ -90,7 +91,7 @@ DoInitializeOptions::DoInitializeOptions(
       restored_keystore_key_for_bootstrapping(
           restored_keystore_key_for_bootstrapping),
       internal_components_factory(internal_components_factory.Pass()),
-      unrecoverable_error_handler(unrecoverable_error_handler.Pass()),
+      unrecoverable_error_handler(unrecoverable_error_handler),
       report_unrecoverable_error_function(report_unrecoverable_error_function),
       saved_nigori_state(saved_nigori_state.Pass()),
       clear_data_option(clear_data_option),
@@ -463,8 +464,7 @@ void SyncBackendHostCore::DoInitialize(
   args.internal_components_factory =
       options->internal_components_factory.Pass();
   args.encryptor = &encryptor_;
-  args.unrecoverable_error_handler =
-      options->unrecoverable_error_handler.Pass();
+  args.unrecoverable_error_handler = options->unrecoverable_error_handler;
   args.report_unrecoverable_error_function =
       options->report_unrecoverable_error_function;
   args.cancelation_signal = &stop_syncing_signal_;

chrome/browser/sync/glue/sync_backend_host_core.h

@@ -39,7 +39,8 @@ struct DoInitializeOptions {
       const std::string& restored_key_for_bootstrapping,
       const std::string& restored_keystore_key_for_bootstrapping,
       scoped_ptr<syncer::InternalComponentsFactory> internal_components_factory,
-      scoped_ptr<syncer::UnrecoverableErrorHandler> unrecoverable_error_handler,
+      const syncer::WeakHandle<syncer::UnrecoverableErrorHandler>&
+          unrecoverable_error_handler,
       const base::Closure& report_unrecoverable_error_function,
       scoped_ptr<syncer::SyncEncryptionHandler::NigoriState> saved_nigori_state,
       syncer::PassphraseTransitionClearDataOption clear_data_option,
@@ -63,7 +64,8 @@ struct DoInitializeOptions {
   std::string restored_key_for_bootstrapping;
   std::string restored_keystore_key_for_bootstrapping;
   scoped_ptr<syncer::InternalComponentsFactory> internal_components_factory;
-  scoped_ptr<syncer::UnrecoverableErrorHandler> unrecoverable_error_handler;
+  const syncer::WeakHandle<syncer::UnrecoverableErrorHandler>
+      unrecoverable_error_handler;
   base::Closure report_unrecoverable_error_function;
   scoped_ptr<syncer::SyncEncryptionHandler::NigoriState> saved_nigori_state;
   const syncer::PassphraseTransitionClearDataOption clear_data_option;

chrome/browser/sync/glue/sync_backend_host_impl.cc

@@ -105,7 +105,8 @@ void SyncBackendHostImpl::Initialize(
     const syncer::SyncCredentials& credentials,
     bool delete_sync_data_folder,
     scoped_ptr<syncer::SyncManagerFactory> sync_manager_factory,
-    scoped_ptr<syncer::UnrecoverableErrorHandler> unrecoverable_error_handler,
+    const syncer::WeakHandle<syncer::UnrecoverableErrorHandler>&
+        unrecoverable_error_handler,
     const base::Closure& report_unrecoverable_error_function,
     syncer::NetworkResources* network_resources,
     scoped_ptr<syncer::SyncEncryptionHandler::NigoriState> saved_nigori_state) {
@@ -159,7 +160,7 @@ void SyncBackendHostImpl::Initialize(
       scoped_ptr<InternalComponentsFactory>(
           new syncer::InternalComponentsFactoryImpl(factory_switches))
           .Pass(),
-      unrecoverable_error_handler.Pass(), report_unrecoverable_error_function,
+      unrecoverable_error_handler, report_unrecoverable_error_function,
       saved_nigori_state.Pass(), clear_data_option, invalidation_versions));
   InitCore(init_opts.Pass());
 }

chrome/browser/sync/glue/sync_backend_host_impl.h

@@ -24,7 +24,6 @@
 #include "sync/internal_api/public/sessions/sync_session_snapshot.h"
 #include "sync/internal_api/public/sessions/type_debug_info_observer.h"
 #include "sync/internal_api/public/sync_manager.h"
-#include "sync/internal_api/public/util/unrecoverable_error_handler.h"
 #include "sync/internal_api/public/util/weak_handle.h"
 #include "sync/protocol/encryption.pb.h"
 #include "sync/protocol/sync_protocol_error.h"
@@ -44,6 +43,7 @@ class InvalidationService;
 namespace syncer {
 class NetworkResources;
 class SyncManagerFactory;
+class UnrecoverableErrorHandler;
 }
 
 namespace sync_driver {
@@ -86,7 +86,8 @@ class SyncBackendHostImpl
       const syncer::SyncCredentials& credentials,
       bool delete_sync_data_folder,
       scoped_ptr<syncer::SyncManagerFactory> sync_manager_factory,
-      scoped_ptr<syncer::UnrecoverableErrorHandler> unrecoverable_error_handler,
+      const syncer::WeakHandle<syncer::UnrecoverableErrorHandler>&
+          unrecoverable_error_handler,
       const base::Closure& report_unrecoverable_error_function,
       syncer::NetworkResources* network_resources,
       scoped_ptr<syncer::SyncEncryptionHandler::NigoriState> saved_nigori_state)

chrome/browser/sync/glue/sync_backend_host_impl_unittest.cc

@@ -218,7 +218,7 @@ class SyncBackendHostTest : public testing::Test {
         credentials_,
         true,
         fake_manager_factory_.Pass(),
-        make_scoped_ptr(new syncer::TestUnrecoverableErrorHandler),
+        MakeWeakHandle(test_unrecoverable_error_handler_.GetWeakPtr()),
         base::Closure(),
         network_resources_.get(),
         saved_nigori_state_.Pass());
@@ -289,6 +289,7 @@ class SyncBackendHostTest : public testing::Test {
   syncer::SyncCredentials credentials_;
   TestingProfileManager profile_manager_;
   TestingProfile* profile_;
+  syncer::TestUnrecoverableErrorHandler test_unrecoverable_error_handler_;
   scoped_ptr<sync_driver::SyncPrefs> sync_prefs_;
   scoped_ptr<SyncBackendHostImpl> backend_;
   scoped_ptr<FakeSyncManagerFactory> fake_manager_factory_;

chrome/browser/sync/glue/sync_backend_host_mock.cc

@@ -21,7 +21,8 @@ void SyncBackendHostMock::Initialize(
     const syncer::SyncCredentials& credentials,
     bool delete_sync_data_folder,
     scoped_ptr<syncer::SyncManagerFactory> sync_manager_factory,
-    scoped_ptr<syncer::UnrecoverableErrorHandler> unrecoverable_error_handler,
+    const syncer::WeakHandle<syncer::UnrecoverableErrorHandler>&
+        unrecoverable_error_handler,
     const base::Closure& report_unrecoverable_error_function,
     syncer::NetworkResources* network_resources,
     scoped_ptr<syncer::SyncEncryptionHandler::NigoriState> saved_nigori_state) {

chrome/browser/sync/glue/sync_backend_host_mock.h

@@ -33,7 +33,8 @@ class SyncBackendHostMock : public SyncBackendHost {
       const syncer::SyncCredentials& credentials,
       bool delete_sync_data_folder,
       scoped_ptr<syncer::SyncManagerFactory> sync_manager_factory,
-      scoped_ptr<syncer::UnrecoverableErrorHandler> unrecoverable_error_handler,
+      const syncer::WeakHandle<syncer::UnrecoverableErrorHandler>&
+          unrecoverable_error_handler,
       const base::Closure& report_unrecoverable_error_function,
       syncer::NetworkResources* network_resources,
       scoped_ptr<syncer::SyncEncryptionHandler::NigoriState> saved_nigori_state)

chrome/browser/sync/profile_sync_service.cc

@@ -525,17 +525,12 @@ void ProfileSyncService::InitializeBackend(bool delete_stale_data) {
   if (backend_mode_ == SYNC && delete_stale_data)
     ClearStaleErrors();
 
-  scoped_ptr<syncer::UnrecoverableErrorHandler>
-      backend_unrecoverable_error_handler(
-          new browser_sync::BackendUnrecoverableErrorHandler(
-              MakeWeakHandle(weak_factory_.GetWeakPtr())));
-
   backend_->Initialize(this, sync_thread_.Pass(), GetJsEventHandler(),
                        sync_service_url_, credentials, delete_stale_data,
                        scoped_ptr<syncer::SyncManagerFactory>(
                            new syncer::SyncManagerFactory(GetManagerType()))
                            .Pass(),
-                       backend_unrecoverable_error_handler.Pass(),
+                       MakeWeakHandle(weak_factory_.GetWeakPtr()),
                        base::Bind(browser_sync::ChromeReportUnrecoverableError),
                        network_resources_.get(), saved_nigori_state_.Pass());
 }

chrome/browser/sync/profile_sync_service.h

@@ -23,7 +23,6 @@
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/browsing_data/browsing_data_remover.h"
-#include "chrome/browser/sync/backend_unrecoverable_error_handler.h"
 #include "chrome/browser/sync/backup_rollback_controller.h"
 #include "chrome/browser/sync/glue/sync_backend_host.h"
 #include "chrome/browser/sync/sessions/sessions_sync_manager.h"

chrome/browser/sync/profile_sync_service_unittest.cc

@@ -115,7 +115,8 @@ class SyncBackendHostNoReturn : public SyncBackendHostMock {
       const syncer::SyncCredentials& credentials,
       bool delete_sync_data_folder,
       scoped_ptr<syncer::SyncManagerFactory> sync_manager_factory,
-      scoped_ptr<syncer::UnrecoverableErrorHandler> unrecoverable_error_handler,
+      const syncer::WeakHandle<syncer::UnrecoverableErrorHandler>&
+          unrecoverable_error_handler,
       const base::Closure& report_unrecoverable_error_function,
       syncer::NetworkResources* network_resources,
       scoped_ptr<syncer::SyncEncryptionHandler::NigoriState> saved_nigori_state)
@@ -136,7 +137,8 @@ class SyncBackendHostMockCollectDeleteDirParam : public SyncBackendHostMock {
       const syncer::SyncCredentials& credentials,
       bool delete_sync_data_folder,
       scoped_ptr<syncer::SyncManagerFactory> sync_manager_factory,
-      scoped_ptr<syncer::UnrecoverableErrorHandler> unrecoverable_error_handler,
+      const syncer::WeakHandle<syncer::UnrecoverableErrorHandler>&
+          unrecoverable_error_handler,
       const base::Closure& report_unrecoverable_error_function,
       syncer::NetworkResources* network_resources,
       scoped_ptr<syncer::SyncEncryptionHandler::NigoriState> saved_nigori_state)
@@ -146,7 +148,7 @@ class SyncBackendHostMockCollectDeleteDirParam : public SyncBackendHostMock {
                                     event_handler, service_url, credentials,
                                     delete_sync_data_folder,
                                     sync_manager_factory.Pass(),
-                                    unrecoverable_error_handler.Pass(),
+                                    unrecoverable_error_handler,
                                     report_unrecoverable_error_function,
                                     network_resources,
                                     saved_nigori_state.Pass());

chrome/chrome_browser.gypi

@@ -2862,8 +2862,6 @@
     'chrome_browser_sync_sources': [
       'browser/sync/about_sync_util.cc',
       'browser/sync/about_sync_util.h',
-      'browser/sync/backend_unrecoverable_error_handler.cc',
-      'browser/sync/backend_unrecoverable_error_handler.h',
       'browser/sync/backup_rollback_controller.cc',
       'browser/sync/backup_rollback_controller.h',
       'browser/sync/glue/autofill_data_type_controller.cc',

sync/internal_api/public/sync_manager.h

@@ -30,7 +30,6 @@
 #include "sync/internal_api/public/shutdown_reason.h"
 #include "sync/internal_api/public/sync_context_proxy.h"
 #include "sync/internal_api/public/sync_encryption_handler.h"
-#include "sync/internal_api/public/util/unrecoverable_error_handler.h"
 #include "sync/internal_api/public/util/weak_handle.h"
 #include "sync/protocol/sync_protocol_error.h"
 
@@ -54,6 +53,7 @@ class ProtocolEvent;
 class SyncEncryptionHandler;
 class SyncScheduler;
 class TypeDebugInfoObserver;
+class UnrecoverableErrorHandler;
 struct Experiments;
 struct UserShare;
 
@@ -260,7 +260,7 @@ class SYNC_EXPORT SyncManager {
     // Must outlive SyncManager.
     Encryptor* encryptor;
 
-    scoped_ptr<UnrecoverableErrorHandler> unrecoverable_error_handler;
+    WeakHandle<UnrecoverableErrorHandler> unrecoverable_error_handler;
     base::Closure report_unrecoverable_error_function;
 
     // Carries shutdown requests across threads and will be used to cut short

sync/internal_api/sync_backup_manager.cc

@@ -24,7 +24,7 @@ void SyncBackupManager::Init(InitArgs* args) {
           args->database_location,
           args->internal_components_factory.get(),
           InternalComponentsFactory::STORAGE_ON_DISK_DEFERRED,
-          args->unrecoverable_error_handler.Pass(),
+          args->unrecoverable_error_handler,
           args->report_unrecoverable_error_function)) {
     GetUserShare()->directory->CollectMetaHandleCounts(
         &status_.num_entries_by_type, &status_.num_to_delete_entries_by_type);

sync/internal_api/sync_manager_impl.cc

@@ -242,7 +242,6 @@ void SyncManagerImpl::Init(InitArgs* args) {
 
   database_path_ = args->database_location.Append(
       syncable::Directory::kSyncDatabaseFilename);
-  unrecoverable_error_handler_ = args->unrecoverable_error_handler.Pass();
   report_unrecoverable_error_function_ =
       args->report_unrecoverable_error_function;
 
@@ -267,7 +266,7 @@ void SyncManagerImpl::Init(InitArgs* args) {
   share_.directory.reset(
       new syncable::Directory(
           backing_store.release(),
-          unrecoverable_error_handler_.get(),
+          args->unrecoverable_error_handler,
           report_unrecoverable_error_function_,
           sync_encryption_handler_.get(),
           sync_encryption_handler_->GetCryptographerUnsafe()));

sync/internal_api/sync_manager_impl.h

@@ -335,7 +335,6 @@ class SYNC_EXPORT_PRIVATE SyncManagerImpl
 
   ProtocolEventBuffer protocol_event_buffer_;
 
-  scoped_ptr<UnrecoverableErrorHandler> unrecoverable_error_handler_;
   base::Closure report_unrecoverable_error_function_;
 
   // Sync's encryption handler. It tracks the set of encrypted types, manages

sync/internal_api/sync_manager_impl_unittest.cc

@@ -832,8 +832,7 @@ class SyncManagerTest : public testing::Test,
   };
 
   SyncManagerTest()
-      : sync_manager_("Test sync manager"),
-        mock_unrecoverable_error_handler_(NULL) {
+      : sync_manager_("Test sync manager") {
     switches_.encryption_method =
         InternalComponentsFactory::ENCRYPTION_KEYSTORE;
   }
@@ -883,8 +882,8 @@ class SyncManagerTest : public testing::Test,
     args.invalidator_client_id = "fake_invalidator_client_id";
     args.internal_components_factory.reset(GetFactory());
     args.encryptor = &encryptor_;
-    mock_unrecoverable_error_handler_ = new MockUnrecoverableErrorHandler();
-    args.unrecoverable_error_handler.reset(mock_unrecoverable_error_handler_);
+    args.unrecoverable_error_handler =
+        MakeWeakHandle(mock_unrecoverable_error_handler_.GetWeakPtr());
     args.cancelation_signal = &cancelation_signal_;
     sync_manager_.Init(&args);
 
@@ -1079,9 +1078,7 @@ class SyncManagerTest : public testing::Test,
   }
 
   bool HasUnrecoverableError() {
-    if (mock_unrecoverable_error_handler_)
-      return mock_unrecoverable_error_handler_->invocation_count() > 0;
-    return false;
+    return mock_unrecoverable_error_handler_.invocation_count() > 0;
   }
 
  private:
@@ -1103,9 +1100,7 @@ class SyncManagerTest : public testing::Test,
   StrictMock<SyncEncryptionHandlerObserverMock> encryption_observer_;
   InternalComponentsFactory::Switches switches_;
   InternalComponentsFactory::StorageOption storage_used_;
-
-  // Not owned (ownership is passed to the SyncManager).
-  MockUnrecoverableErrorHandler* mock_unrecoverable_error_handler_;
+  MockUnrecoverableErrorHandler mock_unrecoverable_error_handler_;
 };
 
 TEST_F(SyncManagerTest, GetAllNodesForTypeTest) {