Don't create services that need a DB thread if there is no DB thread.

Right now, this CL is vacuous, since |db_thread_| is always set.

However, while changing the WebDataService to move off the DB thread, I'll be
changing ChromeSyncClient to get the correct thread from the WebDataService.  If
there is no WebDataService, then the easiest thing to do is this CL (don't
bother creating things that need a WebDataService to function, and ensure that
tests that need these created create the WebDataService first).

If we didn't do this, then one of two things would happen:
* ChromeSyncClient would pass a null db_thread_ in, which would get passed into
  the various individual datatype services here, which would then run into
  problems because at least some of them DCHECK that they have a model thread.
  These services would have to be tweaked to instead DCHECK that no one tries to
  _use_ them when there's no model thread.  I went this route first, then
  decided it made no sense to create services that weren't allowed to be used.
* ChromeSyncClient would make up a random task runner and pass it down as the
  model thread.  Again, it felt misleading to create a task runner that no one
  could run tasks on (because if someone did, it would mean they were trying to
  talk to a database that didn't exist).

So, this seemed like the clearest precursor to the future changes.

While there, I also tweaked how the conditional works for history-based services
so it would be structured similarly.

Bug: 689520
Change-Id: Ic1aaa5947e7328b322bc508e3092ab389b6f73c3
Reviewed-on: https://chromium-review.googlesource.com/578929Reviewed-by: Nicolas Zea <zea@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488388}

chrome/browser/sync/profile_sync_service_factory_unittest.cc

@@ -28,6 +28,12 @@ using browser_sync::ProfileSyncService;
 using syncer::DataTypeController;
 
 class ProfileSyncServiceFactoryTest : public testing::Test {
+ public:
+  void SetUp() override {
+    // Some services will only be created if there is a WebDataService.
+    profile_->CreateWebDataService();
+  }
+
  protected:
   ProfileSyncServiceFactoryTest() : profile_(new TestingProfile()) {}
 
@@ -113,7 +119,7 @@ class ProfileSyncServiceFactoryTest : public testing::Test {
 
  private:
   content::TestBrowserThreadBundle thread_bundle_;
-  std::unique_ptr<Profile> profile_;
+  std::unique_ptr<TestingProfile> profile_;
 };
 
 // Verify that the disable sync flag disables creation of the sync service.

components/browser_sync/profile_sync_components_factory_impl.cc

@@ -154,48 +154,51 @@ void ProfileSyncComponentsFactoryImpl::RegisterCommonDataTypes(
             sync_service->GetLocalDeviceInfoProvider()));
   }
 
