Make SyncManagerTests use in-memory sqlite

Add a flag to the SyncManager to allow it to initialize an in-memory
directory.  This is to be used only in tests.

This is an ugly change, but there's not much alternative available right
now.  We really ought to figure out a better way to test the SyncManager
that doesn't require cluttering the main class with flag variables.
That task is left to another day.

Using an in-memory database results in significant speedups.  The time
taken to run sync_unit_tests on my desktop went from 13s to 6s.  It
seems to cut 10-20s off the runtime of unit_test
--gtest_filter='*Sync*', too.

BUG=116328, 117836
TEST=


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

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

10 files changed, 70 insertions, 43 deletions

diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc
index de47c855..4aad465 100644
--- a/chrome/browser/sync/glue/sync_backend_host.cc
+++ b/chrome/browser/sync/glue/sync_backend_host.cc
@@ -366,7 +366,7 @@ void SyncBackendHost::Initialize(
       &sync_notifier_factory_,
       delete_sync_data_folder,
       sync_prefs_->GetEncryptionBootstrapToken(),
-      false,
+      sync_api::SyncManager::NON_TEST,
       unrecoverable_error_handler,
       report_unrecoverable_error_function));
 }
@@ -771,7 +771,7 @@ SyncBackendHost::DoInitializeOptions::DoInitializeOptions(
     sync_notifier::SyncNotifierFactory* sync_notifier_factory,
     bool delete_sync_data_folder,
     const std::string& restored_key_for_bootstrapping,
-    bool setup_for_test_mode,
+    sync_api::SyncManager::TestingMode testing_mode,
     UnrecoverableErrorHandler* unrecoverable_error_handler,
     ReportUnrecoverableErrorFunction report_unrecoverable_error_function)
     : sync_loop(sync_loop),
@@ -785,7 +785,7 @@ SyncBackendHost::DoInitializeOptions::DoInitializeOptions(
       sync_notifier_factory(sync_notifier_factory),
       delete_sync_data_folder(delete_sync_data_folder),
       restored_key_for_bootstrapping(restored_key_for_bootstrapping),
-      setup_for_test_mode(setup_for_test_mode),
+      testing_mode(testing_mode),
       unrecoverable_error_handler(unrecoverable_error_handler),
       report_unrecoverable_error_function(
           report_unrecoverable_error_function) {
@@ -1026,7 +1026,7 @@ void SyncBackendHost::Core::DoInitialize(const DoInitializeOptions& options) {
           options.chrome_sync_notification_bridge,
           options.sync_notifier_factory->CreateSyncNotifier()),
       options.restored_key_for_bootstrapping,
-      options.setup_for_test_mode,
+      options.testing_mode,
       &encryptor_,
       options.unrecoverable_error_handler,
       options.report_unrecoverable_error_function);
diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h
index 5d886d1..7c712e3 100644
--- a/chrome/browser/sync/glue/sync_backend_host.h
+++ b/chrome/browser/sync/glue/sync_backend_host.h
@@ -293,7 +293,7 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
         sync_notifier::SyncNotifierFactory* sync_notifier_factory,
         bool delete_sync_data_folder,
         const std::string& restored_key_for_bootstrapping,
-        bool setup_for_test_mode,
+        sync_api::SyncManager::TestingMode testing_mode,
         UnrecoverableErrorHandler* unrecoverable_error_handler,
         ReportUnrecoverableErrorFunction report_unrecoverable_error_function);
     ~DoInitializeOptions();
@@ -311,7 +311,7 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
     std::string lsid;
     bool delete_sync_data_folder;
     std::string restored_key_for_bootstrapping;
-    bool setup_for_test_mode;
+    sync_api::SyncManager::TestingMode testing_mode;
     UnrecoverableErrorHandler* unrecoverable_error_handler;
     ReportUnrecoverableErrorFunction report_unrecoverable_error_function;
   };
diff --git a/chrome/browser/sync/internal_api/sync_manager.cc b/chrome/browser/sync/internal_api/sync_manager.cc
index d2265f0..ebe072f 100644
--- a/chrome/browser/sync/internal_api/sync_manager.cc
+++ b/chrome/browser/sync/internal_api/sync_manager.cc
@@ -135,7 +135,7 @@ class SyncManager::SyncInternal
         registrar_(NULL),
         change_delegate_(NULL),
         initialized_(false),
