Prevent duplicate URLs from entering the prefetch pipeline

If the same URL is suggested multiple times at once, it should only be added
once. I stumbled into this condition when using the test data for Feed.

Bug: 841516
Change-Id: I26b95e4c1b4e7ace1516e0a84187d9960d327aaa
Reviewed-on: https://chromium-review.googlesource.com/c/1351275
Commit-Queue: Dan H <harringtond@google.com>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612862}

components/offline_pages/core/prefetch/tasks/add_unique_urls_task.cc

@@ -101,7 +101,7 @@ Result AddUniqueUrlsSync(
 
   std::map<std::string, int64_t> existing_items =
       GetAllUrlsAndIdsFromNamespaceSync(db, name_space);
-
+  std::set<std::string> added_urls;
   int added_row_count = 0;
   base::Time now = OfflineClock()->Now();
   // Insert rows in reverse order to ensure that the beginning of the list has
@@ -109,7 +109,12 @@ Result AddUniqueUrlsSync(
   for (auto candidate_iter = candidate_prefetch_urls.rbegin();
        candidate_iter != candidate_prefetch_urls.rend(); ++candidate_iter) {
     const PrefetchURL& prefetch_url = *candidate_iter;
-    auto existing_iter = existing_items.find(prefetch_url.url.spec());
+    const std::string url_spec = prefetch_url.url.spec();
+    // Don't add the same URL more than once.
+    if (!added_urls.insert(url_spec).second)
+      continue;
+
+    auto existing_iter = existing_items.find(url_spec);
     if (existing_iter != existing_items.end()) {
       // An existing item is still being suggested so update its timestamps (and
       // therefore priority).

components/offline_pages/core/prefetch/tasks/add_unique_urls_task_unittest.cc

@@ -96,8 +96,11 @@ TEST_F(AddUniqueUrlsTaskTest, AddTaskInEmptyStore) {
 }
 
 TEST_F(AddUniqueUrlsTaskTest, SingleDuplicateUrlNotAdded) {
-  std::vector<PrefetchURL> urls;
-  urls.push_back(PrefetchURL{kClientId1, kTestURL1, kTestTitle1});
+  // Add the same URL twice in a single round. Only one entry should be added.
+  std::vector<PrefetchURL> urls = {
+      {kClientId1, kTestURL1, kTestTitle1},
+      {kClientId2, kTestURL1, kTestTitle2},
+  };
   RunTask(std::make_unique<AddUniqueUrlsTask>(dispatcher(), store(),
                                               kTestNamespace, urls));
   EXPECT_EQ(1, dispatcher()->task_schedule_count);
@@ -112,6 +115,7 @@ TEST_F(AddUniqueUrlsTaskTest, SingleDuplicateUrlNotAdded) {
                                               kTestNamespace, urls));
   // The task schedule count should not have changed with no new URLs.
   EXPECT_EQ(1, dispatcher()->task_schedule_count);
+  EXPECT_EQ(1UL, GetAllItems().size());
 }
 
 TEST_F(AddUniqueUrlsTaskTest, DontAddURLIfItAlreadyExists) {