-  // Autocomplete sync is enabled by default.  Register unless explicitly
-  // disabled.
-  if (!disabled_types.Has(syncer::AUTOFILL)) {
-    if (FeatureList::IsEnabled(switches::kSyncUSSAutocomplete)) {
-      sync_service->RegisterDataTypeController(
-          base::MakeUnique<autofill::WebDataModelTypeController>(
-              syncer::AUTOFILL, sync_client_, db_thread_, web_data_service_,
-              base::Bind(
-                  &autofill::AutocompleteSyncBridge::FromWebDataService)));
-    } else {
+  // These features are enabled only if there's a DB thread to post tasks to.
+  if (db_thread_) {
+    // Autocomplete sync is enabled by default.  Register unless explicitly
+    // disabled.
+    if (!disabled_types.Has(syncer::AUTOFILL)) {
+      if (FeatureList::IsEnabled(switches::kSyncUSSAutocomplete)) {
+        sync_service->RegisterDataTypeController(
+            base::MakeUnique<autofill::WebDataModelTypeController>(
+                syncer::AUTOFILL, sync_client_, db_thread_, web_data_service_,
+                base::Bind(
+                    &autofill::AutocompleteSyncBridge::FromWebDataService)));
+      } else {
+        sync_service->RegisterDataTypeController(
+            base::MakeUnique<AutofillDataTypeController>(
+                db_thread_, error_callback, sync_client_, web_data_service_));
+      }
+    }
+
+    // Autofill sync is enabled by default.  Register unless explicitly
+    // disabled.
+    if (!disabled_types.Has(syncer::AUTOFILL_PROFILE)) {
       sync_service->RegisterDataTypeController(
-          base::MakeUnique<AutofillDataTypeController>(
+          base::MakeUnique<AutofillProfileDataTypeController>(
               db_thread_, error_callback, sync_client_, web_data_service_));
     }
-  }
 
-  // Autofill sync is enabled by default.  Register unless explicitly
-  // disabled.
-  if (!disabled_types.Has(syncer::AUTOFILL_PROFILE)) {
-    sync_service->RegisterDataTypeController(
-        base::MakeUnique<AutofillProfileDataTypeController>(
-            db_thread_, error_callback, sync_client_, web_data_service_));
-  }
-
-  // Wallet data sync is enabled by default, but behind a syncer experiment
-  // enforced by the datatype controller. Register unless explicitly disabled.
-  bool wallet_disabled = disabled_types.Has(syncer::AUTOFILL_WALLET_DATA);
-  if (!wallet_disabled) {
-    sync_service->RegisterDataTypeController(
-        base::MakeUnique<AutofillWalletDataTypeController>(
-            syncer::AUTOFILL_WALLET_DATA, db_thread_, error_callback,
-            sync_client_, web_data_service_));
-  }
+    // Wallet data sync is enabled by default, but behind a syncer experiment
+    // enforced by the datatype controller. Register unless explicitly disabled.
+    bool wallet_disabled = disabled_types.Has(syncer::AUTOFILL_WALLET_DATA);
+    if (!wallet_disabled) {
+      sync_service->RegisterDataTypeController(
+          base::MakeUnique<AutofillWalletDataTypeController>(
+              syncer::AUTOFILL_WALLET_DATA, db_thread_, error_callback,
+              sync_client_, web_data_service_));
+    }
 
-  // Wallet metadata sync depends on Wallet data sync. Register if Wallet data
-  // is syncing and metadata sync is not explicitly disabled.
-  if (!wallet_disabled &&
-      !disabled_types.Has(syncer::AUTOFILL_WALLET_METADATA)) {
-    sync_service->RegisterDataTypeController(
-        base::MakeUnique<AutofillWalletDataTypeController>(
-            syncer::AUTOFILL_WALLET_METADATA, db_thread_, error_callback,
-            sync_client_, web_data_service_));
+    // Wallet metadata sync depends on Wallet data sync. Register if Wallet data
+    // is syncing and metadata sync is not explicitly disabled.
+    if (!wallet_disabled &&
+        !disabled_types.Has(syncer::AUTOFILL_WALLET_METADATA)) {
+      sync_service->RegisterDataTypeController(
+          base::MakeUnique<AutofillWalletDataTypeController>(
+              syncer::AUTOFILL_WALLET_METADATA, db_thread_, error_callback,
+              sync_client_, web_data_service_));
+    }
   }
 
   // Bookmark sync is enabled by default.  Register unless explicitly
@@ -206,55 +209,54 @@ void ProfileSyncComponentsFactoryImpl::RegisterCommonDataTypes(
                                                      sync_client_));
   }
 
-  const bool history_disabled =
-      sync_client_->GetPrefService()->GetBoolean(history_disabled_pref_);
-  // TypedUrl sync is enabled by default.  Register unless explicitly disabled,
-  // or if saving history is disabled.
-  if (!disabled_types.Has(syncer::TYPED_URLS) && !history_disabled) {
-    if (base::FeatureList::IsEnabled(switches::kSyncUSSTypedURL)) {
-      // TODO(gangwu): Register controller here once typed url controller
-      // implemented.
-    } else {
-      sync_service->RegisterDataTypeController(
-          base::MakeUnique<TypedUrlDataTypeController>(
-              error_callback, sync_client_, history_disabled_pref_));
+  // These features are enabled only if history is not disabled.
+  if (!sync_client_->GetPrefService()->GetBoolean(history_disabled_pref_)) {
+    // TypedUrl sync is enabled by default.  Register unless explicitly
+    // disabled.
+    if (!disabled_types.Has(syncer::TYPED_URLS)) {
+      if (base::FeatureList::IsEnabled(switches::kSyncUSSTypedURL)) {
+        // TODO(gangwu): Register controller here once typed url controller
+        // implemented.
+      } else {
+        sync_service->RegisterDataTypeController(
+            base::MakeUnique<TypedUrlDataTypeController>(
+                error_callback, sync_client_, history_disabled_pref_));
+      }
     }
-  }
 
-  // Delete directive sync is enabled by default.  Register unless full history
-  // sync is disabled.
-  if (!disabled_types.Has(syncer::HISTORY_DELETE_DIRECTIVES) &&
-      !history_disabled) {
-    sync_service->RegisterDataTypeController(
-        base::MakeUnique<HistoryDeleteDirectivesDataTypeController>(
-            error_callback, sync_client_));
-  }
+    // Delete directive sync is enabled by default.
+    if (!disabled_types.Has(syncer::HISTORY_DELETE_DIRECTIVES)) {
+      sync_service->RegisterDataTypeController(
+          base::MakeUnique<HistoryDeleteDirectivesDataTypeController>(
+              error_callback, sync_client_));
+    }
 
-  // Session sync is enabled by default.  Register unless explicitly disabled.
-  // This is also disabled if the browser history is disabled, because the
-  // tab sync data is added to the web history on the server.
-  if (!disabled_types.Has(syncer::PROXY_TABS) && !history_disabled) {
-    sync_service->RegisterDataTypeController(
-        base::MakeUnique<ProxyDataTypeController>(syncer::PROXY_TABS));
-    sync_service->RegisterDataTypeController(
-        base::MakeUnique<SessionDataTypeController>(
-            error_callback, sync_client_,
-            sync_service->GetLocalDeviceInfoProvider(),
-            history_disabled_pref_));
-  }
+    // Session sync is enabled by default.  This is disabled if history is
+    // disabled because the tab sync data is added to the web history on the
+    // server.
+    if (!disabled_types.Has(syncer::PROXY_TABS)) {
+      sync_service->RegisterDataTypeController(
+          base::MakeUnique<ProxyDataTypeController>(syncer::PROXY_TABS));
+      sync_service->RegisterDataTypeController(
+          base::MakeUnique<SessionDataTypeController>(
+              error_callback, sync_client_,
+              sync_service->GetLocalDeviceInfoProvider(),
+              history_disabled_pref_));
+    }
 
-  // Favicon sync is enabled by default. Register unless explicitly disabled.
-  if (!disabled_types.Has(syncer::FAVICON_IMAGES) &&
-      !disabled_types.Has(syncer::FAVICON_TRACKING) && !history_disabled) {
-    // crbug/384552. We disable error uploading for this data types for now.
-    sync_service->RegisterDataTypeController(
-        base::MakeUnique<AsyncDirectoryTypeController>(
-            syncer::FAVICON_IMAGES, base::Closure(), sync_client_,
-            syncer::GROUP_UI, ui_thread_));
-    sync_service->RegisterDataTypeController(
-        base::MakeUnique<AsyncDirectoryTypeController>(
-            syncer::FAVICON_TRACKING, base::Closure(), sync_client_,
-            syncer::GROUP_UI, ui_thread_));
+    // Favicon sync is enabled by default. Register unless explicitly disabled.
+    if (!disabled_types.Has(syncer::FAVICON_IMAGES) &&
+        !disabled_types.Has(syncer::FAVICON_TRACKING)) {
+      // crbug/384552. We disable error uploading for this data types for now.
+      sync_service->RegisterDataTypeController(
+          base::MakeUnique<AsyncDirectoryTypeController>(
+              syncer::FAVICON_IMAGES, base::Closure(), sync_client_,
+              syncer::GROUP_UI, ui_thread_));
+      sync_service->RegisterDataTypeController(
+          base::MakeUnique<AsyncDirectoryTypeController>(
+              syncer::FAVICON_TRACKING, base::Closure(), sync_client_,
+              syncer::GROUP_UI, ui_thread_));
+    }
   }
 
   // Password sync is enabled by default.  Register unless explicitly

ios/chrome/browser/sync/ios_chrome_profile_sync_service_factory_unittest.cc

@@ -26,6 +26,11 @@ class IOSChromeProfileSyncServiceFactoryTest : public testing::Test {
     chrome_browser_state_ = browser_state_builder.Build();
   }
 
+  void SetUp() override {
+    // Some services will only be created if there is a WebDataService.
+    chrome_browser_state_->CreateWebDataService();
+  }
+
  protected:
   // Returns the collection of default datatypes.
   std::vector<syncer::ModelType> DefaultDatatypes() {