[NTP Snippets] Refactor background scheduling for remote suggestions

components/ntp_snippets/remote/remote_suggestions_provider_impl.cc

Index: components/ntp_snippets/remote/remote_suggestions_provider_impl.cc
diff --git a/components/ntp_snippets/remote/remote_suggestions_provider.cc b/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc
similarity index 81%
copy from components/ntp_snippets/remote/remote_suggestions_provider.cc
copy to components/ntp_snippets/remote/remote_suggestions_provider_impl.cc
index 3f8ce09352cb049d65d9ae44e45a984b32e0c6bb..3b31aa9e8d3d942cb6d912253c0b1eacaa06fcca 100644
--- a/components/ntp_snippets/remote/remote_suggestions_provider.cc
+++ b/components/ntp_snippets/remote/remote_suggestions_provider_impl.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/ntp_snippets/remote/remote_suggestions_provider.h"
+#include "components/ntp_snippets/remote/remote_suggestions_provider_impl.h"
 
 #include <algorithm>
 #include <iterator>
@@ -29,7 +29,6 @@
 #include "components/ntp_snippets/pref_names.h"
 #include "components/ntp_snippets/remote/remote_suggestions_database.h"
 #include "components/ntp_snippets/switches.h"
-#include "components/ntp_snippets/user_classifier.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/variations/variations_associated_data.h"
@@ -48,22 +47,6 @@ const int kMaxSnippetCount = 10;
 // Number of archived snippets we keep around in memory.
 const int kMaxArchivedSnippetCount = 200;
 
-// Default values for fetching intervals, fallback and wifi.
-const double kDefaultFetchingIntervalRareNtpUser[] = {48.0, 24.0};
-const double kDefaultFetchingIntervalActiveNtpUser[] = {24.0, 6.0};
-const double kDefaultFetchingIntervalActiveSuggestionsConsumer[] = {24.0, 6.0};
-
-// Variation parameters than can override the default fetching intervals.
-const char* kFetchingIntervalParamNameRareNtpUser[] = {
-    "fetching_interval_hours-fallback-rare_ntp_user",
-    "fetching_interval_hours-wifi-rare_ntp_user"};
-const char* kFetchingIntervalParamNameActiveNtpUser[] = {
-    "fetching_interval_hours-fallback-active_ntp_user",
-    "fetching_interval_hours-wifi-active_ntp_user"};
-const char* kFetchingIntervalParamNameActiveSuggestionsConsumer[] = {
-    "fetching_interval_hours-fallback-active_suggestions_consumer",
-    "fetching_interval_hours-wifi-active_suggestions_consumer"};
-
 // Keys for storing CategoryContent info in prefs.
 const char kCategoryContentId[] = "id";
 const char kCategoryContentTitle[] = "title";
@@ -73,42 +56,6 @@ const char kCategoryContentAllowFetchingMore[] = "allow_fetching_more";
 // TODO(treib): Remove after M57.
 const char kDeprecatedSnippetHostsPref[] = "ntp_snippets.hosts";
 
-base::TimeDelta GetFetchingInterval(bool is_wifi,
-                                    UserClassifier::UserClass user_class) {
-  double value_hours = 0.0;
-
-  const int index = is_wifi ? 1 : 0;
-  const char* param_name = "";
-  switch (user_class) {
-    case UserClassifier::UserClass::RARE_NTP_USER:
-      value_hours = kDefaultFetchingIntervalRareNtpUser[index];
-      param_name = kFetchingIntervalParamNameRareNtpUser[index];
-      break;
-    case UserClassifier::UserClass::ACTIVE_NTP_USER:
-      value_hours = kDefaultFetchingIntervalActiveNtpUser[index];
-      param_name = kFetchingIntervalParamNameActiveNtpUser[index];
-      break;
-    case UserClassifier::UserClass::ACTIVE_SUGGESTIONS_CONSUMER:
-      value_hours = kDefaultFetchingIntervalActiveSuggestionsConsumer[index];
-      param_name = kFetchingIntervalParamNameActiveSuggestionsConsumer[index];
-      break;
-  }
-
-  // The default value can be overridden by a variation parameter.
-  std::string param_value_str = variations::GetVariationParamValueByFeature(
-      ntp_snippets::kArticleSuggestionsFeature, param_name);
-  if (!param_value_str.empty()) {
-    double param_value_hours = 0.0;
-    if (base::StringToDouble(param_value_str, &param_value_hours)) {
-      value_hours = param_value_hours;
-    } else {
-      LOG(WARNING) << "Invalid value for variation parameter " << param_name;
-    }
-  }
-
-  return base::TimeDelta::FromSecondsD(value_hours * 3600.0);
-}
-
 std::unique_ptr<std::vector<std::string>> GetSnippetIDVector(
     const NTPSnippet::PtrVector& snippets) {
   auto result = base::MakeUnique<std::vector<std::string>>();
@@ -277,9 +224,8 @@ void CachedImageFetcher::OnSnippetImageFetchedFromDatabase(
   // |image_decoder_| is null in tests.
   if (image_decoder_ && !data.empty()) {
     image_decoder_->DecodeImage(
-        data, base::Bind(
-                  &CachedImageFetcher::OnSnippetImageDecodedFromDatabase,
-                  base::Unretained(this), callback, suggestion_id, url));
+        data, base::Bind(&CachedImageFetcher::OnSnippetImageDecodedFromDatabase,
+                         base::Unretained(this), callback, suggestion_id, url));
     return;
   }
   // Fetching from the DB failed; start a network fetch.
@@ -320,26 +266,22 @@ void CachedImageFetcher::FetchSnippetImageFromNetwork(
                  base::Unretained(this), callback));
 }
 
