[Sync] NonBlocking type registration clear instead of crashes when also registered as pssive.

components/sync/driver/glue/sync_backend_registrar_unittest.cc

 // Copyright 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.
 
 #include "components/sync/driver/glue/sync_backend_registrar.h"
 
 #include "base/location.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "components/sync/base/model_type.h"
@@ skipping 15 matching lines @@
 using ::testing::Return;
 using ::testing::StrictMock;
 using syncer::FIRST_REAL_MODEL_TYPE;
 using syncer::AUTOFILL;
 using syncer::BOOKMARKS;
 using syncer::PREFERENCES;
 using syncer::THEMES;
 using syncer::NIGORI;
 using syncer::PASSWORDS;
 using syncer::MODEL_TYPE_COUNT;
+using syncer::ModelSafeGroup;
+using syncer::ModelSafeRoutingInfo;
 using syncer::ModelTypeSet;
 using syncer::ModelType;
 using syncer::ModelTypeFromInt;
 
 void TriggerChanges(SyncBackendRegistrar* registrar, ModelType type) {
   registrar->OnChangesApplied(type, 0, NULL,
                               syncer::ImmutableChangeRecordList());
   registrar->OnChangesComplete(type);
 }
 
 class RegistrarSyncClient : public sync_driver::FakeSyncClient {
  public:
   RegistrarSyncClient(
       const scoped_refptr<base::SingleThreadTaskRunner>& ui_task_runner,
       const scoped_refptr<base::SingleThreadTaskRunner>& db_task_runner,
       const scoped_refptr<base::SingleThreadTaskRunner>& file_task_runner)
       : ui_task_runner_(ui_task_runner),
         db_task_runner_(db_task_runner),
         file_task_runner_(file_task_runner) {}
 
   scoped_refptr<syncer::ModelSafeWorker> CreateModelWorkerForGroup(
-      syncer::ModelSafeGroup group,
+      ModelSafeGroup group,
       syncer::WorkerLoopDestructionObserver* observer) override {
     switch (group) {
       case syncer::GROUP_UI:
         return new BrowserThreadModelWorker(ui_task_runner_, group, observer);
       case syncer::GROUP_DB:
         return new BrowserThreadModelWorker(db_task_runner_, group, observer);
       case syncer::GROUP_FILE:
         return new BrowserThreadModelWorker(file_task_runner_, group, observer);
       case syncer::GROUP_PASSIVE:
         return new syncer::PassiveModelWorker(observer);
       default:
         return nullptr;
     }
   }
 
  private:
   const scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
   const scoped_refptr<base::SingleThreadTaskRunner> db_task_runner_;
   const scoped_refptr<base::SingleThreadTaskRunner> file_task_runner_;
 };
 
 // Flaky: https://crbug.com/498238
 class SyncBackendRegistrarTest : public testing::Test {
  public:
   void TestNonUIDataTypeActivationAsync(sync_driver::ChangeProcessor* processor,
                                         base::WaitableEvent* done) {
     registrar_->ActivateDataType(AUTOFILL, syncer::GROUP_DB, processor,
                                  test_user_share_.user_share());
-    syncer::ModelSafeRoutingInfo expected_routing_info;
-    expected_routing_info[AUTOFILL] = syncer::GROUP_DB;
-    ExpectRoutingInfo(registrar_.get(), expected_routing_info);
+    ExpectRoutingInfo(registrar_.get(), {{AUTOFILL, syncer::GROUP_DB}});
     ExpectHasProcessorsForTypes(*registrar_, ModelTypeSet(AUTOFILL));
     TriggerChanges(registrar_.get(), AUTOFILL);
     done->Signal();
   }
 
  protected:
   SyncBackendRegistrarTest()
       : db_thread_("DBThreadForTest"),
         file_thread_("FileThreadForTest"),
         sync_thread_(NULL) {}
@@ skipping 15 matching lines @@
   void TearDown() override {
     registrar_->RequestWorkerStopOnUIThread();
     test_user_share_.TearDown();
     sync_thread_->task_runner()->PostTask(
         FROM_HERE, base::Bind(&SyncBackendRegistrar::Shutdown,
                               base::Unretained(registrar_.release())));
     sync_thread_->WaitUntilThreadStarted();
     base::RunLoop().RunUntilIdle();
   }
 
-  void ExpectRoutingInfo(
-      SyncBackendRegistrar* registrar,
-      const syncer::ModelSafeRoutingInfo& expected_routing_info) {
-    syncer::ModelSafeRoutingInfo routing_info;
-    registrar->GetModelSafeRoutingInfo(&routing_info);
-    EXPECT_EQ(expected_routing_info, routing_info);
+  void ExpectRoutingInfo(SyncBackendRegistrar* registrar,
+                         const ModelSafeRoutingInfo& expected_routing_info) {
+    ModelSafeRoutingInfo actual_routing_info;
+    registrar->GetModelSafeRoutingInfo(&actual_routing_info);
+    EXPECT_EQ(expected_routing_info, actual_routing_info);
   }
 
   void ExpectHasProcessorsForTypes(const SyncBackendRegistrar& registrar,
                                    ModelTypeSet types) {
     for (int i = FIRST_REAL_MODEL_TYPE; i < MODEL_TYPE_COUNT; ++i) {
       ModelType model_type = ModelTypeFromInt(i);
       EXPECT_EQ(types.Has(model_type),
                 registrar_->IsTypeActivatedForTest(model_type));
     }
   }
 
+  size_t GetWorkersSize() {
+    std::vector<scoped_refptr<syncer::ModelSafeWorker>> workers;
+    registrar_->GetWorkers(&workers);
+    return workers.size();
+  }
+
   const scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner() {
     return message_loop_.task_runner();
   }
 
   const scoped_refptr<base::SingleThreadTaskRunner> db_task_runner() {
     return db_thread_.task_runner();
   }
 
   const scoped_refptr<base::SingleThreadTaskRunner> file_task_runner() {
     return db_thread_.task_runner();
   }
 
   base::MessageLoop message_loop_;
   base::Thread db_thread_;
   base::Thread file_thread_;
 
   syncer::TestUserShare test_user_share_;
   std::unique_ptr<RegistrarSyncClient> sync_client_;
   std::unique_ptr<SyncBackendRegistrar> registrar_;
 
   base::Thread* sync_thread_;
 };
 
 TEST_F(SyncBackendRegistrarTest, ConstructorEmpty) {
   registrar_->SetInitialTypes(ModelTypeSet());
   EXPECT_FALSE(registrar_->IsNigoriEnabled());
-  {
-    std::vector<scoped_refptr<syncer::ModelSafeWorker>> workers;
-    registrar_->GetWorkers(&workers);
-    EXPECT_EQ(4u, workers.size());
-  }
-  ExpectRoutingInfo(registrar_.get(), syncer::ModelSafeRoutingInfo());
+  EXPECT_EQ(4u, GetWorkersSize());
+  ExpectRoutingInfo(registrar_.get(), ModelSafeRoutingInfo());
   ExpectHasProcessorsForTypes(*registrar_, ModelTypeSet());
 }
 
 TEST_F(SyncBackendRegistrarTest, ConstructorNonEmpty) {
-  const ModelTypeSet initial_types(BOOKMARKS, NIGORI, PASSWORDS);
   registrar_->RegisterNonBlockingType(BOOKMARKS);
-  registrar_->SetInitialTypes(initial_types);
+  registrar_->SetInitialTypes(ModelTypeSet(BOOKMARKS, NIGORI, PASSWORDS));
   EXPECT_TRUE(registrar_->IsNigoriEnabled());
-  {
-    std::vector<scoped_refptr<syncer::ModelSafeWorker>> workers;
-    registrar_->GetWorkers(&workers);
-    EXPECT_EQ(4u, workers.size());
-  }
-  {
-    syncer::ModelSafeRoutingInfo expected_routing_info;
-    expected_routing_info[BOOKMARKS] = syncer::GROUP_NON_BLOCKING;
-    expected_routing_info[NIGORI] = syncer::GROUP_PASSIVE;
-    // Passwords dropped because of no password store.
-    ExpectRoutingInfo(registrar_.get(), expected_routing_info);
-  }
+  EXPECT_EQ(4u, GetWorkersSize());
+  EXPECT_EQ(ModelTypeSet(NIGORI), registrar_->GetLastConfiguredTypes());
+  // Bookmarks dropped because it is nonblocking.
+  // Passwords dropped because of no password store.
+  ExpectRoutingInfo(registrar_.get(), {{NIGORI, syncer::GROUP_PASSIVE}});
   ExpectHasProcessorsForTypes(*registrar_, ModelTypeSet());
 }
 
+TEST_F(SyncBackendRegistrarTest, ConstructorNonEmptyReversedInitialization) {
+  // The blocking types get to set initial types before NonBlocking types here.
+  registrar_->SetInitialTypes(ModelTypeSet(BOOKMARKS, NIGORI, PASSWORDS));
+  registrar_->RegisterNonBlockingType(BOOKMARKS);
+  EXPECT_TRUE(registrar_->IsNigoriEnabled());
+  EXPECT_EQ(4u, GetWorkersSize());
+  EXPECT_EQ(ModelTypeSet(NIGORI), registrar_->GetLastConfiguredTypes());
+  // Bookmarks dropped because it is nonblocking.
+  // Passwords dropped because of no password store.
+  ExpectRoutingInfo(registrar_.get(), {{NIGORI, syncer::GROUP_PASSIVE}});
+  ExpectHasProcessorsForTypes(*registrar_, ModelTypeSet());
+}
+
 TEST_F(SyncBackendRegistrarTest, ConfigureDataTypes) {
   registrar_->RegisterNonBlockingType(BOOKMARKS);
   registrar_->SetInitialTypes(ModelTypeSet());
 
   // Add.
   const ModelTypeSet types1(BOOKMARKS, NIGORI, AUTOFILL);
   EXPECT_EQ(types1, registrar_->ConfigureDataTypes(types1, ModelTypeSet()));
-  {
-    syncer::ModelSafeRoutingInfo expected_routing_info;
-    expected_routing_info[BOOKMARKS] = syncer::GROUP_NON_BLOCKING;
-    expected_routing_info[NIGORI] = syncer::GROUP_PASSIVE;
-    expected_routing_info[AUTOFILL] = syncer::GROUP_PASSIVE;
-    ExpectRoutingInfo(registrar_.get(), expected_routing_info);
-  }
+  ExpectRoutingInfo(registrar_.get(), {{BOOKMARKS, syncer::GROUP_NON_BLOCKING},
+                                       {NIGORI, syncer::GROUP_PASSIVE},
+                                       {AUTOFILL, syncer::GROUP_PASSIVE}});
   ExpectHasProcessorsForTypes(*registrar_, ModelTypeSet());
   EXPECT_EQ(types1, registrar_->GetLastConfiguredTypes());
 
   // Add and remove.
   const ModelTypeSet types2(PREFERENCES, THEMES);
   EXPECT_EQ(types2, registrar_->ConfigureDataTypes(types2, types1));
-  {
-    syncer::ModelSafeRoutingInfo expected_routing_info;
-    expected_routing_info[PREFERENCES] = syncer::GROUP_PASSIVE;
-    expected_routing_info[THEMES] = syncer::GROUP_PASSIVE;
-    ExpectRoutingInfo(registrar_.get(), expected_routing_info);
-  }
+
+  ExpectRoutingInfo(registrar_.get(), {{PREFERENCES, syncer::GROUP_PASSIVE},
+                                       {THEMES, syncer::GROUP_PASSIVE}});
   ExpectHasProcessorsForTypes(*registrar_, ModelTypeSet());
   EXPECT_EQ(types2, registrar_->GetLastConfiguredTypes());
 
   // Remove.
   EXPECT_TRUE(registrar_->ConfigureDataTypes(ModelTypeSet(), types2).Empty());
-  ExpectRoutingInfo(registrar_.get(), syncer::ModelSafeRoutingInfo());
+  ExpectRoutingInfo(registrar_.get(), ModelSafeRoutingInfo());
   ExpectHasProcessorsForTypes(*registrar_, ModelTypeSet());
   EXPECT_EQ(ModelTypeSet(), registrar_->GetLastConfiguredTypes());
 }
 
 TEST_F(SyncBackendRegistrarTest, ActivateDeactivateUIDataType) {
   InSequence in_sequence;
   registrar_->SetInitialTypes(ModelTypeSet());
 
   // Should do nothing.
   TriggerChanges(registrar_.get(), BOOKMARKS);
 
   StrictMock<sync_driver::ChangeProcessorMock> change_processor_mock;
   EXPECT_CALL(change_processor_mock, StartImpl());
   EXPECT_CALL(change_processor_mock, IsRunning()).WillRepeatedly(Return(true));
   EXPECT_CALL(change_processor_mock, ApplyChangesFromSyncModel(NULL, _, _));
   EXPECT_CALL(change_processor_mock, IsRunning()).WillRepeatedly(Return(true));
   EXPECT_CALL(change_processor_mock, CommitChangesFromSyncModel());
   EXPECT_CALL(change_processor_mock, IsRunning()).WillRepeatedly(Return(false));
 
   const ModelTypeSet types(BOOKMARKS);
   EXPECT_EQ(types, registrar_->ConfigureDataTypes(types, ModelTypeSet()));
   registrar_->ActivateDataType(BOOKMARKS, syncer::GROUP_UI,
                                &change_processor_mock,
                                test_user_share_.user_share());
-  {
-    syncer::ModelSafeRoutingInfo expected_routing_info;
-    expected_routing_info[BOOKMARKS] = syncer::GROUP_UI;
-    ExpectRoutingInfo(registrar_.get(), expected_routing_info);
-  }
+  ExpectRoutingInfo(registrar_.get(), {{BOOKMARKS, syncer::GROUP_UI}});
   ExpectHasProcessorsForTypes(*registrar_, types);
 
   TriggerChanges(registrar_.get(), BOOKMARKS);
 
   registrar_->DeactivateDataType(BOOKMARKS);
-  ExpectRoutingInfo(registrar_.get(), syncer::ModelSafeRoutingInfo());
+  ExpectRoutingInfo(registrar_.get(), ModelSafeRoutingInfo());
   ExpectHasProcessorsForTypes(*registrar_, ModelTypeSet());
 
   // Should do nothing.
   TriggerChanges(registrar_.get(), BOOKMARKS);
 }
 
 TEST_F(SyncBackendRegistrarTest, ActivateDeactivateNonUIDataType) {
   InSequence in_sequence;
   registrar_->SetInitialTypes(ModelTypeSet());
 
@@ skipping 13 matching lines @@
 
   base::WaitableEvent done(base::WaitableEvent::ResetPolicy::AUTOMATIC,
                            base::WaitableEvent::InitialState::NOT_SIGNALED);
   db_task_runner()->PostTask(
       FROM_HERE,
       base::Bind(&SyncBackendRegistrarTest::TestNonUIDataTypeActivationAsync,
                  base::Unretained(this), &change_processor_mock, &done));
   done.Wait();
 
   registrar_->DeactivateDataType(AUTOFILL);
-  ExpectRoutingInfo(registrar_.get(), syncer::ModelSafeRoutingInfo());
+  ExpectRoutingInfo(registrar_.get(), ModelSafeRoutingInfo());
   ExpectHasProcessorsForTypes(*registrar_, ModelTypeSet());
 
   // Should do nothing.
   TriggerChanges(registrar_.get(), AUTOFILL);
 }
 
 // Tests that registration and configuration of non-blocking data types is
 // handled correctly in SyncBackendRegistrar.
 TEST_F(SyncBackendRegistrarTest, ConfigureNonBlockingDataType) {
   registrar_->RegisterNonBlockingType(AUTOFILL);
   registrar_->RegisterNonBlockingType(BOOKMARKS);
 
-  ExpectRoutingInfo(registrar_.get(), syncer::ModelSafeRoutingInfo());
+  ExpectRoutingInfo(registrar_.get(), ModelSafeRoutingInfo());
   // Simulate that initial sync was already done for AUTOFILL.
   registrar_->AddRestoredNonBlockingType(AUTOFILL);
   // It should be added to routing info and set of configured types.
   EXPECT_EQ(ModelTypeSet(AUTOFILL), registrar_->GetLastConfiguredTypes());
-  {
-    syncer::ModelSafeRoutingInfo expected_routing_info;
-    expected_routing_info[AUTOFILL] = syncer::GROUP_NON_BLOCKING;
-    ExpectRoutingInfo(registrar_.get(), expected_routing_info);
-  }
+  ExpectRoutingInfo(registrar_.get(), {{AUTOFILL, syncer::GROUP_NON_BLOCKING}});
 
   // Configure two non-blocking types. Initial sync wasn't done for BOOKMARKS so
   // it should be included in types to be downloaded.
   ModelTypeSet types_to_add(AUTOFILL, BOOKMARKS);
   ModelTypeSet newly_added_types =
       registrar_->ConfigureDataTypes(types_to_add, ModelTypeSet());
   EXPECT_EQ(ModelTypeSet(BOOKMARKS), newly_added_types);
   EXPECT_EQ(types_to_add, registrar_->GetLastConfiguredTypes());
-  {
-    syncer::ModelSafeRoutingInfo expected_routing_info;
-    expected_routing_info[AUTOFILL] = syncer::GROUP_NON_BLOCKING;
-    expected_routing_info[BOOKMARKS] = syncer::GROUP_NON_BLOCKING;
-    ExpectRoutingInfo(registrar_.get(), expected_routing_info);
-  }
+  ExpectRoutingInfo(registrar_.get(),
+                    {{AUTOFILL, syncer::GROUP_NON_BLOCKING},
+                     {BOOKMARKS, syncer::GROUP_NON_BLOCKING}});
 }
 
 class SyncBackendRegistrarShutdownTest : public testing::Test {
  public:
   void BlockDBThread() {
     EXPECT_FALSE(db_thread_lock_.Try());
 
     db_thread_blocked_.Signal();
     base::AutoLock l(db_thread_lock_);
   }
@@ skipping 109 matching lines @@
   db_thread_lock_.Release();
 
   // Run the main thread loop until all workers have been removed and the
   // registrar destroyed.
   run_loop_.Run();
 }
 
 }  // namespace
 
 }  // namespace browser_sync