Revert 150990 - [Sync] Avoid unregistering object IDs on shutdown

chrome/browser/sync/glue/sync_backend_host_unittest.cc

Index: chrome/browser/sync/glue/sync_backend_host_unittest.cc
===================================================================
--- chrome/browser/sync/glue/sync_backend_host_unittest.cc	(revision 150991)
+++ chrome/browser/sync/glue/sync_backend_host_unittest.cc	(working copy)
@@ -89,44 +89,24 @@
 
 class FakeSyncManagerFactory : public syncer::SyncManagerFactory {
  public:
-  FakeSyncManagerFactory() : fake_manager_(NULL) {}
+  FakeSyncManagerFactory() : manager_(NULL) {}
   virtual ~FakeSyncManagerFactory() {}
 
-  // SyncManagerFactory implementation.  Called on the sync thread.
-  virtual scoped_ptr<SyncManager> CreateSyncManager(
-      std::string name) OVERRIDE {
-    DCHECK(!fake_manager_);
-    fake_manager_ = new FakeSyncManager(initial_sync_ended_types_,
-                                        progress_marker_types_,
-                                        configure_fail_types_);
-    return scoped_ptr<SyncManager>(fake_manager_);
+  // Takes ownership of |manager|.
+  void SetSyncManager(FakeSyncManager* manager) {
+    DCHECK(!manager_.get());
+    manager_.reset(manager);
   }
 
-  // Returns NULL until CreateSyncManager() is called on the sync
-  // thread.  Called on the main thread, but only after
-  // OnBackendInitialized() is called (which is strictly after
-  // CreateSyncManager is called on the sync thread).
-  FakeSyncManager* fake_manager() {
-    return fake_manager_;
+  // Passes ownership of |manager_|.
+  // SyncManagerFactory implementation.
+  virtual scoped_ptr<SyncManager> CreateSyncManager(std::string name) OVERRIDE {
+    DCHECK(manager_.get());
+    return manager_.Pass();
   }
 
-  void set_initial_sync_ended_types(syncer::ModelTypeSet types) {
-    initial_sync_ended_types_ = types;
-  }
-
-  void set_progress_marker_types(syncer::ModelTypeSet types) {
-    progress_marker_types_ = types;
-  }
-
-  void set_configure_fail_types(syncer::ModelTypeSet types) {
-    configure_fail_types_ = types;
-  }
-
  private:
-  syncer::ModelTypeSet initial_sync_ended_types_;
-  syncer::ModelTypeSet progress_marker_types_;
-  syncer::ModelTypeSet configure_fail_types_;
-  FakeSyncManager* fake_manager_;
+  scoped_ptr<SyncManager> manager_;
 };
 
 class SyncBackendHostTest : public testing::Test {
@@ -151,6 +131,8 @@
         invalidator_storage_->AsWeakPtr()));
     credentials_.email = "user@example.com";
     credentials_.sync_token = "sync_token";
+    fake_manager_ = new FakeSyncManager();
+    fake_sync_manager_factory_.SetSyncManager(fake_manager_);
 
     // NOTE: We can't include Passwords or Typed URLs due to the Sync Backend
     // Registrar removing them if it can't find their model workers.
@@ -163,10 +145,8 @@
   }
 
   virtual void TearDown() OVERRIDE {
-    if (backend_.get()) {
-      backend_->StopSyncingForShutdown();
-      backend_->Shutdown(false);
-    }
+    backend_->StopSyncingForShutdown();
+    backend_->Shutdown(false);
     backend_.reset();
     sync_prefs_.reset();
     invalidator_storage_.reset();
@@ -188,17 +168,12 @@
                          GURL(""),
                          credentials_,
                          true,
-                         &fake_manager_factory_,
+                         &fake_sync_manager_factory_,
                          &handler_,
                          NULL);
     ui_loop_.PostDelayedTask(FROM_HERE,
         ui_loop_.QuitClosure(), TestTimeouts::action_timeout());
     ui_loop_.Run();
-    // |fake_manager_factory_|'s fake_manager() is set on the sync
-    // thread, but we can rely on the message loop barriers to
-    // guarantee that we see the updated value.
-    fake_manager_ = fake_manager_factory_.fake_manager();
-    DCHECK(fake_manager_);
   }
 
   // Synchronously configures the backend's datatypes.
@@ -231,15 +206,15 @@
   MessageLoop ui_loop_;
   content::TestBrowserThread ui_thread_;
   content::TestBrowserThread io_thread_;
