Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(465)

Unified Diff: components/ntp_snippets/remote/remote_suggestions_provider_impl.h

Issue 2557363002: [NTP Snippets] Refactor background scheduling for remote suggestions (Closed)
Patch Set: Fixing the last changes :) Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/ntp_snippets/remote/remote_suggestions_provider_impl.h
diff --git a/components/ntp_snippets/remote/remote_suggestions_provider.h b/components/ntp_snippets/remote/remote_suggestions_provider_impl.h
similarity index 87%
copy from components/ntp_snippets/remote/remote_suggestions_provider.h
copy to components/ntp_snippets/remote/remote_suggestions_provider_impl.h
index e743b42951a75c8322bf0bcf98ac7a4fd16d3fbb..9301dd83cdf8e776b9117a3f0cb0affba4f75011 100644
--- a/components/ntp_snippets/remote/remote_suggestions_provider.h
+++ b/components/ntp_snippets/remote/remote_suggestions_provider_impl.h
@@ -2,8 +2,8 @@
// 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>
@@ -27,7 +27,7 @@
#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"
@@ -45,9 +45,8 @@ class ImageFetcher;
namespace ntp_snippets {
-class RemoteSuggestionsDatabase;
class CategoryRanker;
-class UserClassifier;
+class RemoteSuggestionsDatabase;
// CachedImageFetcher takes care of fetching images from the network and caching
// them in the database.
@@ -107,26 +106,24 @@ class CachedImageFetcher : public image_fetcher::ImageFetcherDelegate {
// 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);
@@ -135,33 +132,22 @@ class RemoteSuggestionsProvider final : public ContentSuggestionsProvider {
// 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();
-
- // 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();
+ // RemoteSuggestionsProvider implementation.
+ void SetProviderStatusCallback(
+ std::unique_ptr<ProviderStatusCallback> callback) override;
+ void RefetchInTheBackground(
+ std::unique_ptr<FetchStatusCallback> callback) override;
- // 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();
- }
-
- // (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);
+ const NTPSnippetsFetcher* snippets_fetcher_for_testing_and_debugging()
+ const override;
- // ContentSuggestionsProvider implementation
+ // ContentSuggestionsProvider implementation.
CategoryStatus GetCategoryStatus(Category category) override;
CategoryInfo GetCategoryInfo(Category category) override;
void DismissSuggestion(const ContentSuggestion::ID& suggestion_id) override;
@@ -170,6 +156,7 @@ class RemoteSuggestionsProvider final : public ContentSuggestionsProvider {
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,
@@ -206,16 +193,22 @@ class RemoteSuggestionsProvider final : public ContentSuggestionsProvider {
CachedImageFetcher& GetImageFetcherForTesting() { return image_fetcher_; }
private:
- friend class RemoteSuggestionsProviderTest;
-
- FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderTest,
+ friend class RemoteSuggestionsProviderImplTest;
+
+ 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:
@@ -296,8 +289,10 @@ class RemoteSuggestionsProvider final : public ContentSuggestionsProvider {
// 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
@@ -316,6 +311,7 @@ class RemoteSuggestionsProvider final : public ContentSuggestionsProvider {
// Callback for regular fetch requests with the NTPSnippetsFetcher.
void OnFetchFinished(
+ std::unique_ptr<FetchStatusCallback> callback,
bool interactive_request,
Status status,
NTPSnippetsFetcher::OptionalFetchedCategories fetched_categories);
@@ -368,6 +364,10 @@ class RemoteSuggestionsProvider final : public ContentSuggestionsProvider {
// 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();
@@ -415,12 +415,6 @@ class RemoteSuggestionsProvider final : public ContentSuggestionsProvider {
// 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_;
@@ -436,8 +430,15 @@ class RemoteSuggestionsProvider final : public ContentSuggestionsProvider {
// 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.
+ 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.
@@ -446,9 +447,9 @@ class RemoteSuggestionsProvider final : public ContentSuggestionsProvider {
// 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_

Powered by Google App Engine
This is Rietveld 408576698