[Sync] Invalidate BookmarkSpecifics with invalid GUID

BookmarkSpecifics are currently accepted even if their GUID field holds
an invalid value. However, we want to discard updates containing an
invalid GUID, unless such GUID is empty, as we handle this situation
separately. This CL makes the corresponding changes for this to happen.


Bug: 978430
Change-Id: I39c32cef051e07b36f4dc20027c5ab3414f1c1b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1807332
Commit-Queue: Pauline Leitao <psivieroleitao@google.com>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697570}

components/sync_bookmarks/bookmark_specifics_conversions.cc

@@ -242,6 +242,10 @@ bool IsValidBookmarkSpecifics(const sync_pb::BookmarkSpecifics& specifics,
     LogInvalidSpecifics(InvalidBookmarkSpecificsError::kEmptySpecifics);
     is_valid = false;
   }
+  if (!base::IsValidGUID(specifics.guid()) && !specifics.guid().empty()) {
+    DLOG(ERROR) << "Invalid bookmark: invalid GUID in the specifics.";
+    return false;
+  }
   if (!is_folder) {
     if (!GURL(specifics.url()).is_valid()) {
       DLOG(ERROR) << "Invalid bookmark: invalid url in the specifics.";

components/sync_bookmarks/bookmark_specifics_conversions_unittest.cc

@@ -395,6 +395,24 @@ TEST(BookmarkSpecificsConversionsTest,
   EXPECT_FALSE(IsValidBookmarkSpecifics(*bm_specifics, /*is_folder=*/false));
 }
 
+TEST(BookmarkSpecificsConversionsTest,
+     ShouldBeInvalidBookmarkSpecificsWithInvalidGUID) {
+  sync_pb::EntitySpecifics specifics;
+  sync_pb::BookmarkSpecifics* bm_specifics = specifics.mutable_bookmark();
+
+  // Add empty GUID.
+  bm_specifics->set_guid("");
+  EXPECT_TRUE(IsValidBookmarkSpecifics(*bm_specifics, /*is_folder=*/true));
+
+  // Add invalid GUID.
+  bm_specifics->set_guid("INVALID GUID");
+  EXPECT_FALSE(IsValidBookmarkSpecifics(*bm_specifics, /*is_folder=*/true));
+
+  // Add valid GUID.
+  bm_specifics->set_guid(base::GenerateGUID());
+  EXPECT_TRUE(IsValidBookmarkSpecifics(*bm_specifics, /*is_folder=*/true));
+}
+
 TEST(BookmarkSpecificsConversionsTest, ShouldBeInvalidBookmarkSpecifics) {
   sync_pb::EntitySpecifics specifics;
   sync_pb::BookmarkSpecifics* bm_specifics = specifics.mutable_bookmark();