Reland "Enforce lower-case use of GUIDs for sync"

This reverts commit 7a6d0761.

Reason for revert: the underlying issue that caused the revert
was addressed in crrev.com/c/2062328 by lowercasing GUIDs that are
populated via sync's originator client item ID.

Bug: 1052789

Original change's description:
> Revert "Enforce lower-case use of GUIDs for sync"
>
> This reverts commit b6a3c9f3.
>
> Reason for revert: bookmarks created around 2016, between [M44..M51]
> use an uppercase GUID as originator client item ID.
>
> Original change's description:
> > Enforce lower-case use of GUIDs for sync
> >
> > base::GenerateGUID() produces lower-case GUIDs and they should get
> > exposed to the sync protocol exclusively in this canonical form to avoid
> > running into issues.
> >
> > This patch replaces all occurrences in sync codebase with:
> > git grep -l 'IsValidGUID' -- components/sync* | \
> >   xargs sed -i 's/IsValidGUID/IsValidGUIDOutputString/g'
> >
> > In addition, the implicit GUIDs used for bookmark sync, in particular
> > for permanent nodes, are updated to adopt lower-case GUIDs. These are
> > never committed to the server so there are no backward-compatibility
> > considerations.
> >
> > Change-Id: I0d0468f3475e1a27a161e2af15853c757f53e201
> > Bug: 978430
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2054154
> > Reviewed-by: Marc Treib <treib@chromium.org>
> > Reviewed-by: Scott Violet <sky@chromium.org>
> > Commit-Queue: Mikel Astiz <mastiz@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#741381}
>
> TBR=sky@chromium.org,treib@chromium.org,mastiz@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: 978430
> Change-Id: If3a135e24cb61d15cfc7616dd1160e76ce9dd3e3
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2061749
> Reviewed-by: Mikel Astiz <mastiz@chromium.org>
> Commit-Queue: Mikel Astiz <mastiz@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#742118}

TBR=sky@chromium.org,treib@chromium.org,mastiz@chromium.org

Bug: 978430
Change-Id: I9bb6813c5b3edba7b6204e515475d6e644212c1c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2064931Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743164}

components/sync/engine_impl/bookmark_update_preprocessing.cc

@@ -100,7 +100,7 @@ std::string InferGuidForLegacyBookmark(
   static_assert(base::kSHA1Length >= 16, "16 bytes needed to infer GUID");
 
   const std::string guid = ComputeGuidFromBytes(base::make_span(hash));
-  DCHECK(base::IsValidGUID(guid));
+  DCHECK(base::IsValidGUIDOutputString(guid));
   return guid;
 }
 

components/sync_bookmarks/bookmark_model_merger.cc

