Add more integration tests for backup/rollback.

chrome/browser/sync/test/integration/single_client_backup_rollback_test.cc

 // Copyright 2014 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 "base/command_line.h"
 #include "base/message_loop/message_loop.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/test/integration/bookmarks_helper.h"
@@ skipping 11 matching lines @@
 using bookmarks_helper::ModelMatchesVerifier;
 using bookmarks_helper::Move;
 using bookmarks_helper::Remove;
 using sync_integration_test_util::AwaitCommitActivityCompletion;
 
 class SingleClientBackupRollbackTest : public SyncTest {
  public:
   SingleClientBackupRollbackTest() : SyncTest(SINGLE_CLIENT) {}
   virtual ~SingleClientBackupRollbackTest() {}
 
-  virtual void SetUp() OVERRIDE {
+  void DisableBackup() {
+    CommandLine::ForCurrentProcess()->AppendSwitch(
+          switches::kSyncDisableBackup);
+  }
+
+  void EnableRollback() {
     CommandLine::ForCurrentProcess()->AppendSwitch(
           switches::kSyncEnableRollback);
-    SyncTest::SetUp();
   }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(SingleClientBackupRollbackTest);
 };
 
 class BackupModeChecker {
  public:
   explicit BackupModeChecker(ProfileSyncService* service,
                              base::TimeDelta timeout)
@@ skipping 26 matching lines @@
     return pss_->backend_mode() == ProfileSyncService::BACKUP &&
         pss_->ShouldPushChanges();
   }
 
   ProfileSyncService* pss_;
   base::TimeDelta timeout_;
   base::TimeTicks expiration_;
 };
 
 #if defined(ENABLE_PRE_SYNC_BACKUP)
