Add support for components/ntp_tiles in InstantService

This will allow us to get server-side suggestions on the local NTP.

BUG=514752
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2532103002
Cr-Commit-Position: refs/heads/master@{#437576}

chrome/browser/prefs/browser_prefs.cc

@@ -83,6 +83,7 @@
 #include "components/ntp_snippets/remote/request_throttler.h"
 #include "components/ntp_snippets/sessions/foreign_sessions_suggestions_provider.h"
 #include "components/ntp_snippets/user_classifier.h"
+#include "components/ntp_tiles/most_visited_sites.h"
 #include "components/omnibox/browser/zero_suggest_provider.h"
 #include "components/password_manager/core/browser/password_bubble_experiment.h"
 #include "components/password_manager/core/browser/password_manager.h"
@@ -155,7 +156,6 @@
 #if BUILDFLAG(ANDROID_JAVA_UI)
 #include "chrome/browser/android/bookmarks/partner_bookmarks_shim.h"
 #include "chrome/browser/android/ntp/new_tab_page_prefs.h"
-#include "components/ntp_tiles/most_visited_sites.h"
 #include "components/ntp_tiles/popular_sites.h"
 #else
 #include "chrome/browser/ui/startup/startup_browser_creator.h"
@@ -501,6 +501,7 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
   ntp_snippets::ContentSuggestionsService::RegisterProfilePrefs(registry);
   ntp_snippets::RequestThrottler::RegisterProfilePrefs(registry);
   ntp_snippets::UserClassifier::RegisterProfilePrefs(registry);
+  ntp_tiles::MostVisitedSites::RegisterProfilePrefs(registry);
   password_bubble_experiment::RegisterPrefs(registry);
   password_manager::PasswordManager::RegisterProfilePrefs(registry);
   PrefProxyConfigTrackerImpl::RegisterProfilePrefs(registry);
@@ -563,7 +564,6 @@ void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) {
 
 #if BUILDFLAG(ANDROID_JAVA_UI)
   variations::VariationsService::RegisterProfilePrefs(registry);
-  ntp_tiles::MostVisitedSites::RegisterProfilePrefs(registry);
   ntp_tiles::PopularSites::RegisterProfilePrefs(registry);
   NewTabPagePrefs::RegisterProfilePrefs(registry);
   PartnerBookmarksShim::RegisterProfilePrefs(registry);

chrome/browser/resources/local_ntp/most_visited_single.js

@@ -294,7 +294,6 @@ var addTile = function(args) {
       return;
 
     data.tid = data.rid;
-    data.tileSource = NTPTileSource.TOP_SITES;
     if (!data.faviconUrl) {
       data.faviconUrl = 'chrome-search://favicon/size/16@' +
           window.devicePixelRatio + 'x/' + data.renderViewId + '/' + data.tid;

chrome/browser/search/instant_service.cc

@@ -6,10 +6,12 @@
 
 #include <stddef.h>
 
-#include "base/strings/string_util.h"
+#include "base/bind.h"
+#include "base/feature_list.h"
 #include "build/build_config.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/favicon/fallback_icon_service_factory.h"
+#include "chrome/browser/favicon/favicon_service_factory.h"
 #include "chrome/browser/favicon/large_icon_service_factory.h"
 #include "chrome/browser/history/top_sites_factory.h"
 #include "chrome/browser/profiles/profile.h"
@@ -17,6 +19,8 @@
 #include "chrome/browser/search/instant_service_observer.h"
 #include "chrome/browser/search/most_visited_iframe_source.h"
 #include "chrome/browser/search/search.h"
+#include "chrome/browser/search/suggestions/image_decoder_impl.h"
+#include "chrome/browser/search/suggestions/suggestions_service_factory.h"
 #include "chrome/browser/search/thumbnail_source.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
 #include "chrome/browser/search_engines/ui_thread_search_terms_data.h"
@@ -31,21 +35,19 @@
 #include "components/favicon/core/fallback_icon_service.h"
 #include "components/favicon/core/large_icon_service.h"
 #include "components/history/core/browser/top_sites.h"
+#include "components/image_fetcher/image_fetcher_impl.h"
 #include "components/keyed_service/core/service_access_type.h"
+#include "components/ntp_tiles/icon_cacher.h"
 #include "components/search/search.h"
 #include "components/search_engines/template_url_service.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/url_data_source.h"
-#include "content/public/common/url_constants.h"
-#include "extensions/common/constants.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/image/image_skia.h"
-#include "url/url_constants.h"
 
 #if !defined(OS_ANDROID)
 #include "chrome/browser/search/local_ntp_source.h"
@@ -57,6 +59,13 @@
 #include "chrome/browser/themes/theme_service_factory.h"
 #endif  // defined(ENABLE_THEMES)
 
+namespace {
+
+const base::Feature kNtpTilesFeature{"NTPTilesInInstantService",
+                                     base::FEATURE_DISABLED_BY_DEFAULT};
+
+}  // namespace
+
 InstantService::InstantService(Profile* profile)
     : profile_(profile),
       template_url_service_(TemplateURLServiceFactory::GetForProfile(profile_)),
@@ -93,13 +102,30 @@ InstantService::InstantService(Profile* profile)
                  content::NOTIFICATION_RENDERER_PROCESS_TERMINATED,
                  content::NotificationService::AllSources());
 
-  scoped_refptr<history::TopSites> top_sites =
-      TopSitesFactory::GetForProfile(profile_);
-  if (top_sites) {
-    top_sites->AddObserver(this);
-    // Immediately query the TopSites state.
-    TopSitesChanged(top_sites.get(),
-                    history::TopSitesObserver::ChangeReason::MOST_VISITED);
+  if (base::FeatureList::IsEnabled(kNtpTilesFeature)) {
+    most_visited_sites_ = base::MakeUnique<ntp_tiles::MostVisitedSites>(
+        profile_->GetPrefs(), TopSitesFactory::GetForProfile(profile_),
+        suggestions::SuggestionsServiceFactory::GetForProfile(profile_),
+        /*popular_sites=*/nullptr,
+        base::MakeUnique<ntp_tiles::IconCacher>(
+            FaviconServiceFactory::GetForProfile(
+                profile, ServiceAccessType::IMPLICIT_ACCESS),
+            base::MakeUnique<image_fetcher::ImageFetcherImpl>(
+                base::MakeUnique<suggestions::ImageDecoderImpl>(),
+                profile->GetRequestContext())),
+        /*supervisor=*/nullptr);
+    // TODO(treib): Add supervisor.
+    // TODO(sfiera): Share this with Android in a factory.
+
+    most_visited_sites_->SetMostVisitedURLsObserver(this, 8);
+  } else {
+    top_sites_ = TopSitesFactory::GetForProfile(profile_);
+    if (top_sites_) {
+      top_sites_->AddObserver(this);
+      // Immediately query the TopSites state.
+      TopSitesChanged(top_sites_.get(),
+                      history::TopSitesObserver::ChangeReason::MOST_VISITED);
+    }
   }
 
   if (profile_ && profile_->GetResourceContext()) {
@@ -170,31 +196,35 @@ void InstantService::RemoveObserver(InstantServiceObserver* observer) {
 }
 
 void InstantService::OnNewTabPageOpened() {
-  scoped_refptr<history::TopSites> top_sites =
-      TopSitesFactory::GetForProfile(profile_);
-  if (top_sites)
-    top_sites->SyncWithHistory();
+  if (most_visited_sites_) {
+    most_visited_sites_->Refresh();
+  } else if (top_sites_) {
+    top_sites_->SyncWithHistory();
+  }
 }
 
 void InstantService::DeleteMostVisitedItem(const GURL& url) {
-  scoped_refptr<history::TopSites> top_sites =
-      TopSitesFactory::GetForProfile(profile_);
-  if (top_sites)
-    top_sites->AddBlacklistedURL(url);
+  if (most_visited_sites_) {
+    most_visited_sites_->AddOrRemoveBlacklistedUrl(url, true);
+  } else if (top_sites_) {
+    top_sites_->AddBlacklistedURL(url);
+  }
 }
 
 void InstantService::UndoMostVisitedDeletion(const GURL& url) {
-  scoped_refptr<history::TopSites> top_sites =
-      TopSitesFactory::GetForProfile(profile_);
-  if (top_sites)
-    top_sites->RemoveBlacklistedURL(url);
+  if (most_visited_sites_) {
+    most_visited_sites_->AddOrRemoveBlacklistedUrl(url, false);
+  } else if (top_sites_) {
+    top_sites_->RemoveBlacklistedURL(url);
+  }
 }
 
 void InstantService::UndoAllMostVisitedDeletions() {
-  scoped_refptr<history::TopSites> top_sites =
-      TopSitesFactory::GetForProfile(profile_);
-  if (top_sites)
-    top_sites->ClearBlacklistedURLs();
+  if (most_visited_sites_) {
+    most_visited_sites_->ClearBlacklistedUrls();
+  } else if (top_sites_) {
+    top_sites_->ClearBlacklistedURLs();
+  }
 }
 
 void InstantService::UpdateThemeInfo() {
@@ -236,10 +266,12 @@ void InstantService::Shutdown() {
                    instant_io_context_));
   }
 
-  scoped_refptr<history::TopSites> top_sites =
-      TopSitesFactory::GetForProfile(profile_);
-  if (top_sites)
-    top_sites->RemoveObserver(this);
+  if (most_visited_sites_) {
+    most_visited_sites_.reset();
+  } else if (top_sites_) {
+    top_sites_->RemoveObserver(this);
+    top_sites_ = nullptr;
+  }
 
   instant_io_context_ = NULL;
 }
@@ -277,20 +309,39 @@ void InstantService::OnRendererProcessTerminated(int process_id) {
   }
 }
 
-void InstantService::OnMostVisitedItemsReceived(
+void InstantService::OnTopSitesReceived(
     const history::MostVisitedURLList& data) {
   most_visited_items_.clear();
   for (const history::MostVisitedURL& mv_url : data) {
     InstantMostVisitedItem item;
     item.url = mv_url.url;
     item.title = mv_url.title;
-    item.is_server_side_suggestion = false;
+    item.source = ntp_tiles::NTPTileSource::TOP_SITES;
+    most_visited_items_.push_back(item);
+  }
+
+  NotifyAboutMostVisitedItems();
+}
+
+void InstantService::OnMostVisitedURLsAvailable(
+    const ntp_tiles::NTPTilesVector& tiles) {
+  DCHECK(most_visited_sites_);
+  most_visited_items_.clear();
+  for (const ntp_tiles::NTPTile& tile : tiles) {
+    InstantMostVisitedItem item;
+    item.url = tile.url;
+    item.title = tile.title;
+    item.thumbnail = tile.thumbnail_url;
+    item.favicon = tile.favicon_url;
+    item.source = tile.source;
     most_visited_items_.push_back(item);
   }
 
   NotifyAboutMostVisitedItems();
 }
 
+void InstantService::OnIconMadeAvailable(const GURL& site_url) {}
+
 void InstantService::NotifyAboutMostVisitedItems() {
   for (InstantServiceObserver& observer : observers_)
     observer.MostVisitedItemsChanged(most_visited_items_);
@@ -451,17 +502,20 @@ void InstantService::OnTemplateURLServiceChanged() {
 }
 
 void InstantService::TopSitesLoaded(history::TopSites* top_sites) {
+  DCHECK(!most_visited_sites_);
+  DCHECK_EQ(top_sites_.get(), top_sites);
 }
 
 void InstantService::TopSitesChanged(history::TopSites* top_sites,
                                      ChangeReason change_reason) {
+  DCHECK(!most_visited_sites_);
+  DCHECK_EQ(top_sites_.get(), top_sites);
   // As forced urls already come from tiles, we can safely ignore those updates.
   if (change_reason == history::TopSitesObserver::ChangeReason::FORCED_URL)
     return;
-  top_sites->GetMostVisitedURLs(
-      base::Bind(&InstantService::OnMostVisitedItemsReceived,
-                 weak_ptr_factory_.GetWeakPtr()),
-      false);
+  top_sites_->GetMostVisitedURLs(base::Bind(&InstantService::OnTopSitesReceived,
+                                            weak_ptr_factory_.GetWeakPtr()),
+                                 false);
 }
 
 void InstantService::ResetInstantSearchPrerendererIfNecessary() {

chrome/browser/search/instant_service.h

@@ -11,11 +11,14 @@
 
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "components/history/core/browser/history_types.h"
 #include "components/history/core/browser/top_sites_observer.h"
 #include "components/keyed_service/core/keyed_service.h"
+#include "components/ntp_tiles/most_visited_sites.h"
+#include "components/ntp_tiles/ntp_tile.h"
 #include "components/search_engines/template_url_service_observer.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
@@ -34,11 +37,16 @@ namespace content {
 class RenderProcessHost;
 }
 
+namespace history {
+class TopSites;
+}
+
 // Tracks render process host IDs that are associated with Instant.
 class InstantService : public KeyedService,
                        public content::NotificationObserver,
                        public TemplateURLServiceObserver,
-                       public history::TopSitesObserver {
+                       public history::TopSitesObserver,
+                       public ntp_tiles::MostVisitedSites::Observer {
  public:
   explicit InstantService(Profile* profile);
   ~InstantService() override;
@@ -127,7 +135,12 @@ class InstantService : public KeyedService,
   // Called when we get new most visited items from TopSites, registered as an
   // async callback. Parses them and sends them to the renderer via
   // NotifyAboutMostVisitedItems.
-  void OnMostVisitedItemsReceived(const history::MostVisitedURLList& data);
+  void OnTopSitesReceived(const history::MostVisitedURLList& data);
+
+  // ntp_tiles::MostVisitedSites::Observer implementation.
+  void OnMostVisitedURLsAvailable(
+      const ntp_tiles::NTPTilesVector& tiles) override;
+  void OnIconMadeAvailable(const GURL& site_url) override;
 
   // Notifies the observer about the last known most visited items.
   void NotifyAboutMostVisitedItems();
@@ -168,6 +181,11 @@ class InstantService : public KeyedService,
   std::unique_ptr<TemplateURLData> previous_default_search_provider_;
   GURL previous_google_base_url_;
 
+  // Data sources for NTP tiles (aka Most Visited tiles). Only one of these will
+  // be non-null.
+  std::unique_ptr<ntp_tiles::MostVisitedSites> most_visited_sites_;
+  scoped_refptr<history::TopSites> top_sites_;
+
   // Used for Top Sites async retrieval.
   base::WeakPtrFactory<InstantService> weak_ptr_factory_;
 

chrome/browser/search/instant_service_unittest.cc

@@ -179,9 +179,9 @@ TEST_F(InstantServiceTest, GetSuggestionFromClientSide) {
   history::MostVisitedURLList url_list;
   url_list.push_back(history::MostVisitedURL());
 
-  instant_service_->OnMostVisitedItemsReceived(url_list);
+  instant_service_->OnTopSitesReceived(url_list);
 
   auto items = instant_service_->most_visited_items_;
   ASSERT_EQ(1, (int)items.size());
-  ASSERT_FALSE(items[0].is_server_side_suggestion);
+  ASSERT_EQ(ntp_tiles::NTPTileSource::TOP_SITES, items[0].source);
 }

chrome/common/render_messages.h

@@ -137,6 +137,7 @@ IPC_STRUCT_TRAITS_BEGIN(InstantMostVisitedItem)
   IPC_STRUCT_TRAITS_MEMBER(title)
   IPC_STRUCT_TRAITS_MEMBER(thumbnail)
   IPC_STRUCT_TRAITS_MEMBER(favicon)
+  IPC_STRUCT_TRAITS_MEMBER(source)
 IPC_STRUCT_TRAITS_END()
 
 IPC_STRUCT_TRAITS_BEGIN(RendererContentSettingRules)

chrome/common/search/instant_types.cc

@@ -88,7 +88,8 @@ const char kRLZParameterKey[] = "rlz";
 const char kInputEncodingKey[] = "ie";
 const char kAssistedQueryStatsKey[] = "aqs";
 
-InstantMostVisitedItem::InstantMostVisitedItem() {}
+InstantMostVisitedItem::InstantMostVisitedItem()
+    : source(ntp_tiles::NTPTileSource::TOP_SITES) {}
 
 InstantMostVisitedItem::InstantMostVisitedItem(
     const InstantMostVisitedItem& other) = default;

chrome/common/search/instant_types.h

@@ -11,6 +11,7 @@
 #include <utility>
 
 #include "base/strings/string16.h"
+#include "components/ntp_tiles/ntp_tile_source.h"
 #include "url/gurl.h"
 
 // ID used by Instant code to refer to objects (e.g. Autocomplete results, Most
@@ -141,9 +142,8 @@ struct InstantMostVisitedItem {
   // The external URL of the favicon associated with this page.
   GURL favicon;
 
-  // True if it's a server side suggestion.
-  // Otherwise, it's a client side suggestion.
-  bool is_server_side_suggestion;
+  // The source of the item, e.g. server-side or client-side.
+  ntp_tiles::NTPTileSource source;
 };
 
 // An InstantMostVisitedItem along with its assigned restricted ID.

chrome/renderer/searchbox/searchbox_extension.cc

@@ -169,6 +169,7 @@ v8::Local<v8::Object> GenerateMostVisitedItem(
   // Otherwise, we just create an array with the generated one.
   if (!mv_item.thumbnail.spec().empty()) {
     v8::Local<v8::Array> thumbs = v8::Array::New(isolate, 2);
+    // Note: The "thumb2" source captures a thumbnail on the next visit.
     thumbs->Set(0, GenerateThumb2URL(isolate, mv_item.url.spec()));
     thumbs->Set(1, UTF8ToV8String(isolate, mv_item.thumbnail.spec()));
     obj->Set(v8::String::NewFromUtf8(isolate, "thumbnailUrls"), thumbs);
@@ -185,6 +186,9 @@ v8::Local<v8::Object> GenerateMostVisitedItem(
              UTF8ToV8String(isolate, mv_item.favicon.spec()));
   }
 
+  obj->Set(v8::String::NewFromUtf8(isolate, "tileSource"),
+           v8::Integer::New(isolate, static_cast<int>(mv_item.source)));
+
   if (IsIconNTPEnabled()) {
     // Update website http://www.chromium.org/embeddedsearch when we make this
     // permanent.

components/ntp_tiles/most_visited_sites.cc

@@ -109,6 +109,10 @@ void MostVisitedSites::SetMostVisitedURLsObserver(Observer* observer,
   // SuggestionsService's cache or, if that is empty, sites from TopSites.
   BuildCurrentTiles();
   // Also start a request for fresh suggestions.
+  Refresh();
+}
+
+void MostVisitedSites::Refresh() {
   suggestions_service_->FetchSuggestionsData();
 }
 
@@ -131,6 +135,18 @@ void MostVisitedSites::AddOrRemoveBlacklistedUrl(const GURL& url,
   }
 }
 
+void MostVisitedSites::ClearBlacklistedUrls() {
+  if (top_sites_) {
+    // Always update the blacklist in the local TopSites.
+    top_sites_->ClearBlacklistedURLs();
+  }
+
+  // Only update the server-side blacklist if it's active.
+  if (mv_source_ == NTPTileSource::SUGGESTIONS_SERVICE) {
+    suggestions_service_->ClearBlacklist();
+  }
+}
+
 void MostVisitedSites::OnBlockedSitesChanged() {
   BuildCurrentTiles();
 }
@@ -219,6 +235,8 @@ void MostVisitedSites::OnSuggestionsProfileAvailable(
     tile.url = url;
     tile.source = NTPTileSource::SUGGESTIONS_SERVICE;
     tile.whitelist_icon_path = GetWhitelistLargeIconPath(url);
+    tile.thumbnail_url = GURL(suggestion_pb.thumbnail());
+    tile.favicon_url = GURL(suggestion_pb.favicon_url());
 
     tiles.push_back(std::move(tile));
   }

components/ntp_tiles/most_visited_sites.h

@@ -102,11 +102,17 @@ class MostVisitedSites : public history::TopSitesObserver,
 
   ~MostVisitedSites() override;
 
+  // Sets the observer, and immediately fetches the current suggestions.
   // Does not take ownership of |observer|, which must outlive this object and
   // must not be null.
   void SetMostVisitedURLsObserver(Observer* observer, int num_sites);
 
+  // Requests an asynchronous refresh of the suggestions. Notifies the observer
+  // once the request completes.
+  void Refresh();
+
   void AddOrRemoveBlacklistedUrl(const GURL& url, bool add_url);
+  void ClearBlacklistedUrls();
 
   // MostVisitedSitesSupervisor::Observer implementation.
   void OnBlockedSitesChanged() override;

components/ntp_tiles/ntp_tile.h

@@ -26,6 +26,12 @@ struct NTPTile {
   // is also available from another, higher-priority source.
   base::FilePath whitelist_icon_path;
 
+  // Only valid for source == SUGGESTIONS_SERVICE (empty otherwise).
+  // May point to a local chrome:// URL or to a remote one. May be empty.
+  GURL thumbnail_url;
+  // This won't be empty, but might 404 etc.
+  GURL favicon_url;
+
   NTPTile();
   NTPTile(const NTPTile&);
   ~NTPTile();