@@ -203,7 +203,7 @@ void BookmarkModelMerger::RemoteTreeNode::EmplaceSelfAndDescendantsByGUID(
 
   if (entity().server_defined_unique_tag.empty()) {
     const std::string& guid = entity().specifics.bookmark().guid();
-    DCHECK(base::IsValidGUID(guid));
+    DCHECK(base::IsValidGUIDOutputString(guid));
 
     // Duplicate GUIDs have been sorted out before.
     bool success = guid_to_remote_node_map->emplace(guid, this).second;
@@ -358,7 +358,7 @@ BookmarkModelMerger::FindGuidMatchesOrReassignLocal(
       bookmark_model->root_node());
   while (iterator.has_next()) {
     const bookmarks::BookmarkNode* const node = iterator.Next();
-    DCHECK(base::IsValidGUID(node->guid()));
+    DCHECK(base::IsValidGUIDOutputString(node->guid()));
 
     const auto remote_it = guid_to_remote_node_map.find(node->guid());
     if (remote_it == guid_to_remote_node_map.end()) {
@@ -598,7 +598,7 @@ void BookmarkModelMerger::ProcessLocalCreation(
   // server id upon receiving commit response.
   const bookmarks::BookmarkNode* node = parent->children()[index].get();
   DCHECK(!FindMatchingRemoteNodeByGUID(node));
-  DCHECK(base::IsValidGUID(node->guid()));
+  DCHECK(base::IsValidGUIDOutputString(node->guid()));
 
   // The node's GUID cannot run into collisions because
   // FindGuidMatchesOrReassignLocal() takes care of reassigning local GUIDs if

components/sync_bookmarks/bookmark_model_merger_unittest.cc

@@ -1061,7 +1061,8 @@ TEST(BookmarkModelMergerTest, ShouldReplaceBookmarkGUIDWithConflictingURLs) {
   EXPECT_EQ(bookmark_bar_node->children()[0]->guid(), kGuid);
   EXPECT_EQ(bookmark_bar_node->children()[0]->url(), kUrl2);
   EXPECT_NE(bookmark_bar_node->children()[1]->guid(), kGuid);
-  EXPECT_TRUE(base::IsValidGUID(bookmark_bar_node->children()[1]->guid()));
+  EXPECT_TRUE(
+      base::IsValidGUIDOutputString(bookmark_bar_node->children()[1]->guid()));
   EXPECT_EQ(bookmark_bar_node->children()[1]->url(), kUrl1);
 }
 
@@ -1113,7 +1114,8 @@ TEST(BookmarkModelMergerTest, ShouldReplaceBookmarkGUIDWithConflictingTypes) {
   EXPECT_EQ(bookmark_bar_node->children()[0]->guid(), kGuid);
   EXPECT_TRUE(bookmark_bar_node->children()[0]->is_folder());
   EXPECT_NE(bookmark_bar_node->children()[1]->guid(), kGuid);
-  EXPECT_TRUE(base::IsValidGUID(bookmark_bar_node->children()[1]->guid()));
+  EXPECT_TRUE(
+      base::IsValidGUIDOutputString(bookmark_bar_node->children()[1]->guid()));
   EXPECT_FALSE(bookmark_bar_node->children()[1]->is_folder());
 }
 

components/sync_bookmarks/bookmark_model_observer_impl.cc

@@ -95,7 +95,7 @@ void BookmarkModelObserverImpl::BookmarkNodeAdded(
   // https://cs.chromium.org/chromium/src/components/sync/syncable/mutable_entry.cc?l=237&gsn=CreateEntryKernel
   // 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()));
+  DCHECK(base::IsValidGUIDOutputString(node->guid()));
 
   // Local bookmark creations should have used a random GUID so it's safe to
   // use it as originator client item ID, without the risk for collision.

components/sync_bookmarks/bookmark_remote_updates_handler.cc

@@ -135,7 +135,8 @@ void ApplyRemoteUpdate(
   // replace the entire node in order to use it, as GUIDs are immutable. Further
   // updates are then applied to the new node instead.
   if (update_entity.specifics.bookmark().guid() != node->guid() &&
-      base::IsValidGUID(update_entity.specifics.bookmark().guid())) {
+      base::IsValidGUIDOutputString(
+          update_entity.specifics.bookmark().guid())) {
     const bookmarks::BookmarkNode* old_node = node;
     node = ReplaceBookmarkNodeGUID(
         node, update_entity.specifics.bookmark().guid(), model);
@@ -233,7 +234,7 @@ void BookmarkRemoteUpdatesHandler::Process(
     if (tracked_entity && !update_entity.is_deleted() &&
         update_entity.server_defined_unique_tag.empty() &&
         !tracked_entity->final_guid_matches(remote_guid)) {
-      DCHECK(base::IsValidGUID(remote_guid));
+      DCHECK(base::IsValidGUIDOutputString(remote_guid));
       bookmark_tracker_->PopulateFinalGuid(update_entity.id, remote_guid);
       // In many cases final_guid_matches() may return false because a final
       // GUID is not known for sure, but actually it matches the local GUID.

components/sync_bookmarks/bookmark_specifics_conversions.cc

@@ -176,7 +176,7 @@ std::string InferGuidForLegacyBookmark(
   static_assert(base::kSHA1Length >= 16, "16 bytes needed to infer GUID");
 
   const std::string guid = ComputeGuidFromBytes(base::make_span(hash));
-  DCHECK(base::IsValidGUID(guid));
+  DCHECK(base::IsValidGUIDOutputString(guid));
   return guid;
 }
 
@@ -194,7 +194,8 @@ sync_pb::EntitySpecifics CreateSpecificsFromBookmarkNode(
   }
 
   DCHECK(!node->guid().empty());
-  DCHECK(base::IsValidGUID(node->guid())) << "Actual: " << node->guid();
+  DCHECK(base::IsValidGUIDOutputString(node->guid()))
+      << "Actual: " << node->guid();
 
   if (include_guid) {
     bm_specifics->set_guid(node->guid());
@@ -248,7 +249,7 @@ const bookmarks::BookmarkNode* CreateBookmarkNodeFromSpecifics(
   DCHECK(parent);
   DCHECK(model);
   DCHECK(favicon_service);
-  DCHECK(base::IsValidGUID(specifics.guid()));
+  DCHECK(base::IsValidGUIDOutputString(specifics.guid()));
 
   bookmarks::BookmarkNode::MetaInfoMap metainfo =
       GetBookmarkMetaInfo(specifics);
@@ -285,7 +286,7 @@ void UpdateBookmarkNodeFromSpecifics(
   // resolving any conflict in GUID. Either GUIDs are the same, or the GUID in
   // specifics is invalid, and hence we can ignore it.
   DCHECK(specifics.guid() == node->guid() ||
-         !base::IsValidGUID(specifics.guid()) ||
+         !base::IsValidGUIDOutputString(specifics.guid()) ||
          !base::FeatureList::IsEnabled(
              switches::kUpdateBookmarkGUIDWithNodeReplacement));
 
@@ -309,7 +310,7 @@ const bookmarks::BookmarkNode* ReplaceBookmarkNodeGUID(
     return node;
   }
   const bookmarks::BookmarkNode* new_node;
-  DCHECK(base::IsValidGUID(guid));
+  DCHECK(base::IsValidGUIDOutputString(guid));
 
   if (node->guid() == guid) {
     // Nothing to do.
@@ -341,7 +342,7 @@ bool IsValidBookmarkSpecifics(const sync_pb::BookmarkSpecifics& specifics,
     LogInvalidSpecifics(InvalidBookmarkSpecificsError::kEmptySpecifics);
     is_valid = false;
   }
-  if (!base::IsValidGUID(specifics.guid())) {
+  if (!base::IsValidGUIDOutputString(specifics.guid())) {
     DLOG(ERROR) << "Invalid bookmark: invalid GUID in the specifics.";
     LogInvalidSpecifics(InvalidBookmarkSpecificsError::kInvalidGUID);
     is_valid = false;
@@ -383,7 +384,7 @@ bool IsValidBookmarkSpecifics(const sync_pb::BookmarkSpecifics& specifics,
 bool HasExpectedBookmarkGuid(const sync_pb::BookmarkSpecifics& specifics,
                              const std::string& originator_cache_guid,
                              const std::string& originator_client_item_id) {
-  DCHECK(base::IsValidGUID(specifics.guid()));
+  DCHECK(base::IsValidGUIDOutputString(specifics.guid()));
 
   if (originator_client_item_id.empty()) {
     // This could be a future bookmark with a client tag instead of an