[NTP Snippets] Refactor background scheduling for remote suggestions

components/ntp_snippets/remote/remote_suggestions_provider_impl.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_REMOTE_REMOTE_SUGGESTIONS_PROVIDER_H_
-#define COMPONENTS_NTP_SNIPPETS_REMOTE_REMOTE_SUGGESTIONS_PROVIDER_H_
+#ifndef COMPONENTS_NTP_SNIPPETS_REMOTE_REMOTE_SUGGESTIONS_PROVIDER_IMPL_H_
+#define COMPONENTS_NTP_SNIPPETS_REMOTE_REMOTE_SUGGESTIONS_PROVIDER_IMPL_H_
 
 #include <cstddef>
 #include <deque>
 #include <map>
 #include <memory>
 #include <set>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/callback_forward.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/time/clock.h"
 #include "base/time/time.h"
 #include "components/image_fetcher/image_fetcher_delegate.h"
 #include "components/ntp_snippets/category.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/remote/ntp_snippet.h"
 #include "components/ntp_snippets/remote/ntp_snippets_fetcher.h"
 #include "components/ntp_snippets/remote/ntp_snippets_request_params.h"
-#include "components/ntp_snippets/remote/ntp_snippets_scheduler.h"
+#include "components/ntp_snippets/remote/remote_suggestions_provider.h"
 #include "components/ntp_snippets/remote/remote_suggestions_status_service.h"
 #include "components/ntp_snippets/remote/request_throttler.h"
 
 class PrefRegistrySimple;
 class PrefService;
 
 namespace gfx {
 class Image;
 }  // namespace gfx
 
 namespace image_fetcher {
 class ImageDecoder;
 class ImageFetcher;
 }  // namespace image_fetcher
 
 namespace ntp_snippets {
 
+class CategoryRanker;
 class RemoteSuggestionsDatabase;
-class CategoryRanker;
-class UserClassifier;
 
 // CachedImageFetcher takes care of fetching images from the network and caching
 // them in the database.
 // TODO(tschumann): Move into a separate library and inject the
 // CachedImageFetcher into the RemoteSuggestionsProvider. This allows us to get
 // rid of exposing this member for testing and lets us test the caching logic
 // separately.
 class CachedImageFetcher : public image_fetcher::ImageFetcherDelegate {
  public:
   // |pref_service| and |database| need to outlive the created image fetcher
@@ skipping 39 matching lines @@
   RequestThrottler thumbnail_requests_throttler_;
 
   DISALLOW_COPY_AND_ASSIGN(CachedImageFetcher);
 };
 
 // Retrieves fresh content data (articles) from the server, stores them and
 // provides them as content suggestions.
 // This class is final because it does things in its constructor which make it
 // unsafe to derive from it.
 // TODO(treib): Introduce two-phase initialization and make the class not final?
-class RemoteSuggestionsProvider final : public ContentSuggestionsProvider {
+class RemoteSuggestionsProviderImpl final : public RemoteSuggestionsProvider {
  public:
   // |application_language_code| should be a ISO 639-1 compliant string, e.g.
   // 'en' or 'en-US'. Note that this code should only specify the language, not
   // the locale, so 'en_US' (English language with US locale) and 'en-GB_US'
   // (British English person in the US) are not language codes.
-  RemoteSuggestionsProvider(
+  RemoteSuggestionsProviderImpl(
       Observer* observer,
       PrefService* pref_service,
       const std::string& application_language_code,
       CategoryRanker* category_ranker,
-      const UserClassifier* user_classifier,
-      NTPSnippetsScheduler* scheduler,
       std::unique_ptr<NTPSnippetsFetcher> snippets_fetcher,
       std::unique_ptr<image_fetcher::ImageFetcher> image_fetcher,
       std::unique_ptr<image_fetcher::ImageDecoder> image_decoder,
       std::unique_ptr<RemoteSuggestionsDatabase> database,
       std::unique_ptr<RemoteSuggestionsStatusService> status_service);
 
-  ~RemoteSuggestionsProvider() override;
+  ~RemoteSuggestionsProviderImpl() override;
 
   static void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
   // 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.
+  // Returns whether the service is successfully initialized. While this is
+  // false, some calls may trigger DCHECKs.
   bool initialized() const { return ready() || state_ == State::DISABLED; }
 
-  // Fetchs content suggestions from the Content Suggestions server for all
-  // remote categories in the background.
-  void FetchSnippetsInTheBackground();
+  // RemoteSuggestionsProvider implementation.
+  void SetProviderStatusCallback(
+      std::unique_ptr<ProviderStatusCallback> callback) override;
+  void RefetchInTheBackground(
+      std::unique_ptr<FetchStatusCallback> callback) override;
 
-  // Fetchs content suggestions from the Content Suggestions server for all
-  // remote categories. The request to the server is performed as an interactive
-  // request. Interactive requests are used for actions triggered by the user
-  // and request lower latency processing.
-  void FetchSnippetsForAllCategories();
-
-  // Only used in tests and for debugging in snippets-internal/.
   // TODO(fhorschig): Remove this getter when there is an interface for the
   // fetcher that allows better mocks.
-  const NTPSnippetsFetcher* snippets_fetcher() const {
-    return snippets_fetcher_.get();
-  }
+  const NTPSnippetsFetcher* snippets_fetcher_for_testing_and_debugging()
+      const override;
 
-  // (Re)schedules the periodic fetching of snippets. If |force| is true, the
-  // tasks will be re-scheduled even if they already exist and have the correct
-  // periods.
-  void RescheduleFetching(bool force);
-
-  // ContentSuggestionsProvider implementation
+  // ContentSuggestionsProvider implementation.
   CategoryStatus GetCategoryStatus(Category category) override;
   CategoryInfo GetCategoryInfo(Category category) override;
   void DismissSuggestion(const ContentSuggestion::ID& suggestion_id) override;
   void FetchSuggestionImage(const ContentSuggestion::ID& suggestion_id,
                             const ImageFetchedCallback& callback) override;
   void Fetch(const Category& category,
              const std::set<std::string>& known_suggestion_ids,
              const FetchDoneCallback& callback) override;
+  void ReloadSuggestions() override;
   void ClearHistory(
       base::Time begin,
       base::Time end,
       const base::Callback<bool(const GURL& url)>& filter) override;
   void ClearCachedSuggestions(Category category) override;
   void OnSignInStateChanged() override;
   void GetDismissedSuggestionsForDebugging(
       Category category,
       const DismissedSuggestionsCallback& callback) override;
   void ClearDismissedSuggestionsForDebugging(Category category) override;
@@ skipping 16 matching lines @@
   // Overrides internal clock for testing purposes.
   void SetClockForTesting(std::unique_ptr<base::Clock> clock) {
     clock_ = std::move(clock);
   }
 
   // TODO(tschumann): remove this method as soon as we inject the fetcher into
   // the constructor.
   CachedImageFetcher& GetImageFetcherForTesting() { return image_fetcher_; }
 
  private:
-  friend class RemoteSuggestionsProviderTest;
+  friend class RemoteSuggestionsProviderImplTest;
 
-  FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderTest,
+  FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
+                           CallsProviderStatusCallbackWhenReady);
+  FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
+                           CallsProviderStatusCallbackOnError);
+  FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
+                           CallsProviderStatusCallbackWhenDisabled);
+  FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
+                           ShouldNotCrashWhenCallingEmptyCallback);
+  FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
                            DontNotifyIfNotAvailable);
