[NTP] Add isCustomLink property to EmbeddedSearchAPI

The property is used to determine if we need to show the "Restore
default shortcuts" option. Previously this was calculated by checking
the tile source of the most visited items, but this did not account for
the case where there are no available items. As such, if there were no
custom links, the restore default option was disabled.

Bug: 879082
Change-Id: Ia15f4fc863642b520234c061ee5ecf6f7a54c50e
Reviewed-on: https://chromium-review.googlesource.com/1197465
Commit-Queue: Kristi Park <kristipark@chromium.org>
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Reviewed-by: Mathieu Perreault <mathp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587835}

chrome/browser/resources/local_ntp/most_visited_single.js

@@ -154,13 +154,6 @@ let isMDEnabled = false;
 let isCustomLinksEnabled = false;
 
 
-/**
- * True if the tiles to be shown are custom links.
- * @type {boolean}
- */
-let tilesAreCustomLinks = false;
-
-
 /**
  * Log an event on the NTP.
  * @param {number} eventType Event from LOG_TYPE.
@@ -213,19 +206,18 @@ var countLoad = function() {
   if (loadedCounter <= 0) {
     swapInNewTiles();
     logEvent(LOG_TYPE.NTP_ALL_TILES_LOADED);
+    let tilesAreCustomLinks =
+        isCustomLinksEnabled && chrome.embeddedSearch.newTabPage.isCustomLinks;
     // Note that it's easiest to capture this when all custom links are loaded,
     // rather than when the impression for each link is logged.
-    if (isCustomLinksEnabled && tilesAreCustomLinks) {
+    if (tilesAreCustomLinks) {
       chrome.embeddedSearch.newTabPage.logEvent(
           LOG_TYPE.NTP_SHORTCUT_CUSTOMIZED);
     }
     // Tell the parent page whether to show the restore default shortcuts option
     // in the menu.
     window.parent.postMessage(
-        {
-          cmd: 'loaded',
-          showRestoreDefault: (isCustomLinksEnabled && tilesAreCustomLinks)
-        },
+        {cmd: 'loaded', showRestoreDefault: tilesAreCustomLinks},
         DOMAIN_ORIGIN);
     tilesAreCustomLinks = false;
     // Reset to 1, so that any further 'show' message will cause us to swap in
@@ -671,9 +663,6 @@ function renderMaterialDesignTile(data) {
   }
   mdTileContainer.className = CLASSES.MD_TILE_CONTAINER;
 
-  if (data.isCustomLink)
-    tilesAreCustomLinks = true;
-
   // The tile will be appended to tiles.
   const position = tiles.children.length;
   // This is set in the load/error event for the favicon image.

chrome/browser/search/instant_service.cc

@@ -388,8 +388,11 @@ void InstantService::OnURLsAvailable(
 void InstantService::OnIconMadeAvailable(const GURL& site_url) {}
 
 void InstantService::NotifyAboutMostVisitedItems() {
+  bool is_custom_links = features::IsCustomLinksEnabled() && most_visited_sites_
+                             ? most_visited_sites_->IsCustomLinksInitialized()
+                             : false;
   for (InstantServiceObserver& observer : observers_)
-    observer.MostVisitedItemsChanged(most_visited_items_);
+    observer.MostVisitedItemsChanged(most_visited_items_, is_custom_links);
 }
 
 void InstantService::NotifyAboutThemeInfo() {

chrome/browser/search/instant_service_observer.cc

@@ -8,5 +8,5 @@ void InstantServiceObserver::ThemeInfoChanged(const ThemeBackgroundInfo&) {
 }
 
 void InstantServiceObserver::MostVisitedItemsChanged(
-    const std::vector<InstantMostVisitedItem>&) {
-}
+    const std::vector<InstantMostVisitedItem>&,
+    bool is_custom_links) {}

chrome/browser/search/instant_service_observer.h

@@ -22,9 +22,11 @@ class InstantServiceObserver {
   // Indicates that the user's custom theme has changed in some way.
   virtual void ThemeInfoChanged(const ThemeBackgroundInfo&);
 
-  // Indicates that the most visited items has changed.
+  // Indicates that the most visited items has changed. |is_custom_links| is
+  // true if the items are custom links.
   virtual void MostVisitedItemsChanged(
-      const std::vector<InstantMostVisitedItem>&);
+      const std::vector<InstantMostVisitedItem>&,
+      bool is_custom_links);
 
  protected:
   virtual ~InstantServiceObserver() {}

chrome/browser/ui/search/local_ntp_backgrounds_browsertest.cc

@@ -72,8 +72,8 @@ class TestThemeInfoObserver : public InstantServiceObserver {
     }
   }
 
-  void MostVisitedItemsChanged(
-      const std::vector<InstantMostVisitedItem>&) override {}
+  void MostVisitedItemsChanged(const std::vector<InstantMostVisitedItem>&,
+                               bool is_custom_links) override {}
 
   InstantService* const service_;
 

chrome/browser/ui/search/local_ntp_browsertest.cc

@@ -84,8 +84,8 @@ class TestMostVisitedObserver : public InstantServiceObserver {
  private:
   void ThemeInfoChanged(const ThemeBackgroundInfo&) override {}
 
-  void MostVisitedItemsChanged(
-      const std::vector<InstantMostVisitedItem>& items) override {
+  void MostVisitedItemsChanged(const std::vector<InstantMostVisitedItem>& items,
+                               bool is_custom_links) override {
     items_ = items;
 
     if (quit_closure_ && items_.size() == expected_count_) {

chrome/browser/ui/search/search_ipc_router.cc

@@ -124,11 +124,12 @@ void SearchIPCRouter::OmniboxFocusChanged(OmniboxFocusState state,
 }
 
 void SearchIPCRouter::SendMostVisitedItems(
-    const std::vector<InstantMostVisitedItem>& items) {
+    const std::vector<InstantMostVisitedItem>& items,
+    bool is_custom_links) {
   if (!policy_->ShouldSendMostVisitedItems())
     return;
 
-  embedded_search_client()->MostVisitedChanged(items);
+  embedded_search_client()->MostVisitedChanged(items, is_custom_links);
 }
 
 void SearchIPCRouter::SendThemeBackgroundInfo(

chrome/browser/ui/search/search_ipc_router.h

@@ -178,7 +178,8 @@ class SearchIPCRouter : public content::WebContentsObserver,
                            OmniboxFocusChangeReason reason);
 
   // Tells the renderer about the most visited items.
-  void SendMostVisitedItems(const std::vector<InstantMostVisitedItem>& items);
+  void SendMostVisitedItems(const std::vector<InstantMostVisitedItem>& items,
+                            bool is_custom_links);
 
   // Tells the renderer about the current theme background.
   void SendThemeBackgroundInfo(const ThemeBackgroundInfo& theme_info);

chrome/browser/ui/search/search_ipc_router_unittest.cc

@@ -726,9 +726,9 @@ TEST_F(SearchIPCRouterTest, SendMostVisitedItemsMsg) {
       .Times(1)
       .WillOnce(Return(true));
 
-  EXPECT_CALL(*mock_embedded_search_client(), MostVisitedChanged(_));
+  EXPECT_CALL(*mock_embedded_search_client(), MostVisitedChanged(_, false));
   GetSearchIPCRouter().SendMostVisitedItems(
-      std::vector<InstantMostVisitedItem>());
+      std::vector<InstantMostVisitedItem>(), false);
 }
 
 TEST_F(SearchIPCRouterTest, DoNotSendMostVisitedItemsMsg) {
@@ -739,9 +739,10 @@ TEST_F(SearchIPCRouterTest, DoNotSendMostVisitedItemsMsg) {
       .Times(1)
       .WillOnce(Return(false));
 
-  EXPECT_CALL(*mock_embedded_search_client(), MostVisitedChanged(_)).Times(0);
+  EXPECT_CALL(*mock_embedded_search_client(), MostVisitedChanged(_, false))
+      .Times(0);
   GetSearchIPCRouter().SendMostVisitedItems(
-      std::vector<InstantMostVisitedItem>());
+      std::vector<InstantMostVisitedItem>(), false);
 }
 
 TEST_F(SearchIPCRouterTest, SendThemeBackgroundInfoMsg) {

chrome/browser/ui/search/search_tab_helper.cc

@@ -239,8 +239,9 @@ void SearchTabHelper::ThemeInfoChanged(const ThemeBackgroundInfo& theme_info) {
 }
 
 void SearchTabHelper::MostVisitedItemsChanged(
-    const std::vector<InstantMostVisitedItem>& items) {
-  ipc_router_.SendMostVisitedItems(items);
+    const std::vector<InstantMostVisitedItem>& items,
+    bool is_custom_links) {
+  ipc_router_.SendMostVisitedItems(items, is_custom_links);
 }
 
 void SearchTabHelper::FocusOmnibox(OmniboxFocusState state) {

chrome/browser/ui/search/search_tab_helper.h

@@ -125,8 +125,8 @@ class SearchTabHelper : public content::WebContentsObserver,
 
   // Overridden from InstantServiceObserver:
   void ThemeInfoChanged(const ThemeBackgroundInfo& theme_info) override;
-  void MostVisitedItemsChanged(
-      const std::vector<InstantMostVisitedItem>& items) override;
+  void MostVisitedItemsChanged(const std::vector<InstantMostVisitedItem>& items,
+                               bool is_custom_links) override;
 
   // Overridden from SelectFileDialog::Listener:
   void FileSelected(const base::FilePath& path,

chrome/common/search.mojom

@@ -120,7 +120,7 @@ interface EmbeddedSearchClient {
   FocusChanged(OmniboxFocusState new_focus_state,
                OmniboxFocusChangeReason reason);
 
-  MostVisitedChanged(array<InstantMostVisitedItem> items);
+  MostVisitedChanged(array<InstantMostVisitedItem> items, bool is_custom_links);
 
   SetInputInProgress(bool input_in_progress);
 

chrome/common/search/mock_embedded_search_client.h

@@ -17,8 +17,8 @@ class MockEmbeddedSearchClient : public chrome::mojom::EmbeddedSearchClient {
   MOCK_METHOD2(ChromeIdentityCheckResult, void(const base::string16&, bool));
   MOCK_METHOD2(FocusChanged, void(OmniboxFocusState, OmniboxFocusChangeReason));
   MOCK_METHOD1(HistorySyncCheckResult, void(bool));
-  MOCK_METHOD1(MostVisitedChanged,
-               void(const std::vector<InstantMostVisitedItem>&));
+  MOCK_METHOD2(MostVisitedChanged,
+               void(const std::vector<InstantMostVisitedItem>&, bool));
   MOCK_METHOD1(SetInputInProgress, void(bool));
   MOCK_METHOD1(ThemeChanged, void(const ThemeBackgroundInfo&));
   MOCK_METHOD0(SelectLocalImageSuccess, void());

chrome/renderer/searchbox/searchbox.cc

@@ -340,6 +340,10 @@ void SearchBox::UndoMostVisitedDeletion(
   embedded_search_service_->UndoMostVisitedDeletion(page_seq_no_, url);
 }
 
+bool SearchBox::IsCustomLinks() const {
+  return is_custom_links_;
+}
+
 void SearchBox::AddCustomLink(const GURL& url, const std::string& title) {
   if (!url.is_valid()) {
     AddCustomLinkResult(false);
@@ -477,8 +481,10 @@ void SearchBox::DeleteCustomLinkResult(bool success) {
 }
 
 void SearchBox::MostVisitedChanged(
-    const std::vector<InstantMostVisitedItem>& items) {
+    const std::vector<InstantMostVisitedItem>& items,
+    bool is_custom_links) {
   has_received_most_visited_ = true;
+  is_custom_links_ = is_custom_links;
 
   std::vector<InstantMostVisitedItemIDPair> last_known_items;
   GetMostVisitedItems(&last_known_items);

chrome/renderer/searchbox/searchbox.h

@@ -119,6 +119,9 @@ class SearchBox : public content::RenderFrameObserver,
   // Sends UndoMostVisitedDeletion to the browser.
   void UndoMostVisitedDeletion(InstantRestrictedID most_visited_item_id);
 
+  // Returns true if the most visited items are custom links.
+  bool IsCustomLinks() const;
+
   // Sends AddCustomLink to the browser.
   void AddCustomLink(const GURL& url, const std::string& title);
 
@@ -167,8 +170,8 @@ class SearchBox : public content::RenderFrameObserver,
   void SetPageSequenceNumber(int page_seq_no) override;
   void FocusChanged(OmniboxFocusState new_focus_state,
                     OmniboxFocusChangeReason reason) override;
-  void MostVisitedChanged(
-      const std::vector<InstantMostVisitedItem>& items) override;
+  void MostVisitedChanged(const std::vector<InstantMostVisitedItem>& items,
+                          bool is_custom_links) override;
   void SetInputInProgress(bool input_in_progress) override;
   void ThemeChanged(const ThemeBackgroundInfo& theme_info) override;
 
@@ -206,6 +209,8 @@ class SearchBox : public content::RenderFrameObserver,
   bool is_key_capture_enabled_;
   InstantRestrictedIDCache<InstantMostVisitedItem> most_visited_items_cache_;
   bool has_received_most_visited_;
+  // True if the most visited items are custom links.
+  bool is_custom_links_ = false;
   ThemeBackgroundInfo theme_info_;
 
   base::WeakPtrFactory<SearchBox> weak_ptr_factory_;

chrome/renderer/searchbox/searchbox_extension.cc

@@ -169,8 +169,6 @@ v8::Local<v8::Object> GenerateMostVisitedItemData(
       .Set("thumbnailUrl", thumbnail_url)
       .Set("tileTitleSource", static_cast<int>(mv_item.title_source))
       .Set("tileSource", static_cast<int>(mv_item.source))
-      .Set("isCustomLink",
-           mv_item.source == ntp_tiles::TileSource::CUSTOM_LINKS)
       .Set("title", title)
       .Set("domain", mv_item.url.host())
       .Set("direction", base::StringPiece(direction))
@@ -605,6 +603,7 @@ class NewTabPageBindings : public gin::Wrappable<NewTabPageBindings> {
   static v8::Local<v8::Value> GetMostVisited(v8::Isolate* isolate);
   static bool GetMostVisitedAvailable(v8::Isolate* isolate);
   static v8::Local<v8::Value> GetThemeBackgroundInfo(v8::Isolate* isolate);
+  static bool GetIsCustomLinks();
 
   // Handlers for JS functions visible to all NTPs.
   static void CheckIsUserSignedInToChromeAs(const std::string& identity);
@@ -664,6 +663,7 @@ gin::ObjectTemplateBuilder NewTabPageBindings::GetObjectTemplateBuilder(
                    &NewTabPageBindings::GetMostVisitedAvailable)
       .SetProperty("themeBackgroundInfo",
                    &NewTabPageBindings::GetThemeBackgroundInfo)
+      .SetProperty("isCustomLinks", &NewTabPageBindings::GetIsCustomLinks)
       .SetMethod("checkIsUserSignedIntoChromeAs",
                  &NewTabPageBindings::CheckIsUserSignedInToChromeAs)
       .SetMethod("checkIsUserSyncingHistory",
@@ -759,6 +759,15 @@ v8::Local<v8::Value> NewTabPageBindings::GetThemeBackgroundInfo(
   return GenerateThemeBackgroundInfo(isolate, theme_info);
 }
 
+// static
+bool NewTabPageBindings::GetIsCustomLinks() {
+  const SearchBox* search_box = GetSearchBoxForCurrentContext();
+  if (!search_box || !HasOrigin(GURL(chrome::kChromeSearchMostVisitedUrl)))
+    return false;
+
+  return search_box->IsCustomLinks();
+}
+
 // static
 void NewTabPageBindings::CheckIsUserSignedInToChromeAs(
     const std::string& identity) {