+#define MAYBE_TestBackupDisabled TestBackupDisabled
+#else
+#define MAYBE_TestBackupDisabled DISABLED_TestBackupDisabled
+#endif
+IN_PROC_BROWSER_TEST_F(SingleClientBackupRollbackTest,
+                       MAYBE_TestBackupDisabled) {
+  DisableBackup();
+
+  ASSERT_TRUE(SetupClients()) << "SetupClients() failed.";
+
+  BackupModeChecker checker(GetSyncService(0),
+                            base::TimeDelta::FromSeconds(15));
+  ASSERT_FALSE(checker.Wait());

[pval...(no longer on Chromium), 2014/06/28 00:12:00]

does Wait() have to wait the entire timeout to eventually return false?

[haitaol1, 2014/07/01 16:49:36]

Unfortunately yes because backup controller won't do anything if disabled.

On 2014/06/28 00:12:00, pvalenzuela wrote:

+
+  ASSERT_EQ(ProfileSyncService::IDLE, GetSyncService(0)->backend_mode());
+}
+
+#if defined(ENABLE_PRE_SYNC_BACKUP)
+#define MAYBE_TestBackupOnly TestBackupOnly
+#else
+#define MAYBE_TestBackupOnly DISABLED_TestBackupOnly
+#endif
+IN_PROC_BROWSER_TEST_F(SingleClientBackupRollbackTest,
+                       MAYBE_TestBackupOnly) {
+  ASSERT_TRUE(SetupClients()) << "SetupClients() failed.";
+
+  // Starting state:
+  // other_node
+  //    -> http://mail.google.com  "url0"
+  //    -> http://www.nhl.com "url1"
+  ASSERT_TRUE(AddURL(0, GetOtherNode(0), 0, "url0",
+                     GURL("http://mail.google.com")));
+  ASSERT_TRUE(AddURL(0, GetOtherNode(0), 1, "url1",
+                     GURL("http://www.nhl.com")));
+
+  BackupModeChecker checker(GetSyncService(0),
+                            base::TimeDelta::FromSeconds(15));
+  ASSERT_TRUE(checker.Wait());
+
+  // Setup sync, wait for its completion, and make sure changes were synced.
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_TRUE(AwaitCommitActivityCompletion(GetSyncService((0))));
+  ASSERT_TRUE(ModelMatchesVerifier(0));
+
+  // Made bookmark changes while sync is on.
+  Remove(0, GetOtherNode(0), 1);
+  ASSERT_TRUE(AddURL(0, GetOtherNode(0), 1, "url2",
+                     GURL("http://www.yahoo.com")));
+  ASSERT_TRUE(AwaitCommitActivityCompletion(GetSyncService((0))));
+  ASSERT_TRUE(ModelMatchesVerifier(0));
+
+  // Let server to return rollback command on next sync request.
+  GetFakeServer()->TriggerError(sync_pb::SyncEnums::USER_ROLLBACK);
+
+  // Make another change to trigger downloading of rollback command.
+  Remove(0, GetOtherNode(0), 0);

[pval...(no longer on Chromium), 2014/06/28 00:12:00]

Is there another way we can trigger a sync that wouldn't mess with the bookmark
model?

I'm thinking that we should maybe trigger an invalidation on calls to
TriggerError (this would be fairly easy within FakeServer). Thoughts?

[haitaol1, 2014/07/01 16:49:36]

Can you point me to a sample to trigger invalidation?

On 2014/06/28 00:12:00, pvalenzuela wrote:

[pval...(no longer on Chromium), 2014/07/02 18:10:59]

Let's save it for another CL. Add a TODO here (in my name) to trigger an
invaldiation as part of the rollback triggering.

+
+  // Wait for sync to switch to backup mode after finishing rollback.

[pval...(no longer on Chromium), 2014/06/28 00:12:00]

Update comment to reflect that a rollback isn't actually happening

[haitaol1, 2014/07/01 16:49:36]

Done.

+  ASSERT_TRUE(checker.Wait());
+
+  // With rollback disabled, bookmarks in backup DB should not be restored.
+  // Only bookmark added during sync is present.
+  ASSERT_EQ(1, GetOtherNode(0)->child_count());
+  ASSERT_EQ(GURL("http://www.yahoo.com"),
+            GetOtherNode(0)->GetChild(0)->url());
+}
+
+#if defined(ENABLE_PRE_SYNC_BACKUP)
 #define MAYBE_TestBackupRollback TestBackupRollback
 #else
 #define MAYBE_TestBackupRollback DISABLED_TestBackupRollback
 #endif
 IN_PROC_BROWSER_TEST_F(SingleClientBackupRollbackTest,
                        MAYBE_TestBackupRollback) {
+  EnableRollback();
+
   ASSERT_TRUE(SetupClients()) << "SetupClients() failed.";
 
   // Starting state:
   // other_node
   //    -> top
   //      -> tier1_a
   //        -> http://mail.google.com  "tier1_a_url0"
   //      -> tier1_b
   //        -> http://www.nhl.com "tier1_b_url0"
   const BookmarkNode* top = AddFolder(0, GetOtherNode(0), 0, "top");
@@ skipping 41 matching lines @@
 
 #if defined(ENABLE_PRE_SYNC_BACKUP)
 #define MAYBE_TestPrefBackupRollback TestPrefBackupRollback
 #else
 #define MAYBE_TestPrefBackupRollback DISABLED_TestPrefBackupRollback
 #endif
 // Verify local preferences are not affected by preferences in backup DB under
 // backup mode.
 IN_PROC_BROWSER_TEST_F(SingleClientBackupRollbackTest,
                        MAYBE_TestPrefBackupRollback) {
+  EnableRollback();
+
   const char kUrl1[] = "http://www.google.com";
   const char kUrl2[] = "http://map.google.com";
   const char kUrl3[] = "http://plus.google.com";
 
   ASSERT_TRUE(SetupClients()) << "SetupClients() failed.";
 
   preferences_helper::ChangeStringPref(0, prefs::kHomePage, kUrl1);
 
   BackupModeChecker checker(GetSyncService(0),
                             base::TimeDelta::FromSeconds(15));
@@ skipping 21 matching lines @@
   // Make another change to trigger downloading of rollback command.
   preferences_helper::ChangeStringPref(0, prefs::kHomePage, "");
 
   // Wait for sync to switch to backup mode after finishing rollback.
   ASSERT_TRUE(checker.Wait());
 
   // Verify preference is restored.
   ASSERT_EQ(kUrl2,
             preferences_helper::GetPrefs(0)->GetString(prefs::kHomePage));
 }
+
+#if defined(ENABLE_PRE_SYNC_BACKUP)
+#define MAYBE_RollbackNoBackup RollbackNoBackup
+#else
+#define MAYBE_RollbackNoBackup DISABLED_RollbackNoBackup
+#endif
+IN_PROC_BROWSER_TEST_F(SingleClientBackupRollbackTest,
+                       MAYBE_RollbackNoBackup) {
+  EnableRollback();
+
+  ASSERT_TRUE(SetupClients()) << "SetupClients() failed.";
+
+  // Setup sync, wait for its completion, and make sure changes were synced.
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+
+  // Starting state:
+  // other_node
+  //    -> http://mail.google.com  "url0"
+  //    -> http://www.nhl.com "url1"
+  ASSERT_TRUE(AddURL(0, GetOtherNode(0), 0, "url0",
+                     GURL("http://mail.google.com")));
+  ASSERT_TRUE(AddURL(0, GetOtherNode(0), 1, "url1",
+                     GURL("http://www.nhl.com")));
+
+  ASSERT_TRUE(AwaitCommitActivityCompletion(GetSyncService((0))));
+  ASSERT_TRUE(ModelMatchesVerifier(0));
+
+  // Let server to return rollback command on next sync request.
+  GetFakeServer()->TriggerError(sync_pb::SyncEnums::USER_ROLLBACK);
+
+  // Make another change to trigger downloading of rollback command.
+  Remove(0, GetOtherNode(0), 0);
+
+  // Wait for sync to switch to backup mode after finishing rollback.
+  BackupModeChecker checker(GetSyncService(0),
+                            base::TimeDelta::FromSeconds(15));
+  ASSERT_TRUE(checker.Wait());
+
+  // Without backup DB, bookmarks added during sync shouldn't be removed.
+  ASSERT_EQ(1, GetOtherNode(0)->child_count());
+  ASSERT_EQ(GURL("http://www.nhl.com"),
+            GetOtherNode(0)->GetChild(0)->url());
+}