Commit 01b3cdb5 authored by Moe Ahmadi's avatar Moe Ahmadi Committed by Commit Bot

[NTP][RQ] Integrates Repeatable Queries Service into MostVisitedSites

MostVisitedSites will be serving repeatable queries in the first two or
the last two slots for the Most Visited URLs, if applicable, depending
on the chosen variation.

Fixed: 1145050
Change-Id: I899eb37d5461dc68e43c85bcaf0666830575e025
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2531067Reviewed-by: default avatarKristi Park <kristipark@chromium.org>
Reviewed-by: default avatarRamya Nagarajan <ramyan@chromium.org>
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826600}
parent 54668617
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "chrome/browser/ntp_tiles/chrome_custom_links_manager_factory.h" #include "chrome/browser/ntp_tiles/chrome_custom_links_manager_factory.h"
#include "chrome/browser/ntp_tiles/chrome_popular_sites_factory.h" #include "chrome/browser/ntp_tiles/chrome_popular_sites_factory.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/search/repeatable_queries/repeatable_queries_service_factory.h"
#include "chrome/browser/search/suggestions/suggestions_service_factory.h" #include "chrome/browser/search/suggestions/suggestions_service_factory.h"
#include "chrome/common/buildflags.h" #include "chrome/common/buildflags.h"
#include "components/history/core/browser/top_sites.h" #include "components/history/core/browser/top_sites.h"
...@@ -127,6 +128,11 @@ ChromeMostVisitedSitesFactory::NewForProfile(Profile* profile) { ...@@ -127,6 +128,11 @@ ChromeMostVisitedSitesFactory::NewForProfile(Profile* profile) {
auto most_visited_sites = std::make_unique<ntp_tiles::MostVisitedSites>( auto most_visited_sites = std::make_unique<ntp_tiles::MostVisitedSites>(
profile->GetPrefs(), TopSitesFactory::GetForProfile(profile), profile->GetPrefs(), TopSitesFactory::GetForProfile(profile),
#if defined(OS_ANDROID)
nullptr,
#else
RepeatableQueriesServiceFactory::GetForProfile(profile),
#endif
SuggestionsServiceFactory::GetForProfile(profile), SuggestionsServiceFactory::GetForProfile(profile),
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
ChromePopularSitesFactory::NewForProfile(profile), ChromePopularSitesFactory::NewForProfile(profile),
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "chrome/browser/signin/identity_manager_factory.h" #include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/common/webui_url_constants.h" #include "chrome/common/webui_url_constants.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h" #include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/omnibox/common/omnibox_features.h"
#include "components/search/ntp_features.h" #include "components/search/ntp_features.h"
#include "components/search/repeatable_queries/repeatable_queries_service.h" #include "components/search/repeatable_queries/repeatable_queries_service.h"
#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_context.h"
...@@ -42,7 +43,8 @@ RepeatableQueriesServiceFactory::~RepeatableQueriesServiceFactory() = default; ...@@ -42,7 +43,8 @@ RepeatableQueriesServiceFactory::~RepeatableQueriesServiceFactory() = default;
KeyedService* RepeatableQueriesServiceFactory::BuildServiceInstanceFor( KeyedService* RepeatableQueriesServiceFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const { content::BrowserContext* context) const {
if (!base::FeatureList::IsEnabled(ntp_features::kNtpRepeatableQueries)) { if (!base::FeatureList::IsEnabled(ntp_features::kNtpRepeatableQueries) ||
!base::FeatureList::IsEnabled(omnibox::kNewSearchFeatures)) {
return nullptr; return nullptr;
} }
......
...@@ -60,6 +60,7 @@ static_library("ntp_tiles") { ...@@ -60,6 +60,7 @@ static_library("ntp_tiles") {
"//components/prefs", "//components/prefs",
"//components/rappor/public", "//components/rappor/public",
"//components/resources", "//components/resources",
"//components/search",
"//components/search_engines", "//components/search_engines",
"//components/strings", "//components/strings",
"//components/url_formatter", "//components/url_formatter",
......
include_rules = [ include_rules = [
"+components/favicon",
"+components/favicon_base", "+components/favicon_base",
"+components/favicon",
"+components/google/core", "+components/google/core",
"+components/grit", "+components/grit",
"+components/image_fetcher",
"+components/history/core/browser", "+components/history/core/browser",
"+components/history/core/test", "+components/history/core/test",
"+components/image_fetcher",
"+components/pref_registry", "+components/pref_registry",
"+components/prefs", "+components/prefs",
"+components/rappor", "+components/rappor",
"+components/search_engines", "+components/search_engines",
"+components/search",
"+components/strings/grit/components_strings.h", "+components/strings/grit/components_strings.h",
"+components/suggestions", "+components/suggestions",
"+components/sync_preferences", "+components/sync_preferences",
...@@ -20,6 +21,6 @@ include_rules = [ ...@@ -20,6 +21,6 @@ include_rules = [
"+services/data_decoder/public", "+services/data_decoder/public",
"+services/network/public/cpp", "+services/network/public/cpp",
"+services/network/test", "+services/network/test",
"+ui/gfx",
"+ui/base", "+ui/base",
"+ui/gfx",
] ]
...@@ -26,6 +26,7 @@ ...@@ -26,6 +26,7 @@
#include "components/ntp_tiles/switches.h" #include "components/ntp_tiles/switches.h"
#include "components/pref_registry/pref_registry_syncable.h" #include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/search/ntp_features.h"
using history::TopSites; using history::TopSites;
using suggestions::ChromeSuggestion; using suggestions::ChromeSuggestion;
...@@ -120,6 +121,7 @@ base::string16 GenerateShortTitle(const base::string16& title) { ...@@ -120,6 +121,7 @@ base::string16 GenerateShortTitle(const base::string16& title) {
MostVisitedSites::MostVisitedSites( MostVisitedSites::MostVisitedSites(
PrefService* prefs, PrefService* prefs,
scoped_refptr<history::TopSites> top_sites, scoped_refptr<history::TopSites> top_sites,
RepeatableQueriesService* repeatable_queries,
SuggestionsService* suggestions, SuggestionsService* suggestions,
std::unique_ptr<PopularSites> popular_sites, std::unique_ptr<PopularSites> popular_sites,
std::unique_ptr<CustomLinksManager> custom_links, std::unique_ptr<CustomLinksManager> custom_links,
...@@ -127,6 +129,7 @@ MostVisitedSites::MostVisitedSites( ...@@ -127,6 +129,7 @@ MostVisitedSites::MostVisitedSites(
std::unique_ptr<MostVisitedSitesSupervisor> supervisor) std::unique_ptr<MostVisitedSitesSupervisor> supervisor)
: prefs_(prefs), : prefs_(prefs),
top_sites_(top_sites), top_sites_(top_sites),
repeatable_queries_(repeatable_queries),
suggestions_service_(suggestions), suggestions_service_(suggestions),
popular_sites_(std::move(popular_sites)), popular_sites_(std::move(popular_sites)),
custom_links_(std::move(custom_links)), custom_links_(std::move(custom_links)),
...@@ -215,7 +218,11 @@ void MostVisitedSites::SetMostVisitedURLsObserver(Observer* observer, ...@@ -215,7 +218,11 @@ void MostVisitedSites::SetMostVisitedURLsObserver(Observer* observer,
if (top_sites_) { if (top_sites_) {
// Register as TopSitesObserver so that we can update ourselves when the // Register as TopSitesObserver so that we can update ourselves when the
// TopSites changes. // TopSites changes.
top_sites_observer_.Add(top_sites_.get()); top_sites_observer_.Observe(top_sites_.get());
}
if (repeatable_queries_) {
repeatable_queries_observer_.Observe(repeatable_queries_);
} }
if (custom_links_) { if (custom_links_) {
...@@ -244,6 +251,10 @@ void MostVisitedSites::Refresh() { ...@@ -244,6 +251,10 @@ void MostVisitedSites::Refresh() {
top_sites_->SyncWithHistory(); top_sites_->SyncWithHistory();
} }
if (repeatable_queries_) {
repeatable_queries_->Refresh();
}
suggestions_service_->FetchSuggestionsData(); suggestions_service_->FetchSuggestionsData();
} }
...@@ -389,6 +400,13 @@ void MostVisitedSites::AddOrRemoveBlockedUrl(const GURL& url, bool add_url) { ...@@ -389,6 +400,13 @@ void MostVisitedSites::AddOrRemoveBlockedUrl(const GURL& url, bool add_url) {
base::UserMetricsAction("Suggestions.Site.RemovalUndone")); base::UserMetricsAction("Suggestions.Site.RemovalUndone"));
} }
if (repeatable_queries_) {
if (add_url)
repeatable_queries_->DeleteQueryWithDestinationURL(url);
else
NOTREACHED() << "Deleted repeatable queries cannot be restored.";
}
if (top_sites_) { if (top_sites_) {
if (add_url) if (add_url)
top_sites_->AddBlockedUrl(url); top_sites_->AddBlockedUrl(url);
...@@ -481,7 +499,37 @@ void MostVisitedSites::OnMostVisitedURLsAvailable( ...@@ -481,7 +499,37 @@ void MostVisitedSites::OnMostVisitedURLsAvailable(
} }
mv_source_ = TileSource::TOP_SITES; mv_source_ = TileSource::TOP_SITES;
InitiateNotificationForNewTiles(std::move(tiles)); InitiateNotificationForNewTiles(InsertRepeatableQueryTiles(std::move(tiles)));
}
NTPTilesVector MostVisitedSites::InsertRepeatableQueryTiles(
NTPTilesVector tiles) {
if (!repeatable_queries_)
return tiles;
const std::vector<RepeatableQuery>& repeatable_queries =
repeatable_queries_->repeatable_queries();
// Make room for the repeatable query tiles, if necessary.
int num_overflow_tiles =
tiles.size() + repeatable_queries.size() - GetMaxNumSites();
if (num_overflow_tiles > 0)
tiles.resize(tiles.size() - num_overflow_tiles);
auto insert_position = (ntp_features::GetRepeatableQueriesInsertPosition() ==
ntp_features::RepeatableQueriesInsertPosition::kStart)
? tiles.begin()
: tiles.end();
for (const auto& repeatable_query : repeatable_queries) {
NTPTile tile;
tile.title = repeatable_query.query;
tile.url = repeatable_query.destination_url;
tile.source = TileSource::REPEATABLE_QUERIES_SERVICE;
auto inserted_position = tiles.insert(insert_position, tile);
insert_position = inserted_position + 1;
}
return tiles;
} }
void MostVisitedSites::OnSuggestionsProfileChanged( void MostVisitedSites::OnSuggestionsProfileChanged(
...@@ -888,6 +936,20 @@ void MostVisitedSites::TopSitesChanged(TopSites* top_sites, ...@@ -888,6 +936,20 @@ void MostVisitedSites::TopSitesChanged(TopSites* top_sites,
} }
} }
void MostVisitedSites::OnRepeatableQueriesUpdated() {
// Repeatable Queries are shown along with the most visited URLs only.
// Simulate a change to the most visited urls. This will result in
// MostVisitedSites::OnMostVisitedURLsAvailable to be called synchronously or
// asynchronously depending on whether the most visited URLs are cached.
if (top_sites_) {
TopSitesChanged(top_sites_.get(), ChangeReason::MOST_VISITED);
}
}
void MostVisitedSites::OnRepeatableQueriesServiceShuttingDown() {
repeatable_queries_observer_.RemoveObservation();
}
bool MostVisitedSites::ShouldAddHomeTile() const { bool MostVisitedSites::ShouldAddHomeTile() const {
return GetMaxNumSites() > 0u && return GetMaxNumSites() > 0u &&
homepage_client_ && // No platform-specific implementation - no tile. homepage_client_ && // No platform-specific implementation - no tile.
......
...@@ -21,7 +21,7 @@ ...@@ -21,7 +21,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/scoped_observer.h" #include "base/scoped_observation.h"
#include "base/strings/string16.h" #include "base/strings/string16.h"
#include "components/history/core/browser/history_types.h" #include "components/history/core/browser/history_types.h"
#include "components/history/core/browser/top_sites.h" #include "components/history/core/browser/top_sites.h"
...@@ -31,6 +31,8 @@ ...@@ -31,6 +31,8 @@
#include "components/ntp_tiles/popular_sites.h" #include "components/ntp_tiles/popular_sites.h"
#include "components/ntp_tiles/section_type.h" #include "components/ntp_tiles/section_type.h"
#include "components/ntp_tiles/tile_source.h" #include "components/ntp_tiles/tile_source.h"
#include "components/search/repeatable_queries/repeatable_queries_service.h"
#include "components/search/repeatable_queries/repeatable_queries_service_observer.h"
#include "components/suggestions/proto/suggestions.pb.h" #include "components/suggestions/proto/suggestions.pb.h"
#include "components/suggestions/suggestions_service.h" #include "components/suggestions/suggestions_service.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -82,7 +84,8 @@ class MostVisitedSitesSupervisor { ...@@ -82,7 +84,8 @@ class MostVisitedSitesSupervisor {
// Tracks the list of most visited sites. // Tracks the list of most visited sites.
class MostVisitedSites : public history::TopSitesObserver, class MostVisitedSites : public history::TopSitesObserver,
public MostVisitedSitesSupervisor::Observer { public MostVisitedSitesSupervisor::Observer,
public RepeatableQueriesServiceObserver {
public: public:
// The observer to be notified when the list of most visited sites changes. // The observer to be notified when the list of most visited sites changes.
class Observer { class Observer {
...@@ -123,6 +126,7 @@ class MostVisitedSites : public history::TopSitesObserver, ...@@ -123,6 +126,7 @@ class MostVisitedSites : public history::TopSitesObserver,
// optional and if null, the associated features will be disabled. // optional and if null, the associated features will be disabled.
MostVisitedSites(PrefService* prefs, MostVisitedSites(PrefService* prefs,
scoped_refptr<history::TopSites> top_sites, scoped_refptr<history::TopSites> top_sites,
RepeatableQueriesService* repeatable_queries,
suggestions::SuggestionsService* suggestions, suggestions::SuggestionsService* suggestions,
std::unique_ptr<PopularSites> popular_sites, std::unique_ptr<PopularSites> popular_sites,
std::unique_ptr<CustomLinksManager> custom_links, std::unique_ptr<CustomLinksManager> custom_links,
...@@ -260,6 +264,8 @@ class MostVisitedSites : public history::TopSitesObserver, ...@@ -260,6 +264,8 @@ class MostVisitedSites : public history::TopSitesObserver,
void OnMostVisitedURLsAvailable( void OnMostVisitedURLsAvailable(
const history::MostVisitedURLList& visited_list); const history::MostVisitedURLList& visited_list);
NTPTilesVector InsertRepeatableQueryTiles(NTPTilesVector tiles);
// Callback for when an update is reported by the SuggestionsService. // Callback for when an update is reported by the SuggestionsService.
void OnSuggestionsProfileChanged( void OnSuggestionsProfileChanged(
const suggestions::SuggestionsProfile& suggestions_profile); const suggestions::SuggestionsProfile& suggestions_profile);
...@@ -341,8 +347,13 @@ class MostVisitedSites : public history::TopSitesObserver, ...@@ -341,8 +347,13 @@ class MostVisitedSites : public history::TopSitesObserver,
void TopSitesChanged(history::TopSites* top_sites, void TopSitesChanged(history::TopSites* top_sites,
ChangeReason change_reason) override; ChangeReason change_reason) override;
// RepeatableQueriesServiceObserver implementation.
void OnRepeatableQueriesUpdated() override;
void OnRepeatableQueriesServiceShuttingDown() override;
PrefService* prefs_; PrefService* prefs_;
scoped_refptr<history::TopSites> top_sites_; scoped_refptr<history::TopSites> top_sites_;
RepeatableQueriesService* repeatable_queries_;
suggestions::SuggestionsService* suggestions_service_; suggestions::SuggestionsService* suggestions_service_;
std::unique_ptr<PopularSites> const popular_sites_; std::unique_ptr<PopularSites> const popular_sites_;
std::unique_ptr<CustomLinksManager> const custom_links_; std::unique_ptr<CustomLinksManager> const custom_links_;
...@@ -368,9 +379,13 @@ class MostVisitedSites : public history::TopSitesObserver, ...@@ -368,9 +379,13 @@ class MostVisitedSites : public history::TopSitesObserver,
suggestions::SuggestionsService::ResponseCallbackList::Subscription> suggestions::SuggestionsService::ResponseCallbackList::Subscription>
suggestions_subscription_; suggestions_subscription_;
ScopedObserver<history::TopSites, history::TopSitesObserver> base::ScopedObservation<history::TopSites, history::TopSitesObserver>
top_sites_observer_{this}; top_sites_observer_{this};
base::ScopedObservation<RepeatableQueriesService,
RepeatableQueriesServiceObserver>
repeatable_queries_observer_{this};
std::unique_ptr<base::CallbackList<void()>::Subscription> std::unique_ptr<base::CallbackList<void()>::Subscription>
custom_links_subscription_; custom_links_subscription_;
......
...@@ -474,9 +474,9 @@ class MostVisitedSitesTest ...@@ -474,9 +474,9 @@ class MostVisitedSitesTest
EXPECT_CALL(*icon_cacher, StartFetchMostLikely(_, _)).Times(AtLeast(0)); EXPECT_CALL(*icon_cacher, StartFetchMostLikely(_, _)).Times(AtLeast(0));
most_visited_sites_ = std::make_unique<MostVisitedSites>( most_visited_sites_ = std::make_unique<MostVisitedSites>(
&pref_service_, mock_top_sites_, &mock_suggestions_service_, &pref_service_, mock_top_sites_, /*repeatable_queries=*/nullptr,
popular_sites_factory_.New(), std::move(mock_custom_links), &mock_suggestions_service_, popular_sites_factory_.New(),
std::move(icon_cacher), std::move(mock_custom_links), std::move(icon_cacher),
/*supervisor=*/nullptr); /*supervisor=*/nullptr);
} }
......
...@@ -25,6 +25,7 @@ IOSMostVisitedSitesFactory::NewForBrowserState( ...@@ -25,6 +25,7 @@ IOSMostVisitedSitesFactory::NewForBrowserState(
return std::make_unique<ntp_tiles::MostVisitedSites>( return std::make_unique<ntp_tiles::MostVisitedSites>(
browser_state->GetPrefs(), browser_state->GetPrefs(),
ios::TopSitesFactory::GetForBrowserState(browser_state), ios::TopSitesFactory::GetForBrowserState(browser_state),
/*repeatable_queries=*/nullptr,
suggestions::SuggestionsServiceFactory::GetForBrowserState(browser_state), suggestions::SuggestionsServiceFactory::GetForBrowserState(browser_state),
IOSPopularSitesFactory::NewForBrowserState(browser_state), IOSPopularSitesFactory::NewForBrowserState(browser_state),
/*custom_links=*/nullptr, /*custom_links=*/nullptr,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment