[NTP] Add undo functionality for all custom link actions and added notifications

Add the ability to undo add, edit, and delete for custom links. Also
added separate notifications for each action.

Screenshots:
https://screenshot.googleplex.com/rmGyjvf4AD9.png
https://screenshot.googleplex.com/fvp9f4zFMJ8.png
https://screenshot.googleplex.com/7FRRUa3OGuN.png

Bug: 856394, 851293
Change-Id: I42f218e6ad2f4b384491ddc6abf26cabd23afb18
Reviewed-on: https://chromium-review.googlesource.com/1155633Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Kristi Park <kristipark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579949}

chrome/browser/resources/local_ntp/local_ntp.js

@@ -523,9 +523,45 @@ function reloadTiles() {
 
 
 /**
- * Shows the blacklist notification and triggers a delay to hide it.
+ * Callback for embeddedSearch.newTabPage.onaddcustomlinkdone. Called when the
+ * custom link was successfully added. Shows the "Shortcut added" notification.
+ * @param {string} success True if the link was successfully added.
  */
-function showNotification() {
+function onAddCustomLinkDone(success) {
+  showNotification(configData.translatedStrings.linkAddedMsg);
+}
+
+
+/**
+ * Callback for embeddedSearch.newTabPage.onupdatecustomlinkdone. Called when
+ * the custom link was successfully updated. Shows the "Shortcut edited"
+ * notification.
+ * @param {string} success True if the link was successfully updated.
+ */
+function onUpdateCustomLinkDone(success) {
+  showNotification(configData.translatedStrings.linkEditedMsg);
+}
+
+
+/**
+ * Callback for embeddedSearch.newTabPage.ondeletecustomlinkdone. Called when
+ * the custom link was successfully deleted. Shows the "Shortcut deleted"
+ * notification.
+ * @param {string} success True if the link was successfully deleted.
+ */
+function onDeleteCustomLinkDone(success) {
+  showNotification(configData.translatedStrings.linkRemovedMsg);
+}
+
+
+/**
+ * Shows the pop-up notification and triggers a delay to hide it. The message
+ * will be set to |msg|.
+ * @param {string} msg The notification message.
+ */
+function showNotification(msg) {
+  $(IDS.NOTIFICATION_MESSAGE).textContent = msg;
+
   if (configData.isMDIconsEnabled || configData.isMDUIEnabled) {
     $(IDS.NOTIFICATION).classList.remove(CLASSES.HIDE_NOTIFICATION);
     // Timeout is required for the "float up" transition to work. Modifying the
@@ -549,7 +585,7 @@ function showNotification() {
 
 
 /**
- * Hides the blacklist notification.
+ * Hides the pop-up notification.
  */
 function hideNotification() {
   if (configData.isMDIconsEnabled || configData.isMDUIEnabled) {
@@ -581,9 +617,10 @@ function hideNotification() {
  */
 function onUndo() {
   hideNotification();
-  if (lastBlacklistedTile != null) {
+  if (configData.isCustomLinksEnabled)
+    ntpApiHandle.undoCustomLinkAction();
+  else if (lastBlacklistedTile != null)
     ntpApiHandle.undoMostVisitedDeletion(lastBlacklistedTile);
-  }
 }
 
 
@@ -593,7 +630,10 @@ function onUndo() {
  */
 function onRestoreAll() {
   hideNotification();
-  ntpApiHandle.undoAllMostVisitedDeletions();
+  if (configData.isCustomLinksEnabled)
+    ntpApiHandle.resetCustomLinks();
+  else
+    ntpApiHandle.undoAllMostVisitedDeletions();
 }
 
 
@@ -713,7 +753,7 @@ function handlePostMessage(event) {
               !args.showRestoreDefault);
     }
   } else if (cmd === 'tileBlacklisted') {
-    showNotification();
+    showNotification(configData.translatedStrings.linkRemovedMsg);
     lastBlacklistedTile = args.tid;
 
     ntpApiHandle.deleteMostVisitedItem(args.tid);
@@ -876,6 +916,12 @@ function init() {
       enableMDIcons();
     }
 
+    if (configData.isCustomLinksEnabled) {
+      ntpApiHandle.onaddcustomlinkdone = onAddCustomLinkDone;
+      ntpApiHandle.onupdatecustomlinkdone = onUpdateCustomLinkDone;
+      ntpApiHandle.ondeletecustomlinkdone = onDeleteCustomLinkDone;
+    }
+
     if (configData.isCustomBackgroundsEnabled ||
         configData.isCustomLinksEnabled) {
       customBackgrounds.init();
@@ -896,6 +942,7 @@ function init() {
       }
     }
 
+
     // Set up the fakebox (which only exists on the Google NTP).
     ntpApiHandle.oninputstart = onInputStart;
     ntpApiHandle.oninputcancel = onInputCancel;

chrome/browser/search/instant_service.cc

@@ -196,36 +196,39 @@ void InstantService::UndoAllMostVisitedDeletions() {
   }
 }
 
-void InstantService::AddCustomLink(const GURL& url, const std::string& title) {
+bool InstantService::AddCustomLink(const GURL& url, const std::string& title) {
   if (most_visited_sites_) {
     // Initializes custom links if they have not been initialized yet.
     most_visited_sites_->InitializeCustomLinks();
-    most_visited_sites_->AddCustomLink(url, base::UTF8ToUTF16(title));
+    return most_visited_sites_->AddCustomLink(url, base::UTF8ToUTF16(title));
   }
+  return false;
 }
 
-void InstantService::UpdateCustomLink(const GURL& url,
+bool InstantService::UpdateCustomLink(const GURL& url,
                                       const GURL& new_url,
                                       const std::string& new_title) {
   if (most_visited_sites_) {
     // Initializes custom links if they have not been initialized yet.
     most_visited_sites_->InitializeCustomLinks();
-    most_visited_sites_->UpdateCustomLink(url, new_url,
-                                          base::UTF8ToUTF16(new_title));
+    return most_visited_sites_->UpdateCustomLink(url, new_url,
+                                                 base::UTF8ToUTF16(new_title));
   }
+  return false;
 }
 
-void InstantService::DeleteCustomLink(const GURL& url) {
+bool InstantService::DeleteCustomLink(const GURL& url) {
   if (most_visited_sites_) {
     // Initializes custom links if they have not been initialized yet.
     most_visited_sites_->InitializeCustomLinks();
-    most_visited_sites_->DeleteCustomLink(url);
+    return most_visited_sites_->DeleteCustomLink(url);
   }
+  return false;
 }
 
-void InstantService::UndoDeleteCustomLink() {
+void InstantService::UndoCustomLinkAction() {
   if (most_visited_sites_) {
-    most_visited_sites_->UndoDeleteCustomLink();
+    most_visited_sites_->UndoCustomLinkAction();
   }
 }
 

chrome/browser/search/instant_service.h

@@ -81,16 +81,16 @@ class InstantService : public KeyedService,
   // Invoked when the Instant page wants to undo all Most Visited deletions.
   void UndoAllMostVisitedDeletions();
   // Invoked when the Instant page wants to add a custom link.
-  void AddCustomLink(const GURL& url, const std::string& title);
+  bool AddCustomLink(const GURL& url, const std::string& title);
   // Invoked when the Instant page wants to update a custom link.
-  void UpdateCustomLink(const GURL& url,
+  bool UpdateCustomLink(const GURL& url,
                         const GURL& new_url,
                         const std::string& new_title);
   // Invoked when the Instant page wants to delete a custom link.
-  void DeleteCustomLink(const GURL& url);
-  // Invoked when the Instant page wants to restore the previously deleted
-  // custom link.
-  void UndoDeleteCustomLink();
+  bool DeleteCustomLink(const GURL& url);
+  // Invoked when the Instant page wants to undo the previous custom link
+  // action.
+  void UndoCustomLinkAction();
   // Invoked when the Instant page wants to delete all custom links and use Most
   // Visited sites instead.
   void ResetCustomLinks();

chrome/browser/ui/search/search_ipc_router.cc

@@ -190,47 +190,50 @@ void SearchIPCRouter::UndoAllMostVisitedDeletions(int page_seq_no) {
 
 void SearchIPCRouter::AddCustomLink(int page_seq_no,
                                     const GURL& url,
-                                    const std::string& title) {
-  if (page_seq_no != commit_counter_)
-    return;
-
-  if (!policy_->ShouldProcessAddCustomLink())
-    return;
+                                    const std::string& title,
+                                    AddCustomLinkCallback callback) {
+  bool result = false;
+  if (page_seq_no == commit_counter_ && policy_->ShouldProcessAddCustomLink()) {
+    result = delegate_->OnAddCustomLink(url, title);
+  }
 
-  delegate_->OnAddCustomLink(url, title);
+  std::move(callback).Run(result);
 }
 
 void SearchIPCRouter::UpdateCustomLink(int page_seq_no,
                                        const GURL& url,
                                        const GURL& new_url,
-                                       const std::string& new_title) {
-  if (page_seq_no != commit_counter_)
-    return;
-
-  if (!policy_->ShouldProcessUpdateCustomLink())
-    return;
+                                       const std::string& new_title,
+                                       UpdateCustomLinkCallback callback) {
+  bool result = false;
+  if (page_seq_no == commit_counter_ &&
+      policy_->ShouldProcessUpdateCustomLink()) {
+    result = delegate_->OnUpdateCustomLink(url, new_url, new_title);
+  }
 
-  delegate_->OnUpdateCustomLink(url, new_url, new_title);
+  std::move(callback).Run(result);
 }
 
-void SearchIPCRouter::DeleteCustomLink(int page_seq_no, const GURL& url) {
-  if (page_seq_no != commit_counter_)
-    return;
-
-  if (!policy_->ShouldProcessDeleteCustomLink())
-    return;
+void SearchIPCRouter::DeleteCustomLink(int page_seq_no,
+                                       const GURL& url,
+                                       DeleteCustomLinkCallback callback) {
+  bool result = false;
+  if (page_seq_no == commit_counter_ &&
+      policy_->ShouldProcessDeleteCustomLink()) {
+    result = delegate_->OnDeleteCustomLink(url);
+  }
 
-  delegate_->OnDeleteCustomLink(url);
+  std::move(callback).Run(result);
 }
 
-void SearchIPCRouter::UndoDeleteCustomLink(int page_seq_no) {
+void SearchIPCRouter::UndoCustomLinkAction(int page_seq_no) {
   if (page_seq_no != commit_counter_)
     return;
 
-  if (!policy_->ShouldProcessUndoDeleteCustomLink())
+  if (!policy_->ShouldProcessUndoCustomLinkAction())
     return;
 
-  delegate_->OnUndoDeleteCustomLink();
+  delegate_->OnUndoCustomLinkAction();
 }
 
 void SearchIPCRouter::ResetCustomLinks(int page_seq_no) {

chrome/browser/ui/search/search_ipc_router.h

@@ -56,19 +56,19 @@ class SearchIPCRouter : public content::WebContentsObserver,
     virtual void OnUndoAllMostVisitedDeletions() = 0;
 
     // Called when the EmbeddedSearch wants to add a custom link.
-    virtual void OnAddCustomLink(const GURL& url, const std::string& title) = 0;
+    virtual bool OnAddCustomLink(const GURL& url, const std::string& title) = 0;
 
     // Called when the EmbeddedSearch wants to update a custom link.
-    virtual void OnUpdateCustomLink(const GURL& url,
+    virtual bool OnUpdateCustomLink(const GURL& url,
                                     const GURL& new_url,
                                     const std::string& new_title) = 0;
 
     // Called when the EmbeddedSearch wants to delete a custom link.
-    virtual void OnDeleteCustomLink(const GURL& url) = 0;
+    virtual bool OnDeleteCustomLink(const GURL& url) = 0;
 
-    // Called when the EmbeddedSearch wants to restore the previously deleted
-    // custom link.
-    virtual void OnUndoDeleteCustomLink() = 0;
+    // Called when the EmbeddedSearch wants to undo the previous custom link
+    // action.
+    virtual void OnUndoCustomLinkAction() = 0;
 
     // Called when the EmbeddedSearch wants to delete all custom links and
     // use Most Visited sites instead.
@@ -134,7 +134,7 @@ class SearchIPCRouter : public content::WebContentsObserver,
     virtual bool ShouldProcessAddCustomLink() = 0;
     virtual bool ShouldProcessUpdateCustomLink() = 0;
     virtual bool ShouldProcessDeleteCustomLink() = 0;
-    virtual bool ShouldProcessUndoDeleteCustomLink() = 0;
+    virtual bool ShouldProcessUndoCustomLinkAction() = 0;
     virtual bool ShouldProcessResetCustomLinks() = 0;
     virtual bool ShouldProcessLogEvent() = 0;
     virtual bool ShouldProcessPasteIntoOmnibox(bool is_active_tab) = 0;
@@ -196,13 +196,17 @@ class SearchIPCRouter : public content::WebContentsObserver,
   void UndoAllMostVisitedDeletions(int page_seq_no) override;
   void AddCustomLink(int page_seq_no,
                      const GURL& url,
-                     const std::string& title) override;
+                     const std::string& title,
+                     AddCustomLinkCallback callback) override;
   void UpdateCustomLink(int page_seq_no,
                         const GURL& url,
                         const GURL& new_url,
-                        const std::string& new_title) override;
-  void DeleteCustomLink(int page_seq_no, const GURL& url) override;
-  void UndoDeleteCustomLink(int page_seq_no) override;
+                        const std::string& new_title,
+                        UpdateCustomLinkCallback callback) override;
+  void DeleteCustomLink(int page_seq_no,
+                        const GURL& url,
+                        DeleteCustomLinkCallback callback) override;
+  void UndoCustomLinkAction(int page_seq_no) override;
   void ResetCustomLinks(int page_seq_no) override;
   void LogEvent(int page_seq_no,
                 NTPLoggingEventType event,

chrome/browser/ui/search/search_ipc_router_policy_impl.cc

@@ -50,7 +50,7 @@ bool SearchIPCRouterPolicyImpl::ShouldProcessDeleteCustomLink() {
   return !is_incognito_ && search::IsInstantNTP(web_contents_);
 }
 
-bool SearchIPCRouterPolicyImpl::ShouldProcessUndoDeleteCustomLink() {
+bool SearchIPCRouterPolicyImpl::ShouldProcessUndoCustomLinkAction() {
   return !is_incognito_ && search::IsInstantNTP(web_contents_);
 }
 

chrome/browser/ui/search/search_ipc_router_policy_impl.h

@@ -34,7 +34,7 @@ class SearchIPCRouterPolicyImpl : public SearchIPCRouter::Policy {
   bool ShouldProcessAddCustomLink() override;
   bool ShouldProcessUpdateCustomLink() override;
   bool ShouldProcessDeleteCustomLink() override;
-  bool ShouldProcessUndoDeleteCustomLink() override;
+  bool ShouldProcessUndoCustomLinkAction() override;
   bool ShouldProcessResetCustomLinks() override;
   bool ShouldProcessLogEvent() override;
   bool ShouldProcessPasteIntoOmnibox(bool is_active_tab) override;

chrome/browser/ui/search/search_ipc_router_unittest.cc

@@ -61,13 +61,13 @@ class MockSearchIPCRouterDelegate : public SearchIPCRouter::Delegate {
   MOCK_METHOD1(OnUndoMostVisitedDeletion, void(const GURL& url));
   MOCK_METHOD0(OnUndoAllMostVisitedDeletions, void());
   MOCK_METHOD2(OnAddCustomLink,
-               void(const GURL& url, const std::string& title));
+               bool(const GURL& url, const std::string& title));
   MOCK_METHOD3(OnUpdateCustomLink,
-               void(const GURL& url,
+               bool(const GURL& url,
                     const GURL& new_url,
                     const std::string& new_title));
-  MOCK_METHOD1(OnDeleteCustomLink, void(const GURL& url));
-  MOCK_METHOD0(OnUndoDeleteCustomLink, void());
+  MOCK_METHOD1(OnDeleteCustomLink, bool(const GURL& url));
+  MOCK_METHOD0(OnUndoCustomLinkAction, void());
   MOCK_METHOD0(OnResetCustomLinks, void());
   MOCK_METHOD2(OnLogEvent, void(NTPLoggingEventType event,
                                 base::TimeDelta time));
@@ -98,7 +98,7 @@ class MockSearchIPCRouterPolicy : public SearchIPCRouter::Policy {
   MOCK_METHOD0(ShouldProcessAddCustomLink, bool());
   MOCK_METHOD0(ShouldProcessUpdateCustomLink, bool());
   MOCK_METHOD0(ShouldProcessDeleteCustomLink, bool());
-  MOCK_METHOD0(ShouldProcessUndoDeleteCustomLink, bool());
+  MOCK_METHOD0(ShouldProcessUndoCustomLinkAction, bool());
   MOCK_METHOD0(ShouldProcessResetCustomLinks, bool());
   MOCK_METHOD0(ShouldProcessLogEvent, bool());
   MOCK_METHOD1(ShouldProcessPasteIntoOmnibox, bool(bool));
@@ -480,13 +480,17 @@ TEST_F(SearchIPCRouterTest, ProcessAddCustomLinkMsg) {
   MockSearchIPCRouterPolicy* policy = GetSearchIPCRouterPolicy();
   GURL item_url("www.foo.com");
   std::string item_title("foo");
-  EXPECT_CALL(*mock_delegate(), OnAddCustomLink(item_url, item_title)).Times(1);
+  EXPECT_CALL(*mock_delegate(), OnAddCustomLink(item_url, item_title))
+      .Times(1)
+      .WillOnce(Return(true));
   EXPECT_CALL(*policy, ShouldProcessAddCustomLink())
       .Times(1)
       .WillOnce(Return(true));
 
+  base::MockCallback<SearchIPCRouter::AddCustomLinkCallback> callback;
+  EXPECT_CALL(callback, Run(true));
   GetSearchIPCRouter().AddCustomLink(GetSearchIPCRouterSeqNo(), item_url,
-                                     item_title);
+                                     item_title, callback.Get());
 }
 
 TEST_F(SearchIPCRouterTest, IgnoreAddCustomLinkMsg) {
@@ -500,8 +504,10 @@ TEST_F(SearchIPCRouterTest, IgnoreAddCustomLinkMsg) {
       .Times(1)
       .WillOnce(Return(false));
 
+  base::MockCallback<SearchIPCRouter::AddCustomLinkCallback> callback;
+  EXPECT_CALL(callback, Run(false));
   GetSearchIPCRouter().AddCustomLink(GetSearchIPCRouterSeqNo(), item_url,
-                                     item_title);
+                                     item_title, callback.Get());
 }
 
 TEST_F(SearchIPCRouterTest, ProcessUpdateCustomLinkMsg) {
@@ -513,13 +519,16 @@ TEST_F(SearchIPCRouterTest, ProcessUpdateCustomLinkMsg) {
   std::string new_title("foo");
   EXPECT_CALL(*mock_delegate(),
               OnUpdateCustomLink(item_url, new_url, new_title))
-      .Times(1);
+      .Times(1)
+      .WillOnce(Return(true));
   EXPECT_CALL(*policy, ShouldProcessUpdateCustomLink())
       .Times(1)
       .WillOnce(Return(true));
 
+  base::MockCallback<SearchIPCRouter::AddCustomLinkCallback> callback;
+  EXPECT_CALL(callback, Run(true));
   GetSearchIPCRouter().UpdateCustomLink(GetSearchIPCRouterSeqNo(), item_url,
-                                        new_url, new_title);
+                                        new_url, new_title, callback.Get());
 }
 
 TEST_F(SearchIPCRouterTest, IgnoreUpdateCustomLinkMsg) {
@@ -536,8 +545,10 @@ TEST_F(SearchIPCRouterTest, IgnoreUpdateCustomLinkMsg) {
       .Times(1)
       .WillOnce(Return(false));
 
+  base::MockCallback<SearchIPCRouter::AddCustomLinkCallback> callback;
+  EXPECT_CALL(callback, Run(false));
   GetSearchIPCRouter().UpdateCustomLink(GetSearchIPCRouterSeqNo(), item_url,
-                                        new_url, new_title);
+                                        new_url, new_title, callback.Get());
 }
 
 TEST_F(SearchIPCRouterTest, ProcessDeleteCustomLinkMsg) {
@@ -545,12 +556,17 @@ TEST_F(SearchIPCRouterTest, ProcessDeleteCustomLinkMsg) {
   SetupMockDelegateAndPolicy();
   MockSearchIPCRouterPolicy* policy = GetSearchIPCRouterPolicy();
   GURL item_url("www.foo.com");
-  EXPECT_CALL(*mock_delegate(), OnDeleteCustomLink(item_url)).Times(1);
+  EXPECT_CALL(*mock_delegate(), OnDeleteCustomLink(item_url))
+      .Times(1)
+      .WillOnce(Return(true));
   EXPECT_CALL(*policy, ShouldProcessDeleteCustomLink())
       .Times(1)
       .WillOnce(Return(true));
 
-  GetSearchIPCRouter().DeleteCustomLink(GetSearchIPCRouterSeqNo(), item_url);
+  base::MockCallback<SearchIPCRouter::AddCustomLinkCallback> callback;
+  EXPECT_CALL(callback, Run(true));
+  GetSearchIPCRouter().DeleteCustomLink(GetSearchIPCRouterSeqNo(), item_url,
+                                        callback.Get());
 }
 
 TEST_F(SearchIPCRouterTest, IgnoreDeleteCustomLinkMsg) {
@@ -563,31 +579,34 @@ TEST_F(SearchIPCRouterTest, IgnoreDeleteCustomLinkMsg) {
       .Times(1)
       .WillOnce(Return(false));
 
-  GetSearchIPCRouter().DeleteCustomLink(GetSearchIPCRouterSeqNo(), item_url);
+  base::MockCallback<SearchIPCRouter::AddCustomLinkCallback> callback;
+  EXPECT_CALL(callback, Run(false));
+  GetSearchIPCRouter().DeleteCustomLink(GetSearchIPCRouterSeqNo(), item_url,
+                                        callback.Get());
 }
 
-TEST_F(SearchIPCRouterTest, ProcessUndoDeleteCustomLinkMsg) {
+TEST_F(SearchIPCRouterTest, ProcessUndoCustomLinkActionMsg) {
   NavigateAndCommitActiveTab(GURL("chrome-search://foo/bar"));
   SetupMockDelegateAndPolicy();
   MockSearchIPCRouterPolicy* policy = GetSearchIPCRouterPolicy();
-  EXPECT_CALL(*mock_delegate(), OnUndoDeleteCustomLink()).Times(1);
-  EXPECT_CALL(*policy, ShouldProcessUndoDeleteCustomLink())
+  EXPECT_CALL(*mock_delegate(), OnUndoCustomLinkAction()).Times(1);
+  EXPECT_CALL(*policy, ShouldProcessUndoCustomLinkAction())
       .Times(1)
       .WillOnce(Return(true));
 
-  GetSearchIPCRouter().UndoDeleteCustomLink(GetSearchIPCRouterSeqNo());
+  GetSearchIPCRouter().UndoCustomLinkAction(GetSearchIPCRouterSeqNo());
 }
 
-TEST_F(SearchIPCRouterTest, IgnoreUndoDeleteCustomLinkMsg) {
+TEST_F(SearchIPCRouterTest, IgnoreUndoCustomLinkActionMsg) {
   NavigateAndCommitActiveTab(GURL("chrome-search://foo/bar"));
   SetupMockDelegateAndPolicy();
   MockSearchIPCRouterPolicy* policy = GetSearchIPCRouterPolicy();
-  EXPECT_CALL(*mock_delegate(), OnUndoDeleteCustomLink()).Times(0);
-  EXPECT_CALL(*policy, ShouldProcessUndoDeleteCustomLink())
+  EXPECT_CALL(*mock_delegate(), OnUndoCustomLinkAction()).Times(0);
+  EXPECT_CALL(*policy, ShouldProcessUndoCustomLinkAction())
       .Times(1)
       .WillOnce(Return(false));
 
-  GetSearchIPCRouter().UndoDeleteCustomLink(GetSearchIPCRouterSeqNo());
+  GetSearchIPCRouter().UndoCustomLinkAction(GetSearchIPCRouterSeqNo());
 }
 
 TEST_F(SearchIPCRouterTest, ProcessResetCustomLinksMsg) {

chrome/browser/ui/search/search_tab_helper.cc

@@ -298,30 +298,33 @@ void SearchTabHelper::OnUndoAllMostVisitedDeletions() {
     instant_service_->UndoAllMostVisitedDeletions();
 }
 
-void SearchTabHelper::OnAddCustomLink(const GURL& url,
+bool SearchTabHelper::OnAddCustomLink(const GURL& url,
                                       const std::string& title) {
   DCHECK(!url.is_empty());
   if (instant_service_)
-    instant_service_->AddCustomLink(url, title);
+    return instant_service_->AddCustomLink(url, title);
+  return false;
 }
 
-void SearchTabHelper::OnUpdateCustomLink(const GURL& url,
+bool SearchTabHelper::OnUpdateCustomLink(const GURL& url,
                                          const GURL& new_url,
                                          const std::string& new_title) {
   DCHECK(!url.is_empty());
   if (instant_service_)
-    instant_service_->UpdateCustomLink(url, new_url, new_title);
+    return instant_service_->UpdateCustomLink(url, new_url, new_title);
+  return false;
 }
 
-void SearchTabHelper::OnDeleteCustomLink(const GURL& url) {
+bool SearchTabHelper::OnDeleteCustomLink(const GURL& url) {
   DCHECK(!url.is_empty());
   if (instant_service_)
-    instant_service_->DeleteCustomLink(url);
+    return instant_service_->DeleteCustomLink(url);
+  return false;
 }
 
-void SearchTabHelper::OnUndoDeleteCustomLink() {
+void SearchTabHelper::OnUndoCustomLinkAction() {
   if (instant_service_)
-    instant_service_->UndoDeleteCustomLink();
+    instant_service_->UndoCustomLinkAction();
 }
 
 void SearchTabHelper::OnResetCustomLinks() {

chrome/browser/ui/search/search_tab_helper.h

@@ -98,12 +98,12 @@ class SearchTabHelper : public content::WebContentsObserver,
   void OnDeleteMostVisitedItem(const GURL& url) override;
   void OnUndoMostVisitedDeletion(const GURL& url) override;
   void OnUndoAllMostVisitedDeletions() override;
-  void OnAddCustomLink(const GURL& url, const std::string& title) override;
-  void OnUpdateCustomLink(const GURL& url,
+  bool OnAddCustomLink(const GURL& url, const std::string& title) override;
+  bool OnUpdateCustomLink(const GURL& url,
                           const GURL& new_url,
                           const std::string& new_title) override;
-  void OnDeleteCustomLink(const GURL& url) override;
-  void OnUndoDeleteCustomLink() override;
+  bool OnDeleteCustomLink(const GURL& url) override;
+  void OnUndoCustomLinkAction() override;
   void OnResetCustomLinks() override;
   void OnLogEvent(NTPLoggingEventType event, base::TimeDelta time) override;
   void OnLogMostVisitedImpression(

chrome/browser/ui/search/search_tab_helper_unittest.cc

@@ -60,13 +60,13 @@ class MockSearchIPCRouterDelegate : public SearchIPCRouter::Delegate {
   MOCK_METHOD1(OnUndoMostVisitedDeletion, void(const GURL& url));
   MOCK_METHOD0(OnUndoAllMostVisitedDeletions, void());
   MOCK_METHOD2(OnAddCustomLink,
-               void(const GURL& url, const std::string& title));
+               bool(const GURL& url, const std::string& title));
   MOCK_METHOD3(OnUpdateCustomLink,
-               void(const GURL& url,
+               bool(const GURL& url,
                     const GURL& new_url,
                     const std::string& new_title));
-  MOCK_METHOD1(OnDeleteCustomLink, void(const GURL& url));
-  MOCK_METHOD0(OnUndoDeleteCustomLink, void());
+  MOCK_METHOD1(OnDeleteCustomLink, bool(const GURL& url));
+  MOCK_METHOD0(OnUndoCustomLinkAction, void());
   MOCK_METHOD0(OnResetCustomLinks, void());
   MOCK_METHOD2(OnLogEvent, void(NTPLoggingEventType event,
                                 base::TimeDelta time));

chrome/common/search.mojom

@@ -45,20 +45,22 @@ interface EmbeddedSearch {
   // Tells InstantExtended to undo one most visited item deletion.
   UndoMostVisitedDeletion(int32 page_seq_no, url.mojom.Url url);
 
-  // Tells InstantExtended to add a custom link.
-  AddCustomLink(int32 page_seq_no, url.mojom.Url url, string title);
+  // Tells InstantExtended to add a custom link. Returns true if successful.
+  AddCustomLink(int32 page_seq_no, url.mojom.Url url, string title)
+      => (bool success);
 
-  // Tells InstantExtended to update a custom link.
+  // Tells InstantExtended to update a custom link. Returns true if successful.
   UpdateCustomLink(int32 page_seq_no,
                    url.mojom.Url url,
                    url.mojom.Url new_url,
-                   string new_title);
+                   string new_title) => (bool success);
 
-  // Tells InstantExtended to delete a custom link.
-  DeleteCustomLink(int32 page_seq_no, url.mojom.Url url);
+  // Tells InstantExtended to delete a custom link. Returns true if successful.
+  DeleteCustomLink(int32 page_seq_no, url.mojom.Url url) => (bool success);
 
-  // Tells InstantExtended to restore the previously deleted custom link.
-  UndoDeleteCustomLink(int32 page_seq_no);
+  // Tells InstantExtended to undo the previous custom link action (add, edit,
+  // delete, etc.).
+  UndoCustomLinkAction(int32 page_seq_no);
 
   // Tells InstantExtended to delete all custom links and use most visited sites
   // instead.

chrome/renderer/searchbox/searchbox.cc

@@ -323,30 +323,44 @@ void SearchBox::UndoMostVisitedDeletion(
 }
 
 void SearchBox::AddCustomLink(const GURL& url, const std::string& title) {
-  if (!url.is_valid())
+  if (!url.is_valid()) {
+    AddCustomLinkResult(false);
     return;
-  embedded_search_service_->AddCustomLink(page_seq_no_, url, title);
+  }
+  embedded_search_service_->AddCustomLink(
+      page_seq_no_, url, title,
+      base::BindOnce(&SearchBox::AddCustomLinkResult,
+                     weak_ptr_factory_.GetWeakPtr()));
 }
 
 void SearchBox::UpdateCustomLink(InstantRestrictedID link_id,
                                  const GURL& new_url,
                                  const std::string& new_title) {
   GURL url = GetURLForMostVisitedItem(link_id);
-  if (!url.is_valid())
+  if (!url.is_valid()) {
+    UpdateCustomLinkResult(false);
     return;
-  embedded_search_service_->UpdateCustomLink(page_seq_no_, url, new_url,
-                                             new_title);
+  }
+  embedded_search_service_->UpdateCustomLink(
+      page_seq_no_, url, new_url, new_title,
+      base::BindOnce(&SearchBox::UpdateCustomLinkResult,
+                     weak_ptr_factory_.GetWeakPtr()));
 }
 
 void SearchBox::DeleteCustomLink(InstantRestrictedID most_visited_item_id) {
   GURL url = GetURLForMostVisitedItem(most_visited_item_id);
-  if (!url.is_valid())
+  if (!url.is_valid()) {
+    DeleteCustomLinkResult(false);
     return;
-  embedded_search_service_->DeleteCustomLink(page_seq_no_, url);
+  }
+  embedded_search_service_->DeleteCustomLink(
+      page_seq_no_, url,
+      base::BindOnce(&SearchBox::DeleteCustomLinkResult,
+                     weak_ptr_factory_.GetWeakPtr()));
 }
 
-void SearchBox::UndoDeleteCustomLink() {
-  embedded_search_service_->UndoDeleteCustomLink(page_seq_no_);
+void SearchBox::UndoCustomLinkAction() {
+  embedded_search_service_->UndoCustomLinkAction(page_seq_no_);
 }
 
 void SearchBox::ResetCustomLinks() {
@@ -419,6 +433,27 @@ void SearchBox::HistorySyncCheckResult(bool sync_history) {
   }
 }
 
+void SearchBox::AddCustomLinkResult(bool success) {
+  if (can_run_js_in_renderframe_) {
+    SearchBoxExtension::DispatchAddCustomLinkResult(
+        render_frame()->GetWebFrame(), success);
+  }
+}
+
+void SearchBox::UpdateCustomLinkResult(bool success) {
+  if (can_run_js_in_renderframe_) {
+    SearchBoxExtension::DispatchUpdateCustomLinkResult(
+        render_frame()->GetWebFrame(), success);
+  }
+}
+
+void SearchBox::DeleteCustomLinkResult(bool success) {
+  if (can_run_js_in_renderframe_) {
+    SearchBoxExtension::DispatchDeleteCustomLinkResult(
+        render_frame()->GetWebFrame(), success);
+  }
+}
+
 void SearchBox::MostVisitedChanged(
     const std::vector<InstantMostVisitedItem>& items) {
   has_received_most_visited_ = true;

chrome/renderer/searchbox/searchbox.h

@@ -130,8 +130,8 @@ class SearchBox : public content::RenderFrameObserver,
   // Sends DeleteCustomLink to the browser.
   void DeleteCustomLink(InstantRestrictedID most_visited_item_id);
 
-  // Sends UndoDeleteCustomLink to the browser.
-  void UndoDeleteCustomLink();
+  // Sends UndoCustomLinkAction to the browser.
+  void UndoCustomLinkAction();
 
   // Sends ResetCustomLinks to the browser.
   void ResetCustomLinks();
@@ -170,6 +170,10 @@ class SearchBox : public content::RenderFrameObserver,
   void HistorySyncCheckResult(bool sync_history);
   void ChromeIdentityCheckResult(const base::string16& identity,
                                  bool identity_match);
+  void AddCustomLinkResult(bool success);
+  void UpdateCustomLinkResult(bool success);
+  void DeleteCustomLinkResult(bool success);
+  void UndoCustomLinkActionResult(bool success);
 
   // Returns the URL of the Most Visited item specified by the |item_id|.
   GURL GetURLForMostVisitedItem(InstantRestrictedID item_id) const;

chrome/renderer/searchbox/searchbox_extension.cc

@@ -410,6 +410,39 @@ static const char kDispatchHistorySyncCheckResult[] =
     "  true;"
     "}";
 
+static const char kDispatchAddCustomLinkResult[] =
+    "if (window.chrome &&"
+    "    window.chrome.embeddedSearch &&"
+    "    window.chrome.embeddedSearch.newTabPage &&"
+    "    window.chrome.embeddedSearch.newTabPage.onaddcustomlinkdone &&"
+    "    typeof window.chrome.embeddedSearch.newTabPage"
+    "        .onaddcustomlinkdone === 'function') {"
+    "  window.chrome.embeddedSearch.newTabPage.onaddcustomlinkdone(%s);"
+    "  true;"
+    "}";
+
+static const char kDispatchUpdateCustomLinkResult[] =
+    "if (window.chrome &&"
+    "    window.chrome.embeddedSearch &&"
+    "    window.chrome.embeddedSearch.newTabPage &&"
+    "    window.chrome.embeddedSearch.newTabPage.onupdatecustomlinkdone &&"
+    "    typeof window.chrome.embeddedSearch.newTabPage"
+    "        .onupdatecustomlinkdone === 'function') {"
+    "  window.chrome.embeddedSearch.newTabPage.onupdatecustomlinkdone(%s);"
+    "  true;"
+    "}";
+
+static const char kDispatchDeleteCustomLinkResult[] =
+    "if (window.chrome &&"
+    "    window.chrome.embeddedSearch &&"
+    "    window.chrome.embeddedSearch.newTabPage &&"
+    "    window.chrome.embeddedSearch.newTabPage.ondeletecustomlinkdone &&"
+    "    typeof window.chrome.embeddedSearch.newTabPage"
+    "        .ondeletecustomlinkdone === 'function') {"
+    "  window.chrome.embeddedSearch.newTabPage.ondeletecustomlinkdone(%s);"
+    "  true;"
+    "}";
+
 static const char kDispatchInputCancelScript[] =
     "if (window.chrome &&"
     "    window.chrome.embeddedSearch &&"
@@ -592,6 +625,7 @@ class NewTabPageBindings : public gin::Wrappable<NewTabPageBindings> {
   static void UpdateCustomLink(int rid,
                                const std::string& url,
                                const std::string& title);
+  static void UndoCustomLinkAction();
   static void ResetCustomLinks();
   static void LogEvent(int event);
   static void LogMostVisitedImpression(
@@ -645,6 +679,8 @@ gin::ObjectTemplateBuilder NewTabPageBindings::GetObjectTemplateBuilder(
       .SetMethod("getMostVisitedItemData",
                  &NewTabPageBindings::GetMostVisitedItemData)
       .SetMethod("updateCustomLink", &NewTabPageBindings::UpdateCustomLink)
+      .SetMethod("undoCustomLinkAction",
+                 &NewTabPageBindings::UndoCustomLinkAction)
       .SetMethod("resetCustomLinks", &NewTabPageBindings::ResetCustomLinks)
       .SetMethod("logEvent", &NewTabPageBindings::LogEvent)
       .SetMethod("logMostVisitedImpression",
@@ -782,15 +818,7 @@ void NewTabPageBindings::UndoMostVisitedDeletion(
   if (!search_box)
     return;
 
-  // Treat the Most Visited item as a custom link if called from the Most
-  // Visited or edit custom link iframes, and if custom links is enabled. This
-  // will not initialize custom links.
-  if (ntp_tiles::IsCustomLinksEnabled() &&
-      HasOrigin(GURL(chrome::kChromeSearchMostVisitedUrl))) {
-    search_box->UndoDeleteCustomLink();
-  } else {
-    search_box->UndoMostVisitedDeletion(*rid);
-  }
+  search_box->UndoMostVisitedDeletion(*rid);
 }
 
 // static
@@ -836,6 +864,18 @@ void NewTabPageBindings::UpdateCustomLink(int rid,
   }
 }
 
+// static
+void NewTabPageBindings::UndoCustomLinkAction() {
+  if (!ntp_tiles::IsCustomLinksEnabled())
+    return;
+  SearchBox* search_box = GetSearchBoxForCurrentContext();
+  if (!search_box || !(HasOrigin(GURL(chrome::kChromeSearchMostVisitedUrl)) ||
+                       HasOrigin(GURL(chrome::kChromeSearchLocalNtpUrl)))) {
+    return;
+  }
+  search_box->UndoCustomLinkAction();
+}
+
 // static
 void NewTabPageBindings::ResetCustomLinks() {
   if (!ntp_tiles::IsCustomLinksEnabled())
@@ -993,6 +1033,33 @@ void SearchBoxExtension::DispatchHistorySyncCheckResult(
   Dispatch(frame, script);
 }
 
+// static
+void SearchBoxExtension::DispatchAddCustomLinkResult(
+    blink::WebLocalFrame* frame,
+    bool success) {
+  blink::WebString script(blink::WebString::FromUTF8(base::StringPrintf(
+      kDispatchAddCustomLinkResult, success ? "true" : "false")));
+  Dispatch(frame, script);
+}
+
+// static
+void SearchBoxExtension::DispatchUpdateCustomLinkResult(
+    blink::WebLocalFrame* frame,
+    bool success) {
+  blink::WebString script(blink::WebString::FromUTF8(base::StringPrintf(
+      kDispatchUpdateCustomLinkResult, success ? "true" : "false")));
+  Dispatch(frame, script);
+}
+
+// static
+void SearchBoxExtension::DispatchDeleteCustomLinkResult(
+    blink::WebLocalFrame* frame,
+    bool success) {
+  blink::WebString script(blink::WebString::FromUTF8(base::StringPrintf(
+      kDispatchDeleteCustomLinkResult, success ? "true" : "false")));
+  Dispatch(frame, script);
+}
+
 // static
 void SearchBoxExtension::DispatchInputCancel(blink::WebLocalFrame* frame) {
   Dispatch(frame, kDispatchInputCancelScript);

chrome/renderer/searchbox/searchbox_extension.h

@@ -25,6 +25,12 @@ class SearchBoxExtension {
   static void DispatchFocusChange(blink::WebLocalFrame* frame);
   static void DispatchHistorySyncCheckResult(blink::WebLocalFrame* frame,
                                              bool sync_history);
+  static void DispatchAddCustomLinkResult(blink::WebLocalFrame* frame,
+                                          bool success);
+  static void DispatchUpdateCustomLinkResult(blink::WebLocalFrame* frame,
+                                             bool success);
+  static void DispatchDeleteCustomLinkResult(blink::WebLocalFrame* frame,
+                                             bool success);
   static void DispatchInputCancel(blink::WebLocalFrame* frame);
   static void DispatchInputStart(blink::WebLocalFrame* frame);
   static void DispatchKeyCaptureChange(blink::WebLocalFrame* frame);

components/ntp_tiles/custom_links_manager.h

@@ -61,9 +61,10 @@ class CustomLinksManager {
   // if custom links is not initialized, |url| is invalid, or |url| does not
   // exist in the list.
   virtual bool DeleteLink(const GURL& url) = 0;
-  // Replaces the previously removed link. Returns false and does nothing if
-  // custom links is not initialized or there is no previously deleted link.
-  virtual bool UndoDeleteLink() = 0;
+  // Restores the previous state of the list of links. Used to undo the previous
+  // action (add, edit, delete, etc.). Returns false and does nothing if custom
+  // links is not initialized or there is no previous state to restore.
+  virtual bool UndoAction() = 0;
 };
 
 }  // namespace ntp_tiles

components/ntp_tiles/custom_links_manager_impl.cc

@@ -65,6 +65,7 @@ bool CustomLinksManagerImpl::AddLink(const GURL& url,
   if (FindLinkWithUrl(url) != current_links_.end())
     return false;
 
+  previous_links_ = current_links_;
   current_links_.emplace_back(Link{url, title});
   store_.StoreLinks(current_links_);
   return true;
@@ -78,19 +79,22 @@ bool CustomLinksManagerImpl::UpdateLink(const GURL& url,
     return false;
   }
 
+  // Do not update if |new_url| is invalid or already exists in the list.
+  if (!new_url.is_empty() &&
+      (!new_url.is_valid() ||
+       FindLinkWithUrl(new_url) != current_links_.end())) {
+    return false;
+  }
+
   auto it = FindLinkWithUrl(url);
   if (it == current_links_.end())
     return false;
 
-  if (!new_url.is_empty()) {
-    // Do not update if |new_url| already exists in the list.
-    if (!new_url.is_valid() ||
-        FindLinkWithUrl(new_url) != current_links_.end()) {
-      return false;
-    }
-    it->url = new_url;
-  }
+  // At this point, we will be modifying at least one of the values.
+  previous_links_ = current_links_;
 
+  if (!new_url.is_empty())
+    it->url = new_url;
   if (!new_title.empty())
     it->title = new_title;
 
@@ -106,24 +110,19 @@ bool CustomLinksManagerImpl::DeleteLink(const GURL& url) {
   if (it == current_links_.end())
     return false;
 
-  prev_deleted_link_ =
-      std::make_pair(std::distance(current_links_.begin(), it), *it);
+  previous_links_ = current_links_;
   current_links_.erase(it);
   store_.StoreLinks(current_links_);
   return true;
 }
 
-bool CustomLinksManagerImpl::UndoDeleteLink() {
-  if (!IsInitialized() || !prev_deleted_link_.has_value() ||
-      current_links_.size() == kMaxNumLinks) {
+bool CustomLinksManagerImpl::UndoAction() {
+  if (!IsInitialized() || !previous_links_.has_value())
     return false;
-  }
-  DCHECK_LE(prev_deleted_link_->first, current_links_.size());
 
-  // Inserts the previously deleted link where it was located before.
-  current_links_.insert(current_links_.begin() + prev_deleted_link_->first,
-                        std::move(prev_deleted_link_->second));
-  prev_deleted_link_ = base::nullopt;
+  // Replace the current links with the previous state.
+  current_links_ = *previous_links_;
+  previous_links_ = base::nullopt;
   store_.StoreLinks(current_links_);
   return true;
 }
@@ -131,7 +130,7 @@ bool CustomLinksManagerImpl::UndoDeleteLink() {
 void CustomLinksManagerImpl::ClearLinks() {
   store_.ClearLinks();
   current_links_.clear();
-  prev_deleted_link_ = base::nullopt;
+  previous_links_ = base::nullopt;
 }
 
 std::vector<CustomLinksManager::Link>::iterator

components/ntp_tiles/custom_links_manager_impl.h

@@ -43,7 +43,7 @@ class CustomLinksManagerImpl : public CustomLinksManager {
                   const GURL& new_url,
                   const base::string16& new_title) override;
   bool DeleteLink(const GURL& url) override;
-  bool UndoDeleteLink() override;
+  bool UndoAction() override;
 
   // Register preferences used by this class.
   static void RegisterProfilePrefs(
@@ -57,8 +57,9 @@ class CustomLinksManagerImpl : public CustomLinksManager {
   PrefService* const prefs_;
   CustomLinksStore store_;
   std::vector<Link> current_links_;
-  // Contains the deleted link's data and the index it was located at.
-  base::Optional<std::pair<size_t, Link>> prev_deleted_link_;
+  // The state of the current list of links before the last action was
+  // performed.
+  base::Optional<std::vector<Link>> previous_links_;
 
   base::WeakPtrFactory<CustomLinksManagerImpl> weak_ptr_factory_;
 

components/ntp_tiles/custom_links_manager_impl_unittest.cc

@@ -287,6 +287,72 @@ TEST_F(CustomLinksManagerImplTest, DeleteLinkWhenUrlDoesNotExist) {
   EXPECT_TRUE(custom_links_->GetLinks().empty());
 }
 
+TEST_F(CustomLinksManagerImplTest, UndoAddLink) {
+  NTPTilesVector initial_tiles = FillTestTiles(kTestCase1);
+  std::vector<CustomLinksManager::Link> initial_links =
+      FillTestLinks(kTestCase1);
+  std::vector<CustomLinksManager::Link> expected_links = initial_links;
+  expected_links.emplace_back(
+      CustomLinksManager::Link{GURL(kTestUrl), base::UTF8ToUTF16(kTestTitle)});
+
+  // Initialize.
+  ASSERT_TRUE(custom_links_->Initialize(initial_tiles));
+  ASSERT_EQ(initial_links, custom_links_->GetLinks());
+
+  // Try to undo before add is called. This should fail and not modify the list.
+  EXPECT_FALSE(custom_links_->UndoAction());
+  EXPECT_EQ(initial_links, custom_links_->GetLinks());
+
+  // Add link.
+  EXPECT_TRUE(
+      custom_links_->AddLink(GURL(kTestUrl), base::UTF8ToUTF16(kTestTitle)));
+  EXPECT_EQ(expected_links, custom_links_->GetLinks());
+
+  // Undo add link.
+  EXPECT_TRUE(custom_links_->UndoAction());
+  EXPECT_EQ(initial_links, custom_links_->GetLinks());
+
+  // Try to undo again. This should fail and not modify the list.
+  EXPECT_FALSE(custom_links_->UndoAction());
+  EXPECT_EQ(initial_links, custom_links_->GetLinks());
+}
+
+TEST_F(CustomLinksManagerImplTest, UndoUpdateLink) {
+  NTPTilesVector initial_tiles = FillTestTiles(kTestCase1);
+  std::vector<CustomLinksManager::Link> initial_links =
+      FillTestLinks(kTestCase1);
+  std::vector<CustomLinksManager::Link> links_after_update_url(initial_links);
+  links_after_update_url[0].url = GURL(kTestUrl);
+  std::vector<CustomLinksManager::Link> links_after_update_title(initial_links);
+  links_after_update_title[0].title = base::UTF8ToUTF16(kTestTitle);
+
+  // Initialize.
+  ASSERT_TRUE(custom_links_->Initialize(initial_tiles));
+  ASSERT_EQ(initial_links, custom_links_->GetLinks());
+
+  // Update the link's URL.
+  EXPECT_TRUE(custom_links_->UpdateLink(GURL(kTestCase1[0].url), GURL(kTestUrl),
+                                        base::string16()));
+  EXPECT_EQ(links_after_update_url, custom_links_->GetLinks());
+
+  // Undo update link.
+  EXPECT_TRUE(custom_links_->UndoAction());
+  EXPECT_EQ(initial_links, custom_links_->GetLinks());
+
+  // Update the link's title.
+  EXPECT_TRUE(custom_links_->UpdateLink(GURL(kTestCase1[0].url), GURL(),
+                                        base::UTF8ToUTF16(kTestTitle)));
+  EXPECT_EQ(links_after_update_title, custom_links_->GetLinks());
+
+  // Undo update link.
+  EXPECT_TRUE(custom_links_->UndoAction());
+  EXPECT_EQ(initial_links, custom_links_->GetLinks());
+
+  // Try to undo again. This should fail and not modify the list.
+  EXPECT_FALSE(custom_links_->UndoAction());
+  EXPECT_EQ(initial_links, custom_links_->GetLinks());
+}
+
 TEST_F(CustomLinksManagerImplTest, UndoDeleteLink) {
   NTPTilesVector initial_tiles;
   AddTile(&initial_tiles, kTestUrl, kTestTitle);
@@ -298,17 +364,12 @@ TEST_F(CustomLinksManagerImplTest, UndoDeleteLink) {
   ASSERT_TRUE(custom_links_->Initialize(initial_tiles));
   ASSERT_EQ(expected_links, custom_links_->GetLinks());
 
-  // Try to undo delete before delete is called. This should fail and not modify
-  // the list.
-  EXPECT_FALSE(custom_links_->UndoDeleteLink());
-  EXPECT_EQ(expected_links, custom_links_->GetLinks());
-
   // Delete link.
   ASSERT_TRUE(custom_links_->DeleteLink(GURL(kTestUrl)));
   ASSERT_TRUE(custom_links_->GetLinks().empty());
 
   // Undo delete link.
-  EXPECT_TRUE(custom_links_->UndoDeleteLink());
+  EXPECT_TRUE(custom_links_->UndoAction());
   EXPECT_EQ(expected_links, custom_links_->GetLinks());
 }
 
@@ -332,36 +393,8 @@ TEST_F(CustomLinksManagerImplTest, UndoDeleteLinkAfterAdd) {
   ASSERT_TRUE(custom_links_->GetLinks().empty());
 
   // Undo delete link.
-  EXPECT_TRUE(custom_links_->UndoDeleteLink());
+  EXPECT_TRUE(custom_links_->UndoAction());
   EXPECT_EQ(expected_links, custom_links_->GetLinks());
 }
 
-TEST_F(CustomLinksManagerImplTest, UndoDeleteLinkWhenAtMaxLinks) {
-  NTPTilesVector initial_tiles = FillTestTiles(kTestCaseMax);
-  std::vector<CustomLinksManager::Link> intial_links =
-      FillTestLinks(kTestCaseMax);
-  std::vector<CustomLinksManager::Link> links_after_delete(intial_links);
-  links_after_delete.pop_back();
-  std::vector<CustomLinksManager::Link> links_after_add(links_after_delete);
-  links_after_add.emplace_back(
-      CustomLinksManager::Link{GURL(kTestUrl), base::UTF8ToUTF16(kTestTitle)});
-
-  // Initialize.
-  ASSERT_TRUE(custom_links_->Initialize(initial_tiles));
-  ASSERT_EQ(intial_links, custom_links_->GetLinks());
-
-  // Delete link.
-  ASSERT_TRUE(custom_links_->DeleteLink(GURL(kTestCaseMax[9].url)));
-  ASSERT_EQ(links_after_delete, custom_links_->GetLinks());
-
-  // Add link. Should be at max links.
-  ASSERT_TRUE(
-      custom_links_->AddLink(GURL(kTestUrl), base::UTF8ToUTF16(kTestTitle)));
-  ASSERT_EQ(links_after_add, custom_links_->GetLinks());
-
-  // Try to undo delete link. This should fail and not modify the list.
-  EXPECT_FALSE(custom_links_->UndoDeleteLink());
-  EXPECT_EQ(links_after_add, custom_links_->GetLinks());
-}
-
 }  // namespace ntp_tiles

components/ntp_tiles/most_visited_sites.cc

@@ -234,38 +234,44 @@ bool MostVisitedSites::IsCustomLinksInitialized() {
   return custom_links_->IsInitialized();
 }
 
-void MostVisitedSites::AddCustomLink(const GURL& url,
+bool MostVisitedSites::AddCustomLink(const GURL& url,
                                      const base::string16& title) {
   if (!custom_links_)
-    return;
+    return false;
 
-  if (custom_links_->AddLink(url, title))
+  bool success = custom_links_->AddLink(url, title);
+  if (success)
     BuildCurrentTiles();
+  return success;
 }
 
-void MostVisitedSites::UpdateCustomLink(const GURL& url,
+bool MostVisitedSites::UpdateCustomLink(const GURL& url,
                                         const GURL& new_url,
                                         const base::string16& new_title) {
   if (!custom_links_)
-    return;
+    return false;
 
-  if (custom_links_->UpdateLink(url, new_url, new_title))
+  bool success = custom_links_->UpdateLink(url, new_url, new_title);
+  if (success)
     BuildCurrentTiles();
+  return success;
 }
 
-void MostVisitedSites::DeleteCustomLink(const GURL& url) {
+bool MostVisitedSites::DeleteCustomLink(const GURL& url) {
   if (!custom_links_)
-    return;
+    return false;
 
-  if (custom_links_->DeleteLink(url))
+  bool success = custom_links_->DeleteLink(url);
+  if (success)
     BuildCurrentTiles();
+  return success;
 }
 
-void MostVisitedSites::UndoDeleteCustomLink() {
+void MostVisitedSites::UndoCustomLinkAction() {
   if (!custom_links_)
     return;
 
-  if (custom_links_->UndoDeleteLink())
+  if (custom_links_->UndoAction())
     BuildCurrentTiles();
 }
 

components/ntp_tiles/most_visited_sites.h

@@ -168,21 +168,23 @@ class MostVisitedSites : public history::TopSitesObserver,
   void UninitializeCustomLinks();
   // Returns true if custom links has been initialized, false otherwise.
   bool IsCustomLinksInitialized();
-  // Adds a custom link. If the number of current links is maxed, does nothing.
-  // Custom links must be enabled.
-  void AddCustomLink(const GURL& url, const base::string16& title);
+  // Adds a custom link. If the number of current links is maxed, returns false
+  // and does nothing. Custom links must be enabled.
+  bool AddCustomLink(const GURL& url, const base::string16& title);
   // Updates the URL and/or title of the custom link specified by |url|. If
   // |url| does not exist or |new_url| already exists in the custom link list,
-  // does nothing. Custom links must be enabled.
-  void UpdateCustomLink(const GURL& url,
+  // returns false and does nothing. Custom links must be enabled.
+  bool UpdateCustomLink(const GURL& url,
                         const GURL& new_url,
                         const base::string16& new_title);
   // Deletes the custom link with the specified |url|. If |url| does not exist
-  // in the custom link list, does nothing. Custom links must be enabled.
-  void DeleteCustomLink(const GURL& url);
-  // Restores the previously deleted custom link. If there is no such link, does
-  // nothing. Custom links must be enabled.
-  void UndoDeleteCustomLink();
+  // in the custom link list, returns false and does nothing. Custom links must
+  // be enabled.
+  bool DeleteCustomLink(const GURL& url);
+  // Restores the previous state of custom links before the last action that
+  // modified them. If there was no action, does nothing. Custom links must be
+  // enabled.
+  void UndoCustomLinkAction();
 
   void AddOrRemoveBlacklistedUrl(const GURL& url, bool add_url);
   void ClearBlacklistedUrls();

components/ntp_tiles/most_visited_sites_unittest.cc

@@ -277,7 +277,7 @@ class MockCustomLinksManager : public CustomLinksManager {
                     const GURL& new_url,
                     const base::string16& new_title));
   MOCK_METHOD1(DeleteLink, bool(const GURL& url));
-  MOCK_METHOD0(UndoDeleteLink, bool());
+  MOCK_METHOD0(UndoAction, bool());
 };
 
 class PopularSitesFactoryForTest {