New snippets now replace old snippets and do not merge

components/ntp_snippets/ntp_snippets_service.h

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_NTP_SNIPPETS_NTP_SNIPPETS_SERVICE_H_
 #define COMPONENTS_NTP_SNIPPETS_NTP_SNIPPETS_SERVICE_H_
 
 #include <stddef.h>
 
+#include <map>
 #include <memory>
 #include <set>
 #include <string>
 #include <vector>
 
 #include "base/callback_forward.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/scoped_observer.h"
 #include "base/timer/timer.h"

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

Not needed anymore I think :)

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

Done.

 #include "components/image_fetcher/image_fetcher_delegate.h"
 #include "components/ntp_snippets/category.h"
 #include "components/ntp_snippets/category_factory.h"
 #include "components/ntp_snippets/category_status.h"
 #include "components/ntp_snippets/content_suggestion.h"
 #include "components/ntp_snippets/content_suggestions_provider.h"
 #include "components/ntp_snippets/ntp_snippet.h"
 #include "components/ntp_snippets/ntp_snippets_fetcher.h"
 #include "components/ntp_snippets/ntp_snippets_scheduler.h"
 #include "components/ntp_snippets/ntp_snippets_status_service.h"
@@ skipping 67 matching lines @@
 
   // Returns whether the service is ready. While this is false, the list of
   // snippets will be empty, and all modifications to it (fetch, dismiss, etc)
   // will be ignored.
   bool ready() const { return state_ == State::READY; }
 
   // Returns whether the service is initialized. While this is false, some
   // 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.
   void FetchSnippets(bool interactive_request);
 
   // Fetches snippets from the server for specified hosts (overriding
   // suggestions from the suggestion service) and adds them to the current ones.
   // Only called from chrome://snippets-internals, DO NOT USE otherwise!
   // Ignored while |loaded()| is false.
   void FetchSnippetsFromHosts(const std::set<std::string>& hosts,
@@ skipping 74 matching lines @@
     // - READY: when the global Chrome state changes, for example after
     //          |OnStateChanged| is called and sync is enabled.
     // - ERROR_OCCURRED: when an unrecoverable error occurred.
     DISABLED,
 
     // The service or one of its dependencies encountered an unrecoverable error
     // and the service can't be used anymore.
     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;
 
   // Callbacks for the NTPSnippetsDatabase.
   void OnDatabaseLoaded(NTPSnippet::PtrVector snippets);
   void OnDatabaseError();
 
   // Callback for the SuggestionsService.
   void OnSuggestionsChanged(const suggestions::SuggestionsProfile& suggestions);
 
   // 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();
 
   // Completes the initialization phase of the service, registering the last
   // observers. This is done after construction, once the database is loaded.
   void FinishInitialization();
 
   void OnSnippetImageFetchedFromDatabase(const ImageFetchedCallback& callback,
                                          const std::string& suggestion_id,
@@ skipping 55 matching lines @@
 
     // The title of the section, localized to the running UI language.
     base::string16 localized_title;
 
     // True iff the server returned results in this category in the last fetch.
     // We never remove categories that the server still provides, but if the
     // server stops providing a category, we won't yet report it as NOT_PROVIDED
     // 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;
 
     CategoryContent();
     CategoryContent(CategoryContent&&);
     ~CategoryContent();
     CategoryContent& operator=(CategoryContent&&);
   };
   std::map<Category, CategoryContent, Category::CompareByID> categories_;
 
   // The ISO 639-1 code of the language used by the application.
   const std::string application_language_code_;
 
   // Scheduler for fetching snippets. Not owned.
   NTPSnippetsScheduler* scheduler_;
 
   // The subscription to the SuggestionsService. When the suggestions change,
   // SuggestionsService will call |OnSuggestionsChanged|, which triggers an
   // update to the set of snippets.
   using SuggestionsSubscription =
       suggestions::SuggestionsService::ResponseCallbackList::Subscription;
   std::unique_ptr<SuggestionsSubscription> suggestions_service_subscription_;
 
   // 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_;
 
   // The database for persisting snippets.
   std::unique_ptr<NTPSnippetsDatabase> database_;
 
   // The service that provides events and data about the signin and sync state.
   std::unique_ptr<NTPSnippetsStatusService> snippets_status_service_;
 
   // Set to true if FetchSnippets is called before the database has been loaded.
   // The fetch will be executed after the database load finishes.
   bool fetch_after_load_;
 
   // Set to true if NukeAllSnippets is called before the database has been
   // loaded. The nuke will be executed after the database load finishes.
   bool nuke_after_load_;
 
   // Request throttler for limiting requests to thumbnail images.
   RequestThrottler thumbnail_requests_throttler_;
 
   DISALLOW_COPY_AND_ASSIGN(NTPSnippetsService);
 };
 
 }  // namespace ntp_snippets
 
 #endif  // COMPONENTS_NTP_SNIPPETS_NTP_SNIPPETS_SERVICE_H_