sync: avoid creating a HistoryModelWorker unless it is needed.

chrome/browser/sync/glue/sync_backend_host.cc

 // Copyright (c) 2010 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 "build/build_config.h"
 
 #include <algorithm>
 
 #include "base/command_line.h"
 #include "base/file_util.h"
@@ skipping 79 matching lines @@
   if (!core_thread_.Start())
     return;
 
   // Create a worker for the UI thread and route bookmark changes to it.
   // TODO(tim): Pull this into a method to reuse.  For now we don't even
   // need to lock because we init before the syncapi exists and we tear down
   // after the syncapi is destroyed.  Make sure to NULL-check workers_ indices
   // when a new type is synced as the worker may already exist and you just
   // need to update routing_info_.
   registrar_.workers[GROUP_DB] = new DatabaseModelWorker();
-  registrar_.workers[GROUP_HISTORY] =
-      new HistoryModelWorker(
-          profile_->GetHistoryService(Profile::IMPLICIT_ACCESS));
   registrar_.workers[GROUP_UI] = new UIModelWorker(frontend_loop_);
   registrar_.workers[GROUP_PASSIVE] = new ModelSafeWorker();
 
+  if (CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableSyncTypedUrls || types.count(syncable::TYPED_URLS))) {

[lipalani, 2011/01/12 22:06:51]

Also is this code compiling. It looks like you need to have a closing parantheis
after switches::kEnableSyncTypedUrls.

[tim (not reviewing), 2011/01/12 23:41:28]

Doh. Hadn't uploaded the patch set with this fixed :) Did now.

+    // TODO(tim): Bug 53916.  HistoryModelWorker crashes, so avoid adding it
+    // unless specifically requested until bug is fixed.
+    registrar_.workers[GROUP_HISTORY] =
+        new HistoryModelWorker(
+            profile_->GetHistoryService(Profile::IMPLICIT_ACCESS));
+  }

[lipalani, 2011/01/12 22:02:53]

may be HasSwitch check alone is sufficient. The types comes from the registered
data types. And it would not be registered unless the command line was passed. 
The problem with the types check is that when somebody fixes the typed urls to
be default, this code would need no modification, so they would not look for it
and it would stay on forever until some tester tests by disabling typed url and
then reenables and finds things crashing.
Does that make sense?

[tim (not reviewing), 2011/01/12 23:41:28]

The reason for the types check is unittests. See the case below with password
store.

[lipalani, 2011/01/13 00:31:46]

sounds good.
On 2011/01/12 23:41:28, timsteele wrote:

+
   PasswordStore* password_store =
       profile_->GetPasswordStore(Profile::IMPLICIT_ACCESS);
   if (password_store) {
     registrar_.workers[GROUP_PASSWORD] =
         new PasswordModelWorker(password_store);
   } else {
     LOG(WARNING) << "Password store not initialized, cannot sync passwords";
   }
 
   // Any datatypes that we want the syncer to pull down must
@@ skipping 766 matching lines @@
 }
 
 void SyncBackendHost::Core::DeleteSyncDataFolder() {
   if (file_util::DirectoryExists(host_->sync_data_folder_path())) {
     if (!file_util::Delete(host_->sync_data_folder_path(), true))
       LOG(DFATAL) << "Could not delete the Sync Data folder.";
   }
 }
 
 }  // namespace browser_sync