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

13 files changed, 304 insertions, 139 deletions

diff --git a/chrome/browser/sync/engine/syncapi.cc b/chrome/browser/sync/engine/syncapi.cc
index 76f492b..f83b144 100644..100755
--- a/chrome/browser/sync/engine/syncapi.cc
+++ b/chrome/browser/sync/engine/syncapi.cc
@@ -13,7 +13,6 @@
 
 #include "base/basictypes.h"
 #include "base/base64.h"
-#include "base/histogram.h"
 #include "base/lock.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
@@ -1036,22 +1035,6 @@ class SyncManager::SyncInternal
   // Called only by our NetworkChangeNotifier.
   virtual void OnIPAddressChanged();
 
- private:
-  // Try to authenticate using a LSID cookie.
-  void AuthenticateWithLsid(const std::string& lsid);
-
-  // Try to authenticate using persisted credentials from a previous successful
-  // authentication. If no such credentials exist, calls OnAuthError on the
-  // client to collect credentials. Otherwise, there exist local credentials
-  // that were once used for a successful auth, so we'll try to re-use these.
-  // Failure of that attempt will be communicated as normal using OnAuthError.
-  // Since this entry point will bypass normal GAIA authentication and try to
-  // authenticate directly with the sync service using a cached token,
-  // authentication failure will generally occur due to expired credentials, or
-  // possibly because of a password change.
-  bool AuthenticateForUser(const std::string& username,
-                           const std::string& auth_token);
-
   bool InitialSyncEndedForAllEnabledTypes() {
     syncable::ScopedDirLookup lookup(dir_manager(), username_for_share());
     if (!lookup.good()) {
@@ -1069,6 +1052,22 @@ class SyncManager::SyncInternal
     return true;
   }
 
+ private:
+  // Try to authenticate using a LSID cookie.
+  void AuthenticateWithLsid(const std::string& lsid);
+
+  // Try to authenticate using persisted credentials from a previous successful
+  // authentication. If no such credentials exist, calls OnAuthError on the
+  // client to collect credentials. Otherwise, there exist local credentials
+  // that were once used for a successful auth, so we'll try to re-use these.
+  // Failure of that attempt will be communicated as normal using OnAuthError.
+  // Since this entry point will bypass normal GAIA authentication and try to
+  // authenticate directly with the sync service using a cached token,
+  // authentication failure will generally occur due to expired credentials, or
+  // possibly because of a password change.
+  bool AuthenticateForUser(const std::string& username,
+                           const std::string& auth_token);
+
   // Helper to call OnAuthError when no authentication credentials are
   // available.
   void RaiseAuthNeededEvent();
@@ -1269,6 +1268,10 @@ void SyncManager::Authenticate(const char* username, const char* password,
                       std::string(captcha));
 }
 
+bool SyncManager::InitialSyncEndedForAllEnabledTypes() {
+  return data_->InitialSyncEndedForAllEnabledTypes();
+}
+
 void SyncManager::StartSyncing() {
   data_->StartSyncing();
 }
@@ -1522,17 +1525,8 @@ bool SyncManager::SyncInternal::AuthenticateForUser(
   // We optimize by opening the directory before the "fresh" authentication
   // attempt completes so that we can immediately begin processing changes.
   if (!dir_manager()->Open(username_for_share())) {
-    // TODO(tim): Cue a refresh if the db was corrupt.
     if (observer_)
       observer_->OnStopSyncingPermanently();
-
-#if defined(OS_WIN)
-      UMA_HISTOGRAM_COUNTS_100("Sync.DirectoryOpenFailedWin", 1);
-#elif defined(OS_MACOSX)
-      UMA_HISTOGRAM_COUNTS_100("Sync.DirectoryOpenFailedMac", 1);
-#else
-      UMA_HISTOGRAM_COUNTS_100("Sync.DirectoryOpenFailedNotWinMac", 1);
-#endif
     return false;
   }
 
diff --git a/chrome/browser/sync/engine/syncapi.h b/chrome/browser/sync/engine/syncapi.h
index 5cf71bc..ec28386 100644
--- a/chrome/browser/sync/engine/syncapi.h
+++ b/chrome/browser/sync/engine/syncapi.h
@@ -754,6 +754,12 @@ class SyncManager {
   // Returns empty if there is no such username.
   const std::string& GetAuthenticatedUsername();
 
+  // Check if the database has been populated with a full "initial" download of
+  // sync items for each data type currently present in the routing info.
+  // Prerequisite for calling this is that OnInitializationComplete has been
+  // called.
+  bool InitialSyncEndedForAllEnabledTypes();
+
   // Submit credentials to GAIA for verification. On success, both |username|
   // and the obtained auth token are persisted on disk for future re-use.
   // If authentication fails, OnAuthProblem is called on our Observer.
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);
 };
 
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index 24855b2..c6fd726 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -294,6 +294,12 @@ void ProfileSyncService::InitializeBackend(bool delete_sync_data_folder) {
                        notification_method_);
 }
 
