Revert 142563 - Handle sync database corruption with re-download

A recent change allowed us to handle a missing sync database by creating
a new directory and repopulating it by requesting an initial sync from
the server.

This commit builds on that work by intentionally deleting the sync
database if it is found to be corrupt or unusable.  The client will then
recognize that the database is empty and repopulate it, just as it would
had the directory not existed in the first place.

BUG=109668
TEST=Manual.  Three cases, see below:
- Delete 'Sync Data/SyncData.sqlite3' from the profile directory.  Start
  chromium and observe that all data is redownloaded from the server.
- Corrupt an existing sync directory.  The SQL command
  'DELETE FROM metas WHERE metahandle = 1' should be sufficient.  Start
  chromium and observe that the directory is deleted then recreated
  and all data is redownloaded from the server.
- Ensure that no directory can be created.  You could do this by marking
  the 'Sync Data' folder as read-only.  Start chrome, and observe that
  sync is unusable, but the browser still works.
  NOTE: This only works in release builds, debug builds will DCHECK.
  It's debatable whether or not this scenario is worth testing.


Review URL: https://chromiumcodereview.appspot.com/10554016

TBR=rlarocque@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10578003

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

2 files changed, 25 insertions, 17 deletions

diff --git a/sync/internal_api/sync_manager.cc b/sync/internal_api/sync_manager.cc
index e9d3434..8c95292 100644
--- a/sync/internal_api/sync_manager.cc
+++ b/sync/internal_api/sync_manager.cc
@@ -11,7 +11,6 @@
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
-#include "base/file_util.h"
 #include "base/json/json_writer.h"
 #include "base/memory/ref_counted.h"
 #include "base/metrics/histogram.h"
@@ -905,6 +904,10 @@ bool SyncManager::SyncInternal::Init(
   encryptor_ = encryptor;
   unrecoverable_error_handler_ = unrecoverable_error_handler;
   report_unrecoverable_error_function_ = report_unrecoverable_error_function;
+  share_.directory.reset(
+      new syncable::Directory(encryptor_,
+                              unrecoverable_error_handler_,
+                              report_unrecoverable_error_function_));
 
   connection_manager_.reset(new SyncAPIServerConnectionManager(
       sync_server_and_path, port, use_ssl, user_agent, post_factory));
@@ -914,8 +917,6 @@ bool SyncManager::SyncInternal::Init(
 
   connection_manager()->AddListener(this);
 
-  bool signed_in = SignIn(credentials);
-
   // Test mode does not use a syncer context or syncer thread.
   if (testing_mode_ == NON_TEST) {
     // Build a SyncSessionContext and store the worker in it.
@@ -937,6 +938,8 @@ bool SyncManager::SyncInternal::Init(
     scheduler_.reset(new SyncScheduler(name_, session_context(), new Syncer()));
   }
 
+  bool signed_in = SignIn(credentials);
+
   if (signed_in) {
     if (scheduler()) {
       scheduler()->Start(SyncScheduler::CONFIGURATION_MODE);
@@ -1125,10 +1128,6 @@ void SyncManager::SyncInternal::StartSyncingNormally(
 
 bool SyncManager::SyncInternal::OpenDirectory() {
   DCHECK(!initialized_) << "Should only happen once";
-  share_.directory.reset(
-      new syncable::Directory(encryptor_,
-                              unrecoverable_error_handler_,
-                              report_unrecoverable_error_function_));
 
   // Set before Open().
   change_observer_ =
@@ -1143,16 +1142,6 @@ bool SyncManager::SyncInternal::OpenDirectory() {
   } else {
     open_result = directory()->Open(
         database_path_, username_for_share(), this, transaction_observer);
-    // If at first we don't succeed, delete the DB and try again.
-    if (open_result != syncable::OPENED) {
-      file_util::Delete(database_path_, false);
-      share_.directory.reset(
-          new syncable::Directory(encryptor_,
-                                  unrecoverable_error_handler_,
-                                  report_unrecoverable_error_function_));
-      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();
diff --git a/sync/syncable/directory_backing_store_unittest.cc b/sync/syncable/directory_backing_store_unittest.cc
index 65dd1e8..5161817 100644
--- a/sync/syncable/directory_backing_store_unittest.cc
+++ b/sync/syncable/directory_backing_store_unittest.cc
@@ -2084,6 +2084,25 @@ TEST_F(DirectoryBackingStoreTest, ModelTypeIds) {
   }
 }
 
+// TODO(109668): This had to be disabled because the latest API will
+// intentionally crash when a database is this badly corrupted.
+TEST_F(DirectoryBackingStoreTest, DISABLED_Corruption) {
+  {
+    scoped_ptr<OnDiskDirectoryBackingStore> dbs(
+        new OnDiskDirectoryBackingStore(GetUsername(), GetDatabasePath()));
+    EXPECT_TRUE(LoadAndIgnoreReturnedData(dbs.get()));
+  }
+  std::string bad_data("BAD DATA");
+  EXPECT_TRUE(file_util::WriteFile(GetDatabasePath(), bad_data.data(),
+                                   bad_data.size()));
+  {
+    scoped_ptr<OnDiskDirectoryBackingStore> dbs(
+        new OnDiskDirectoryBackingStore(GetUsername(), GetDatabasePath()));
+
+    EXPECT_FALSE(LoadAndIgnoreReturnedData(dbs.get()));
+  }
+}
+
 TEST_F(DirectoryBackingStoreTest, DeleteEntries) {
   sql::Connection connection;
   ASSERT_TRUE(connection.OpenInMemory());