Commit f549216f authored by Yuheng Huang's avatar Yuheng Huang Committed by Commit Bot

Tab Search: Implement TabUpdated() API

The TabUpdated() API will be called from
from TabStripModelObserver.TabChangedAt()
It will send out UpdateTabInfo for the updated tab
so no more additional API calls are needed.

Related CL:
https://chrome-internal-review.googlesource.com/c/chrome/browser/resources/tab_search/+/3217588

Bug: 1099917
Change-Id: Id46b18827a8ef4f3c02d5a992ef4a3cc8ae2f6f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2357958Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarThomas Lukaszewicz <tluk@chromium.org>
Reviewed-by: default avatarJohn Lee <johntlee@chromium.org>
Commit-Queue: Yuheng Huang <yuhengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800399}
parent acdecba0
...@@ -74,6 +74,10 @@ interface PageHandler { ...@@ -74,6 +74,10 @@ interface PageHandler {
// WebUI-side handler for requests from the browser. // WebUI-side handler for requests from the browser.
interface Page { interface Page {
// Callback when any tabs in the current profile are // Callback when any tabs in the current profile are
// added/changed/removed/navigated to new URL // selected/inserted/removed/moved/replaced
TabsChanged(); TabsChanged();
// Callback when a tab is updated within the same web contents.
// e.g. url, title or favicon changed.
TabUpdated(Tab tab);
}; };
...@@ -195,8 +195,12 @@ void TabSearchPageHandler::TabChangedAt(content::WebContents* contents, ...@@ -195,8 +195,12 @@ void TabSearchPageHandler::TabChangedAt(content::WebContents* contents,
TabChangeType change_type) { TabChangeType change_type) {
// TODO(crbug.com/1112496): Support more values for TabChangeType and filter // TODO(crbug.com/1112496): Support more values for TabChangeType and filter
// out the changes we are not interested in. // out the changes we are not interested in.
if (change_type == TabChangeType::kAll) if (change_type != TabChangeType::kAll)
ScheduleDebounce(); return;
Browser* browser = chrome::FindBrowserWithWebContents(contents);
if (!browser)
return;
page_->TabUpdated(GetTabData(browser->tab_strip_model(), contents, index));
} }
void TabSearchPageHandler::ScheduleDebounce() { void TabSearchPageHandler::ScheduleDebounce() {
......
...@@ -18,6 +18,9 @@ ...@@ -18,6 +18,9 @@
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "ui/gfx/color_utils.h" #include "ui/gfx/color_utils.h"
using testing::_;
using testing::Truly;
namespace { namespace {
constexpr char kTabUrl1[] = "http://foo/1"; constexpr char kTabUrl1[] = "http://foo/1";
...@@ -44,6 +47,7 @@ class MockPage : public tab_search::mojom::Page { ...@@ -44,6 +47,7 @@ class MockPage : public tab_search::mojom::Page {
mojo::Receiver<tab_search::mojom::Page> receiver_{this}; mojo::Receiver<tab_search::mojom::Page> receiver_{this};
MOCK_METHOD0(TabsChanged, void()); MOCK_METHOD0(TabsChanged, void());
MOCK_METHOD1(TabUpdated, void(tab_search::mojom::TabPtr));
}; };
void ExpectNewTab(const tab_search::mojom::Tab* tab, void ExpectNewTab(const tab_search::mojom::Tab* tab,
...@@ -179,6 +183,7 @@ TEST_F(TabSearchPageHandlerTest, GetTabs) { ...@@ -179,6 +183,7 @@ TEST_F(TabSearchPageHandlerTest, GetTabs) {
AddTabWithTitle(browser1(), GURL(kTabUrl1), kTabName1); AddTabWithTitle(browser1(), GURL(kTabUrl1), kTabName1);
EXPECT_CALL(page_, TabsChanged()).Times(1); EXPECT_CALL(page_, TabsChanged()).Times(1);
EXPECT_CALL(page_, TabUpdated(_)).Times(2);
handler()->mock_debounce_timer()->Fire(); handler()->mock_debounce_timer()->Fire();
int32_t tab_id2 = 0; int32_t tab_id2 = 0;
...@@ -246,6 +251,7 @@ TEST_F(TabSearchPageHandlerTest, GetTabs) { ...@@ -246,6 +251,7 @@ TEST_F(TabSearchPageHandlerTest, GetTabs) {
// timer fires. // timer fires.
TEST_F(TabSearchPageHandlerTest, TabsChanged) { TEST_F(TabSearchPageHandlerTest, TabsChanged) {
EXPECT_CALL(page_, TabsChanged()).Times(4); EXPECT_CALL(page_, TabsChanged()).Times(4);
EXPECT_CALL(page_, TabUpdated(_)).Times(1);
FireTimer(); // Will call TabsChanged(). FireTimer(); // Will call TabsChanged().
// Add 2 tabs in browser1. // Add 2 tabs in browser1.
...@@ -276,6 +282,7 @@ TEST_F(TabSearchPageHandlerTest, TabsChanged) { ...@@ -276,6 +282,7 @@ TEST_F(TabSearchPageHandlerTest, TabsChanged) {
// will not call TabsChanged(). // will not call TabsChanged().
TEST_F(TabSearchPageHandlerTest, TabsNotChanged) { TEST_F(TabSearchPageHandlerTest, TabsNotChanged) {
EXPECT_CALL(page_, TabsChanged()).Times(1); EXPECT_CALL(page_, TabsChanged()).Times(1);
EXPECT_CALL(page_, TabUpdated(_)).Times(0);
FireTimer(); // Will call TabsChanged(). FireTimer(); // Will call TabsChanged().
ASSERT_FALSE(IsTimerRunning()); ASSERT_FALSE(IsTimerRunning());
AddTabWithTitle(browser3(), GURL(kTabUrl1), AddTabWithTitle(browser3(), GURL(kTabUrl1),
...@@ -286,6 +293,22 @@ TEST_F(TabSearchPageHandlerTest, TabsNotChanged) { ...@@ -286,6 +293,22 @@ TEST_F(TabSearchPageHandlerTest, TabsNotChanged) {
ASSERT_FALSE(IsTimerRunning()); ASSERT_FALSE(IsTimerRunning());
} }
bool VerifyTabUpdated(const tab_search::mojom::TabPtr& tab) {
ExpectNewTab(tab.get(), kTabUrl1, kTabName1, 1);
return true;
}
// Verify tab update event is called correctly with data
TEST_F(TabSearchPageHandlerTest, TabUpdated) {
EXPECT_CALL(page_, TabsChanged()).Times(1);
EXPECT_CALL(page_, TabUpdated(Truly(VerifyTabUpdated))).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
AddTabWithTitle(browser1(), GURL(kTabUrl2), kTabName2);
FireTimer();
}
TEST_F(TabSearchPageHandlerTest, CloseTab) { TEST_F(TabSearchPageHandlerTest, CloseTab) {
AddTabWithTitle(browser1(), GURL(kTabUrl1), kTabName1); AddTabWithTitle(browser1(), GURL(kTabUrl1), kTabName1);
AddTabWithTitle(browser2(), GURL(kTabUrl2), kTabName2); AddTabWithTitle(browser2(), GURL(kTabUrl2), kTabName2);
...@@ -296,6 +319,7 @@ TEST_F(TabSearchPageHandlerTest, CloseTab) { ...@@ -296,6 +319,7 @@ TEST_F(TabSearchPageHandlerTest, CloseTab) {
int tab_id = extensions::ExtensionTabUtil::GetTabId( int tab_id = extensions::ExtensionTabUtil::GetTabId(
browser2()->tab_strip_model()->GetWebContentsAt(0)); browser2()->tab_strip_model()->GetWebContentsAt(0));
EXPECT_CALL(page_, TabsChanged()).Times(1); EXPECT_CALL(page_, TabsChanged()).Times(1);
EXPECT_CALL(page_, TabUpdated(_)).Times(1);
handler()->CloseTab(tab_id); handler()->CloseTab(tab_id);
ASSERT_EQ(1, browser1()->tab_strip_model()->count()); ASSERT_EQ(1, browser1()->tab_strip_model()->count());
ASSERT_EQ(1, browser2()->tab_strip_model()->count()); ASSERT_EQ(1, browser2()->tab_strip_model()->count());
......
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