Tab Search: Implement TabsRemoved API

This change makes close tab more efficient. According to the performance
benchmark https://pinpoint-dot-chromeperf.appspot.com/job/15a9cbdd520000
It reduces close tab latency from 105ms to 37ms.

Bug: 1099917
Change-Id: I987a98006d5b8788af8ab844a8871458a620b20c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2527625
Commit-Queue: Yuheng Huang <yuhengh@chromium.org>
Reviewed-by: Thomas Lukaszewicz <tluk@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826147}

chrome/browser/resources/tab_search/app.js

@@ -137,7 +137,9 @@ export class TabSearchAppElement extends PolymerElement {
     const callbackRouter = this.apiProxy_.getCallbackRouter();
     this.listenerIds_.push(
         callbackRouter.tabsChanged.addListener(() => this.updateTabs_()),
-        callbackRouter.tabUpdated.addListener(tab => this.onTabUpdated_(tab)));
+        callbackRouter.tabUpdated.addListener(tab => this.onTabUpdated_(tab)),
+        callbackRouter.tabsRemoved.addListener(
+            tabIds => this.onTabsRemoved_(tabIds)));
     this.updateTabs_();
   }
 
@@ -206,6 +208,21 @@ export class TabSearchAppElement extends PolymerElement {
     }
   }
 