-        setup_for_test_mode_(false),
+        testing_mode_(NON_TEST),
         observing_ip_address_changes_(false),
         encryptor_(NULL),
         unrecoverable_error_handler_(NULL),
@@ -192,7 +192,7 @@ class SyncManager::SyncInternal
             bool enable_sync_tabs_for_other_clients,
             sync_notifier::SyncNotifier* sync_notifier,
             const std::string& restored_key_for_bootstrapping,
-            bool setup_for_test_mode,
+            TestingMode testing_mode,
             Encryptor* encryptor,
             UnrecoverableErrorHandler* unrecoverable_error_handler,
             ReportUnrecoverableErrorFunction
@@ -579,11 +579,12 @@ class SyncManager::SyncInternal
   // Set to true once Init has been called.
   bool initialized_;
 
-  // True if the SyncManager should be running in test mode (no sync
-  // scheduler actually communicating with the server).
-  bool setup_for_test_mode_;
+  // Controls the disabling of certain SyncManager features.
+  // Can be used to disable communication with the server and the use of an
+  // on-disk file for maintaining syncer state.
+  // TODO(117836): Clean up implementation of SyncManager unit tests.
+  TestingMode testing_mode_;
 
-  // Whether we should respond to an IP address change notification.
   bool observing_ip_address_changes_;
 
   // Map used to store the notification info to be displayed in
