Use multiple IDs when discarding or merging snippets.

components/ntp_snippets/ntp_snippets_service.cc

Index: components/ntp_snippets/ntp_snippets_service.cc
diff --git a/components/ntp_snippets/ntp_snippets_service.cc b/components/ntp_snippets/ntp_snippets_service.cc
index 448c31a6acce280820f5d0f8ce06910b45af1311..62595a9af201682de59e2c686bbb4591f8a11d26 100644
--- a/components/ntp_snippets/ntp_snippets_service.cc
+++ b/components/ntp_snippets/ntp_snippets_service.cc
@@ -161,13 +161,14 @@ std::unique_ptr<base::ListValue> SnippetsToListValue(
   return list;
 }
 
-bool ContainsSnippet(const NTPSnippet::PtrVector& haystack,
-                     const std::unique_ptr<NTPSnippet>& needle) {
-  const std::string& id = needle->id();
-  return std::find_if(haystack.begin(), haystack.end(),
-                      [&id](const std::unique_ptr<NTPSnippet>& snippet) {
-                        return snippet->id() == id;
-                      }) != haystack.end();
+void InsertAllIDs(const NTPSnippet::PtrVector& snippets,
+                  std::set<std::string>* ids) {
+  for (const std::unique_ptr<NTPSnippet>& snippet : snippets) {
+    ids->insert(snippet->id());
+    for (const SnippetSource& source : snippet->sources()) {

[Marc Treib, 2016/05/18 18:58:05]

nit: no braces required (we usually leave them out if both the condition and the
body fit on one line each)

[tschumann, 2016/05/18 19:23:28]

hmmm... Ok, done for consistency.
One day (in the not-so-distant future) I will challenge this convention, though
=)

[Marc Treib, 2016/05/18 19:29:29]

You'll have to fight Bernhard over it :D
Personally, I don't really mind either way, as long as it's consistent.

+      ids->insert(source.url.spec());
+    }
+  }
 }
 
 void WrapImageFetchedCallback(
@@ -393,12 +394,23 @@ void NTPSnippetsService::OnFetchFinished(
 
 void NTPSnippetsService::MergeSnippets(NTPSnippet::PtrVector new_snippets) {
   // Remove new snippets that we already have, or that have been discarded.
+  std::set<std::string> old_snippet_ids;
+  InsertAllIDs(discarded_snippets_, &old_snippet_ids);
+  InsertAllIDs(snippets_, &old_snippet_ids);
   new_snippets.erase(
-      std::remove_if(new_snippets.begin(), new_snippets.end(),
-                     [this](const std::unique_ptr<NTPSnippet>& snippet) {
-                       return ContainsSnippet(discarded_snippets_, snippet) ||
-                              ContainsSnippet(snippets_, snippet);
-                     }),
+      std::remove_if(
+          new_snippets.begin(), new_snippets.end(),
+          [&old_snippet_ids](const std::unique_ptr<NTPSnippet>& snippet) {
+            if (old_snippet_ids.count(snippet->id())) {

[Marc Treib, 2016/05/18 18:58:05]

Also here: no braces required

[tschumann, 2016/05/18 19:23:28]

Done.

+              return true;
+            }
+            for (const SnippetSource& source : snippet->sources()) {
+              if (old_snippet_ids.count(source.url.spec())) {

[Marc Treib, 2016/05/18 18:58:05]

And here. (The "for" should keep the braces though, since its body has two
lines)

[tschumann, 2016/05/18 19:23:28]

Done.

+                return true;
+              }
+            }
+            return false;
+          }),
       new_snippets.end());
 
   // Fill in default publish/expiry dates where required.
@@ -433,7 +445,6 @@ void NTPSnippetsService::MergeSnippets(NTPSnippet::PtrVector new_snippets) {
                                   num_snippets_discarded);
     }
   }
-
   // Insert the new snippets at the front.
   snippets_.insert(snippets_.begin(),
                    std::make_move_iterator(new_snippets.begin()),