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

Side by Side 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, 9 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 unified diff | Download patch
OLDNEW
1 // Copyright 2015 The Chromium Authors. All rights reserved. 1 // Copyright 2015 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #ifndef COMPONENTS_NTP_SNIPPETS_REMOTE_REMOTE_SUGGESTIONS_PROVIDER_IMPL_H_ 5 #ifndef COMPONENTS_NTP_SNIPPETS_REMOTE_REMOTE_SUGGESTIONS_PROVIDER_IMPL_H_
6 #define COMPONENTS_NTP_SNIPPETS_REMOTE_REMOTE_SUGGESTIONS_PROVIDER_IMPL_H_ 6 #define COMPONENTS_NTP_SNIPPETS_REMOTE_REMOTE_SUGGESTIONS_PROVIDER_IMPL_H_
7 7
8 #include <cstddef> 8 #include <cstddef>
9 #include <deque> 9 #include <deque>
10 #include <map> 10 #include <map>
(...skipping 29 matching lines...) Expand all
40 40
41 namespace image_fetcher { 41 namespace image_fetcher {
42 class ImageDecoder; 42 class ImageDecoder;
43 class ImageFetcher; 43 class ImageFetcher;
44 } // namespace image_fetcher 44 } // namespace image_fetcher
45 45
46 namespace ntp_snippets { 46 namespace ntp_snippets {
47 47
48 class CategoryRanker; 48 class CategoryRanker;
49 class RemoteSuggestionsDatabase; 49 class RemoteSuggestionsDatabase;
50 class RemoteSuggestionsScheduler;
50 51
51 // CachedImageFetcher takes care of fetching images from the network and caching 52 // CachedImageFetcher takes care of fetching images from the network and caching
52 // them in the database. 53 // them in the database.
53 // TODO(tschumann): Move into a separate library and inject the 54 // TODO(tschumann): Move into a separate library and inject the
54 // CachedImageFetcher into the RemoteSuggestionsProvider. This allows us to get 55 // CachedImageFetcher into the RemoteSuggestionsProvider. This allows us to get
55 // rid of exposing this member for testing and lets us test the caching logic 56 // rid of exposing this member for testing and lets us test the caching logic
56 // separately. 57 // separately.
57 class CachedImageFetcher : public image_fetcher::ImageFetcherDelegate { 58 class CachedImageFetcher : public image_fetcher::ImageFetcherDelegate {
58 public: 59 public:
59 // |pref_service| and |database| need to outlive the created image fetcher 60 // |pref_service| and |database| need to outlive the created image fetcher
(...skipping 69 matching lines...) Expand 10 before | Expand all | Expand 10 after
129 130
130 // Returns whether the service is ready. While this is false, the list of 131 // Returns whether the service is ready. While this is false, the list of
131 // suggestions will be empty, and all modifications to it (fetch, dismiss, 132 // suggestions will be empty, and all modifications to it (fetch, dismiss,
132 // etc) will be ignored. 133 // etc) will be ignored.
133 bool ready() const { return state_ == State::READY; } 134 bool ready() const { return state_ == State::READY; }
134 135
135 // Returns whether the service is successfully initialized. While this is 136 // Returns whether the service is successfully initialized. While this is
136 // false, some calls may trigger DCHECKs. 137 // false, some calls may trigger DCHECKs.
137 bool initialized() const { return ready() || state_ == State::DISABLED; } 138 bool initialized() const { return ready() || state_ == State::DISABLED; }
138 139
140 // Set the scheduler to be notified whenever the provider becomes active /
141 // in-active and whenever history is deleted. The initial change is also
142 // notified (switching from an initial undecided status). If the scheduler is
143 // set after the first change, it is called back immediately.
144 void SetRemoteSuggestionsScheduler(RemoteSuggestionsScheduler* scheduler);
145
139 // RemoteSuggestionsProvider implementation. 146 // RemoteSuggestionsProvider implementation.
140 void SetProviderStatusCallback(
141 std::unique_ptr<ProviderStatusCallback> callback) override;
142 void RefetchInTheBackground( 147 void RefetchInTheBackground(
143 std::unique_ptr<FetchStatusCallback> callback) override; 148 std::unique_ptr<FetchStatusCallback> callback) override;
144 149
145 // TODO(fhorschig): Remove this getter when there is an interface for the 150 // TODO(fhorschig): Remove this getter when there is an interface for the
146 // fetcher that allows better mocks. 151 // fetcher that allows better mocks.
147 const RemoteSuggestionsFetcher* suggestions_fetcher_for_debugging() 152 const RemoteSuggestionsFetcher* suggestions_fetcher_for_debugging()
148 const override; 153 const override;
149 154
150 // ContentSuggestionsProvider implementation. 155 // ContentSuggestionsProvider implementation.
151 CategoryStatus GetCategoryStatus(Category category) override; 156 CategoryStatus GetCategoryStatus(Category category) override;
(...skipping 37 matching lines...) Expand 10 before | Expand all | Expand 10 after
189 clock_ = std::move(clock); 194 clock_ = std::move(clock);
190 } 195 }
191 196
192 // TODO(tschumann): remove this method as soon as we inject the fetcher into 197 // TODO(tschumann): remove this method as soon as we inject the fetcher into
193 // the constructor. 198 // the constructor.
194 CachedImageFetcher& GetImageFetcherForTesting() { return image_fetcher_; } 199 CachedImageFetcher& GetImageFetcherForTesting() { return image_fetcher_; }
195 200
196 private: 201 private:
197 friend class RemoteSuggestionsProviderImplTest; 202 friend class RemoteSuggestionsProviderImplTest;
198 203
204 // TODO(jkrcal): Mock the database to trigger the error naturally (or remove
205 // the error state and get rid of the test).
199 FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest, 206 FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
200 CallsProviderStatusCallbackWhenReady); 207 CallsSchedulerOnError);
208 // TODO(jkrcal): Mock the status service and remove these friend declarations.
201 FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest, 209 FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
202 CallsProviderStatusCallbackOnError); 210 CallsSchedulerWhenDisabled);
203 FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
204 CallsProviderStatusCallbackWhenDisabled);
205 FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
206 ShouldNotCrashWhenCallingEmptyCallback);
207 FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest, 211 FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
208 DontNotifyIfNotAvailable); 212 DontNotifyIfNotAvailable);
209 FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest, 213 FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
210 RemoveExpiredDismissedContent); 214 CallsSchedulerWhenSignedIn);
211 FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest, StatusChanges);
212 FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest, 215 FRIEND_TEST_ALL_PREFIXES(RemoteSuggestionsProviderImplTest,
213 SuggestionsFetchedOnSignInAndSignOut); 216 CallsSchedulerWhenSignedOut);
214 217
215 // Possible state transitions: 218 // Possible state transitions:
216 // NOT_INITED --------+ 219 // NOT_INITED --------+
217 // / \ | 220 // / \ |
218 // v v | 221 // v v |
219 // READY <--> DISABLED | 222 // READY <--> DISABLED |
220 // \ / | 223 // \ / |
221 // v v | 224 // v v |
222 // ERROR_OCCURRED <-----+ 225 // ERROR_OCCURRED <-----+
226 // TODO(jkrcal): Do we need to keep the distinction between states DISABLED
227 // and ERROR_OCCURED?
223 enum class State { 228 enum class State {
224 // The service has just been created. Can change to states: 229 // The service has just been created. Can change to states:
225 // - DISABLED: After the database is done loading, 230 // - DISABLED: After the database is done loading,
226 // GetStateForDependenciesStatus can identify the next state to 231 // GetStateForDependenciesStatus can identify the next state to
227 // be DISABLED. 232 // be DISABLED.
228 // - READY: if GetStateForDependenciesStatus returns it, after the database 233 // - READY: if GetStateForDependenciesStatus returns it, after the database
229 // is done loading. 234 // is done loading.
230 // - ERROR_OCCURRED: when an unrecoverable error occurred. 235 // - ERROR_OCCURRED: when an unrecoverable error occurred.
231 NOT_INITED, 236 NOT_INITED,
232 237
(...skipping 112 matching lines...) Expand 10 before | Expand all | Expand 10 after
345 350
346 // Removes expired dismissed suggestions from the service and the database. 351 // Removes expired dismissed suggestions from the service and the database.
347 void ClearExpiredDismissedSuggestions(); 352 void ClearExpiredDismissedSuggestions();
348 353
349 // Removes images from the DB that are not referenced from any known 354 // Removes images from the DB that are not referenced from any known
350 // suggestion. Needs to iterate the whole suggestion database -- so do it 355 // suggestion. Needs to iterate the whole suggestion database -- so do it
351 // often enough to keep it small but not too often as it still iterates over 356 // often enough to keep it small but not too often as it still iterates over
352 // the file system. 357 // the file system.
353 void ClearOrphanedImages(); 358 void ClearOrphanedImages();
354 359
360 // Clears suggestions because any history item has been removed.
361 void ClearHistoryDependentState();
362
363 // Clears suggestions for any non-history related reason (e.g., sign-in status
364 // change, etc.).
365 void ClearSuggestions();
366
355 // Clears all stored suggestions and updates the observer. 367 // Clears all stored suggestions and updates the observer.
356 void NukeAllSuggestions(); 368 void NukeAllSuggestions();
357 369
358 // Completes the initialization phase of the service, registering the last 370 // Completes the initialization phase of the service, registering the last
359 // observers. This is done after construction, once the database is loaded. 371 // observers. This is done after construction, once the database is loaded.
360 void FinishInitialization(); 372 void FinishInitialization();
361 373
362 // Triggers a state transition depending on the provided status. This method 374 // Triggers a state transition depending on the provided status. This method
363 // is called when a change is detected by |status_service_|. 375 // is called when a change is detected by |status_service_|.
364 void OnStatusChanged(RemoteSuggestionsStatus old_status, 376 void OnStatusChanged(RemoteSuggestionsStatus old_status,
(...skipping 70 matching lines...) Expand 10 before | Expand all | Expand 10 after
435 447
436 // Set to true if FetchSuggestions is called while the service isn't ready. 448 // Set to true if FetchSuggestions is called while the service isn't ready.
437 // The fetch will be executed once the service enters the READY state. 449 // The fetch will be executed once the service enters the READY state.
438 // TODO(jkrcal): create a struct and have here just one base::Optional<>? 450 // TODO(jkrcal): create a struct and have here just one base::Optional<>?
439 bool fetch_when_ready_; 451 bool fetch_when_ready_;
440 452
441 // The parameters for the fetch to perform later. 453 // The parameters for the fetch to perform later.
442 bool fetch_when_ready_interactive_; 454 bool fetch_when_ready_interactive_;
443 std::unique_ptr<FetchStatusCallback> fetch_when_ready_callback_; 455 std::unique_ptr<FetchStatusCallback> fetch_when_ready_callback_;
444 456
445 std::unique_ptr<ProviderStatusCallback> provider_status_callback_; 457 RemoteSuggestionsScheduler* remote_suggestions_scheduler_;
446 458
447 // Set to true if NukeAllSuggestions is called while the service isn't ready. 459 // Set to true if ClearHistoryDependentState is called while the service isn't
448 // The nuke will be executed once the service finishes initialization or 460 // ready. The nuke will be executed once the service finishes initialization
449 // enters the READY state. 461 // or enters the READY state.
450 bool nuke_when_initialized_; 462 bool clear_history_dependent_state_when_initialized_;
451 463
452 // A clock for getting the time. This allows to inject a clock in tests. 464 // A clock for getting the time. This allows to inject a clock in tests.
453 std::unique_ptr<base::Clock> clock_; 465 std::unique_ptr<base::Clock> clock_;
454 466
455 DISALLOW_COPY_AND_ASSIGN(RemoteSuggestionsProviderImpl); 467 DISALLOW_COPY_AND_ASSIGN(RemoteSuggestionsProviderImpl);
456 }; 468 };
457 469
458 } // namespace ntp_snippets 470 } // namespace ntp_snippets
459 471
460 #endif // COMPONENTS_NTP_SNIPPETS_REMOTE_REMOTE_SUGGESTIONS_PROVIDER_IMPL_H_ 472 #endif // COMPONENTS_NTP_SNIPPETS_REMOTE_REMOTE_SUGGESTIONS_PROVIDER_IMPL_H_
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698