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

Side by Side Diff: components/ntp_snippets/remote/remote_suggestions_provider_impl.cc

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 #include "components/ntp_snippets/remote/remote_suggestions_provider_impl.h" 5 #include "components/ntp_snippets/remote/remote_suggestions_provider_impl.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 #include <iterator> 8 #include <iterator>
9 #include <utility> 9 #include <utility>
10 10
11 #include "base/bind.h" 11 #include "base/bind.h"
12 #include "base/command_line.h" 12 #include "base/command_line.h"
13 #include "base/location.h" 13 #include "base/location.h"
14 #include "base/memory/ptr_util.h" 14 #include "base/memory/ptr_util.h"
15 #include "base/metrics/histogram_macros.h" 15 #include "base/metrics/histogram_macros.h"
16 #include "base/metrics/sparse_histogram.h" 16 #include "base/metrics/sparse_histogram.h"
17 #include "base/stl_util.h" 17 #include "base/stl_util.h"
18 #include "base/strings/string_number_conversions.h" 18 #include "base/strings/string_number_conversions.h"
19 #include "base/threading/thread_task_runner_handle.h" 19 #include "base/threading/thread_task_runner_handle.h"
20 #include "base/time/default_clock.h" 20 #include "base/time/default_clock.h"
21 #include "base/time/time.h" 21 #include "base/time/time.h"
22 #include "base/values.h" 22 #include "base/values.h"
23 #include "components/data_use_measurement/core/data_use_user_data.h" 23 #include "components/data_use_measurement/core/data_use_user_data.h"
24 #include "components/image_fetcher/image_decoder.h" 24 #include "components/image_fetcher/image_decoder.h"
25 #include "components/image_fetcher/image_fetcher.h" 25 #include "components/image_fetcher/image_fetcher.h"
26 #include "components/ntp_snippets/category_rankers/category_ranker.h" 26 #include "components/ntp_snippets/category_rankers/category_ranker.h"
27 #include "components/ntp_snippets/pref_names.h" 27 #include "components/ntp_snippets/pref_names.h"
28 #include "components/ntp_snippets/remote/remote_suggestions_database.h" 28 #include "components/ntp_snippets/remote/remote_suggestions_database.h"
29 #include "components/ntp_snippets/remote/remote_suggestions_scheduler.h"
29 #include "components/ntp_snippets/switches.h" 30 #include "components/ntp_snippets/switches.h"
30 #include "components/prefs/pref_registry_simple.h" 31 #include "components/prefs/pref_registry_simple.h"
31 #include "components/prefs/pref_service.h" 32 #include "components/prefs/pref_service.h"
32 #include "components/strings/grit/components_strings.h" 33 #include "components/strings/grit/components_strings.h"
33 #include "ui/gfx/image/image.h" 34 #include "ui/gfx/image/image.h"
34 35
35 namespace ntp_snippets { 36 namespace ntp_snippets {
36 37
37 namespace { 38 namespace {
38 39
(...skipping 238 matching lines...) Expand 10 before | Expand all | Expand 10 after
277 suggestions_fetcher_(std::move(suggestions_fetcher)), 278 suggestions_fetcher_(std::move(suggestions_fetcher)),
278 database_(std::move(database)), 279 database_(std::move(database)),
279 image_fetcher_(std::move(image_fetcher), 280 image_fetcher_(std::move(image_fetcher),
280 std::move(image_decoder), 281 std::move(image_decoder),
281 pref_service, 282 pref_service,
282 database_.get()), 283 database_.get()),
283 status_service_(std::move(status_service)), 284 status_service_(std::move(status_service)),
284 fetch_when_ready_(false), 285 fetch_when_ready_(false),
285 fetch_when_ready_interactive_(false), 286 fetch_when_ready_interactive_(false),
286 fetch_when_ready_callback_(nullptr), 287 fetch_when_ready_callback_(nullptr),
287 provider_status_callback_(nullptr), 288 remote_suggestions_scheduler_(nullptr),
288 nuke_when_initialized_(false), 289 clear_history_dependent_state_when_initialized_(false),
289 clock_(base::MakeUnique<base::DefaultClock>()) { 290 clock_(base::MakeUnique<base::DefaultClock>()) {
290 RestoreCategoriesFromPrefs(); 291 RestoreCategoriesFromPrefs();
291 // The articles category always exists. Add it if we didn't get it from prefs. 292 // The articles category always exists. Add it if we didn't get it from prefs.
292 // TODO(treib): Rethink this. 293 // TODO(treib): Rethink this.
293 category_contents_.insert( 294 category_contents_.insert(
294 std::make_pair(articles_category_, 295 std::make_pair(articles_category_,
295 CategoryContent(BuildArticleCategoryInfo(base::nullopt)))); 296 CategoryContent(BuildArticleCategoryInfo(base::nullopt))));
296 // Tell the observer about all the categories. 297 // Tell the observer about all the categories.
297 for (const auto& entry : category_contents_) { 298 for (const auto& entry : category_contents_) {
298 observer->OnCategoryStatusChanged(this, entry.first, entry.second.status); 299 observer->OnCategoryStatusChanged(this, entry.first, entry.second.status);
(...skipping 20 matching lines...) Expand all
319 320
320 // static 321 // static
321 void RemoteSuggestionsProviderImpl::RegisterProfilePrefs( 322 void RemoteSuggestionsProviderImpl::RegisterProfilePrefs(
322 PrefRegistrySimple* registry) { 323 PrefRegistrySimple* registry) {
323 registry->RegisterListPref(prefs::kRemoteSuggestionCategories); 324 registry->RegisterListPref(prefs::kRemoteSuggestionCategories);
324 registry->RegisterInt64Pref(prefs::kLastSuccessfulBackgroundFetchTime, 0); 325 registry->RegisterInt64Pref(prefs::kLastSuccessfulBackgroundFetchTime, 0);
325 326
326 RemoteSuggestionsStatusService::RegisterProfilePrefs(registry); 327 RemoteSuggestionsStatusService::RegisterProfilePrefs(registry);
327 } 328 }
328 329
329 void RemoteSuggestionsProviderImpl::SetProviderStatusCallback( 330 void RemoteSuggestionsProviderImpl::SetRemoteSuggestionsScheduler(
330 std::unique_ptr<ProviderStatusCallback> callback) { 331 RemoteSuggestionsScheduler* scheduler) {
331 provider_status_callback_ = std::move(callback); 332 remote_suggestions_scheduler_ = scheduler;
332 // Call the observer right away if we've reached any final state. 333 // Call the observer right away if we've reached any final state.
333 NotifyStateChanged(); 334 NotifyStateChanged();
334 } 335 }
335 336
336 void RemoteSuggestionsProviderImpl::ReloadSuggestions() { 337 void RemoteSuggestionsProviderImpl::ReloadSuggestions() {
337 FetchSuggestions(/*interactive_request=*/true, 338 FetchSuggestions(/*interactive_request=*/true,
338 /*callback=*/nullptr); 339 /*callback=*/nullptr);
339 } 340 }
340 341
341 void RemoteSuggestionsProviderImpl::RefetchInTheBackground( 342 void RemoteSuggestionsProviderImpl::RefetchInTheBackground(
(...skipping 98 matching lines...) Expand 10 before | Expand all | Expand 10 after
440 suggestion_id.id_within_category()); 441 suggestion_id.id_within_category());
441 } 442 }
442 443
443 void RemoteSuggestionsProviderImpl::ClearHistory( 444 void RemoteSuggestionsProviderImpl::ClearHistory(
444 base::Time begin, 445 base::Time begin,
445 base::Time end, 446 base::Time end,
446 const base::Callback<bool(const GURL& url)>& filter) { 447 const base::Callback<bool(const GURL& url)>& filter) {
447 // Both time range and the filter are ignored and all suggestions are removed, 448 // Both time range and the filter are ignored and all suggestions are removed,
448 // because it is not known which history entries were used for the suggestions 449 // because it is not known which history entries were used for the suggestions
449 // personalization. 450 // personalization.
450 if (!ready()) { 451 ClearHistoryDependentState();
451 // No need to refresh the UI afterwards as we didn't provide any data to the
452 // UI so far.
453 nuke_when_initialized_ = true;
454 } else {
455 NukeAllSuggestions();
456 }
457 } 452 }
458 453
459 void RemoteSuggestionsProviderImpl::ClearCachedSuggestions(Category category) { 454 void RemoteSuggestionsProviderImpl::ClearCachedSuggestions(Category category) {
460 if (!initialized()) { 455 if (!initialized()) {
461 return; 456 return;
462 } 457 }
463 458
464 auto content_it = category_contents_.find(category); 459 auto content_it = category_contents_.find(category);
465 if (content_it == category_contents_.end()) { 460 if (content_it == category_contents_.end()) {
466 return; 461 return;
(...skipping 407 matching lines...) Expand 10 before | Expand all | Expand 10 after
874 for (const auto& suggestion_ptr : content.suggestions) { 869 for (const auto& suggestion_ptr : content.suggestions) {
875 alive_suggestions->insert(suggestion_ptr->id()); 870 alive_suggestions->insert(suggestion_ptr->id());
876 } 871 }
877 for (const auto& suggestion_ptr : content.dismissed) { 872 for (const auto& suggestion_ptr : content.dismissed) {
878 alive_suggestions->insert(suggestion_ptr->id()); 873 alive_suggestions->insert(suggestion_ptr->id());
879 } 874 }
880 } 875 }
881 database_->GarbageCollectImages(std::move(alive_suggestions)); 876 database_->GarbageCollectImages(std::move(alive_suggestions));
882 } 877 }
883 878
879 void RemoteSuggestionsProviderImpl::ClearHistoryDependentState() {
880 if (!initialized()) {
881 clear_history_dependent_state_when_initialized_ = true;
882 return;
883 }
884
885 NukeAllSuggestions();
886 if (remote_suggestions_scheduler_) {
887 remote_suggestions_scheduler_->OnHistoryCleared();
888 }
889 }
890
891 void RemoteSuggestionsProviderImpl::ClearSuggestions() {
892 DCHECK(initialized());
893
894 NukeAllSuggestions();
895 if (remote_suggestions_scheduler_) {
896 remote_suggestions_scheduler_->OnSuggestionsCleared();
897 }
898 }
899
884 void RemoteSuggestionsProviderImpl::NukeAllSuggestions() { 900 void RemoteSuggestionsProviderImpl::NukeAllSuggestions() {
885 for (const auto& item : category_contents_) { 901 for (const auto& item : category_contents_) {
886 Category category = item.first; 902 Category category = item.first;
887 const CategoryContent& content = item.second; 903 const CategoryContent& content = item.second;
888 904
889 ClearCachedSuggestions(category); 905 ClearCachedSuggestions(category);
890 // Update listeners about the new (empty) state. 906 // Update listeners about the new (empty) state.
891 if (IsCategoryStatusAvailable(content.status)) { 907 if (IsCategoryStatusAvailable(content.status)) {
892 NotifyNewSuggestions(category, content); 908 NotifyNewSuggestions(category, content);
893 } 909 }
(...skipping 18 matching lines...) Expand all
912 // find it in the database (and also can't fetch it remotely). Cut the 928 // find it in the database (and also can't fetch it remotely). Cut the
913 // lookup short and return directly. 929 // lookup short and return directly.
914 base::ThreadTaskRunnerHandle::Get()->PostTask( 930 base::ThreadTaskRunnerHandle::Get()->PostTask(
915 FROM_HERE, base::Bind(callback, gfx::Image())); 931 FROM_HERE, base::Bind(callback, gfx::Image()));
916 return; 932 return;
917 } 933 }
918 image_fetcher_.FetchSuggestionImage(suggestion_id, image_url, callback); 934 image_fetcher_.FetchSuggestionImage(suggestion_id, image_url, callback);
919 } 935 }
920 936
921 void RemoteSuggestionsProviderImpl::EnterStateReady() { 937 void RemoteSuggestionsProviderImpl::EnterStateReady() {
922 if (nuke_when_initialized_) { 938 if (clear_history_dependent_state_when_initialized_) {
923 NukeAllSuggestions(); 939 clear_history_dependent_state_when_initialized_ = false;
924 nuke_when_initialized_ = false; 940 ClearHistoryDependentState();
925 } 941 }
926 942
927 auto article_category_it = category_contents_.find(articles_category_); 943 auto article_category_it = category_contents_.find(articles_category_);
928 DCHECK(article_category_it != category_contents_.end()); 944 DCHECK(article_category_it != category_contents_.end());
929 if (article_category_it->second.suggestions.empty() || fetch_when_ready_) { 945 if (fetch_when_ready_) {
930 // TODO(jkrcal): Fetching suggestions automatically upon creation of this
931 // lazily created service can cause troubles, e.g. in unit tests where
932 // network I/O is not allowed.
933 // Either add a DCHECK here that we actually are allowed to do network I/O
934 // or change the logic so that some explicit call is always needed for the
935 // network request.
936 FetchSuggestions(fetch_when_ready_interactive_, 946 FetchSuggestions(fetch_when_ready_interactive_,
937 std::move(fetch_when_ready_callback_)); 947 std::move(fetch_when_ready_callback_));
938 fetch_when_ready_ = false; 948 fetch_when_ready_ = false;
939 } 949 }
940 950
941 for (const auto& item : category_contents_) { 951 for (const auto& item : category_contents_) {
942 Category category = item.first; 952 Category category = item.first;
943 const CategoryContent& content = item.second; 953 const CategoryContent& content = item.second;
944 // FetchSuggestions has set the status to |AVAILABLE_LOADING| if relevant, 954 // FetchSuggestions has set the status to |AVAILABLE_LOADING| if relevant,
945 // otherwise we transition to |AVAILABLE| here. 955 // otherwise we transition to |AVAILABLE| here.
946 if (content.status != CategoryStatus::AVAILABLE_LOADING) { 956 if (content.status != CategoryStatus::AVAILABLE_LOADING) {
947 UpdateCategoryStatus(category, CategoryStatus::AVAILABLE); 957 UpdateCategoryStatus(category, CategoryStatus::AVAILABLE);
948 } 958 }
949 } 959 }
950 } 960 }
951 961
952 void RemoteSuggestionsProviderImpl::EnterStateDisabled() { 962 void RemoteSuggestionsProviderImpl::EnterStateDisabled() {
953 NukeAllSuggestions(); 963 ClearSuggestions();
954 } 964 }
955 965
956 void RemoteSuggestionsProviderImpl::EnterStateError() { 966 void RemoteSuggestionsProviderImpl::EnterStateError() {
957 status_service_.reset(); 967 status_service_.reset();
958 } 968 }
959 969
960 void RemoteSuggestionsProviderImpl::FinishInitialization() { 970 void RemoteSuggestionsProviderImpl::FinishInitialization() {
961 if (nuke_when_initialized_) { 971 if (clear_history_dependent_state_when_initialized_) {
962 // We nuke here in addition to EnterStateReady, so that it happens even if 972 // We clear here in addition to EnterStateReady, so that it happens even if
963 // we enter the DISABLED state below. 973 // we enter the DISABLED state below.
964 NukeAllSuggestions(); 974 clear_history_dependent_state_when_initialized_ = false;
965 nuke_when_initialized_ = false; 975 ClearHistoryDependentState();
966 } 976 }
967 977
968 // Note: Initializing the status service will run the callback right away with 978 // Note: Initializing the status service will run the callback right away with
969 // the current state. 979 // the current state.
970 status_service_->Init(base::Bind( 980 status_service_->Init(base::Bind(
971 &RemoteSuggestionsProviderImpl::OnStatusChanged, base::Unretained(this))); 981 &RemoteSuggestionsProviderImpl::OnStatusChanged, base::Unretained(this)));
972 982
973 // Always notify here even if we got nothing from the database, because we 983 // Always notify here even if we got nothing from the database, because we
974 // don't know how long the fetch will take or if it will even complete. 984 // don't know how long the fetch will take or if it will even complete.
975 for (const auto& item : category_contents_) { 985 for (const auto& item : category_contents_) {
976 Category category = item.first; 986 Category category = item.first;
977 const CategoryContent& content = item.second; 987 const CategoryContent& content = item.second;
978 // Note: We might be in a non-available status here, e.g. DISABLED due to 988 // Note: We might be in a non-available status here, e.g. DISABLED due to
979 // enterprise policy. 989 // enterprise policy.
980 if (IsCategoryStatusAvailable(content.status)) { 990 if (IsCategoryStatusAvailable(content.status)) {
981 NotifyNewSuggestions(category, content); 991 NotifyNewSuggestions(category, content);
982 } 992 }
983 } 993 }
984 } 994 }
985 995
986 void RemoteSuggestionsProviderImpl::OnStatusChanged( 996 void RemoteSuggestionsProviderImpl::OnStatusChanged(
987 RemoteSuggestionsStatus old_status, 997 RemoteSuggestionsStatus old_status,
988 RemoteSuggestionsStatus new_status) { 998 RemoteSuggestionsStatus new_status) {
989 switch (new_status) { 999 switch (new_status) {
990 case RemoteSuggestionsStatus::ENABLED_AND_SIGNED_IN: 1000 case RemoteSuggestionsStatus::ENABLED_AND_SIGNED_IN:
991 if (old_status == RemoteSuggestionsStatus::ENABLED_AND_SIGNED_OUT) { 1001 if (old_status == RemoteSuggestionsStatus::ENABLED_AND_SIGNED_OUT) {
992 DCHECK(state_ == State::READY); 1002 DCHECK(state_ == State::READY);
993 // Clear nonpersonalized suggestions. 1003 // Clear nonpersonalized suggestions (and notify the scheduler there are
994 NukeAllSuggestions(); 1004 // no suggestions).
995 // Fetch personalized ones. 1005 ClearSuggestions();
996 // TODO(jkrcal): Loop in SchedulingRemoteSuggestionsProvider somehow.
997 FetchSuggestions(/*interactive_request=*/true,
998 /*callback=*/nullptr);
999 } else { 1006 } else {
1000 // Do not change the status. That will be done in EnterStateReady(). 1007 // Do not change the status. That will be done in EnterStateReady().
1001 EnterState(State::READY); 1008 EnterState(State::READY);
1002 } 1009 }
1003 break; 1010 break;
1004 1011
1005 case RemoteSuggestionsStatus::ENABLED_AND_SIGNED_OUT: 1012 case RemoteSuggestionsStatus::ENABLED_AND_SIGNED_OUT:
1006 if (old_status == RemoteSuggestionsStatus::ENABLED_AND_SIGNED_IN) { 1013 if (old_status == RemoteSuggestionsStatus::ENABLED_AND_SIGNED_IN) {
1007 DCHECK(state_ == State::READY); 1014 DCHECK(state_ == State::READY);
1008 // Clear personalized suggestions. 1015 // Clear personalized suggestions (and notify the scheduler there are
1009 NukeAllSuggestions(); 1016 // no suggestions).
1010 // Fetch nonpersonalized ones. 1017 ClearSuggestions();
1011 // TODO(jkrcal): Loop in SchedulingRemoteSuggestionsProvider somehow.
1012 FetchSuggestions(/*interactive_request=*/true,
1013 /*callback=*/nullptr);
1014 } else { 1018 } else {
1015 // Do not change the status. That will be done in EnterStateReady(). 1019 // Do not change the status. That will be done in EnterStateReady().
1016 EnterState(State::READY); 1020 EnterState(State::READY);
1017 } 1021 }
1018 break; 1022 break;
1019 1023
1020 case RemoteSuggestionsStatus::EXPLICITLY_DISABLED: 1024 case RemoteSuggestionsStatus::EXPLICITLY_DISABLED:
1021 EnterState(State::DISABLED); 1025 EnterState(State::DISABLED);
1022 UpdateAllCategoryStatus(CategoryStatus::CATEGORY_EXPLICITLY_DISABLED); 1026 UpdateAllCategoryStatus(CategoryStatus::CATEGORY_EXPLICITLY_DISABLED);
1023 break; 1027 break;
(...skipping 39 matching lines...) Expand 10 before | Expand all | Expand 10 after
1063 1067
1064 case State::COUNT: 1068 case State::COUNT:
1065 NOTREACHED(); 1069 NOTREACHED();
1066 break; 1070 break;
1067 } 1071 }
1068 1072
1069 NotifyStateChanged(); 1073 NotifyStateChanged();
1070 } 1074 }
1071 1075
1072 void RemoteSuggestionsProviderImpl::NotifyStateChanged() { 1076 void RemoteSuggestionsProviderImpl::NotifyStateChanged() {
1073 if (!provider_status_callback_) { 1077 if (!remote_suggestions_scheduler_) {
1074 return; 1078 return;
1075 } 1079 }
1076 1080
1077 switch (state_) { 1081 switch (state_) {
1078 case State::NOT_INITED: 1082 case State::NOT_INITED:
1079 // Initial state, not sure yet whether active or not. 1083 // Initial state, not sure yet whether active or not.
1080 break; 1084 break;
1081 case State::READY: 1085 case State::READY:
1082 provider_status_callback_->Run(ProviderStatus::ACTIVE); 1086 remote_suggestions_scheduler_->OnProviderActivated();
1083 break; 1087 break;
1084 case State::DISABLED: 1088 case State::DISABLED:
1085 provider_status_callback_->Run(ProviderStatus::INACTIVE); 1089 remote_suggestions_scheduler_->OnProviderDeactivated();
1086 break; 1090 break;
1087 case State::ERROR_OCCURRED: 1091 case State::ERROR_OCCURRED:
1088 provider_status_callback_->Run(ProviderStatus::INACTIVE); 1092 remote_suggestions_scheduler_->OnProviderDeactivated();
1089 break; 1093 break;
1090 case State::COUNT: 1094 case State::COUNT:
1091 NOTREACHED(); 1095 NOTREACHED();
1092 break; 1096 break;
1093 } 1097 }
1094 } 1098 }
1095 1099
1096 void RemoteSuggestionsProviderImpl::NotifyNewSuggestions( 1100 void RemoteSuggestionsProviderImpl::NotifyNewSuggestions(
1097 Category category, 1101 Category category,
1098 const CategoryContent& content) { 1102 const CategoryContent& content) {
(...skipping 171 matching lines...) Expand 10 before | Expand all | Expand 10 after
1270 RemoteSuggestionsProviderImpl::CategoryContent::CategoryContent( 1274 RemoteSuggestionsProviderImpl::CategoryContent::CategoryContent(
1271 CategoryContent&&) = default; 1275 CategoryContent&&) = default;
1272 1276
1273 RemoteSuggestionsProviderImpl::CategoryContent::~CategoryContent() = default; 1277 RemoteSuggestionsProviderImpl::CategoryContent::~CategoryContent() = default;
1274 1278
1275 RemoteSuggestionsProviderImpl::CategoryContent& 1279 RemoteSuggestionsProviderImpl::CategoryContent&
1276 RemoteSuggestionsProviderImpl::CategoryContent::operator=(CategoryContent&&) = 1280 RemoteSuggestionsProviderImpl::CategoryContent::operator=(CategoryContent&&) =
1277 default; 1281 default;
1278 1282
1279 } // namespace ntp_snippets 1283 } // namespace ntp_snippets
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698