[Sync] Avoid unregistering object IDs on shutdown

sync/internal_api/sync_manager_impl_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Unit tests for the SyncApi. Note that a lot of the underlying
 // functionality is provided by the Syncable layer, which has its own
 // unit tests. We'll test SyncApi specific things in this harness.
 
 #include <cstddef>
 #include <map>
@@ skipping 678 matching lines @@
   MOCK_METHOD1(OnUpdatedToken, void(const std::string&));  // NOLINT
   MOCK_METHOD2(OnEncryptedTypesChanged,
                void(ModelTypeSet, bool));  // NOLINT
   MOCK_METHOD0(OnEncryptionComplete, void());  // NOLINT
   MOCK_METHOD1(OnActionableError,
                void(const SyncProtocolError&));  // NOLINT
 };
 
 class SyncNotifierMock : public SyncNotifier {
  public:
+  MOCK_METHOD1(RegisterHandler, void(SyncNotifierObserver*));
   MOCK_METHOD2(UpdateRegisteredIds,
                void(SyncNotifierObserver*, const ObjectIdSet&));
+  MOCK_METHOD1(UnregisterHandler, void(SyncNotifierObserver*));
   MOCK_METHOD1(SetUniqueId, void(const std::string&));
   MOCK_METHOD1(SetStateDeprecated, void(const std::string&));
   MOCK_METHOD2(UpdateCredentials,
                void(const std::string&, const std::string&));
   MOCK_METHOD1(SendNotification, void(ModelTypeSet));
 };
 
 }  // namespace
 
 class SyncManagerTest : public testing::Test,
@@ skipping 24 matching lines @@
 
     SyncCredentials credentials;
     credentials.email = "foo@bar.com";
     credentials.sync_token = "sometoken";
 
     sync_notifier_mock_ = new StrictMock<SyncNotifierMock>();
     EXPECT_CALL(*sync_notifier_mock_, SetUniqueId(_));
     EXPECT_CALL(*sync_notifier_mock_, SetStateDeprecated(""));
     EXPECT_CALL(*sync_notifier_mock_,
                 UpdateCredentials(credentials.email, credentials.sync_token));
+    EXPECT_CALL(*sync_notifier_mock_, RegisterHandler(_));
+    EXPECT_CALL(*sync_notifier_mock_, UnregisterHandler(_));
 
     sync_manager_.AddObserver(&observer_);
     EXPECT_CALL(observer_, OnInitializationComplete(_, _, _)).
         WillOnce(SaveArg<0>(&js_backend_));
 
     EXPECT_FALSE(js_backend_.IsInitialized());
 
     std::vector<ModelSafeWorker*> workers;
     ModelSafeRoutingInfo routing_info;
     GetModelSafeRoutingInfo(&routing_info);
@@ skipping 20 matching lines @@
     for (ModelSafeRoutingInfo::iterator i = routing_info.begin();
          i != routing_info.end(); ++i) {
       type_roots_[i->first] = MakeServerNodeForType(
           sync_manager_.GetUserShare(), i->first);
     }
     PumpLoop();
   }
 
   void TearDown() {
     sync_manager_.RemoveObserver(&observer_);
-    EXPECT_CALL(*sync_notifier_mock_, UpdateRegisteredIds(_, ObjectIdSet()));
     sync_manager_.ShutdownOnSyncThread();

[msw, 2012/08/09 05:20:26]

nit: comment that StrictMock ensures that the current IDs aren't unregistered?
I didn't nit similarly elsewhere, but here doesn't even have a comment like:
"Ensure nothing happens..." 

[akalin, 2012/08/10 01:28:08]

Done.

     sync_notifier_mock_ = NULL;
     PumpLoop();
   }
 
   void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) {
     (*out)[NIGORI] = GROUP_PASSIVE;
     (*out)[BOOKMARKS] = GROUP_PASSIVE;
     (*out)[THEMES] = GROUP_PASSIVE;
     (*out)[SESSIONS] = GROUP_PASSIVE;
     (*out)[PASSWORDS] = GROUP_PASSIVE;
@@ skipping 152 matching lines @@
   StrictMock<SyncNotifierMock>* sync_notifier_mock_;
   SyncManagerImpl sync_manager_;
   WeakHandle<JsBackend> js_backend_;
   StrictMock<SyncManagerObserverMock> observer_;
 };
 
 TEST_F(SyncManagerTest, UpdateEnabledTypes) {
   ModelSafeRoutingInfo routes;
   GetModelSafeRoutingInfo(&routes);
   const ModelTypeSet enabled_types = GetRoutingInfoTypes(routes);
-
   EXPECT_CALL(*sync_notifier_mock_,
               UpdateRegisteredIds(
                   _, ModelTypeSetToObjectIdSet(enabled_types)));
+
   sync_manager_.UpdateEnabledTypes(enabled_types);
 }
 
+TEST_F(SyncManagerTest, RegisterInvalidationHandler) {
+  EXPECT_CALL(*sync_notifier_mock_, RegisterHandler(NULL));
+  sync_manager_.RegisterInvalidationHandler(NULL);
+}
+
 TEST_F(SyncManagerTest, UpdateRegisteredInvalidationIds) {
-  EXPECT_CALL(*sync_notifier_mock_, UpdateRegisteredIds(NULL, ObjectIdSet()));
+  EXPECT_CALL(*sync_notifier_mock_,
+              UpdateRegisteredIds(NULL, ObjectIdSet()));
   sync_manager_.UpdateRegisteredInvalidationIds(NULL, ObjectIdSet());
 }
 
+TEST_F(SyncManagerTest, UnregisterInvalidationHandler) {
+  EXPECT_CALL(*sync_notifier_mock_, UnregisterHandler(NULL));
+  sync_manager_.UnregisterInvalidationHandler(NULL);
+}
+
 TEST_F(SyncManagerTest, ProcessJsMessage) {
   const JsArgList kNoArgs;
 
   StrictMock<MockJsReplyHandler> reply_handler;
 
   ListValue disabled_args;
   disabled_args.Append(
       Value::CreateStringValue("TRANSIENT_NOTIFICATION_ERROR"));
 
   EXPECT_CALL(reply_handler,
@@ skipping 1802 matching lines @@
 
   // Verify only the non-disabled types remain after cleanup.
   sync_manager_.PurgeDisabledTypes(enabled_types, new_enabled_types);
   EXPECT_TRUE(new_enabled_types.Equals(
       Union(sync_manager_.InitialSyncEndedTypes(), partial_enabled_types)));
   EXPECT_TRUE(disabled_types.Equals(
       sync_manager_.GetTypesWithEmptyProgressMarkerToken(ModelTypeSet::All())));
 }
 
 }  // namespace