+void ProfileSyncService::CreateBackend() {
+  backend_.reset(
+      new SyncBackendHost(this, profile_, profile_->GetPath(),
+                          data_type_controllers_));
+}
+
 void ProfileSyncService::StartUp() {
   // Don't start up multiple times.
   if (backend_.get()) {
@@ -306,10 +312,7 @@ void ProfileSyncService::StartUp() {
   last_synced_time_ = base::Time::FromInternalValue(
       profile_->GetPrefs()->GetInt64(prefs::kSyncLastSyncedTime));
 
-  backend_.reset(
-      new SyncBackendHost(this, profile_, profile_->GetPath(),
-                          data_type_controllers_));
-
+  CreateBackend();
   // Initialize the backend.  Every time we start up a new SyncBackendHost,
   // we'll want to start from a fresh SyncDB, so delete any old one that might
   // be there.
@@ -540,14 +543,14 @@ void ProfileSyncService::ShowChooseDataTypes(gfx::NativeWindow parent_window) {
 }
 
 SyncBackendHost::StatusSummary ProfileSyncService::QuerySyncStatusSummary() {
-  if (backend_.get())
+  if (backend_.get() && backend_initialized_)
     return backend_->GetStatusSummary();
   else
     return SyncBackendHost::Status::OFFLINE_UNUSABLE;
 }
 
 SyncBackendHost::Status ProfileSyncService::QueryDetailedSyncStatus() {
-  if (backend_.get()) {
+  if (backend_.get() && backend_initialized_) {
     return backend_->GetDetailedStatus();
   } else {
     SyncBackendHost::Status status =
@@ -594,7 +597,7 @@ string16 ProfileSyncService::GetLastSyncedTimeString() const {
 }
 
 string16 ProfileSyncService::GetAuthenticatedUsername() const {
-  if (backend_.get())
+  if (backend_.get() && backend_initialized_)
     return backend_->GetAuthenticatedUsername();
   else
     return string16();
@@ -707,11 +710,12 @@ void ProfileSyncService::GetRegisteredDataTypes(
 }
 
 bool ProfileSyncService::IsCryptographerReady() const {
-  return backend_->GetUserShareHandle()->
-      dir_manager->cryptographer()->is_ready();
+  return backend_.get() && backend_initialized_ &&
+      backend_->GetUserShareHandle()->dir_manager->cryptographer()->is_ready();
 }
 
 void ProfileSyncService::SetPassphrase(const std::string& passphrase) {
+  DCHECK(backend_.get());
   backend_->SetPassphrase(passphrase);
 }
 
@@ -734,6 +738,7 @@ void ProfileSyncService::ActivateDataType(
     NOTREACHED();
     return;
   }
+  DCHECK(backend_initialized_);
   change_processor->Start(profile(), backend_->GetUserShareHandle());
   backend_->ActivateDataType(data_type_controller, change_processor);
 }
diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h
index c8c0833..d3f8e79 100644
--- a/chrome/browser/sync/profile_sync_service.h
+++ b/chrome/browser/sync/profile_sync_service.h
@@ -321,10 +321,9 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   void RegisterPreferences();
   void ClearPreferences();
 
-  // Tests need to override this.  If |delete_sync_data_folder| is true, then
-  // this method will delete all previous "Sync Data" folders. (useful if the
-  // folder is partial/corrupt)
-  virtual void InitializeBackend(bool delete_sync_data_folder);
+  // Test need to override this to create backends that allow setting up
+  // initial conditions, such as populating sync nodes.
+  virtual void CreateBackend();
 
   const browser_sync::DataTypeController::TypeMap& data_type_controllers() {
     return data_type_controllers_;
@@ -351,6 +350,10 @@ class ProfileSyncService : public browser_sync::SyncFrontend,
   FRIEND_TEST_ALL_PREFIXES(ProfileSyncServiceTest,
                            UnrecoverableErrorSuspendsService);
 
+  // If |delete_sync_data_folder| is true, then this method will delete all
+  // previous "Sync Data" folders. (useful if the folder is partial/corrupt).
+  void InitializeBackend(bool delete_sync_data_folder);
+
   // Initializes the various settings from the command line.
   void InitSettings();
 
diff --git a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc
index 75b19d1..e970457 100644
--- a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc
@@ -40,6 +40,7 @@ using base::Time;
 using browser_sync::AutofillChangeProcessor;
 using browser_sync::AutofillDataTypeController;
 using browser_sync::AutofillModelAssociator;
+using browser_sync::SyncBackendHostForProfileSyncTest;
 using syncable::WriteTransaction;
 using testing::_;
 using testing::DoAll;
@@ -143,6 +144,9 @@ class ProfileSyncServiceAutofillTest : public AbstractProfileSyncServiceTest {
                                          &profile_,
                                          service_.get());
 
+     SyncBackendHostForProfileSyncTest::
+         SetDefaultExpectationsForWorkerCreation(&profile_);
+
       EXPECT_CALL(factory_, CreateAutofillSyncComponents(_, _, _, _)).
           WillOnce(MakeAutofillSyncComponents(service_.get(),
                                               &web_database_,
diff --git a/chrome/browser/sync/profile_sync_service_password_unittest.cc b/chrome/browser/sync/profile_sync_service_password_unittest.cc
index bdc4037..337375a2 100644
--- a/chrome/browser/sync/profile_sync_service_password_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_password_unittest.cc
@@ -156,8 +156,16 @@ class ProfileSyncServicePasswordTest : public AbstractProfileSyncServiceTest {
       EXPECT_CALL(factory_, CreateDataTypeManager(_, _)).
           WillOnce(ReturnNewDataTypeManager());
 
+      // Creating model safe workers will request the history service and
+      // password store.  I couldn't manage to convince gmock that splitting up
+      // the expectations to match the class responsibilities was a good thing,
+      // so we set them all together here.
+      EXPECT_CALL(profile_, GetHistoryService(_)).
+         WillOnce(Return(static_cast<HistoryService*>(NULL)));
+
       EXPECT_CALL(profile_, GetPasswordStore(_)).
-          WillOnce(Return(password_store_.get()));
+          Times(2).
+          WillRepeatedly(Return(password_store_.get()));
 
       EXPECT_CALL(observer_,
           Observe(
diff --git a/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc b/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc
index 60a4c20..31c25b4 100644
--- a/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc
@@ -170,6 +170,9 @@ class ProfileSyncServiceTypedUrlTest : public AbstractProfileSyncServiceTest {
       EXPECT_CALL(profile_, GetHistoryServiceWithoutCreating()).
           WillRepeatedly(Return(history_service_.get()));
 
+      EXPECT_CALL(profile_, GetPasswordStore(_)).
+          WillOnce(Return(static_cast<PasswordStore*>(NULL)));
+
       EXPECT_CALL(profile_, GetHistoryService(_)).
           WillRepeatedly(Return(history_service_.get()));
 
diff --git a/chrome/browser/sync/profile_sync_service_unittest.cc b/chrome/browser/sync/profile_sync_service_unittest.cc
index ae4986a..11bc2e2 100644
--- a/chrome/browser/sync/profile_sync_service_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_unittest.cc
@@ -240,10 +240,15 @@ class ProfileSyncServiceTest : public testing::Test {
   }
 
   void StartSyncService() {
+    StartSyncServiceAndSetInitialSyncEnded(true);
+  }
+  void StartSyncServiceAndSetInitialSyncEnded(bool set_initial_sync_ended) {
     if (!service_.get()) {
       service_.reset(new TestProfileSyncService(&factory_,
                                                 profile_.get(),
                                                 false, false, NULL));
+      if (!set_initial_sync_ended)
+        service_->dont_set_initial_sync_ended_on_init();
 
       // Register the bookmark data type.
       model_associator_ = new TestBookmarkModelAssociator(service_.get(),
@@ -1305,8 +1310,9 @@ TEST_F(ProfileSyncServiceTestWithData, RecoverAfterDeletingSyncDataDirectory) {
   // Now pretend that the user has deleted this directory from the disk.
   file_util::Delete(sync_data_directory, true);
 
-  // Restart the sync service.
-  StartSyncService();
+  // Restart the sync service.  Don't fake out setting initial sync ended; lets
+  // make sure the system does in fact nudge and wait for this to happen.
+  StartSyncServiceAndSetInitialSyncEnded(false);
 
   // Make sure we're back in sync.  In real life, the user would need
   // to reauthenticate before this happens, but in the test, authentication
@@ -1336,6 +1342,8 @@ TEST_F(ProfileSyncServiceTestWithData, TestStartupWithOldSyncData) {
     service_.reset(
         new TestProfileSyncService(&factory_, profile_.get(),
                                    false, true, NULL));
+
+    service_->dont_set_initial_sync_ended_on_init();
     profile_->GetPrefs()->SetBoolean(prefs::kSyncHasSetupCompleted, false);
 
     model_associator_ = new TestBookmarkModelAssociator(service_.get(),
diff --git a/chrome/browser/sync/syncable/directory_backing_store.cc b/chrome/browser/sync/syncable/directory_backing_store.cc
index 8dc939cd..7f82a73 100644
--- a/chrome/browser/sync/syncable/directory_backing_store.cc
+++ b/chrome/browser/sync/syncable/directory_backing_store.cc
@@ -14,6 +14,7 @@
 
 #include "base/file_util.h"
 #include "base/hash_tables.h"
+#include "base/histogram.h"
 #include "base/logging.h"
 #include "base/stl_util-inl.h"
 #include "base/string_number_conversions.h"
@@ -283,11 +284,34 @@ bool DirectoryBackingStore::BeginLoad() {
   DCHECK(load_dbhandle_ == NULL);
   bool ret = OpenAndConfigureHandleHelper(&load_dbhandle_);
   if (ret)
-    return ret;
+    return true;
   // Something's gone wrong. Nuke the database and try again.
   LOG(ERROR) << "Sync database " << backing_filepath_.value()
-             << " corrupt.";
-  return false;
+             << " corrupt. Deleting and recreating.";
+  file_util::Delete(backing_filepath_, false);

+  bool failed_again = !OpenAndConfigureHandleHelper(&load_dbhandle_);
+
+  // Using failed_again here lets us distinguish from cases where corruption
+  // occurred even when re-opening a fresh directory (they'll go in a separate
+  // double weight histogram bucket).  Failing twice in a row means we disable
+  // sync, so it's useful to see this number separately.
+  int bucket = failed_again ? 2 : 1;
+#if defined(OS_WIN)
+  UMA_HISTOGRAM_COUNTS_100("Sync.DirectoryOpenFailedWin", bucket);
+#elif defined(OS_MACOSX)
+  UMA_HISTOGRAM_COUNTS_100("Sync.DirectoryOpenFailedMac", bucket);
+#else
+  UMA_HISTOGRAM_COUNTS_100("Sync.DirectoryOpenFailedNotWinMac", bucket);
+
+#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
+  UMA_HISTOGRAM_COUNTS_100("Sync.DirectoryOpenFailedLinux", bucket);
+#elif defined(OS_CHROMEOS)
+  UMA_HISTOGRAM_COUNTS_100("Sync.DirectoryOpenFailedCros", bucket);
+#else
+  UMA_HISTOGRAM_COUNTS_100("Sync.DirectoryOpenFailedOther", bucket);
+#endif  // OS_LINUX && !OS_CHROMEOS
+#endif  // OS_WIN
+  return !failed_again;
 }
 
 void DirectoryBackingStore::EndLoad() {
diff --git a/chrome/browser/sync/syncable/directory_backing_store_unittest.cc b/chrome/browser/sync/syncable/directory_backing_store_unittest.cc
index 7ec3d49..c158f9a 100644
--- a/chrome/browser/sync/syncable/directory_backing_store_unittest.cc
+++ b/chrome/browser/sync/syncable/directory_backing_store_unittest.cc
@@ -1019,8 +1019,7 @@ TEST_F(DirectoryBackingStoreTest, ModelTypeIds) {
   }
 }
 
-// TODO(tim): Disabled due to bug 48502.
-TEST_F(DirectoryBackingStoreTest, DISABLED_Corruption) {
+TEST_F(DirectoryBackingStoreTest, Corruption) {
   {
     scoped_ptr<DirectoryBackingStore> dbs(
         new DirectoryBackingStore(GetUsername(), GetDatabasePath()));
diff --git a/chrome/browser/sync/test_profile_sync_service.h b/chrome/browser/sync/test_profile_sync_service.h
index 9cb6973..aba7c42 100644
--- a/chrome/browser/sync/test_profile_sync_service.h
+++ b/chrome/browser/sync/test_profile_sync_service.h
@@ -10,15 +10,29 @@
 
 #include "base/message_loop.h"
 #include "chrome/browser/profile.h"
+#include "chrome/browser/sync/engine/syncapi.h"
 #include "chrome/browser/sync/profile_sync_factory.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/glue/data_type_controller.h"
 #include "chrome/browser/sync/glue/data_type_manager_impl.h"
 #include "chrome/browser/sync/glue/sync_backend_host.h"
+#include "chrome/browser/sync/sessions/session_state.h"
+#include "chrome/browser/sync/syncable/directory_manager.h"
+#include "chrome/browser/sync/syncable/syncable.h"
 #include "chrome/common/notification_service.h"
+#include "chrome/test/profile_mock.h"
 #include "chrome/test/sync/test_http_bridge_factory.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
+using browser_sync::ModelSafeRoutingInfo;
+using browser_sync::sessions::ErrorCounters;
+using browser_sync::sessions::SyncerStatus;
+using browser_sync::sessions::SyncSessionSnapshot;
+using sync_api::UserShare;
+using syncable::DirectoryManager;
+using syncable::ModelType;
+using syncable::ScopedDirLookup;
+
 ACTION_P(CallOnPaused, core) {
   core->OnPaused();
 };
@@ -31,22 +45,35 @@ ACTION(ReturnNewDataTypeManager) {
   return new browser_sync::DataTypeManagerImpl(arg0, arg1);
 }
 
+namespace browser_sync {
+
 // Mocks out the SyncerThread operations (Pause/Resume) since no thread is
 // running in these tests, and allows tests to provide a task on construction
 // to set up initial nodes to mock out an actual server initial sync
 // download.
-class SyncBackendHostForProfileSyncTest : public browser_sync::SyncBackendHost {
+class SyncBackendHostForProfileSyncTest : public SyncBackendHost {
  public:
-  SyncBackendHostForProfileSyncTest(browser_sync::SyncFrontend* frontend,
+  // |initial_condition_setup_task| can be used to populate nodes before the
+  //     OnBackendInitialized callback fires.
+  // |set_initial_sync_ended_on_init| determines whether we pretend that a full
+  //     initial download has occurred and set bits for enabled data types.  If
+  //     this is false, configuring data types will require a syncer nudge.
+  // |synchronous_init| causes initialization to block until the syncapi has
+  //     completed setting itself up and called us back.
+  SyncBackendHostForProfileSyncTest(SyncFrontend* frontend,
       Profile* profile,
       const FilePath& profile_path,
-      const browser_sync::DataTypeController::TypeMap& data_type_controllers,
+      const DataTypeController::TypeMap& data_type_controllers,
       Task* initial_condition_setup_task,
       int num_expected_resumes,
-      int num_expected_pauses)
+      int num_expected_pauses,
+      bool set_initial_sync_ended_on_init,
+      bool synchronous_init)
       : browser_sync::SyncBackendHost(frontend, profile, profile_path,
                                       data_type_controllers),
-        initial_condition_setup_task_(initial_condition_setup_task) {
+        initial_condition_setup_task_(initial_condition_setup_task),
+        set_initial_sync_ended_on_init_(set_initial_sync_ended_on_init),
+        synchronous_init_(synchronous_init) {
     // By default, the RequestPause and RequestResume methods will
     // send the confirmation notification and return true.
     ON_CALL(*this, RequestPause()).
@@ -55,25 +82,106 @@ class SyncBackendHostForProfileSyncTest : public browser_sync::SyncBackendHost {
     ON_CALL(*this, RequestResume()).
         WillByDefault(testing::DoAll(CallOnResumed(core_),
                                      testing::Return(true)));
+    ON_CALL(*this, RequestNudge()).WillByDefault(testing::Invoke(this,
+         &SyncBackendHostForProfileSyncTest::
+             SimulateSyncCycleCompletedInitialSyncEnded));
+
     EXPECT_CALL(*this, RequestPause()).Times(num_expected_pauses);
     EXPECT_CALL(*this, RequestResume()).Times(num_expected_resumes);
+    EXPECT_CALL(*this, RequestNudge()).
+        Times(set_initial_sync_ended_on_init ? 0 : 1);
   }
 
   MOCK_METHOD0(RequestPause, bool());
   MOCK_METHOD0(RequestResume, bool());
+  MOCK_METHOD0(RequestNudge, void());
+
+  void SetInitialSyncEndedForEnabledTypes() {
+    UserShare* user_share = core_->syncapi()->GetUserShare();
+    DirectoryManager* dir_manager = user_share->dir_manager.get();
+
+    ScopedDirLookup dir(dir_manager, user_share->authenticated_name);
+    if (!dir.good())
+      FAIL();
+
+    ModelSafeRoutingInfo enabled_types;
+    GetModelSafeRoutingInfo(&enabled_types);
+    for (ModelSafeRoutingInfo::const_iterator i = enabled_types.begin();
+         i != enabled_types.end(); ++i) {
+      dir->set_initial_sync_ended_for_type(i->first, true);
+    }
+  }
 
   virtual void HandleInitializationCompletedOnFrontendLoop() {
+    set_syncapi_initialized();  // Need to do this asap so task below works.
+
+    // Set up any nodes the test wants around before model association.
     if (initial_condition_setup_task_) {
       initial_condition_setup_task_->Run();
     }
 
+    // Pretend we downloaded initial updates and set initial sync ended bits
+    // if we were asked to.
+    if (set_initial_sync_ended_on_init_)
+      SetInitialSyncEndedForEnabledTypes();
+
     SyncBackendHost::HandleInitializationCompletedOnFrontendLoop();
   }
+
+  // Called when a nudge comes in.
+  void SimulateSyncCycleCompletedInitialSyncEnded() {
+    syncable::ModelTypeBitSet sync_ended;
+    ModelSafeRoutingInfo enabled_types;
+    GetModelSafeRoutingInfo(&enabled_types);
+    for (ModelSafeRoutingInfo::const_iterator i = enabled_types.begin();
+         i != enabled_types.end(); ++i) {
+      sync_ended.set(i->first);
+    }
+    core_->HandleSyncCycleCompletedOnFrontendLoop(new SyncSessionSnapshot(
+        SyncerStatus(), ErrorCounters(), 0, 0, false,
+        sync_ended, false, false, 0, 0, false));
+  }
+
+  virtual sync_api::HttpPostProviderFactory* MakeHttpBridgeFactory(
+      URLRequestContextGetter* getter) {
+    return new browser_sync::TestHttpBridgeFactory;
+  }
+
+  virtual void InitCore(const Core::DoInitializeOptions& options) {
+    std::wstring user = L"testuser";
+    core_loop()->PostTask(FROM_HERE,
+        NewRunnableMethod(core_.get(),
+        &SyncBackendHost::Core::DoInitializeForTest,
+        user,
+        options.http_bridge_factory,
+        options.auth_http_bridge_factory,
+        options.delete_sync_data_folder,
+        browser_sync::kDefaultNotificationMethod));
+
+    // TODO(akalin): Figure out a better way to do this.
+    if (synchronous_init_) {
+      // The SyncBackend posts a task to the current loop when
+      // initialization completes.
+      MessageLoop::current()->Run();
+    }
+  }
+
+  static void SetDefaultExpectationsForWorkerCreation(ProfileMock* profile) {
+    EXPECT_CALL(*profile, GetPasswordStore(testing::_)).
+        WillOnce(testing::Return((PasswordStore*)NULL));
+    EXPECT_CALL(*profile, GetHistoryService(testing::_)).
+        WillOnce(testing::Return((HistoryService*)NULL));
+  }
+
  private:
   Task* initial_condition_setup_task_;
+  bool set_initial_sync_ended_on_init_;
+  bool synchronous_init_;
 
 };
 
+}  // namespace browser_sync
+
 class TestProfileSyncService : public ProfileSyncService {
  public:
   TestProfileSyncService(ProfileSyncFactory* factory,
@@ -87,33 +195,21 @@ class TestProfileSyncService : public ProfileSyncService {
         synchronous_sync_configuration_(false),
         num_expected_resumes_(1),
         num_expected_pauses_(1),
-        initial_condition_setup_task_(initial_condition_setup_task) {
+        initial_condition_setup_task_(initial_condition_setup_task),
+        set_initial_sync_ended_on_init_(true) {
     RegisterPreferences();
     SetSyncSetupCompleted();
   }
   virtual ~TestProfileSyncService() { }
 
-  virtual void InitializeBackend(bool delete_sync_data_folder) {
-    browser_sync::TestHttpBridgeFactory* factory =
-        new browser_sync::TestHttpBridgeFactory();
-    browser_sync::TestHttpBridgeFactory* factory2 =
-        new browser_sync::TestHttpBridgeFactory();
-    backend_->Shutdown(false);
-    backend_.reset(new SyncBackendHostForProfileSyncTest(this, profile(),
+  virtual void CreateBackend() {
+    backend_.reset(new browser_sync::SyncBackendHostForProfileSyncTest(
+        this, profile(),
         profile()->GetPath(), data_type_controllers(),
         initial_condition_setup_task_.release(),
-        num_expected_resumes_, num_expected_pauses_));
-    backend()->InitializeForTestMode(L"testuser", factory, factory2,
-        delete_sync_data_folder, browser_sync::kDefaultNotificationMethod);
-    // TODO(akalin): Figure out a better way to do this.
-    if (synchronous_backend_initialization_) {
-      // The SyncBackend posts a task to the current loop when
-      // initialization completes.
-      MessageLoop::current()->Run();
-      // Initialization is synchronous for test mode, so we should be
-      // good to go.
-      DCHECK(sync_initialized());
-    }
+        num_expected_resumes_, num_expected_pauses_,
+        set_initial_sync_ended_on_init_,
+        synchronous_backend_initialization_));
   }
 
   virtual void OnBackendInitialized() {
@@ -140,7 +236,9 @@ class TestProfileSyncService : public ProfileSyncService {
   void set_num_expected_pauses(int num) {
     num_expected_pauses_ = num;
   }
-
+  void dont_set_initial_sync_ended_on_init() {
+    set_initial_sync_ended_on_init_ = false;
+  }
   void set_synchronous_sync_configuration() {
     synchronous_sync_configuration_ = true;
   }
@@ -162,6 +260,7 @@ class TestProfileSyncService : public ProfileSyncService {
   int num_expected_pauses_;
 
   scoped_ptr<Task> initial_condition_setup_task_;
+  bool set_initial_sync_ended_on_init_;
 };
 
 #endif  // CHROME_BROWSER_SYNC_TEST_PROFILE_SYNC_SERVICE_H_