-  StrictMock<MockSyncFrontend> mock_frontend_;
+  MockSyncFrontend mock_frontend_;
   syncer::SyncCredentials credentials_;
   syncer::TestUnrecoverableErrorHandler handler_;
   scoped_ptr<TestingProfile> profile_;
   scoped_ptr<SyncPrefs> sync_prefs_;
   scoped_ptr<InvalidatorStorage> invalidator_storage_;
   scoped_ptr<SyncBackendHost> backend_;
+  FakeSyncManagerFactory fake_sync_manager_factory_;
   FakeSyncManager* fake_manager_;
-  FakeSyncManagerFactory fake_manager_factory_;
   syncer::ModelTypeSet enabled_types_;
 };
 
@@ -282,8 +257,8 @@
 TEST_F(SyncBackendHostTest, Restart) {
   sync_prefs_->SetSyncSetupCompleted();
   syncer::ModelTypeSet all_but_nigori = enabled_types_;
-  fake_manager_factory_.set_progress_marker_types(enabled_types_);
-  fake_manager_factory_.set_initial_sync_ended_types(enabled_types_);
+  fake_manager_->set_progress_marker_types(enabled_types_);
+  fake_manager_->set_initial_sync_ended_types(enabled_types_);
   InitializeBackend();
   EXPECT_TRUE(fake_manager_->GetAndResetDownloadedTypes().Empty());
   EXPECT_TRUE(Intersection(fake_manager_->GetAndResetCleanedTypes(),
@@ -314,8 +289,8 @@
   syncer::ModelTypeSet partial_types(syncer::NIGORI, syncer::BOOKMARKS);
   syncer::ModelTypeSet full_types =
       Difference(enabled_types_, partial_types);
-  fake_manager_factory_.set_progress_marker_types(enabled_types_);
-  fake_manager_factory_.set_initial_sync_ended_types(full_types);
+  fake_manager_->set_progress_marker_types(enabled_types_);
+  fake_manager_->set_initial_sync_ended_types(full_types);
 
   // Bringing up the backend should purge all partial types, then proceed to
   // download the Nigori.
@@ -501,8 +476,8 @@
   // Set sync manager behavior before passing it down. All types have progress
   // markers and initial sync ended except the new types.
   syncer::ModelTypeSet old_types = enabled_types_;
-  fake_manager_factory_.set_progress_marker_types(old_types);
-  fake_manager_factory_.set_initial_sync_ended_types(old_types);
+  fake_manager_->set_progress_marker_types(old_types);
+  fake_manager_->set_initial_sync_ended_types(old_types);
   syncer::ModelTypeSet new_types(syncer::APP_SETTINGS,
                                  syncer::EXTENSION_SETTINGS);
   enabled_types_.PutAll(new_types);
@@ -542,8 +517,8 @@
   syncer::ModelTypeSet partial_types(syncer::NIGORI, syncer::BOOKMARKS);
   syncer::ModelTypeSet full_types =
       Difference(enabled_types_, partial_types);
-  fake_manager_factory_.set_progress_marker_types(old_types);
-  fake_manager_factory_.set_initial_sync_ended_types(full_types);
+  fake_manager_->set_progress_marker_types(old_types);
+  fake_manager_->set_initial_sync_ended_types(full_types);
   syncer::ModelTypeSet new_types(syncer::APP_SETTINGS,
                                  syncer::EXTENSION_SETTINGS);
   enabled_types_.PutAll(new_types);
@@ -634,35 +609,6 @@
   ui_loop_.Run();
 }
 
-// Call StopSyncingForShutdown() on the backend and fire some notifications
-// before calling Shutdown().  Then start up and shut down the backend again.
-// Those notifications shouldn't propagate to the frontend.
-TEST_F(SyncBackendHostTest, NotificationsAfterStopSyncingForShutdown) {
-  InitializeBackend();
-
-  syncer::ObjectIdSet ids;
-  ids.insert(invalidation::ObjectId(5, "id5"));
-  backend_->UpdateRegisteredInvalidationIds(ids);
-
-  backend_->StopSyncingForShutdown();
-
-  // Should not trigger anything.
-  fake_manager_->DisableNotifications(syncer::TRANSIENT_NOTIFICATION_ERROR);
-  fake_manager_->EnableNotifications();
-  const syncer::ObjectIdPayloadMap& id_payloads =
-      syncer::ObjectIdSetToPayloadMap(ids, "payload");
-  fake_manager_->Invalidate(id_payloads, syncer::REMOTE_NOTIFICATION);
-
-  // Make sure the above calls take effect before we continue.
-  fake_manager_->WaitForSyncThread();
-
-  backend_->Shutdown(false);
-  backend_.reset();
-
-  TearDown();
-  SetUp();
-}
-
 }  // namespace
 
 }  // namespace browser_sync