[bookmarks] Add metrics to reflect counts for non-duplicates

These new metrics can be derived from existing metrics but simplify the
analysis, in line with the general guidance by the metrics team.

Change-Id: Ic0f1c59a590dbb9be43b5ee45f37a808b352cca9
Bug: 1068378
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2517681Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Weilun Shi <sweilun@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824328}

chrome/browser/ui/bookmarks/bookmark_browsertest.cc

@@ -347,9 +347,12 @@ IN_PROC_BROWSER_TEST_F(BookmarkBrowsertest, PRE_EmitUmaForDuplicates) {
 IN_PROC_BROWSER_TEST_F(BookmarkBrowsertest, EmitUmaForDuplicates) {
   WaitForBookmarkModel(browser()->profile());
 
+  // The total number of bookmarks is 7, but it gets rounded down due to
+  // bucketing.
   ASSERT_THAT(
       histogram_tester()->GetAllSamples("Bookmarks.Count.OnProfileLoad"),
       testing::ElementsAre(base::Bucket(/*min=*/6, /*count=*/1)));
+
   // 2 bookmarks have URL http://b.com and 4 have http://c.com. This counts as 4
   // duplicates.
   EXPECT_THAT(histogram_tester()->GetAllSamples(
@@ -366,6 +369,19 @@ IN_PROC_BROWSER_TEST_F(BookmarkBrowsertest, EmitUmaForDuplicates) {
       histogram_tester()->GetAllSamples(
           "Bookmarks.Count.OnProfileLoad.DuplicateUrlAndTitleAndParent"),
       testing::ElementsAre(base::Bucket(/*min=*/1, /*count=*/1)));
+
+  // The remaining histograms are the result of substracting the number of
+  // duplicates from the total, which is 7 despite the bucket for the first
+  // histogram above suggesting 6.
+  EXPECT_THAT(histogram_tester()->GetAllSamples(
+                  "Bookmarks.Count.OnProfileLoad.UniqueUrl"),
+              testing::ElementsAre(base::Bucket(/*min=*/3, /*count=*/1)));
+  EXPECT_THAT(histogram_tester()->GetAllSamples(
+                  "Bookmarks.Count.OnProfileLoad.UniqueUrlAndTitle"),
+              testing::ElementsAre(base::Bucket(/*min=*/5, /*count=*/1)));
+  EXPECT_THAT(histogram_tester()->GetAllSamples(
+                  "Bookmarks.Count.OnProfileLoad.UniqueUrlAndTitleAndParent"),
+              testing::ElementsAre(base::Bucket(/*min=*/6, /*count=*/1)));
 }
 
 #endif  // !defined(OS_CHROMEOS)

components/bookmarks/browser/model_loader.cc

@@ -81,6 +81,12 @@ void LoadBookmarks(const base::FilePath& path,
 
   UrlIndex::Stats stats = details->url_index()->ComputeStats();
 
+  DCHECK_LE(stats.duplicate_url_bookmark_count, stats.total_url_bookmark_count);
+  DCHECK_LE(stats.duplicate_url_and_title_bookmark_count,
+            stats.duplicate_url_bookmark_count);
+  DCHECK_LE(stats.duplicate_url_and_title_and_parent_bookmark_count,
+            stats.duplicate_url_and_title_bookmark_count);
+
   base::UmaHistogramCounts100000(
       "Bookmarks.Count.OnProfileLoad",
       base::saturated_cast<int>(stats.total_url_bookmark_count));
@@ -104,6 +110,21 @@ void LoadBookmarks(const base::FilePath& path,
         base::saturated_cast<int>(
             stats.duplicate_url_and_title_and_parent_bookmark_count));
   }
+
+  // Log derived metrics for convenience.
+  base::UmaHistogramCounts100000(
+      "Bookmarks.Count.OnProfileLoad.UniqueUrl",
+      base::saturated_cast<int>(stats.total_url_bookmark_count -
+                                stats.duplicate_url_bookmark_count));
+  base::UmaHistogramCounts100000(
+      "Bookmarks.Count.OnProfileLoad.UniqueUrlAndTitle",
+      base::saturated_cast<int>(stats.total_url_bookmark_count -
+                                stats.duplicate_url_and_title_bookmark_count));
+  base::UmaHistogramCounts100000(
+      "Bookmarks.Count.OnProfileLoad.UniqueUrlAndTitleAndParent",
+      base::saturated_cast<int>(
+          stats.total_url_bookmark_count -
+          stats.duplicate_url_and_title_and_parent_bookmark_count));
 }
 
 }  // namespace

components/bookmarks/browser/url_index.cc

@@ -42,16 +42,26 @@ void AddStatsForBookmarksWithSameUrl(
               });
   }
 
+  size_t duplicate_title_count = 0;
+  size_t duplicate_title_and_parent_count = 0;
+
   auto prev_i = bookmarks_with_same_url->begin();
   for (auto i = std::next(prev_i); i != bookmarks_with_same_url->end();
        ++i, ++prev_i) {
     DCHECK_EQ((*prev_i)->url(), (*i)->url());
     if ((*prev_i)->GetTitle() == (*i)->GetTitle()) {
-      ++stats->duplicate_url_and_title_bookmark_count;
+      ++duplicate_title_count;
       if ((*prev_i)->parent() == (*i)->parent())
-        ++stats->duplicate_url_and_title_and_parent_bookmark_count;
+        ++duplicate_title_and_parent_count;
     }
   }
+
+  DCHECK_LT(duplicate_title_count, bookmarks_with_same_url->size());
+  DCHECK_LE(duplicate_title_and_parent_count, duplicate_title_count);
+
+  stats->duplicate_url_and_title_bookmark_count += duplicate_title_count;
+  stats->duplicate_url_and_title_and_parent_bookmark_count +=
+      duplicate_title_and_parent_count;
 }
 
 }  // namespace

tools/metrics/histograms/histograms_xml/others/histograms.xml

@@ -1525,6 +1525,36 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="Bookmarks.Count.OnProfileLoad.UniqueUrl" units="bookmarks"
+    expires_after="2021-04-04">
+  <owner>mastiz@chromium.org</owner>
+  <owner>sky@chromium.org</owner>
+  <summary>
+    The number of unique URLs among bookmarks saved by the user. Recorded when
+    bookmarks are loaded into storage from disk.
+  </summary>
+</histogram>
+
+<histogram name="Bookmarks.Count.OnProfileLoad.UniqueUrlAndTitle"
+    units="bookmarks" expires_after="2021-04-04">
+  <owner>mastiz@chromium.org</owner>
+  <owner>sky@chromium.org</owner>
+  <summary>
+    The number of unique URL-title pairs among bookmarks saved by the user.
+    Recorded when bookmarks are loaded into storage from disk.
+  </summary>
+</histogram>
+
+<histogram name="Bookmarks.Count.OnProfileLoad.UniqueUrlAndTitleAndParent"
+    units="bookmarks" expires_after="2021-04-04">
+  <owner>mastiz@chromium.org</owner>
+  <owner>sky@chromium.org</owner>
+  <summary>
+    The number of unique URL-title-parent triples among bookmarks saved by the
+    user. Recorded when bookmarks are loaded into storage from disk.
+  </summary>
+</histogram>
+
 <histogram name="Bookmarks.Count.OpenInIncognito" units="bookmarks"
     expires_after="M82">
   <owner>twellington@google.com</owner>