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

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

Issue 2702223004: [Remote suggestions] Move all decisions to fetch to the scheduler (Closed)
Patch Set: Fixing an embarassing bug :) Created 3 years, 10 months 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_impl.h b/components/ntp_snippets/remote/remote_suggestions_provider_impl.h
index 5fc9f77a9ebd8ccfe401e24fdeb321d0a2522c32..ecd1566b2b3d0478296cd25c2468c382a55e368a 100644
--- a/components/ntp_snippets/remote/remote_suggestions_provider_impl.h
+++ b/components/ntp_snippets/remote/remote_suggestions_provider_impl.h
@@ -47,6 +47,7 @@ namespace ntp_snippets {
class CategoryRanker;
class RemoteSuggestionsDatabase;
+class RemoteSuggestionsScheduler;
// CachedImageFetcher takes care of fetching images from the network and caching
// them in the database.
@@ -136,9 +137,13 @@ class RemoteSuggestionsProviderImpl final : public RemoteSuggestionsProvider {
// false, some calls may trigger DCHECKs.
bool initialized() const { return ready() || state_ == State::DISABLED; }
+ // Set the scheduler to be notified whenever the provider becomes active /
+ // in-active and whenever history is deleted. The initial change is also
+ // notified (switching from an initial undecided status). If the scheduler is
+ // set after the first change, it is called back immediately.
+ void SetRemoteSuggestionsScheduler(RemoteSuggestionsScheduler* scheduler);
+
// RemoteSuggestionsProvider implementation.
- void SetProviderStatusCallback(
- std::unique_ptr<ProviderStatusCallback> callback) override;
void RefetchInTheBackground(
std::unique_ptr<FetchStatusCallback> callback) override;
@@ -196,21 +201,19 @@ class RemoteSuggestionsProviderImpl final : public RemoteSuggestionsProvider {
private:
friend class RemoteSuggestionsProviderImplTest;
+ // TODO(jkrcal): Mock the database to trigger the error naturally (or remove
+ // the error state and get rid of the test).
FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
- CallsProviderStatusCallbackWhenReady);
- FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
- CallsProviderStatusCallbackOnError);
- FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
- CallsProviderStatusCallbackWhenDisabled);
+ CallsSchedulerOnError);
+ // TODO(jkrcal): Mock the status service and remove these friend declarations.
FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
- ShouldNotCrashWhenCallingEmptyCallback);
+ CallsSchedulerWhenDisabled);
FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
DontNotifyIfNotAvailable);
FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
- RemoveExpiredDismissedContent);
- FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest, StatusChanges);
+ CallsSchedulerWhenSignedIn);
FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
- SuggestionsFetchedOnSignInAndSignOut);
+ CallsSchedulerWhenSignedOut);
// Possible state transitions:
// NOT_INITED --------+
@@ -220,6 +223,8 @@ class RemoteSuggestionsProviderImpl final : public RemoteSuggestionsProvider {
// \ / |
// v v |
// ERROR_OCCURRED <-----+
+ // TODO(jkrcal): Do we need to keep the distinction between states DISABLED
+ // and ERROR_OCCURED?
enum class State {
// The service has just been created. Can change to states:
// - DISABLED: After the database is done loading,
@@ -352,6 +357,13 @@ class RemoteSuggestionsProviderImpl final : public RemoteSuggestionsProvider {
// the file system.
void ClearOrphanedImages();
+ // Clears suggestions because any history item has been removed.
+ void ClearHistoryDependentState();
+
+ // Clears suggestions for any non-history related reason (e.g., sign-in status
+ // change, etc.).
+ void ClearSuggestions();
+
// Clears all stored suggestions and updates the observer.
void NukeAllSuggestions();
@@ -442,12 +454,12 @@ class RemoteSuggestionsProviderImpl final : public RemoteSuggestionsProvider {
bool fetch_when_ready_interactive_;
std::unique_ptr<FetchStatusCallback> fetch_when_ready_callback_;
- std::unique_ptr<ProviderStatusCallback> provider_status_callback_;
+ RemoteSuggestionsScheduler* remote_suggestions_scheduler_;
- // Set to true if NukeAllSuggestions is called while the service isn't ready.
- // The nuke will be executed once the service finishes initialization or
- // enters the READY state.
- bool nuke_when_initialized_;
+ // Set to true if ClearHistoryDependentState is called while the service isn't
+ // ready. The nuke will be executed once the service finishes initialization
+ // or enters the READY state.
+ bool clear_history_dependent_state_when_initialized_;
// A clock for getting the time. This allows to inject a clock in tests.
std::unique_ptr<base::Clock> clock_;

Powered by Google App Engine
This is Rietveld 408576698