Reintroduce deterministic synced bookmark creation IDs

The patch reintroduces the logic reverted in
51be6f11, now behind a feature toggle.

switches::kMergeBookmarksUsingGUIDs is reused because actually there's
a fundamental coupling (which was overlooked before and the reason for
crashes): using deterministic tmp IDs *requires* that the merge
algorithm is aware of possible ID collisions (GUID equality).

Bug: 978430,1004205
Change-Id: I7babdcebc2d1eae4433c93316abdcbd374ceba19
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1878148Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709149}

components/sync_bookmarks/bookmark_model_merger.cc

@@ -470,10 +470,10 @@ void BookmarkModelMerger::ProcessLocalCreation(
   const bookmarks::BookmarkNode* node = parent->children()[index].get();
   DCHECK(!FindMatchingRemoteUpdateByGUID(node));
   DCHECK(base::IsValidGUID(node->guid()));
-  // TODO(crbug.com/978430): Consider using |node->guid()| instead of generating
-  // a new random GUID. However, currently that can lead to crashes due to
-  // duplicate server IDs, see crbug.com/1004205.
-  const std::string sync_id = base::GenerateGUID();
+  const std::string sync_id =
+      base::FeatureList::IsEnabled(switches::kMergeBookmarksUsingGUIDs)
+          ? node->guid()
+          : base::GenerateGUID();
   const int64_t server_version = syncer::kUncommittedVersion;
   const base::Time creation_time = base::Time::Now();
   const std::string& suffix = syncer::GenerateSyncableBookmarkHash(

components/sync_bookmarks/bookmark_model_observer_impl.cc

@@ -12,6 +12,7 @@
 #include "components/bookmarks/browser/bookmark_node.h"
 #include "components/sync/base/hash_util.h"
 #include "components/sync/base/unique_position.h"
+#include "components/sync/driver/sync_driver_switches.h"
 #include "components/sync/engine/non_blocking_sync_common.h"
 #include "components/sync_bookmarks/bookmark_specifics_conversions.h"
 #include "components/sync_bookmarks/synced_bookmark_tracker.h"
@@ -95,10 +96,10 @@ void BookmarkModelObserverImpl::BookmarkNodeAdded(
   // Assign a temp server id for the entity. Will be overriden by the actual
   // server id upon receiving commit response.
   DCHECK(base::IsValidGUID(node->guid()));
-  // TODO(crbug.com/978430): Consider using |node->guid()| instead of generating
-  // a new random GUID. However, currently that can lead to crashes due to
-  // duplicate server IDs, see crbug.com/1004205.
-  const std::string sync_id = base::GenerateGUID();
+  const std::string sync_id =
+      base::FeatureList::IsEnabled(switches::kMergeBookmarksUsingGUIDs)
+          ? node->guid()
+          : base::GenerateGUID();
   const int64_t server_version = syncer::kUncommittedVersion;
   const base::Time creation_time = base::Time::Now();
   const sync_pb::UniquePosition unique_position =

components/sync_bookmarks/bookmark_model_observer_impl_unittest.cc

@@ -15,6 +15,7 @@
 #include "components/bookmarks/test/test_bookmark_client.h"
 #include "components/sync/base/time.h"
 #include "components/sync/base/unique_position.h"
+#include "components/sync/driver/sync_driver_switches.h"
 #include "components/sync_bookmarks/synced_bookmark_tracker.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -133,12 +134,10 @@ TEST_F(BookmarkModelObserverImplTest,
       bookmark_tracker()->GetEntitiesWithLocalChanges(kMaxEntries);
   ASSERT_THAT(local_changes.size(), 1U);
   EXPECT_THAT(local_changes[0]->bookmark_node(), Eq(bookmark_node));
-  // TODO(crbug.com/978430): Consider using |node->guid()| instead of generating
-  // new random GUIDs as the temporary server ID, and then reinstate the
-  // following expectation. However, currently that can lead to crashes due to
-  // duplicate server IDs, see crbug.com/1004205.
-  // EXPECT_THAT(local_changes[0]->metadata()->server_id(),
-  //             Eq(bookmark_node->guid()));
+  if (base::FeatureList::IsEnabled(switches::kMergeBookmarksUsingGUIDs)) {
+    EXPECT_THAT(local_changes[0]->metadata()->server_id(),
+                Eq(bookmark_node->guid()));
+  }
 }
 
 TEST_F(BookmarkModelObserverImplTest,