-RemoteSuggestionsProvider::RemoteSuggestionsProvider(
+RemoteSuggestionsProviderImpl::RemoteSuggestionsProviderImpl(
     Observer* observer,
     CategoryFactory* category_factory,
     PrefService* pref_service,
     const std::string& application_language_code,
-    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)
-    : ContentSuggestionsProvider(observer, category_factory),
+    : RemoteSuggestionsProvider(observer, category_factory),
       state_(State::NOT_INITED),
       pref_service_(pref_service),
       articles_category_(
           category_factory->FromKnownCategory(KnownCategories::ARTICLES)),
       application_language_code_(application_language_code),
-      user_classifier_(user_classifier),
-      scheduler_(scheduler),
       snippets_fetcher_(std::move(snippets_fetcher)),
       database_(std::move(database)),
       image_fetcher_(std::move(image_fetcher),
@@ -348,6 +290,7 @@ RemoteSuggestionsProvider::RemoteSuggestionsProvider(
                      database_.get()),
       status_service_(std::move(status_service)),
       fetch_when_ready_(false),
+      fetch_when_ready_interactive_(false),
       nuke_when_initialized_(false),
       clock_(base::MakeUnique<base::DefaultClock>()) {
   pref_service_->ClearPref(kDeprecatedSnippetHostsPref);
@@ -370,46 +313,75 @@ RemoteSuggestionsProvider::RemoteSuggestionsProvider(
   }
 
   database_->SetErrorCallback(base::Bind(
-      &RemoteSuggestionsProvider::OnDatabaseError, base::Unretained(this)));
+      &RemoteSuggestionsProviderImpl::OnDatabaseError, base::Unretained(this)));
 
   // We transition to other states while finalizing the initialization, when the
   // database is done loading.
   database_load_start_ = base::TimeTicks::Now();
-  database_->LoadSnippets(base::Bind(
-      &RemoteSuggestionsProvider::OnDatabaseLoaded, base::Unretained(this)));
+  database_->LoadSnippets(
+      base::Bind(&RemoteSuggestionsProviderImpl::OnDatabaseLoaded,
+                 base::Unretained(this)));
 }
 
-RemoteSuggestionsProvider::~RemoteSuggestionsProvider() = default;
+RemoteSuggestionsProviderImpl::~RemoteSuggestionsProviderImpl() = default;
 
 // static
