sync: Fix backend configuration to support sync db refresh.

Removes the has_new bit in favor of checking if initial_sync_ended for requested types, and recreates the sync db on corruption (as we used to).  These together mean we refresh the sync db on corruption.  Also added some defensive dchecks.

To test this, I ripped out the test-only #ifdefs in SyncBackendHost so the tests use the real Initialize code path, and added a toolbox of stuff to control when we set initial sync ended bits during testing.

BUG=50965
TEST=DirectoryBackingStoreTest, ProfileSyncService* tests.

Review URL: http://codereview.chromium.org/3099001

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@55644 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 78 insertions, 66 deletions

diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc
index c81223b..40782ec 100644..100755
--- a/chrome/browser/sync/glue/sync_backend_host.cc
+++ b/chrome/browser/sync/glue/sync_backend_host.cc
@@ -50,7 +50,8 @@ SyncBackendHost::SyncBackendHost(
       frontend_(frontend),
       sync_data_folder_path_(profile_path.Append(kSyncDataFolderName)),
       data_type_controllers_(data_type_controllers),
-      last_auth_error_(AuthError::None()) {
+      last_auth_error_(AuthError::None()),
+      syncapi_initialized_(false) {
 
   core_ = new Core(this);
 }
@@ -60,7 +61,8 @@ SyncBackendHost::SyncBackendHost()
       frontend_loop_(MessageLoop::current()),
       profile_(NULL),
       frontend_(NULL),
-      last_auth_error_(AuthError::None()) {
+      last_auth_error_(AuthError::None()),
+      syncapi_initialized_(false) {
 }
 
 SyncBackendHost::~SyncBackendHost() {
@@ -106,19 +108,28 @@ void SyncBackendHost::Initialize(
     registrar_.routing_info[(*it)] = GROUP_PASSIVE;
   }
 
+  InitCore(Core::DoInitializeOptions(
+      sync_service_url, lsid.empty(),
+      MakeHttpBridgeFactory(baseline_context_getter),
+      MakeHttpBridgeFactory(baseline_context_getter),
+      lsid,
+      delete_sync_data_folder,
+      invalidate_sync_login,
+      invalidate_sync_xmpp_login,
+      use_chrome_async_socket,
+      try_ssltcp_first,
+      notification_method));
+}
+
+sync_api::HttpPostProviderFactory* SyncBackendHost::MakeHttpBridgeFactory(
+    URLRequestContextGetter* getter) {
+  return new HttpBridgeFactory(getter);
+}
+
+void SyncBackendHost::InitCore(const Core::DoInitializeOptions& options) {
   core_thread_.message_loop()->PostTask(FROM_HERE,
       NewRunnableMethod(core_.get(), &SyncBackendHost::Core::DoInitialize,
-                        Core::DoInitializeOptions(
-                            sync_service_url, lsid.empty(),
-                            new HttpBridgeFactory(baseline_context_getter),
-                            new HttpBridgeFactory(baseline_context_getter),
-                            lsid,
-                            delete_sync_data_folder,
-                            invalidate_sync_login,
-                            invalidate_sync_xmpp_login,
-                            use_chrome_async_socket,
-                            try_ssltcp_first,
-                            notification_method)));
+                        options));
 }
 
 void SyncBackendHost::Authenticate(const std::string& username,
@@ -190,30 +201,31 @@ void SyncBackendHost::ConfigureDataTypes(const syncable::ModelTypeSet& types,
                                          CancelableTask* ready_task) {
   // Only one configure is allowed at a time.
   DCHECK(!configure_ready_task_.get());
-  AutoLock lock(registrar_lock_);
-  bool has_new = false;
-
-  for (DataTypeController::TypeMap::const_iterator it =
-           data_type_controllers_.begin();
-       it != data_type_controllers_.end(); ++it) {
-    syncable::ModelType type = (*it).first;
-
-    // If a type is not specified, remove it from the routing_info.
-    if (types.count(type) == 0) {
-      registrar_.routing_info.erase(type);
-    } else {
-      // Add a newly specified data type as GROUP_PASSIVE into the
-      // routing_info, if it does not already exist.
-      if (registrar_.routing_info.count(type) == 0) {
-        registrar_.routing_info[type] = GROUP_PASSIVE;
-        has_new = true;
+  DCHECK(syncapi_initialized_);
+
+  {
+    AutoLock lock(registrar_lock_);
+    for (DataTypeController::TypeMap::const_iterator it =
+             data_type_controllers_.begin();
+         it != data_type_controllers_.end(); ++it) {
+      syncable::ModelType type = (*it).first;
+
+      // If a type is not specified, remove it from the routing_info.
+      if (types.count(type) == 0) {
+        registrar_.routing_info.erase(type);
+      } else {
+        // Add a newly specified data type as GROUP_PASSIVE into the
+        // routing_info, if it does not already exist.
+        if (registrar_.routing_info.count(type) == 0) {
+          registrar_.routing_info[type] = GROUP_PASSIVE;
+        }
       }
     }
   }
 
   // If no new data types were added to the passive group, no need to
   // wait for the syncer.
-  if (!has_new) {
+  if (core_->syncapi()->InitialSyncEndedForAllEnabledTypes()) {
     ready_task->Run();
     delete ready_task;
     return;
@@ -221,7 +233,7 @@ void SyncBackendHost::ConfigureDataTypes(const syncable::ModelTypeSet& types,
 
   // Save the task here so we can run it when the syncer finishes
   // initializing the new data types.  It will be run only when the
-  // set of initially sycned data types matches the types requested in
+  // set of initially synced data types matches the types requested in
   // this configure.
   configure_ready_task_.reset(ready_task);
   configure_initial_sync_types_ = types;
@@ -231,6 +243,10 @@ void SyncBackendHost::ConfigureDataTypes(const syncable::ModelTypeSet& types,
   // downloading updates for newly added data types.  Once this is
   // complete, the configure_ready_task_ is run via an
   // OnInitializationComplete notification.
+  RequestNudge();
+}
+
+void SyncBackendHost::RequestNudge() {
   core_thread_.message_loop()->PostTask(FROM_HERE,
       NewRunnableMethod(core_.get(), &SyncBackendHost::Core::DoRequestNudge));
 }
@@ -308,18 +324,22 @@ void SyncBackendHost::Core::NotifyPassphraseAccepted() {
 }
 
 SyncBackendHost::UserShareHandle SyncBackendHost::GetUserShareHandle() const {
+  DCHECK(syncapi_initialized_);
   return core_->syncapi()->GetUserShare();
 }
 
 SyncBackendHost::Status SyncBackendHost::GetDetailedStatus() {
+  DCHECK(syncapi_initialized_);
   return core_->syncapi()->GetDetailedStatus();
 }
 
 SyncBackendHost::StatusSummary SyncBackendHost::GetStatusSummary() {
+  DCHECK(syncapi_initialized_);
   return core_->syncapi()->GetStatusSummary();
 }
 
 string16 SyncBackendHost::GetAuthenticatedUsername() const {
+  DCHECK(syncapi_initialized_);
   return UTF8ToUTF16(core_->syncapi()->GetAuthenticatedUsername());
 }
 
@@ -347,6 +367,7 @@ void SyncBackendHost::GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) {
 }
 
 bool SyncBackendHost::HasUnsyncedItems() const {
+  DCHECK(syncapi_initialized_);
   return core_->syncapi()->HasUnsyncedItems();
 }
 
@@ -559,6 +580,7 @@ void SyncBackendHost::Core::HandleInitalizationCompletedOnFrontendLoop() {
 void SyncBackendHost::HandleInitializationCompletedOnFrontendLoop() {
   if (!frontend_)
     return;
+  syncapi_initialized_ = true;
   frontend_->OnBackendInitialized();
 }
 
diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h
index 9dacfab..4e1a9cd 100644
--- a/chrome/browser/sync/glue/sync_backend_host.h
+++ b/chrome/browser/sync/glue/sync_backend_host.h
@@ -182,43 +182,10 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar {
 
   // Determines if the underlying sync engine has made any local changes to
   // items that have not yet been synced with the server.
+  // ONLY CALL THIS IF OnInitializationComplete was called!
   bool HasUnsyncedItems() const;
 
-#if defined(UNIT_TEST)
-  // Called from unit test to bypass authentication and initialize the syncapi
-  // to a state suitable for testing but not production.
-  void InitializeForTestMode(const std::wstring& test_user,
-                             sync_api::HttpPostProviderFactory* factory,
-                             sync_api::HttpPostProviderFactory* auth_factory,
-                             bool delete_sync_data_folder,
-                             NotificationMethod notification_method) {
-    if (!core_thread_.Start())
-      return;
-    registrar_.workers[GROUP_UI] = new UIModelWorker(frontend_loop_);
-    registrar_.routing_info[syncable::BOOKMARKS] = GROUP_PASSIVE;
-    registrar_.routing_info[syncable::PREFERENCES] = GROUP_PASSIVE;
-    registrar_.routing_info[syncable::AUTOFILL] = GROUP_PASSIVE;
-    registrar_.routing_info[syncable::THEMES] = GROUP_PASSIVE;
-    registrar_.routing_info[syncable::TYPED_URLS] = GROUP_PASSIVE;
-    registrar_.routing_info[syncable::NIGORI] = GROUP_PASSIVE;
-    registrar_.routing_info[syncable::PASSWORDS] = GROUP_PASSIVE;
-
-    core_thread_.message_loop()->PostTask(FROM_HERE,
-        NewRunnableMethod(core_.get(),
-        &SyncBackendHost::Core::DoInitializeForTest,
-        test_user,
-        factory,
-        auth_factory,
-        delete_sync_data_folder,
-        notification_method));
-  }
-#endif
  protected:
-  // InitializationComplete passes through the SyncBackendHost to forward
-  // on to |frontend_|, and so that tests can intercept here if they need to
-  // set up initial conditions.
-  virtual void HandleInitializationCompletedOnFrontendLoop();
-
   // The real guts of SyncBackendHost, to keep the public client API clean.
   class Core : public base::RefCountedThreadSafe<SyncBackendHost::Core>,
                public sync_api::SyncManager::Observer {
@@ -352,6 +319,7 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar {
 
    private:
     friend class base::RefCountedThreadSafe<SyncBackendHost::Core>;
+    friend class SyncBackendHostForProfileSyncTest;
 
     ~Core() {}
 
@@ -414,6 +382,25 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar {
     DISALLOW_COPY_AND_ASSIGN(Core);
   };
 
+  // InitializationComplete passes through the SyncBackendHost to forward
+  // on to |frontend_|, and so that tests can intercept here if they need to
+  // set up initial conditions.
+  virtual void HandleInitializationCompletedOnFrontendLoop();
+
+  // Posts a nudge request on the core thread.
+  virtual void RequestNudge();
+
+  // Allows tests to perform alternate core initialization work.
+  virtual void InitCore(const Core::DoInitializeOptions& options);
+
+  // Factory method for HttpPostProviderFactories.
+  virtual sync_api::HttpPostProviderFactory* MakeHttpBridgeFactory(
+      URLRequestContextGetter* getter);
+
+  MessageLoop* core_loop() { return core_thread_.message_loop(); }
+
+  void set_syncapi_initialized() { syncapi_initialized_ = true; }
+
   // Our core, which communicates directly to the syncapi.
   scoped_refptr<Core> core_;
 
@@ -484,6 +471,9 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar {
   // UI-thread cache of the last SyncSessionSnapshot received from syncapi.
   scoped_ptr<sessions::SyncSessionSnapshot> last_snapshot_;
 
+  // Whether we've processed the initialization complete callback.
+  bool syncapi_initialized_;
+
   DISALLOW_COPY_AND_ASSIGN(SyncBackendHost);
 };