[NTP Snippets] Refactor background scheduling for remote suggestions

components/ntp_snippets/remote/remote_suggestions_provider_impl.cc

 // 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.
 
-#include "components/ntp_snippets/remote/remote_suggestions_provider.h"
+#include "components/ntp_snippets/remote/remote_suggestions_provider_impl.h"
 
 #include <algorithm>
 #include <iterator>
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/feature_list.h"
 #include "base/location.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/path_service.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/default_clock.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "components/data_use_measurement/core/data_use_user_data.h"
 #include "components/history/core/browser/history_service.h"
 #include "components/image_fetcher/image_decoder.h"
 #include "components/image_fetcher/image_fetcher.h"
 #include "components/ntp_snippets/features.h"
 #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"
 #include "grit/components_strings.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/image/image.h"
 
 namespace ntp_snippets {
 
 namespace {
 
 // Number of snippets requested to the server. Consider replacing sparse UMA
 // histograms with COUNTS() if this number increases beyond 50.
 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";
 const char kCategoryContentProvidedByServer[] = "provided_by_server";
 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>>();
   for (const auto& snippet : snippets) {
     result->push_back(snippet->id());
   }
   return result;
 }
 
 bool HasIntersection(const std::vector<std::string>& a,
@@ skipping 148 matching lines @@
 }
 
 void CachedImageFetcher::OnSnippetImageFetchedFromDatabase(
     const ImageFetchedCallback& callback,
     const ContentSuggestion::ID& suggestion_id,
     const GURL& url,
     std::string data) {  // SnippetImageCallback requires nonconst reference.
   // |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.
   FetchSnippetImageFromNetwork(suggestion_id, url, callback);
 }
 
 void CachedImageFetcher::OnSnippetImageDecodedFromDatabase(
     const ImageFetchedCallback& callback,
     const ContentSuggestion::ID& suggestion_id,
     const GURL& url,
@@ skipping 20 matching lines @@
     callback.Run(gfx::Image());
     return;
   }
 
   image_fetcher_->StartOrQueueNetworkRequest(
       suggestion_id.id_within_category(), url,
       base::Bind(&CachedImageFetcher::OnImageDecodingDone,
                  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),
                      std::move(image_decoder),
                      pref_service,
                      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);
 
   RestoreCategoriesFromPrefs();
   // The articles category always exists. Add it if we didn't get it from prefs.
   // TODO(treib): Rethink this.
   category_contents_.insert(
       std::make_pair(articles_category_,
                      CategoryContent(BuildArticleCategoryInfo(base::nullopt))));
   // Tell the observer about all the categories.
   for (const auto& entry : category_contents_) {
     observer->OnCategoryStatusChanged(this, entry.first, entry.second.status);
   }
 
   if (database_->IsErrorState()) {
     EnterState(State::ERROR_OCCURRED);
     UpdateAllCategoryStatus(CategoryStatus::LOADING_ERROR);
     return;
   }
 
   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 RemoteSuggestionsProvider::FetchSnippetsForAllCategories() {
-  // TODO(markusheintz): Investigate whether we can call the Fetch method
-  // instead of the FetchSnippets.
-  FetchSnippets(/*interactive_request=*/true);
+void RemoteSuggestionsProviderImpl::ReloadSuggestions() {
+  FetchSnippets(/*interactive_request=*/true, FetchStatusCallback());
 }
 
-void RemoteSuggestionsProvider::FetchSnippets(
-    bool interactive_request) {
+void RemoteSuggestionsProviderImpl::RefetchInTheBackground(
+    const FetchStatusCallback& callback) {
+  FetchSnippets(/*interactive_request=*/false, callback);
+}
+
+const NTPSnippetsFetcher*
+RemoteSuggestionsProviderImpl::snippets_fetcher_for_testing_and_debugging()
+    const {
+  return snippets_fetcher_.get();
+}
+
+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();
 
   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) {
   if (!ready()) {
     CallWithEmptyResults(callback,
                          Status(StatusCode::TEMPORARY_ERROR,
                                 "RemoteSuggestionsProvider is not ready!"));
     return;
   }
   NTPSnippetsFetcher::Params params = BuildFetchParams();
   params.excluded_ids.insert(known_suggestion_ids.begin(),
                              known_suggestion_ids.end());
   params.interactive_request = true;
   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;
   for (const auto& map_entry : category_contents_) {
     const CategoryContent& content = map_entry.second;
     for (const auto& dismissed_snippet : content.dismissed) {
       result.excluded_ids.insert(dismissed_snippet->id());
     }
   }
   return result;
 }
 
-void RemoteSuggestionsProvider::MarkEmptyCategoriesAsLoading() {
+void RemoteSuggestionsProviderImpl::MarkEmptyCategoriesAsLoading() {
   for (const auto& item : category_contents_) {
     Category category = item.first;
     const CategoryContent& content = item.second;
     if (content.snippets.empty()) {
       UpdateCategoryStatus(category, CategoryStatus::AVAILABLE_LOADING);
     }
   }
 }
 
-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;
   }
 
   auto content_it = category_contents_.find(suggestion_id.category());
   DCHECK(content_it != category_contents_.end());
   CategoryContent* content = &content_it->second;
   DismissSuggestionFromCategoryContent(content,
                                        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) {
   // Both time range and the filter are ignored and all suggestions are removed,
   // because it is not known which history entries were used for the suggestions
   // personalization.
   if (!ready()) {
     nuke_when_initialized_ = true;
   } else {
     NukeAllSnippets();
   }
 }
 
-void RemoteSuggestionsProvider::ClearCachedSuggestions(Category category) {
+void RemoteSuggestionsProviderImpl::ClearCachedSuggestions(Category category) {
   if (!initialized()) {
     return;
   }
 
   auto content_it = category_contents_.find(category);
   if (content_it == category_contents_.end()) {
     return;
   }
   CategoryContent* content = &content_it->second;
   if (content->snippets.empty()) {
     return;
   }
 
   database_->DeleteSnippets(GetSnippetIDVector(content->snippets));
   database_->DeleteImages(GetSnippetIDVector(content->snippets));
   content->snippets.clear();
 
   if (IsCategoryStatusAvailable(content->status)) {
     NotifyNewSuggestions(category, *content);
   }
 }
 
-void RemoteSuggestionsProvider::OnSignInStateChanged() {
+void RemoteSuggestionsProviderImpl::OnSignInStateChanged() {
   // Make sure the status service is registered and we already initialised its
   // start state.
   if (!initialized()) {
     return;
   }
 
   status_service_->OnSignInStateChanged();
 }
 
-void RemoteSuggestionsProvider::GetDismissedSuggestionsForDebugging(
+void RemoteSuggestionsProviderImpl::GetDismissedSuggestionsForDebugging(
     Category category,
     const DismissedSuggestionsCallback& callback) {
   auto content_it = category_contents_.find(category);
   DCHECK(content_it != category_contents_.end());
   callback.Run(
       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());
   CategoryContent* content = &content_it->second;
 
   if (!initialized()) {
     return;
   }
 
   if (content->dismissed.empty()) {
     return;
   }
 
   database_->DeleteSnippets(GetSnippetIDVector(content->dismissed));
   // The image got already deleted when the suggestion was dismissed.
 
   content->dismissed.clear();
 }
 
 // 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()));
 
   const CategoryContent& content =
       category_contents_.at(suggestion_id.category());
   const NTPSnippet* snippet =
       content.FindSnippet(suggestion_id.id_within_category());
   if (!snippet) {
     return GURL();
   }
   return snippet->salient_image_url();
 }
 
-void RemoteSuggestionsProvider::OnDatabaseLoaded(
+void RemoteSuggestionsProviderImpl::OnDatabaseLoaded(
     NTPSnippet::PtrVector snippets) {
   if (state_ == State::ERROR_OCCURRED) {
     return;
   }
   DCHECK(state_ == State::NOT_INITED);
   DCHECK(base::ContainsKey(category_contents_, articles_category_));
 
   base::TimeDelta database_load_time =
       base::TimeTicks::Now() - database_load_start_;
   UMA_HISTOGRAM_MEDIUM_TIMES("NewTabPage.Snippets.DatabaseLoadTime",
@@ skipping 35 matching lines @@
   }
 
   // TODO(tschumann): If I move ClearExpiredDismissedSnippets() to the beginning
   // of the function, it essentially does nothing but tests are still green. Fix
   // this!
   ClearExpiredDismissedSnippets();
   ClearOrphanedImages();
   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) {
   if (!fetched_categories) {
     DCHECK(!status.IsSuccess());
     CallWithEmptyResults(fetching_callback, status);
     return;
   }
   if (fetched_categories->size() != 1u) {
     LOG(DFATAL) << "Requested one exclusive category but received "
@@ skipping 39 matching lines @@
   // e.g. to make sure we don't serve results after the sign-out. Revisit this
   // once the snippets fetcher supports concurrent requests. We can then see if
   // Nuke should also cancel outstanding requests or we want to check the
   // status.
   UpdateCategoryStatus(category, CategoryStatus::AVAILABLE);
   // Notify callers and observers.
   fetching_callback.Run(Status::Success(), std::move(result));
   NotifyNewSuggestions(category, *existing_content);
 }
 
-void RemoteSuggestionsProvider::OnFetchFinished(
+void RemoteSuggestionsProviderImpl::OnFetchFinished(
+    const FetchStatusCallback& callback,
     bool interactive_request,
     Status status,
     NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories) {
   if (!ready()) {
     // TODO(tschumann): What happens if this was a user-triggered, interactive
     // request? Is the UI waiting indefinitely now?
     return;
   }
 
   // Record the fetch time of a successfull background fetch.
@@ skipping 54 matching lines @@
   auto content_it = category_contents_.find(articles_category_);
   DCHECK(content_it != category_contents_.end());
   const CategoryContent& content = content_it->second;
   UMA_HISTOGRAM_SPARSE_SLOWLY("NewTabPage.Snippets.NumArticles",
                               content.snippets.size());
   if (content.snippets.empty() && !content.dismissed.empty()) {
     UMA_HISTOGRAM_COUNTS("NewTabPage.Snippets.NumArticlesZeroDueToDiscarded",
                          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.
   content->archived.insert(content->archived.begin(),
                            std::make_move_iterator(to_archive->begin()),
                            std::make_move_iterator(to_archive->end()));
   to_archive->clear();
 
   // If there are more archived snippets than we want to keep, delete the
   // oldest ones by their fetch time (which are always in the back).
   if (content->archived.size() > kMaxArchivedSnippetCount) {
     NTPSnippet::PtrVector to_delete(
         std::make_move_iterator(content->archived.begin() +
                                 kMaxArchivedSnippetCount),
         std::make_move_iterator(content->archived.end()));
     content->archived.resize(kMaxArchivedSnippetCount);
     database_->DeleteImages(GetSnippetIDVector(to_delete));
   }
 }
 
-void RemoteSuggestionsProvider::SanitizeReceivedSnippets(
+void RemoteSuggestionsProviderImpl::SanitizeReceivedSnippets(
     const NTPSnippet::PtrVector& dismissed,
     NTPSnippet::PtrVector* snippets) {
   DCHECK(ready());
   EraseMatchingSnippets(snippets, dismissed);
   RemoveIncompleteSnippets(snippets);
 }
 
-void RemoteSuggestionsProvider::IntegrateSnippets(
+void RemoteSuggestionsProviderImpl::IntegrateSnippets(
     CategoryContent* content,
     NTPSnippet::PtrVector new_snippets) {
   DCHECK(ready());
 
   // Do not touch the current set of snippets if the newly fetched one is empty.
   // TODO(tschumann): This should go. If we get empty results we should update
   // accordingly and remove the old one (only of course if this was not received
   // through a fetch-more).
   if (new_snippets.empty()) {
     return;
   }
 
   // It's entirely possible that the newly fetched snippets contain articles
   // that have been present before.
   // We need to make sure to only delete and archive snippets that don't
   // appear with the same ID in the new suggestions (it's fine for additional
   // IDs though).
   EraseByPrimaryID(&content->snippets, *GetSnippetIDVector(new_snippets));
   // Do not delete the thumbnail images as they are still handy on open NTPs.
   database_->DeleteSnippets(GetSnippetIDVector(content->snippets));
   // Note, that ArchiveSnippets will clear |content->snippets|.
   ArchiveSnippets(content, &content->snippets);
 
   database_->SaveSnippets(new_snippets);
 
   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(
       content->snippets.begin(), content->snippets.end(),
       [&id_within_category](const std::unique_ptr<NTPSnippet>& snippet) {
         return snippet->id() == id_within_category;
       });
   if (it == content->snippets.end()) {
     return;
   }
 
   (*it)->set_dismissed(true);
 
   database_->SaveSnippet(**it);
 
   content->dismissed.push_back(std::move(*it));
   content->snippets.erase(it);
 }
 
-void RemoteSuggestionsProvider::ClearExpiredDismissedSnippets() {
+void RemoteSuggestionsProviderImpl::ClearExpiredDismissedSnippets() {
   std::vector<Category> categories_to_erase;
 
   const base::Time now = base::Time::Now();
 
   for (auto& item : category_contents_) {
     Category category = item.first;
     CategoryContent* content = &item.second;
 
     NTPSnippet::PtrVector to_delete;
     // Move expired dismissed snippets over into |to_delete|.
@@ skipping 17 matching lines @@
   }
 
   for (Category category : categories_to_erase) {
     UpdateCategoryStatus(category, CategoryStatus::NOT_PROVIDED);
     category_contents_.erase(category);
   }
 
   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;
     for (const auto& snippet_ptr : content.snippets) {
       alive_snippets->insert(snippet_ptr->id());
     }
     for (const auto& snippet_ptr : content.dismissed) {
       alive_snippets->insert(snippet_ptr->id());
     }
   }
   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
   // not be personalized.
   for (const auto& item : category_contents_) {
     Category category = item.first;
 
     ClearCachedSuggestions(category);
     ClearDismissedSuggestionsForDebugging(category);
 
     UpdateCategoryStatus(category, CategoryStatus::NOT_PROVIDED);
 
     // Remove the category entirely; it may or may not reappear.
     if (category != articles_category_) {
       categories_to_erase.push_back(category);
     }
   }
 
   for (Category category : categories_to_erase) {
     category_contents_.erase(category);
   }
 
   StoreCategoriesToPrefs();
 }
 
-void RemoteSuggestionsProvider::FetchSuggestionImage(
+void RemoteSuggestionsProviderImpl::FetchSuggestionImage(
     const ContentSuggestion::ID& suggestion_id,
     const ImageFetchedCallback& callback) {
   if (!base::ContainsKey(category_contents_, suggestion_id.category())) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(callback, gfx::Image()));
     return;
   }
   GURL image_url = FindSnippetImageUrl(suggestion_id);
   if (image_url.is_empty()) {
     // As we don't know the corresponding snippet anymore, we don't expect to
     // find it in the database (and also can't fetch it remotely). Cut the
     // lookup short and return directly.
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(callback, gfx::Image()));
     return;
   }
   image_fetcher_.FetchSuggestionImage(suggestion_id, image_url, callback);
 }
 
-void RemoteSuggestionsProvider::EnterStateReady() {
+void RemoteSuggestionsProviderImpl::EnterStateReady() {
   if (nuke_when_initialized_) {
     NukeAllSnippets();
     nuke_when_initialized_ = false;
   }
 
   auto article_category_it = category_contents_.find(articles_category_);
   DCHECK(article_category_it != category_contents_.end());
   if (article_category_it->second.snippets.empty() || fetch_when_ready_) {
     // TODO(jkrcal): Fetching snippets automatically upon creation of this
     // lazily created service can cause troubles, e.g. in unit tests where
     // network I/O is not allowed.
     // 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;
   }
 
   for (const auto& item : category_contents_) {
     Category category = item.first;
     const CategoryContent& content = item.second;
     // FetchSnippets has set the status to |AVAILABLE_LOADING| if relevant,
     // otherwise we transition to |AVAILABLE| here.
     if (content.status != CategoryStatus::AVAILABLE_LOADING) {
       UpdateCategoryStatus(category, CategoryStatus::AVAILABLE);
     }
   }
 }
 
-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.
     NukeAllSnippets();
     nuke_when_initialized_ = false;
   }
 
   // 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.
   for (const auto& item : category_contents_) {
     Category category = item.first;
     const CategoryContent& content = item.second;
     // Note: We might be in a non-available status here, e.g. DISABLED due to
     // enterprise policy.
     if (IsCategoryStatusAvailable(content.status)) {
       NotifyNewSuggestions(category, content);
     }
   }
 }
 
-void RemoteSuggestionsProvider::OnStatusChanged(
+void RemoteSuggestionsProviderImpl::OnStatusChanged(
     RemoteSuggestionsStatus old_status,
     RemoteSuggestionsStatus new_status) {
   switch (new_status) {
     case RemoteSuggestionsStatus::ENABLED_AND_SIGNED_IN:
       if (old_status == RemoteSuggestionsStatus::ENABLED_AND_SIGNED_OUT) {
         DCHECK(state_ == State::READY);
         // 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);
       }
       break;
 
     case RemoteSuggestionsStatus::ENABLED_AND_SIGNED_OUT:
       if (old_status == RemoteSuggestionsStatus::ENABLED_AND_SIGNED_IN) {
         DCHECK(state_ == State::READY);
         // 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);
       }
       break;
 
     case RemoteSuggestionsStatus::EXPLICITLY_DISABLED:
       EnterState(State::DISABLED);
       UpdateAllCategoryStatus(CategoryStatus::CATEGORY_EXPLICITLY_DISABLED);
       break;
 
     case RemoteSuggestionsStatus::SIGNED_OUT_AND_DISABLED:
       EnterState(State::DISABLED);
       UpdateAllCategoryStatus(CategoryStatus::SIGNED_OUT);
       break;
   }
 }
 
-void RemoteSuggestionsProvider::EnterState(State state) {
+void RemoteSuggestionsProviderImpl::EnterState(State state) {
   if (state == state_) {
     return;
   }
 
   UMA_HISTOGRAM_ENUMERATION("NewTabPage.Snippets.EnteredState",
                             static_cast<int>(state),
                             static_cast<int>(State::COUNT));
 
   switch (state) {
     case State::NOT_INITED:
       // Initial state, it should not be possible to get back there.
       NOTREACHED();
       break;
 
     case State::READY:
       DCHECK(state_ == State::NOT_INITED || state_ == State::DISABLED);
 
       DVLOG(1) << "Entering state: READY";
       state_ = State::READY;
       EnterStateReady();
+      provider_status_callback_.Run(ProviderStatus::ACTIVE);
       break;
 
     case State::DISABLED:
       DCHECK(state_ == State::NOT_INITED || state_ == State::READY);
 
       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));
 
   std::vector<ContentSuggestion> result =
       ConvertToContentSuggestions(category, content.snippets);
 
   DVLOG(1) << "NotifyNewSuggestions(): " << result.size()
            << " items in category " << category;
   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;
 
   if (status == content.status) {
     return;
   }
 
   DVLOG(1) << "UpdateCategoryStatus(): " << category.id() << ": "
            << static_cast<int>(content.status) << " -> "
            << static_cast<int>(status);
   content.status = status;
   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);
   }
 }
 
 namespace {
 
 template <typename T>
 typename T::const_iterator FindSnippetInContainer(
     const T& container,
     const std::string& id_within_category) {
   return std::find_if(
       container.begin(), container.end(),
       [&id_within_category](const std::unique_ptr<NTPSnippet>& snippet) {
         return snippet->id() == id_within_category;
       });
 }
 
 }  // 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);
   if (it != snippets.end()) {
     return it->get();
   }
   auto archived_it = FindSnippetInContainer(archived, id_within_category);
   if (archived_it != archived.end()) {
     return archived_it->get();
   }
   auto dismissed_it = FindSnippetInContainer(dismissed, id_within_category);
   if (dismissed_it != dismissed.end()) {
     return dismissed_it->get();
   }
   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_
                      .insert(std::make_pair(category, CategoryContent(info)))
                      .first;
   } else {
     content_it->second.info = info;
   }
   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());
 
   const base::ListValue* list =
       pref_service_->GetList(prefs::kRemoteSuggestionCategories);
   for (const std::unique_ptr<base::Value>& entry : *list) {
     const base::DictionaryValue* dict = nullptr;
     if (!entry->GetAsDictionary(&dict)) {
       DLOG(WARNING) << "Invalid category pref value: " << *entry;
       continue;
@@ skipping 30 matching lines @@
     CategoryInfo info =
         category == articles_category_
             ? BuildArticleCategoryInfo(title)
             : BuildRemoteCategoryInfo(title, allow_fetching_more_results);
     CategoryContent* content = UpdateCategoryInfo(category, info);
     content->included_in_last_server_response =
         included_in_last_server_response;
   }
 }
 
-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_) {
     to_store.emplace_back(entry.first, &entry.second);
   }
   // Sort them into the proper category order.
   std::sort(to_store.begin(), to_store.end(),
             [this](const std::pair<Category, const CategoryContent*>& left,
                    const std::pair<Category, const CategoryContent*>& right) {
               return category_factory()->CompareCategories(left.first,
@@ skipping 11 matching lines @@
     dict->SetBoolean(kCategoryContentProvidedByServer,
                      content.included_in_last_server_response);
     dict->SetBoolean(kCategoryContentAllowFetchingMore,
                      content.info.has_more_action());
     list.Append(std::move(dict));
   }
   // Finally, store the result in the pref service.
   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