New snippets now replace old snippets and do not merge

components/ntp_snippets/ntp_snippets_service.h

Index: components/ntp_snippets/ntp_snippets_service.h
diff --git a/components/ntp_snippets/ntp_snippets_service.h b/components/ntp_snippets/ntp_snippets_service.h
index 3d659a402807a52405841380ff92c5c60e4a9fae..d8320688099f5cb518d816d344cec48a08025da5 100644
--- a/components/ntp_snippets/ntp_snippets_service.h
+++ b/components/ntp_snippets/ntp_snippets_service.h
@@ -7,6 +7,7 @@
 
 #include <stddef.h>
 
+#include <map>
 #include <memory>
 #include <set>
 #include <string>
@@ -104,7 +105,7 @@ class NTPSnippetsService : public ContentSuggestionsProvider,
   // calls may trigger DCHECKs.
   bool initialized() const { return ready() || state_ == State::DISABLED; }
 
-  // Fetches snippets from the server and adds them to the current ones.
+  // Fetches snippets from the server and replaces old snippets by the new ones.
   // Requests can be marked more important by setting |interactive_request| to
   // true (such request might circumvent the daily quota for requests, etc.)
   // Useful for requests triggered by the user.
@@ -199,6 +200,11 @@ class NTPSnippetsService : public ContentSuggestionsProvider,
     ERROR_OCCURRED
   };
 
+  // Returns the URL of the image of a snippet if it is among the current or
+  // among the archived snippets in |category|. Returns empty string, otherwise.

[Marc Treib, 2016/09/21 19:10:17]

nit: Returns an invalid (or empty) URL otherwise.

[jkrcal, 2016/09/22 09:12:42]

Done.

+  GURL FindSnippetImageUrl(Category category,
+                           const std::string& snippet_id) const;
+
   // image_fetcher::ImageFetcherDelegate implementation.
   void OnImageDataFetched(const std::string& snippet_id,
                           const std::string& image_data) override;
@@ -213,15 +219,18 @@ class NTPSnippetsService : public ContentSuggestionsProvider,
   // Callback for the NTPSnippetsFetcher.
   void OnFetchFinished(NTPSnippetsFetcher::OptionalSnippets snippets);
 
-  // Merges newly available snippets with the previously available list.
-  void MergeSnippets(Category category, NTPSnippet::PtrVector new_snippets);
+  // Replace old snippets in |category| by newly available snippets.
+  void ReplaceSnippets(Category category, NTPSnippet::PtrVector new_snippets);
 
   std::set<std::string> GetSnippetHostsFromPrefs() const;
   void StoreSnippetHostsToPrefs(const std::set<std::string>& hosts);
 
-  // Removes the expired snippets (including dismissed) from the service and the
-  // database, and schedules another pass for the next expiration.
-  void ClearExpiredSnippets();
+  // Removes expired dismissed snippets from the service and the database.
+  void ClearExpiredDismissedSnippets();
+
+  // Removes images from the DB that do not have any corresponding snippet
+  // (neither in the current set, nor in the archived set).
+  void ClearOrphanedImages();
 
   // Clears all stored snippets and updates the observer.
   void NukeAllSnippets();
@@ -297,9 +306,14 @@ class NTPSnippetsService : public ContentSuggestionsProvider,
     // while we still have non-expired snippets in it.
     bool provided_by_server = true;
 
-    // All current suggestions (i.e. not dismissed ones).
+    // All suggestions from the most recent fetch (excl. the dismissed ones).

[Marc Treib, 2016/09/21 19:10:17]

nit: To me, "from the most recent fetch" isn't really important here, just that
these are the currently "active" suggestions.

[jkrcal, 2016/09/22 09:12:42]

Done.

     NTPSnippet::PtrVector snippets;
 
+    // All previous snippets that we keep around in memory because they can be

[Marc Treib, 2016/09/21 19:10:17]

nitty nit: s/snippets/suggestions/, to be consistent with above

[jkrcal, 2016/09/22 09:12:42]

Done.

+    // on some open NTP. We do not store this list to any DB so that on new

[Marc Treib, 2016/09/21 19:10:17]

nitty nit 2: "DB" doesn't necessarily imply that they'd be persisted, and it's
not the only way they could be persisted. So I'd just say that they aren't
persisted :)

[jkrcal, 2016/09/22 09:12:42]

Done.

+    // start of Chrome, this is empty.
+    NTPSnippet::PtrVector archived;
+
     // Suggestions that the user dismissed. We keep these around until they
     // expire so we won't re-add them on the next fetch.
     NTPSnippet::PtrVector dismissed;
@@ -327,9 +341,6 @@ class NTPSnippetsService : public ContentSuggestionsProvider,
   // The snippets fetcher.
   std::unique_ptr<NTPSnippetsFetcher> snippets_fetcher_;
 
-  // Timer that calls us back when the next snippet expires.
-  base::OneShotTimer expiry_timer_;
-
   std::unique_ptr<image_fetcher::ImageFetcher> image_fetcher_;
   std::unique_ptr<image_fetcher::ImageDecoder> image_decoder_;