Two fixes regarding to bookmark backup

* Only change parent id when normalizing entry. Use PutParentIdPropertyOnly() instead of PutParentId() because the latter resets predecessor and changes ordering.
* Don't create sync node for mobile bookmarks, which only exists with sync enabled.

BUG=385731

Review URL: https://codereview.chromium.org/374183003

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

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

@@ -24,6 +24,12 @@ using bookmarks_helper::Move;
 using bookmarks_helper::Remove;
 using sync_integration_test_util::AwaitCommitActivityCompletion;
 
+namespace {
+const char kUrl1[] = "http://www.google.com";
+const char kUrl2[] = "http://map.google.com";
+const char kUrl3[] = "http://plus.google.com";
+}  // anonymous namespace
+
 class SingleClientBackupRollbackTest : public SyncTest {
  public:
   SingleClientBackupRollbackTest() : SyncTest(SINGLE_CLIENT) {}
@@ -222,10 +228,6 @@ 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);
@@ -306,3 +308,33 @@ IN_PROC_BROWSER_TEST_F(SingleClientBackupRollbackTest,
   ASSERT_EQ(GURL("http://www.nhl.com"),
             GetOtherNode(0)->GetChild(0)->url());
 }
+
+#if defined(ENABLE_PRE_SYNC_BACKUP)
+#define MAYBE_DontChangeBookmarkOrdering DontChangeBookmarkOrdering
+#else
+#define MAYBE_DontChangeBookmarkOrdering DISABLED_DontChangeBookmarkOrdering
+#endif
+IN_PROC_BROWSER_TEST_F(SingleClientBackupRollbackTest,
+                       MAYBE_DontChangeBookmarkOrdering) {
+  ASSERT_TRUE(SetupClients()) << "SetupClients() failed.";
+
+  const BookmarkNode* sub_folder = AddFolder(0, GetOtherNode(0), 0, "test");
+  ASSERT_TRUE(AddURL(0, sub_folder, 0, "", GURL(kUrl1)));
+  ASSERT_TRUE(AddURL(0, sub_folder, 1, "", GURL(kUrl2)));
+  ASSERT_TRUE(AddURL(0, sub_folder, 2, "", GURL(kUrl3)));
+
+  BackupModeChecker checker(GetSyncService(0),
+                            base::TimeDelta::FromSeconds(15));
+  ASSERT_TRUE(checker.Wait());
+
+  // Restart backup.
+  GetSyncService(0)->StartStopBackupForTesting();
+  GetSyncService(0)->StartStopBackupForTesting();
+  ASSERT_TRUE(checker.Wait());
+
+  // Verify bookmarks are unchanged.
+  ASSERT_EQ(3, sub_folder->child_count());
+  ASSERT_EQ(GURL(kUrl1), sub_folder->GetChild(0)->url());
+  ASSERT_EQ(GURL(kUrl2), sub_folder->GetChild(1)->url());
+  ASSERT_EQ(GURL(kUrl3), sub_folder->GetChild(2)->url());
+}

sync/internal_api/sync_backup_manager.cc

@@ -113,7 +113,7 @@ void SyncBackupManager::NormalizeEntries() {
     if (!entry.GetId().ServerKnows())
       entry.PutId(syncable::Id::CreateFromServerId(entry.GetId().value()));
     if (!entry.GetParentId().ServerKnows()) {
-      entry.PutParentId(syncable::Id::CreateFromServerId(
+      entry.PutParentIdPropertyOnly(syncable::Id::CreateFromServerId(
           entry.GetParentId().value()));
     }
     entry.PutBaseVersion(1);

sync/internal_api/sync_rollback_manager_base.cc

@@ -15,8 +15,8 @@
 namespace {
 
 // Permanent bookmark folders as defined in bookmark_model_associator.cc.
+// No mobile bookmarks because they only exists with sync enabled.
 const char kBookmarkBarTag[] = "bookmark_bar";
-const char kMobileBookmarksTag[] = "synced_bookmarks";
 const char kOtherBookmarksTag[] = "other_bookmarks";
 
 class DummyEntryptionHandler : public syncer::SyncEncryptionHandler {
@@ -120,7 +120,6 @@ void SyncRollbackManagerBase::ConfigureSyncer(
     if (InitTypeRootNode(type.Get())) {
       if (type.Get() == BOOKMARKS) {
         InitBookmarkFolder(kBookmarkBarTag);
-        InitBookmarkFolder(kMobileBookmarksTag);
         InitBookmarkFolder(kOtherBookmarksTag);
       }
     }

sync/internal_api/sync_rollback_manager_base_unittest.cc

@@ -58,7 +58,7 @@ TEST_F(SyncRollbackManagerBaseTest, InitTypeOnConfiguration) {
   EXPECT_EQ(BaseNode::INIT_OK,
             bookmark_bar.InitByTagLookupForBookmarks("bookmark_bar"));
   ReadNode bookmark_mobile(&trans);
-  EXPECT_EQ(BaseNode::INIT_OK,
+  EXPECT_EQ(BaseNode::INIT_FAILED_ENTRY_NOT_GOOD,
             bookmark_mobile.InitByTagLookupForBookmarks("synced_bookmarks"));
   ReadNode bookmark_other(&trans);
   EXPECT_EQ(BaseNode::INIT_OK,