-  FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderTest,
+  FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
                            RemoveExpiredDismissedContent);
-  FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderTest,
-                           RescheduleOnStateChange);
-  FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderTest, StatusChanges);
-  FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderTest,
+  FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest, StatusChanges);
+  FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
                            SuggestionsFetchedOnSignInAndSignOut);
 
   // Possible state transitions:
   //       NOT_INITED --------+
   //       /       \          |
   //      v         v         |
   //   READY <--> DISABLED    |
   //       \       /          |
   //        v     v           |
   //     ERROR_OCCURRED <-----+
@@ skipping 60 matching lines @@
 
     explicit CategoryContent(const CategoryInfo& info);
     CategoryContent(CategoryContent&&);
     ~CategoryContent();
     CategoryContent& operator=(CategoryContent&&);
   };
 
   // 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);
+  // Useful for requests triggered by the user. After the fetch finished, the
+  // provided |callback| will be triggered with the status of the fetch.
+  void FetchSnippets(bool interactive_request,
+                     std::unique_ptr<FetchStatusCallback> callback);
 
   // Returns the URL of the image of a snippet if it is among the current or
   // among the archived snippets in the matching category. Returns an empty URL
   // otherwise.
   GURL FindSnippetImageUrl(const ContentSuggestion::ID& suggestion_id) const;
 
   // Callbacks for the RemoteSuggestionsDatabase.
   void OnDatabaseLoaded(NTPSnippet::PtrVector snippets);
   void OnDatabaseError();
 
   // Callback for fetch-more requests with the NTPSnippetsFetcher.
   void OnFetchMoreFinished(
       const FetchDoneCallback& fetching_callback,
       Status status,
       NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories);
 
   // Callback for regular fetch requests with the NTPSnippetsFetcher.
   void OnFetchFinished(
+      std::unique_ptr<FetchStatusCallback> callback,
       bool interactive_request,
       Status status,
       NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories);
 
   // Moves all snippets from |to_archive| into the archive of the |content|.
   // Clears |to_archive|. As the archive is a FIFO buffer of limited size, this
   // function will also delete images from the database in case the associated
   // snippet gets evicted from the archive.
   void ArchiveSnippets(CategoryContent* content,
                        NTPSnippet::PtrVector* to_archive);