+  /**
+   * @param {!Array<number>} tabIds
+   * @private
+   */
+  onTabsRemoved_(tabIds) {
+    const windows = this.openTabs_;
+    if (windows) {
+      const ids = new Set(tabIds);
+      for (const window of windows) {
+        window.tabs = window.tabs.filter(tab => (!ids.has(tab.tabId)));
+      }
+      this.openTabs_ = windows.concat();
+    }
+  }
+
   /**
    * The seleted item's index, or -1 if no item selected.
    * @return {number}

chrome/browser/ui/webui/tab_search/tab_search.mojom

@@ -105,10 +105,13 @@ interface PageHandler {
 // WebUI-side handler for requests from the browser.
 interface Page {
   // Callback when any tabs in the current profile are
-  // selected/inserted/removed/moved/replaced
+  // selected/inserted/moved/replaced
   TabsChanged();
 
   // Callback when a tab is updated within the same web contents.
   // e.g. url, title or favicon changed.
   TabUpdated(Tab tab);
+
+  // Callback when tabs are removed.
+  TabsRemoved(array<int32> tab_ids);
 };

chrome/browser/ui/webui/tab_search/tab_search_page_handler.cc

@@ -24,6 +24,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/browser/ui/tabs/tab_renderer_data.h"
+#include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
 #include "chrome/browser/ui/webui/util/image_util.h"
 
 namespace {
@@ -207,8 +208,18 @@ void TabSearchPageHandler::OnTabStripModelChanged(
     TabStripModel* tab_strip_model,
     const TabStripModelChange& change,
     const TabStripSelectionChange& selection) {
-  if (!browser_tab_strip_tracker_.is_processing_initial_browsers())
-    ScheduleDebounce();
+  if (browser_tab_strip_tracker_.is_processing_initial_browsers())
+    return;
+  if (change.type() == TabStripModelChange::kRemoved) {
+    std::vector<int> tab_ids;
+    for (auto& content_with_index : change.GetRemove()->contents) {
+      tab_ids.push_back(
+          extensions::ExtensionTabUtil::GetTabId(content_with_index.contents));
+    }
+    page_->TabsRemoved(tab_ids);
+    return;
+  }
+  ScheduleDebounce();
 }
 
 void TabSearchPageHandler::TabChangedAt(content::WebContents* contents,

chrome/browser/ui/webui/tab_search/tab_search_page_handler_unittest.cc

@@ -49,6 +49,7 @@ class MockPage : public tab_search::mojom::Page {
 
   MOCK_METHOD0(TabsChanged, void());
   MOCK_METHOD1(TabUpdated, void(tab_search::mojom::TabPtr));
+  MOCK_METHOD1(TabsRemoved, void(const std::vector<int32_t>& tab_ids));
 };
 
 void ExpectNewTab(const tab_search::mojom::Tab* tab,
@@ -191,6 +192,7 @@ TEST_F(TabSearchPageHandlerTest, GetTabs) {
 
   EXPECT_CALL(page_, TabsChanged()).Times(1);
   EXPECT_CALL(page_, TabUpdated(_)).Times(2);
+  EXPECT_CALL(page_, TabsRemoved(_)).Times(2);
   handler()->mock_debounce_timer()->Fire();
 
   int32_t tab_id2 = 0;
@@ -257,8 +259,9 @@ TEST_F(TabSearchPageHandlerTest, GetTabs) {
 // TabsChanged() and TabsChanged() is only called when the page handler's
 // timer fires.
 TEST_F(TabSearchPageHandlerTest, TabsChanged) {
-  EXPECT_CALL(page_, TabsChanged()).Times(4);
+  EXPECT_CALL(page_, TabsChanged()).Times(3);
   EXPECT_CALL(page_, TabUpdated(_)).Times(1);
+  EXPECT_CALL(page_, TabsRemoved(_)).Times(3);
   FireTimer();  // Will call TabsChanged().
 
   // Add 2 tabs in browser1.
@@ -281,8 +284,7 @@ TEST_F(TabSearchPageHandlerTest, TabsChanged) {
   ASSERT_FALSE(IsTimerRunning());
   browser1()->tab_strip_model()->CloseWebContentsAt(
       0, TabStripModel::CLOSE_CREATE_HISTORICAL_TAB);
-  ASSERT_TRUE(IsTimerRunning());
-  FireTimer();  // Will call TabsChanged().
+  ASSERT_FALSE(IsTimerRunning());
 }
 
 // Ensure that tab model changes in a browser with a different profile
@@ -309,6 +311,7 @@ bool VerifyTabUpdated(const tab_search::mojom::TabPtr& tab) {
 TEST_F(TabSearchPageHandlerTest, TabUpdated) {
   EXPECT_CALL(page_, TabsChanged()).Times(1);
   EXPECT_CALL(page_, TabUpdated(Truly(VerifyTabUpdated))).Times(1);
+  EXPECT_CALL(page_, TabsRemoved(_)).Times(1);
   AddTabWithTitle(browser1(), GURL(kTabUrl1), kTabName1);
   // Adding the following tab will trigger TabUpdated() to the first tab
   // since the tab index will change from 0 to 1
@@ -327,6 +330,7 @@ TEST_F(TabSearchPageHandlerTest, CloseTab) {
       browser2()->tab_strip_model()->GetWebContentsAt(0));
   EXPECT_CALL(page_, TabsChanged()).Times(1);
   EXPECT_CALL(page_, TabUpdated(_)).Times(1);
+  EXPECT_CALL(page_, TabsRemoved(_)).Times(3);
   handler()->CloseTab(tab_id);
   ASSERT_EQ(1, browser1()->tab_strip_model()->count());
   ASSERT_EQ(1, browser2()->tab_strip_model()->count());

chrome/test/data/webui/tab_search/tab_search_app_test.js

@@ -229,6 +229,14 @@ suite('TabSearchAppTest', () => {
     assertEquals('example.com', tabSearchItem.data.hostname);
   });
 
+  test('refresh on tabs removed', async () => {
+    await setupTest(sampleData());
+    verifyTabIds(queryRows(), [1, 5, 6, 2, 3, 4]);
+    testProxy.getCallbackRouterRemote().tabsRemoved([1, 2]);
+    await flushTasks();
+    verifyTabIds(queryRows(), [5, 6, 3, 4]);
+  });
+
   test('Verify initial tab render time is logged correctly', async () => {
     // |metricNames| tracks thow many times recordTime() has been called for
     // a metric.