@@ -738,7 +739,7 @@ bool SyncManager::Init(
     bool enable_sync_tabs_for_other_clients,
     sync_notifier::SyncNotifier* sync_notifier,
     const std::string& restored_key_for_bootstrapping,
-    bool setup_for_test_mode,
+    TestingMode testing_mode,
     Encryptor* encryptor,
     UnrecoverableErrorHandler* unrecoverable_error_handler,
     ReportUnrecoverableErrorFunction report_unrecoverable_error_function) {
@@ -761,7 +762,7 @@ bool SyncManager::Init(
                      enable_sync_tabs_for_other_clients,
                      sync_notifier,
                      restored_key_for_bootstrapping,
-                     setup_for_test_mode,
+                     testing_mode,
                      encryptor,
                      unrecoverable_error_handler,
                      report_unrecoverable_error_function);
@@ -886,7 +887,7 @@ bool SyncManager::SyncInternal::Init(
     bool enable_sync_tabs_for_other_clients,
     sync_notifier::SyncNotifier* sync_notifier,
     const std::string& restored_key_for_bootstrapping,
-    bool setup_for_test_mode,
+    TestingMode testing_mode,
     Encryptor* encryptor,
     UnrecoverableErrorHandler* unrecoverable_error_handler,
     ReportUnrecoverableErrorFunction report_unrecoverable_error_function) {
@@ -902,7 +903,7 @@ bool SyncManager::SyncInternal::Init(
 
   registrar_ = model_safe_worker_registrar;
   change_delegate_ = change_delegate;
-  setup_for_test_mode_ = setup_for_test_mode;
+  testing_mode_ = testing_mode;
 
   enable_sync_tabs_for_other_clients_ = enable_sync_tabs_for_other_clients;
 
@@ -933,7 +934,7 @@ bool SyncManager::SyncInternal::Init(
 
 
   // Test mode does not use a syncer context or syncer thread.
-  if (!setup_for_test_mode_) {
+  if (testing_mode_ == NON_TEST) {
     // Build a SyncSessionContext and store the worker in it.
     DVLOG(1) << "Sync is bringing up SyncSessionContext.";
     std::vector<SyncEngineEventListener*> listeners;
@@ -980,7 +981,7 @@ bool SyncManager::SyncInternal::Init(
                         MakeWeakHandle(weak_ptr_factory_.GetWeakPtr()),
                         signed_in));
 
-  if (!signed_in && !setup_for_test_mode_)
+  if (!signed_in && testing_mode_ == NON_TEST)
     return false;
 
   sync_notifier_->AddObserver(this);
@@ -1104,14 +1105,17 @@ bool SyncManager::SyncInternal::OpenDirectory() {
   // Set before Open().
   change_observer_ =
       browser_sync::MakeWeakHandle(js_mutation_event_observer_.AsWeakPtr());
+  WeakHandle<syncable::TransactionObserver> transaction_observer(
+      browser_sync::MakeWeakHandle(js_mutation_event_observer_.AsWeakPtr()));
 
-  syncable::DirOpenResult open_result =
-      directory()->Open(
-          database_path_,
-          username_for_share(),
-          this,
-          browser_sync::MakeWeakHandle(
-              js_mutation_event_observer_.AsWeakPtr()));
+  syncable::DirOpenResult open_result = syncable::NOT_INITIALIZED;
+  if (testing_mode_ == TEST_IN_MEMORY) {
+    open_result = directory()->OpenInMemoryForTest(
+        username_for_share(), this, transaction_observer);
+  } else {
+    open_result = directory()->Open(
+        database_path_, username_for_share(), this, transaction_observer);
+  }
   if (open_result != syncable::OPENED) {
     LOG(ERROR) << "Could not open share for:" << username_for_share();
     return false;
@@ -1160,7 +1164,7 @@ void SyncManager::SyncInternal::UpdateCredentials(
   if (connection_manager()->set_auth_token(credentials.sync_token)) {
     sync_notifier_->UpdateCredentials(
         credentials.email, credentials.sync_token);
-    if (!setup_for_test_mode_ && initialized_) {
+    if (testing_mode_ == NON_TEST && initialized_) {
       if (scheduler())
         scheduler()->OnCredentialsUpdated();
     }
@@ -1702,7 +1706,6 @@ void SyncManager::SyncInternal::ShutdownOnSyncThread() {
 
   share_.directory.reset();
 
-  setup_for_test_mode_ = false;
   change_delegate_ = NULL;
   registrar_ = NULL;
 
diff --git a/chrome/browser/sync/internal_api/sync_manager.h b/chrome/browser/sync/internal_api/sync_manager.h
index ecf9fc1..b60bd83 100644
--- a/chrome/browser/sync/internal_api/sync_manager.h
+++ b/chrome/browser/sync/internal_api/sync_manager.h
@@ -445,6 +445,12 @@ class SyncManager {
     virtual ~Observer();
   };
 
+  enum TestingMode {
+    NON_TEST,
+    TEST_ON_DISK,
+    TEST_IN_MEMORY,
+  };
+
   // Create an uninitialized SyncManager.  Callers must Init() before using.
   explicit SyncManager(const std::string& name);
   virtual ~SyncManager();
@@ -484,7 +490,7 @@ class SyncManager {
             bool enable_sync_tabs_for_other_clients,
             sync_notifier::SyncNotifier* sync_notifier,
             const std::string& restored_key_for_bootstrapping,
-            bool setup_for_test_mode,
+            TestingMode testing_mode,
             browser_sync::Encryptor* encryptor,
             browser_sync::UnrecoverableErrorHandler*
                 unrecoverable_error_handler,
diff --git a/chrome/browser/sync/internal_api/syncapi_unittest.cc b/chrome/browser/sync/internal_api/syncapi_unittest.cc
index d59d8ef..19f6bc3 100644
--- a/chrome/browser/sync/internal_api/syncapi_unittest.cc
+++ b/chrome/browser/sync/internal_api/syncapi_unittest.cc
@@ -763,7 +763,7 @@ class SyncManagerTest : public testing::Test,
                        credentials,
                        false /* enable_sync_tabs_for_other_clients */,
                        sync_notifier_mock_, "",
-                       true /* setup_for_test_mode */,
+                       sync_api::SyncManager::TEST_IN_MEMORY,
                        &encryptor_,
                        &handler_,
                        NULL);
diff --git a/chrome/browser/sync/profile_sync_service_unittest.cc b/chrome/browser/sync/profile_sync_service_unittest.cc
index 57b30b8..54bd7ac 100644
--- a/chrome/browser/sync/profile_sync_service_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_unittest.cc
@@ -83,7 +83,8 @@ class ProfileSyncServiceTest : public testing::Test {
   // TODO(akalin): Refactor the StartSyncService*() functions below.
 
   void StartSyncService() {
-    StartSyncServiceAndSetInitialSyncEnded(true, true, false, true, true);
+    StartSyncServiceAndSetInitialSyncEnded(
+        true, true, false, true, true, false);
   }
 
   void StartSyncServiceAndSetInitialSyncEnded(
@@ -91,7 +92,8 @@ class ProfileSyncServiceTest : public testing::Test {
       bool issue_auth_token,
       bool synchronous_sync_configuration,
       bool sync_setup_completed,
-      bool expect_create_dtm) {
+      bool expect_create_dtm,
+      bool use_real_database) {
     if (!service_.get()) {
       SigninManager* signin =
           SigninManagerFactory::GetForProfile(profile_.get());
@@ -109,6 +111,8 @@ class ProfileSyncServiceTest : public testing::Test {
         service_->dont_set_initial_sync_ended_on_init();
       if (synchronous_sync_configuration)
         service_->set_synchronous_sync_configuration();
+      if (use_real_database)
+        service_->set_use_real_database();
       if (!sync_setup_completed)
         profile_->GetPrefs()->SetBoolean(prefs::kSyncHasSetupCompleted, false);
 
@@ -250,7 +254,7 @@ TEST_F(ProfileSyncServiceTest, JsControllerHandlersBasic) {
 
 TEST_F(ProfileSyncServiceTest,
        JsControllerHandlersDelayedBackendInitialization) {
-  StartSyncServiceAndSetInitialSyncEnded(true, false, false, true, true);
+  StartSyncServiceAndSetInitialSyncEnded(true, false, false, true, true, false);
 
   StrictMock<MockJsEventHandler> event_handler;
   EXPECT_CALL(event_handler, HandleJsEvent(_, _)).Times(AtLeast(1));
@@ -291,7 +295,7 @@ TEST_F(ProfileSyncServiceTest, JsControllerProcessJsMessageBasic) {
 
 TEST_F(ProfileSyncServiceTest,
        JsControllerProcessJsMessageBasicDelayedBackendInitialization) {
-  StartSyncServiceAndSetInitialSyncEnded(true, false, false, true, true);
+  StartSyncServiceAndSetInitialSyncEnded(true, false, false, true, true, false);
 
   StrictMock<MockJsReplyHandler> reply_handler;
 
@@ -333,7 +337,7 @@ TEST_F(ProfileSyncServiceTest, TestStartupWithOldSyncData) {
   ASSERT_NE(-1,
             file_util::WriteFile(sync_file3, nonsense3, strlen(nonsense3)));
 
-  StartSyncServiceAndSetInitialSyncEnded(false, false, true, false, true);
+  StartSyncServiceAndSetInitialSyncEnded(false, false, true, false, true, true);
   EXPECT_FALSE(service_->HasSyncSetupCompleted());
   EXPECT_FALSE(service_->sync_initialized());
 
@@ -370,7 +374,7 @@ TEST_F(ProfileSyncServiceTest, DISABLED_CorruptDatabase) {
   // Initialize with HasSyncSetupCompleted() set to true and InitialSyncEnded
   // false.  This is to model the scenario that would result when opening the
   // sync database fails.
-  StartSyncServiceAndSetInitialSyncEnded(false, true, true, true, false);
+  StartSyncServiceAndSetInitialSyncEnded(false, true, true, true, false, true);
 
   // The backend is not ready.  Ensure the PSS knows this.
   EXPECT_FALSE(service_->sync_initialized());
diff --git a/chrome/browser/sync/test_profile_sync_service.cc b/chrome/browser/sync/test_profile_sync_service.cc
index 732c5fc..498ca98 100644
--- a/chrome/browser/sync/test_profile_sync_service.cc
+++ b/chrome/browser/sync/test_profile_sync_service.cc
@@ -33,11 +33,13 @@ SyncBackendHostForProfileSyncTest::SyncBackendHostForProfileSyncTest(
     const base::WeakPtr<SyncPrefs>& sync_prefs,
     bool set_initial_sync_ended_on_init,
     bool synchronous_init,
-    bool fail_initial_download)
+    bool fail_initial_download,
+    bool use_real_database)
     : browser_sync::SyncBackendHost(
         profile->GetDebugName(), profile, sync_prefs),
       synchronous_init_(synchronous_init),
-      fail_initial_download_(fail_initial_download) {}
+      fail_initial_download_(fail_initial_download),
+      use_real_database_(use_real_database) {}
 
 SyncBackendHostForProfileSyncTest::~SyncBackendHostForProfileSyncTest() {}
 
@@ -70,7 +72,9 @@ void SyncBackendHostForProfileSyncTest::InitCore(
   test_options.credentials.email = "testuser@gmail.com";
   test_options.credentials.sync_token = "token";
   test_options.restored_key_for_bootstrapping = "";
-  test_options.setup_for_test_mode = true;
+  test_options.testing_mode =
+      use_real_database_ ? sync_api::SyncManager::TEST_ON_DISK
+                         : sync_api::SyncManager::TEST_IN_MEMORY;
   SyncBackendHost::InitCore(test_options);
   // TODO(akalin): Figure out a better way to do this.
   if (synchronous_init_) {
@@ -130,7 +134,8 @@ TestProfileSyncService::TestProfileSyncService(
       synchronous_sync_configuration_(false),
       callback_(callback),
       set_initial_sync_ended_on_init_(true),
-      fail_initial_download_(false) {
+      fail_initial_download_(false),
+      use_real_database_(false) {
   SetSyncSetupCompleted();
 }
 
@@ -212,6 +217,9 @@ void TestProfileSyncService::set_synchronous_sync_configuration() {
 void TestProfileSyncService::fail_initial_download() {
   fail_initial_download_ = true;
 }
+void TestProfileSyncService::set_use_real_database() {
+  use_real_database_ = true;
+}
 
 void TestProfileSyncService::CreateBackend() {
   backend_.reset(new browser_sync::SyncBackendHostForProfileSyncTest(
@@ -219,5 +227,6 @@ void TestProfileSyncService::CreateBackend() {
       sync_prefs_.AsWeakPtr(),
       set_initial_sync_ended_on_init_,
       synchronous_backend_initialization_,
-      fail_initial_download_));
+      fail_initial_download_,
+      use_real_database_));
 }
diff --git a/chrome/browser/sync/test_profile_sync_service.h b/chrome/browser/sync/test_profile_sync_service.h
index 6c09a3b..6869486 100644
--- a/chrome/browser/sync/test_profile_sync_service.h
+++ b/chrome/browser/sync/test_profile_sync_service.h
@@ -36,7 +36,8 @@ class SyncBackendHostForProfileSyncTest : public SyncBackendHost {
       const base::WeakPtr<SyncPrefs>& sync_prefs,
       bool set_initial_sync_ended_on_init,
       bool synchronous_init,
-      bool fail_initial_download);
+      bool fail_initial_download,
+      bool use_real_database);
   virtual ~SyncBackendHostForProfileSyncTest();
 
   MOCK_METHOD1(RequestNudge, void(const tracked_objects::Location&));
@@ -55,6 +56,7 @@ class SyncBackendHostForProfileSyncTest : public SyncBackendHost {
  private:
   bool synchronous_init_;
   bool fail_initial_download_;
+  bool use_real_database_;
 };
 
 }  // namespace browser_sync
@@ -90,6 +92,7 @@ class TestProfileSyncService : public ProfileSyncService {
   void set_synchronous_sync_configuration();
 
   void fail_initial_download();
+  void set_use_real_database();
 
   browser_sync::TestIdFactory* id_factory();
 
@@ -116,6 +119,7 @@ class TestProfileSyncService : public ProfileSyncService {
   bool set_initial_sync_ended_on_init_;
 
   bool fail_initial_download_;
+  bool use_real_database_;
 };
 
 
diff --git a/sync/syncable/dir_open_result.h b/sync/syncable/dir_open_result.h
index 6138a26..f298ff0 100644
--- a/sync/syncable/dir_open_result.h
+++ b/sync/syncable/dir_open_result.h
@@ -7,7 +7,8 @@
 #pragma once
 
 namespace syncable {
-enum DirOpenResult { OPENED,   // success.
+enum DirOpenResult { NOT_INITIALIZED,
+                     OPENED,   // success.
                      FAILED_NEWER_VERSION,  // DB version is too new.
                      FAILED_MAKE_REPOSITORY,  // Couldn't create subdir.
                      FAILED_OPEN_DATABASE,  // sqlite_open() failed.
diff --git a/sync/syncable/directory_backing_store.cc b/sync/syncable/directory_backing_store.cc
index c7395c3..1685610 100644
--- a/sync/syncable/directory_backing_store.cc
+++ b/sync/syncable/directory_backing_store.cc
@@ -326,7 +326,7 @@ bool DirectoryBackingStore::InitializeTables() {
   // crbug.com/105018.
   if (version_on_disk != kCurrentDBVersion) {
     if (version_on_disk > kCurrentDBVersion)
-      return FAILED_NEWER_VERSION;
+      return false;
 
     // Fallback (re-sync everything) migration path.
     DVLOG(1) << "Old/null sync database, version " << version_on_disk;