-void RemoteSuggestionsProvider::RegisterProfilePrefs(
+void RemoteSuggestionsProviderImpl::RegisterProfilePrefs(
     PrefRegistrySimple* registry) {
   // TODO(treib): Remove after M57.
   registry->RegisterListPref(kDeprecatedSnippetHostsPref);
   registry->RegisterListPref(prefs::kRemoteSuggestionCategories);
-  registry->RegisterInt64Pref(prefs::kSnippetBackgroundFetchingIntervalWifi, 0);
-  registry->RegisterInt64Pref(prefs::kSnippetBackgroundFetchingIntervalFallback,
-                              0);
   registry->RegisterInt64Pref(prefs::kLastSuccessfulBackgroundFetchTime, 0);
 
   RemoteSuggestionsStatusService::RegisterProfilePrefs(registry);
 }
 
-void RemoteSuggestionsProvider::FetchSnippetsInTheBackground() {
-  FetchSnippets(/*interactive_request=*/false);
+void RemoteSuggestionsProviderImpl::SetProviderStatusCallback(
+    const ProviderStatusCallback& callback) {
+  provider_status_callback_ = callback;
+
+  // Call the observer right away if we've reached any final state.
+  switch (state_) {

[Marc Treib, 2016/12/19 12:59:23]

Maybe pull this logic into a helper method ("NotifyStateChanged") that can be
reused in EnterState?

[jkrcal, 2016/12/20 16:39:46]

Done.

+    case State::NOT_INITED:
+      // Initial state, not sure yet whether active or not.
+      break;
+    case State::READY:
+      provider_status_callback_.Run(ProviderStatus::ACTIVE);
+      break;
+    case State::DISABLED:
+      provider_status_callback_.Run(ProviderStatus::INACTIVE);
+      break;
+    case State::ERROR_OCCURRED:
+      provider_status_callback_.Run(ProviderStatus::INACTIVE);
+      break;
+    case State::COUNT:
+      NOTREACHED();
+      break;
+  }
+}
+
+void RemoteSuggestionsProviderImpl::ReloadSuggestions() {
+  FetchSnippets(/*interactive_request=*/true, FetchStatusCallback());
+}
+
+void RemoteSuggestionsProviderImpl::RefetchInTheBackground(
+    const FetchStatusCallback& callback) {
+  FetchSnippets(/*interactive_request=*/false, callback);
 }
 
-void RemoteSuggestionsProvider::FetchSnippetsForAllCategories() {
-  // TODO(markusheintz): Investigate whether we can call the Fetch method
-  // instead of the FetchSnippets.
-  FetchSnippets(/*interactive_request=*/true);
+const NTPSnippetsFetcher*
+RemoteSuggestionsProviderImpl::snippets_fetcher_for_testing_and_debugging()
+    const {
+  return snippets_fetcher_.get();
 }
 
-void RemoteSuggestionsProvider::FetchSnippets(
-    bool interactive_request) {
+void RemoteSuggestionsProviderImpl::FetchSnippets(
+    bool interactive_request,
+    const FetchStatusCallback& callback) {
   if (!ready()) {
     fetch_when_ready_ = true;
-    return;

[Marc Treib, 2016/12/19 12:59:23]

I think the "return" should stay?

[jkrcal, 2016/12/20 16:39:46]

Indeed!

+    fetch_when_ready_interactive_ = interactive_request;
+    fetch_when_ready_callback_ = callback;
   }
 
   MarkEmptyCategoriesAsLoading();
@@ -417,11 +389,12 @@ void RemoteSuggestionsProvider::FetchSnippets(
   NTPSnippetsFetcher::Params params = BuildFetchParams();
   params.interactive_request = interactive_request;
   snippets_fetcher_->FetchSnippets(
-      params, base::BindOnce(&RemoteSuggestionsProvider::OnFetchFinished,
-                             base::Unretained(this), interactive_request));
+      params,
+      base::BindOnce(&RemoteSuggestionsProviderImpl::OnFetchFinished,
+                     base::Unretained(this), callback, interactive_request));
 }
 
-void RemoteSuggestionsProvider::Fetch(
+void RemoteSuggestionsProviderImpl::Fetch(
     const Category& category,
     const std::set<std::string>& known_suggestion_ids,
     const FetchDoneCallback& callback) {
@@ -438,12 +411,14 @@ void RemoteSuggestionsProvider::Fetch(
   params.exclusive_category = category;
 
   snippets_fetcher_->FetchSnippets(
-      params, base::BindOnce(&RemoteSuggestionsProvider::OnFetchMoreFinished,
-                             base::Unretained(this), callback));
+      params,
+      base::BindOnce(&RemoteSuggestionsProviderImpl::OnFetchMoreFinished,
+                     base::Unretained(this), callback));
 }
 
 // Builds default fetcher params.
-NTPSnippetsFetcher::Params RemoteSuggestionsProvider::BuildFetchParams() const {
+NTPSnippetsFetcher::Params RemoteSuggestionsProviderImpl::BuildFetchParams()
+    const {
   NTPSnippetsFetcher::Params result;
   result.language_code = application_language_code_;
   result.count_to_fetch = kMaxSnippetCount;
@@ -456,7 +431,7 @@ NTPSnippetsFetcher::Params RemoteSuggestionsProvider::BuildFetchParams() const {
   return result;
 }
 
-void RemoteSuggestionsProvider::MarkEmptyCategoriesAsLoading() {
+void RemoteSuggestionsProviderImpl::MarkEmptyCategoriesAsLoading() {
   for (const auto& item : category_contents_) {
     Category category = item.first;
     const CategoryContent& content = item.second;
@@ -466,58 +441,20 @@ void RemoteSuggestionsProvider::MarkEmptyCategoriesAsLoading() {
   }
 }
 
-void RemoteSuggestionsProvider::RescheduleFetching(bool force) {
-  // The scheduler only exists on Android so far, it's null on other platforms.
-  if (!scheduler_) {
-    return;
-  }
-
-  if (ready()) {
-    base::TimeDelta old_interval_wifi = base::TimeDelta::FromInternalValue(
-        pref_service_->GetInt64(prefs::kSnippetBackgroundFetchingIntervalWifi));
-    base::TimeDelta old_interval_fallback =
-        base::TimeDelta::FromInternalValue(pref_service_->GetInt64(
-            prefs::kSnippetBackgroundFetchingIntervalFallback));
-    UserClassifier::UserClass user_class = user_classifier_->GetUserClass();
-    base::TimeDelta interval_wifi =
-        GetFetchingInterval(/*is_wifi=*/true, user_class);
-    base::TimeDelta interval_fallback =
-        GetFetchingInterval(/*is_wifi=*/false, user_class);
-    if (force || interval_wifi != old_interval_wifi ||
-        interval_fallback != old_interval_fallback) {
-      scheduler_->Schedule(interval_wifi, interval_fallback);
-      pref_service_->SetInt64(prefs::kSnippetBackgroundFetchingIntervalWifi,
-                              interval_wifi.ToInternalValue());
-      pref_service_->SetInt64(prefs::kSnippetBackgroundFetchingIntervalFallback,
-                              interval_fallback.ToInternalValue());
-    }
-  } else {
-    // If we're NOT_INITED, we don't know whether to schedule or unschedule.
-    // If |force| is false, all is well: We'll reschedule on the next state
-    // change anyway. If it's true, then unschedule here, to make sure that the
-    // next reschedule actually happens.
-    if (state_ != State::NOT_INITED || force) {
-      scheduler_->Unschedule();
-      pref_service_->ClearPref(prefs::kSnippetBackgroundFetchingIntervalWifi);
-      pref_service_->ClearPref(
-          prefs::kSnippetBackgroundFetchingIntervalFallback);
-    }
-  }
-}
-
-CategoryStatus RemoteSuggestionsProvider::GetCategoryStatus(Category category) {
+CategoryStatus RemoteSuggestionsProviderImpl::GetCategoryStatus(
+    Category category) {
   auto content_it = category_contents_.find(category);
   DCHECK(content_it != category_contents_.end());
   return content_it->second.status;
 }
 
-CategoryInfo RemoteSuggestionsProvider::GetCategoryInfo(Category category) {
+CategoryInfo RemoteSuggestionsProviderImpl::GetCategoryInfo(Category category) {
   auto content_it = category_contents_.find(category);
   DCHECK(content_it != category_contents_.end());
   return content_it->second.info;
 }
 
-void RemoteSuggestionsProvider::DismissSuggestion(
+void RemoteSuggestionsProviderImpl::DismissSuggestion(
     const ContentSuggestion::ID& suggestion_id) {
   if (!ready()) {
     return;
@@ -530,7 +467,7 @@ void RemoteSuggestionsProvider::DismissSuggestion(
                                        suggestion_id.id_within_category());
 }
 
-void RemoteSuggestionsProvider::ClearHistory(
+void RemoteSuggestionsProviderImpl::ClearHistory(
     base::Time begin,
     base::Time end,
     const base::Callback<bool(const GURL& url)>& filter) {
@@ -544,7 +481,7 @@ void RemoteSuggestionsProvider::ClearHistory(
   }
 }
 
-void RemoteSuggestionsProvider::ClearCachedSuggestions(Category category) {
+void RemoteSuggestionsProviderImpl::ClearCachedSuggestions(Category category) {
   if (!initialized()) {
     return;
   }
@@ -567,7 +504,7 @@ void RemoteSuggestionsProvider::ClearCachedSuggestions(Category category) {
   }
 }
 
-void RemoteSuggestionsProvider::OnSignInStateChanged() {
+void RemoteSuggestionsProviderImpl::OnSignInStateChanged() {
   // Make sure the status service is registered and we already initialised its
   // start state.
   if (!initialized()) {
@@ -577,7 +514,7 @@ void RemoteSuggestionsProvider::OnSignInStateChanged() {
   status_service_->OnSignInStateChanged();
 }
 
-void RemoteSuggestionsProvider::GetDismissedSuggestionsForDebugging(
+void RemoteSuggestionsProviderImpl::GetDismissedSuggestionsForDebugging(
     Category category,
     const DismissedSuggestionsCallback& callback) {
   auto content_it = category_contents_.find(category);
@@ -586,7 +523,7 @@ void RemoteSuggestionsProvider::GetDismissedSuggestionsForDebugging(
       ConvertToContentSuggestions(category, content_it->second.dismissed));
 }
 
-void RemoteSuggestionsProvider::ClearDismissedSuggestionsForDebugging(
+void RemoteSuggestionsProviderImpl::ClearDismissedSuggestionsForDebugging(
     Category category) {
   auto content_it = category_contents_.find(category);
   DCHECK(content_it != category_contents_.end());
@@ -607,14 +544,14 @@ void RemoteSuggestionsProvider::ClearDismissedSuggestionsForDebugging(
 }
 
 // static
-int RemoteSuggestionsProvider::GetMaxSnippetCountForTesting() {
+int RemoteSuggestionsProviderImpl::GetMaxSnippetCountForTesting() {
   return kMaxSnippetCount;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // Private methods
 
-GURL RemoteSuggestionsProvider::FindSnippetImageUrl(
+GURL RemoteSuggestionsProviderImpl::FindSnippetImageUrl(
     const ContentSuggestion::ID& suggestion_id) const {
   DCHECK(base::ContainsKey(category_contents_, suggestion_id.category()));
 
@@ -628,7 +565,7 @@ GURL RemoteSuggestionsProvider::FindSnippetImageUrl(
   return snippet->salient_image_url();
 }
 
-void RemoteSuggestionsProvider::OnDatabaseLoaded(
+void RemoteSuggestionsProviderImpl::OnDatabaseLoaded(
     NTPSnippet::PtrVector snippets) {
   if (state_ == State::ERROR_OCCURRED) {
     return;
@@ -684,12 +621,12 @@ void RemoteSuggestionsProvider::OnDatabaseLoaded(
   FinishInitialization();
 }
 
-void RemoteSuggestionsProvider::OnDatabaseError() {
+void RemoteSuggestionsProviderImpl::OnDatabaseError() {
   EnterState(State::ERROR_OCCURRED);
   UpdateAllCategoryStatus(CategoryStatus::LOADING_ERROR);
 }
 
-void RemoteSuggestionsProvider::OnFetchMoreFinished(
+void RemoteSuggestionsProviderImpl::OnFetchMoreFinished(
     const FetchDoneCallback& fetching_callback,
     Status status,
     NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories) {
@@ -749,7 +686,8 @@ void RemoteSuggestionsProvider::OnFetchMoreFinished(
   NotifyNewSuggestions(category, *existing_content);
 }
 
-void RemoteSuggestionsProvider::OnFetchFinished(
+void RemoteSuggestionsProviderImpl::OnFetchFinished(
+    const FetchStatusCallback& callback,
     bool interactive_request,
     Status status,
     NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories) {
@@ -824,16 +762,10 @@ void RemoteSuggestionsProvider::OnFetchFinished(
                          content.dismissed.size());
   }
 
-  // Reschedule after a successful fetch. This resets all currently scheduled
-  // fetches, to make sure the fallback interval triggers only if no wifi fetch
-  // succeeded, and also that we don't do a background fetch immediately after
-  // a user-initiated one.
-  if (fetched_categories) {
-    RescheduleFetching(true);
-  }
+  callback.Run(status);
 }
 
-void RemoteSuggestionsProvider::ArchiveSnippets(
+void RemoteSuggestionsProviderImpl::ArchiveSnippets(
     CategoryContent* content,
     NTPSnippet::PtrVector* to_archive) {
   // Archive previous snippets - move them at the beginning of the list.
@@ -854,7 +786,7 @@ void RemoteSuggestionsProvider::ArchiveSnippets(
   }
 }
 
-void RemoteSuggestionsProvider::SanitizeReceivedSnippets(
+void RemoteSuggestionsProviderImpl::SanitizeReceivedSnippets(
     const NTPSnippet::PtrVector& dismissed,
     NTPSnippet::PtrVector* snippets) {
   DCHECK(ready());
@@ -862,7 +794,7 @@ void RemoteSuggestionsProvider::SanitizeReceivedSnippets(
   RemoveIncompleteSnippets(snippets);
 }
 
-void RemoteSuggestionsProvider::IntegrateSnippets(
+void RemoteSuggestionsProviderImpl::IntegrateSnippets(
     CategoryContent* content,
     NTPSnippet::PtrVector new_snippets) {
   DCHECK(ready());
@@ -891,7 +823,7 @@ void RemoteSuggestionsProvider::IntegrateSnippets(
   content->snippets = std::move(new_snippets);
 }
 
-void RemoteSuggestionsProvider::DismissSuggestionFromCategoryContent(
+void RemoteSuggestionsProviderImpl::DismissSuggestionFromCategoryContent(
     CategoryContent* content,
     const std::string& id_within_category) {
   auto it = std::find_if(
@@ -911,7 +843,7 @@ void RemoteSuggestionsProvider::DismissSuggestionFromCategoryContent(
   content->snippets.erase(it);
 }
 
-void RemoteSuggestionsProvider::ClearExpiredDismissedSnippets() {
+void RemoteSuggestionsProviderImpl::ClearExpiredDismissedSnippets() {
   std::vector<Category> categories_to_erase;
 
   const base::Time now = base::Time::Now();
@@ -949,7 +881,7 @@ void RemoteSuggestionsProvider::ClearExpiredDismissedSnippets() {
   StoreCategoriesToPrefs();
 }
 
-void RemoteSuggestionsProvider::ClearOrphanedImages() {
+void RemoteSuggestionsProviderImpl::ClearOrphanedImages() {
   auto alive_snippets = base::MakeUnique<std::set<std::string>>();
   for (const auto& entry : category_contents_) {
     const CategoryContent& content = entry.second;
@@ -963,7 +895,7 @@ void RemoteSuggestionsProvider::ClearOrphanedImages() {
   database_->GarbageCollectImages(std::move(alive_snippets));
 }
 
-void RemoteSuggestionsProvider::NukeAllSnippets() {
+void RemoteSuggestionsProviderImpl::NukeAllSnippets() {
   std::vector<Category> categories_to_erase;
 
   // Empty the ARTICLES category and remove all others, since they may or may
@@ -989,7 +921,7 @@ void RemoteSuggestionsProvider::NukeAllSnippets() {
   StoreCategoriesToPrefs();
 }
 
-void RemoteSuggestionsProvider::FetchSuggestionImage(
+void RemoteSuggestionsProviderImpl::FetchSuggestionImage(
     const ContentSuggestion::ID& suggestion_id,
     const ImageFetchedCallback& callback) {
   if (!base::ContainsKey(category_contents_, suggestion_id.category())) {
@@ -1009,7 +941,7 @@ void RemoteSuggestionsProvider::FetchSuggestionImage(
   image_fetcher_.FetchSuggestionImage(suggestion_id, image_url, callback);
 }
 
-void RemoteSuggestionsProvider::EnterStateReady() {
+void RemoteSuggestionsProviderImpl::EnterStateReady() {
   if (nuke_when_initialized_) {
     NukeAllSnippets();
     nuke_when_initialized_ = false;
@@ -1024,7 +956,7 @@ void RemoteSuggestionsProvider::EnterStateReady() {
     // Either add a DCHECK here that we actually are allowed to do network I/O
     // or change the logic so that some explicit call is always needed for the
     // network request.
-    FetchSnippets(/*interactive_request=*/false);
+    FetchSnippets(fetch_when_ready_interactive_, fetch_when_ready_callback_);

[Marc Treib, 2016/12/19 12:59:23]

If we get here because our article suggestions are empty, then
fetch_when_ready_callback_ won't be initialized. Is that okay?

[jkrcal, 2016/12/20 16:39:46]

I believe so. Callbacks keep internally a pointer to the context and the default
initializer sets the pointer to nullptr. Nothing should be called in such a
case.

     fetch_when_ready_ = false;
   }
 
@@ -1039,15 +971,15 @@ void RemoteSuggestionsProvider::EnterStateReady() {
   }
 }
 
-void RemoteSuggestionsProvider::EnterStateDisabled() {
+void RemoteSuggestionsProviderImpl::EnterStateDisabled() {
   NukeAllSnippets();
 }
 
-void RemoteSuggestionsProvider::EnterStateError() {
+void RemoteSuggestionsProviderImpl::EnterStateError() {
   status_service_.reset();
 }
 
-void RemoteSuggestionsProvider::FinishInitialization() {
+void RemoteSuggestionsProviderImpl::FinishInitialization() {
   if (nuke_when_initialized_) {
     // We nuke here in addition to EnterStateReady, so that it happens even if
     // we enter the DISABLED state below.
@@ -1057,8 +989,8 @@ void RemoteSuggestionsProvider::FinishInitialization() {
 
   // Note: Initializing the status service will run the callback right away with
   // the current state.
-  status_service_->Init(base::Bind(&RemoteSuggestionsProvider::OnStatusChanged,
-                                   base::Unretained(this)));
+  status_service_->Init(base::Bind(
+      &RemoteSuggestionsProviderImpl::OnStatusChanged, base::Unretained(this)));
 
   // Always notify here even if we got nothing from the database, because we
   // don't know how long the fetch will take or if it will even complete.
@@ -1073,7 +1005,7 @@ void RemoteSuggestionsProvider::FinishInitialization() {
   }
 }
 
-void RemoteSuggestionsProvider::OnStatusChanged(
+void RemoteSuggestionsProviderImpl::OnStatusChanged(
     RemoteSuggestionsStatus old_status,
     RemoteSuggestionsStatus new_status) {
   switch (new_status) {
@@ -1083,7 +1015,8 @@ void RemoteSuggestionsProvider::OnStatusChanged(
         // Clear nonpersonalized suggestions.
         NukeAllSnippets();
         // Fetch personalized ones.
-        FetchSnippets(/*interactive_request=*/true);
+        // TODO(jkrcal): Loop in SchedulingRemoteSuggestionsProvider somehow.
+        FetchSnippets(/*interactive_request=*/true, FetchStatusCallback());
       } else {
         // Do not change the status. That will be done in EnterStateReady().
         EnterState(State::READY);
@@ -1096,7 +1029,8 @@ void RemoteSuggestionsProvider::OnStatusChanged(
         // Clear personalized suggestions.
         NukeAllSnippets();
         // Fetch nonpersonalized ones.
-        FetchSnippets(/*interactive_request=*/true);
+        // TODO(jkrcal): Loop in SchedulingRemoteSuggestionsProvider somehow.
+        FetchSnippets(/*interactive_request=*/true, FetchStatusCallback());
       } else {
         // Do not change the status. That will be done in EnterStateReady().
         EnterState(State::READY);
@@ -1115,7 +1049,7 @@ void RemoteSuggestionsProvider::OnStatusChanged(
   }
 }
 
-void RemoteSuggestionsProvider::EnterState(State state) {
+void RemoteSuggestionsProviderImpl::EnterState(State state) {
   if (state == state_) {
     return;
   }
@@ -1136,6 +1070,7 @@ void RemoteSuggestionsProvider::EnterState(State state) {
       DVLOG(1) << "Entering state: READY";
       state_ = State::READY;
       EnterStateReady();
+      provider_status_callback_.Run(ProviderStatus::ACTIVE);
       break;
 
     case State::DISABLED:
@@ -1144,24 +1079,23 @@ void RemoteSuggestionsProvider::EnterState(State state) {
       DVLOG(1) << "Entering state: DISABLED";
       state_ = State::DISABLED;
       EnterStateDisabled();
+      provider_status_callback_.Run(ProviderStatus::INACTIVE);
       break;
 
     case State::ERROR_OCCURRED:
       DVLOG(1) << "Entering state: ERROR_OCCURRED";
       state_ = State::ERROR_OCCURRED;
       EnterStateError();
+      provider_status_callback_.Run(ProviderStatus::INACTIVE);
       break;
 
     case State::COUNT:
       NOTREACHED();
       break;
   }
-
-  // Schedule or un-schedule background fetching after each state change.
-  RescheduleFetching(false);
 }
 
-void RemoteSuggestionsProvider::NotifyNewSuggestions(
+void RemoteSuggestionsProviderImpl::NotifyNewSuggestions(
     Category category,
     const CategoryContent& content) {
   DCHECK(IsCategoryStatusAvailable(content.status));
@@ -1174,8 +1108,9 @@ void RemoteSuggestionsProvider::NotifyNewSuggestions(
   observer()->OnNewSuggestions(this, category, std::move(result));
 }
 
-void RemoteSuggestionsProvider::UpdateCategoryStatus(Category category,
-                                                     CategoryStatus status) {
+void RemoteSuggestionsProviderImpl::UpdateCategoryStatus(
+    Category category,
+    CategoryStatus status) {
   auto content_it = category_contents_.find(category);
   DCHECK(content_it != category_contents_.end());
   CategoryContent& content = content_it->second;
@@ -1191,7 +1126,8 @@ void RemoteSuggestionsProvider::UpdateCategoryStatus(Category category,
   observer()->OnCategoryStatusChanged(this, category, content.status);
 }
 
-void RemoteSuggestionsProvider::UpdateAllCategoryStatus(CategoryStatus status) {
+void RemoteSuggestionsProviderImpl::UpdateAllCategoryStatus(
+    CategoryStatus status) {
   for (const auto& category : category_contents_) {
     UpdateCategoryStatus(category.first, status);
   }
@@ -1212,7 +1148,7 @@ typename T::const_iterator FindSnippetInContainer(
 
 }  // namespace
 
-const NTPSnippet* RemoteSuggestionsProvider::CategoryContent::FindSnippet(
+const NTPSnippet* RemoteSuggestionsProviderImpl::CategoryContent::FindSnippet(
     const std::string& id_within_category) const {
   // Search for the snippet in current and archived snippets.
   auto it = FindSnippetInContainer(snippets, id_within_category);
@@ -1230,9 +1166,9 @@ const NTPSnippet* RemoteSuggestionsProvider::CategoryContent::FindSnippet(
   return nullptr;
 }
 
-RemoteSuggestionsProvider::CategoryContent*
-RemoteSuggestionsProvider::UpdateCategoryInfo(Category category,
-                                              const CategoryInfo& info) {
+RemoteSuggestionsProviderImpl::CategoryContent*
+RemoteSuggestionsProviderImpl::UpdateCategoryInfo(Category category,
+                                                  const CategoryInfo& info) {
   auto content_it = category_contents_.find(category);
   if (content_it == category_contents_.end()) {
     content_it = category_contents_
@@ -1244,7 +1180,7 @@ RemoteSuggestionsProvider::UpdateCategoryInfo(Category category,
   return &content_it->second;
 }
 
-void RemoteSuggestionsProvider::RestoreCategoriesFromPrefs() {
+void RemoteSuggestionsProviderImpl::RestoreCategoriesFromPrefs() {
   // This must only be called at startup, before there are any categories.
   DCHECK(category_contents_.empty());
 
@@ -1295,7 +1231,7 @@ void RemoteSuggestionsProvider::RestoreCategoriesFromPrefs() {
   }
 }
 
-void RemoteSuggestionsProvider::StoreCategoriesToPrefs() {
+void RemoteSuggestionsProviderImpl::StoreCategoriesToPrefs() {
   // Collect all the CategoryContents.
   std::vector<std::pair<Category, const CategoryContent*>> to_store;
   for (const auto& entry : category_contents_) {
@@ -1327,17 +1263,17 @@ void RemoteSuggestionsProvider::StoreCategoriesToPrefs() {
   pref_service_->Set(prefs::kRemoteSuggestionCategories, list);
 }
 
-RemoteSuggestionsProvider::CategoryContent::CategoryContent(
+RemoteSuggestionsProviderImpl::CategoryContent::CategoryContent(
     const CategoryInfo& info)
     : info(info) {}
 
-RemoteSuggestionsProvider::CategoryContent::CategoryContent(CategoryContent&&) =
-    default;
+RemoteSuggestionsProviderImpl::CategoryContent::CategoryContent(
+    CategoryContent&&) = default;
 
-RemoteSuggestionsProvider::CategoryContent::~CategoryContent() = default;
+RemoteSuggestionsProviderImpl::CategoryContent::~CategoryContent() = default;
 
-RemoteSuggestionsProvider::CategoryContent&
-RemoteSuggestionsProvider::CategoryContent::operator=(CategoryContent&&) =
+RemoteSuggestionsProviderImpl::CategoryContent&
+RemoteSuggestionsProviderImpl::CategoryContent::operator=(CategoryContent&&) =
     default;
 
 }  // namespace ntp_snippets