[Remote suggestions] Move all decisions to fetch to the scheduler

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_impl.h"
 
 #include <algorithm>
 #include <iterator>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.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/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/threading/thread_task_runner_handle.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/image_fetcher/image_decoder.h"
 #include "components/image_fetcher/image_fetcher.h"
 #include "components/ntp_snippets/category_rankers/category_ranker.h"
 #include "components/ntp_snippets/pref_names.h"
 #include "components/ntp_snippets/remote/remote_suggestions_database.h"
+#include "components/ntp_snippets/remote/remote_suggestions_scheduler.h"
 #include "components/ntp_snippets/switches.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "grit/components_strings.h"
 #include "ui/gfx/image/image.h"
 
 namespace ntp_snippets {
 
 namespace {
 
@@ skipping 238 matching lines @@
       suggestions_fetcher_(std::move(suggestions_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),
       fetch_when_ready_callback_(nullptr),
-      provider_status_callback_(nullptr),
-      nuke_when_initialized_(false),
+      remote_suggestions_scheduler_(nullptr),
+      clear_any_history_when_initialized_(false),
       clock_(base::MakeUnique<base::DefaultClock>()) {
   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);
@@ skipping 20 matching lines @@
 
 // static
 void RemoteSuggestionsProviderImpl::RegisterProfilePrefs(
     PrefRegistrySimple* registry) {
   registry->RegisterListPref(prefs::kRemoteSuggestionCategories);
   registry->RegisterInt64Pref(prefs::kLastSuccessfulBackgroundFetchTime, 0);
 
   RemoteSuggestionsStatusService::RegisterProfilePrefs(registry);
 }
 
-void RemoteSuggestionsProviderImpl::SetProviderStatusCallback(
-    std::unique_ptr<ProviderStatusCallback> callback) {
-  provider_status_callback_ = std::move(callback);
+void RemoteSuggestionsProviderImpl::SetRemoteSuggestionsScheduler(
+    RemoteSuggestionsScheduler* scheduler) {
+  remote_suggestions_scheduler_ = scheduler;
   // Call the observer right away if we've reached any final state.
   NotifyStateChanged();
 }
 
 void RemoteSuggestionsProviderImpl::ReloadSuggestions() {
   FetchSuggestions(/*interactive_request=*/true,
                    /*callback=*/nullptr);
 }
 
 void RemoteSuggestionsProviderImpl::RefetchInTheBackground(
@@ skipping 98 matching lines @@
                                        suggestion_id.id_within_category());
 }
 
 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()) {
-    // No need to refresh the UI afterwards as we didn't provide any data to the
-    // UI so far.
-    nuke_when_initialized_ = true;
-  } else {
-    NukeAllSuggestions();
-  }
+  ClearAnyHistory();
 }
 
 void RemoteSuggestionsProviderImpl::ClearCachedSuggestions(Category category) {
   if (!initialized()) {
     return;
   }
 
   auto content_it = category_contents_.find(category);
   if (content_it == category_contents_.end()) {
     return;
@@ skipping 407 matching lines @@
     for (const auto& suggestion_ptr : content.suggestions) {
       alive_suggestions->insert(suggestion_ptr->id());
     }
     for (const auto& suggestion_ptr : content.dismissed) {
       alive_suggestions->insert(suggestion_ptr->id());
     }
   }
   database_->GarbageCollectImages(std::move(alive_suggestions));
 }
 
+void RemoteSuggestionsProviderImpl::ClearAnyHistory() {
+  if (!ready()) {
+    clear_any_history_when_initialized_ = true;
+    return;
+  }
+
+  if (remote_suggestions_scheduler_) {
+    remote_suggestions_scheduler_->OnHistoryCleared();

[tschumann, 2017/02/21 11:57:32]

this results in 2 calls to the scheduler -- would be nice to have the events
exclusive, wdyt?

[jkrcal, 2017/02/21 15:19:00]

Good point! Renamed the other function and moved its call.

+  }
+  NukeAllSuggestions();
+}
+
 void RemoteSuggestionsProviderImpl::NukeAllSuggestions() {
   for (const auto& item : category_contents_) {
     Category category = item.first;
     const CategoryContent& content = item.second;
 
     ClearCachedSuggestions(category);
     // Update listeners about the new (empty) state.
     if (IsCategoryStatusAvailable(content.status)) {
       NotifyNewSuggestions(category, content);
     }
     // TODO(tschumann): We should not call debug code from production code.
     ClearDismissedSuggestionsForDebugging(category);
   }
 
   StoreCategoriesToPrefs();
+  if (remote_suggestions_scheduler_) {
+    remote_suggestions_scheduler_->OnSuggestionsCleared();
+  }
 }
 
 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 = FindSuggestionImageUrl(suggestion_id);
   if (image_url.is_empty()) {
     // As we don't know the corresponding suggestion 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 RemoteSuggestionsProviderImpl::EnterStateReady() {
-  if (nuke_when_initialized_) {
-    NukeAllSuggestions();
-    nuke_when_initialized_ = false;
+  if (clear_any_history_when_initialized_) {
+    clear_any_history_when_initialized_ = false;
+    ClearAnyHistory();
   }
 
   auto article_category_it = category_contents_.find(articles_category_);
   DCHECK(article_category_it != category_contents_.end());
-  if (article_category_it->second.suggestions.empty() || fetch_when_ready_) {
-    // TODO(jkrcal): Fetching suggestions 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.
+  if (fetch_when_ready_) {
     FetchSuggestions(fetch_when_ready_interactive_,
                      std::move(fetch_when_ready_callback_));
     fetch_when_ready_ = false;
   }
 
   for (const auto& item : category_contents_) {
     Category category = item.first;
     const CategoryContent& content = item.second;
     // FetchSuggestions 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 RemoteSuggestionsProviderImpl::EnterStateDisabled() {
   NukeAllSuggestions();
 }
 
 void RemoteSuggestionsProviderImpl::EnterStateError() {
   status_service_.reset();
 }
 
 void RemoteSuggestionsProviderImpl::FinishInitialization() {
-  if (nuke_when_initialized_) {
-    // We nuke here in addition to EnterStateReady, so that it happens even if
+  if (clear_any_history_when_initialized_) {
+    // We clear here in addition to EnterStateReady, so that it happens even if
     // we enter the DISABLED state below.
-    NukeAllSuggestions();
-    nuke_when_initialized_ = false;
+    clear_any_history_when_initialized_ = false;
+    ClearAnyHistory();
   }
 
   // Note: Initializing the status service will run the callback right away with
   // the current state.
   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 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.
+        // Clear nonpersonalized suggestions (and notify the scheduler there are
+        // no suggestions).
         NukeAllSuggestions();
-        // Fetch personalized ones.
-        // TODO(jkrcal): Loop in SchedulingRemoteSuggestionsProvider somehow.
-        FetchSuggestions(/*interactive_request=*/true,
-                         /*callback=*/nullptr);
       } 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.
+        // Clear personalized suggestions (and notify the scheduler there are
+        // no suggestions).
         NukeAllSuggestions();
-        // Fetch nonpersonalized ones.
-        // TODO(jkrcal): Loop in SchedulingRemoteSuggestionsProvider somehow.
-        FetchSuggestions(/*interactive_request=*/true,
-                         /*callback=*/nullptr);
       } 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;
@@ skipping 39 matching lines @@
 
     case State::COUNT:
       NOTREACHED();
       break;
   }
 
   NotifyStateChanged();
 }
 
 void RemoteSuggestionsProviderImpl::NotifyStateChanged() {
-  if (!provider_status_callback_) {
+  if (!remote_suggestions_scheduler_) {
     return;
   }
 
   switch (state_) {
     case State::NOT_INITED:
       // Initial state, not sure yet whether active or not.
       break;
     case State::READY:
-      provider_status_callback_->Run(ProviderStatus::ACTIVE);
+      remote_suggestions_scheduler_->OnProviderActivated();
       break;
     case State::DISABLED:
-      provider_status_callback_->Run(ProviderStatus::INACTIVE);
+      remote_suggestions_scheduler_->OnProviderInactivated();
       break;
     case State::ERROR_OCCURRED:
-      provider_status_callback_->Run(ProviderStatus::INACTIVE);
+      remote_suggestions_scheduler_->OnProviderInactivated();
       break;
     case State::COUNT:
       NOTREACHED();
       break;
   }
 }
 
 void RemoteSuggestionsProviderImpl::NotifyNewSuggestions(
     Category category,
     const CategoryContent& content) {
@@ skipping 171 matching lines @@
 RemoteSuggestionsProviderImpl::CategoryContent::CategoryContent(
     CategoryContent&&) = default;
 
 RemoteSuggestionsProviderImpl::CategoryContent::~CategoryContent() = default;
 
 RemoteSuggestionsProviderImpl::CategoryContent&
 RemoteSuggestionsProviderImpl::CategoryContent::operator=(CategoryContent&&) =
     default;
 
 }  // namespace ntp_snippets