@@ skipping 32 matching lines @@
   // Triggers a state transition depending on the provided status. This method
   // is called when a change is detected by |status_service_|.
   void OnStatusChanged(RemoteSuggestionsStatus old_status,
                        RemoteSuggestionsStatus new_status);
 
   // Verifies state transitions (see |State|'s documentation) and applies them.
   // Also updates the provider status. Does nothing except updating the provider
   // status if called with the current state.
   void EnterState(State state);
 
+  // Notifies the state change to ProviderStatusCallback specified by
+  // SetProviderStatusCallback().
+  void NotifyStateChanged();
+
   // Enables the service. Do not call directly, use |EnterState| instead.
   void EnterStateReady();
 
   // Disables the service. Do not call directly, use |EnterState| instead.
   void EnterStateDisabled();
 
   // Disables the service permanently because an unrecoverable error occurred.
   // Do not call directly, use |EnterState| instead.
   void EnterStateError();
 
@@ skipping 27 matching lines @@
   const Category articles_category_;
 
   std::map<Category, CategoryContent, Category::CompareByID> category_contents_;
 
   // The ISO 639-1 code of the language used by the application.
   const std::string application_language_code_;
 
   // Ranker that orders the categories. Not owned.
   CategoryRanker* category_ranker_;
 
-  // Classifier that tells us how active the user is. Not owned.
-  const UserClassifier* user_classifier_;
-
-  // Scheduler for fetching snippets. Not owned.
-  NTPSnippetsScheduler* scheduler_;
-
   // The snippets fetcher.
   std::unique_ptr<NTPSnippetsFetcher> snippets_fetcher_;
 
   // The database for persisting snippets.
   std::unique_ptr<RemoteSuggestionsDatabase> database_;
   base::TimeTicks database_load_start_;
 
   // The image fetcher.
   CachedImageFetcher image_fetcher_;
 
   // The service that provides events and data about the signin and sync state.
   std::unique_ptr<RemoteSuggestionsStatusService> status_service_;
 
   // Set to true if FetchSnippets is called while the service isn't ready.
   // The fetch will be executed once the service enters the READY state.
+  // TODO(jkrcal): create a struct and have here just one base::Optional<>?
   bool fetch_when_ready_;
+  // The parameters for the fetch to perform later.

[Bernhard Bauer, 2016/12/21 10:38:42]

Nit: empty line before comments plz :)

[jkrcal, 2016/12/21 11:54:40]

Done.

+  bool fetch_when_ready_interactive_;
+  std::unique_ptr<FetchStatusCallback> fetch_when_ready_callback_;
+
+  std::unique_ptr<ProviderStatusCallback> provider_status_callback_;
 
   // Set to true if NukeAllSnippets is called while the service isn't ready.
   // The nuke will be executed once the service finishes initialization or
   // enters the READY state.
   bool nuke_when_initialized_;
 
   // A clock for getting the time. This allows to inject a clock in tests.
   std::unique_ptr<base::Clock> clock_;
 
-  DISALLOW_COPY_AND_ASSIGN(RemoteSuggestionsProvider);
+  DISALLOW_COPY_AND_ASSIGN(RemoteSuggestionsProviderImpl);
 };
 
 }  // namespace ntp_snippets
 
-#endif  // COMPONENTS_NTP_SNIPPETS_REMOTE_REMOTE_SUGGESTIONS_PROVIDER_H_
+#endif  // COMPONENTS_NTP_SNIPPETS_REMOTE_REMOTE_SUGGESTIONS_PROVIDER_IMPL_H_