Commit ce146697 authored by Tom Lukaszewicz's avatar Tom Lukaszewicz Committed by Josip Sokcevic

Tab Search: Updates WebUI to log tab switch action

This CL updates the WebUI for Tab Search to log whether or not the user
has initiated a tab switch action from a filtered search result list or
an unfiltered list.

The related histogram and enum CL is below:
http://crrev.com/c/2348698

Bug: 1099917
Change-Id: Ie2900b7c058233e353748f0981375186947385ee
Reviewed-on: https://chrome-internal-review.googlesource.com/c/chrome/browser/resources/tab_search/+/3210171Reviewed-by: default avatarIlya Sherman <isherman@chromium.org>
Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Reviewed-by: default avatarYuheng Huang <yuhengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819583}
parent 465d4a1b
...@@ -35,6 +35,7 @@ js_library("tab_search_api_proxy") { ...@@ -35,6 +35,7 @@ js_library("tab_search_api_proxy") {
"//chrome/browser/ui/webui/tab_search:mojo_bindings_js_library_for_compile", "//chrome/browser/ui/webui/tab_search:mojo_bindings_js_library_for_compile",
"//ui/webui/resources/js:cr.m", "//ui/webui/resources/js:cr.m",
] ]
externs_list = [ "$externs_path/metrics_private.js" ]
} }
js_library("tab_search_item") { js_library("tab_search_item") {
......
...@@ -142,7 +142,7 @@ export class TabSearchAppElement extends PolymerElement { ...@@ -142,7 +142,7 @@ export class TabSearchAppElement extends PolymerElement {
*/ */
onItemClick_(e) { onItemClick_(e) {
const tabId = Number.parseInt(e.currentTarget.id, 10); const tabId = Number.parseInt(e.currentTarget.id, 10);
this.apiProxy_.switchToTab({tabId}); this.apiProxy_.switchToTab({tabId}, !!this.searchText_);
} }
/** /**
...@@ -203,7 +203,8 @@ export class TabSearchAppElement extends PolymerElement { ...@@ -203,7 +203,8 @@ export class TabSearchAppElement extends PolymerElement {
break; break;
case 'Enter': case 'Enter':
const selectedItem = this.filteredOpenTabs_[this.selectedIndex_]; const selectedItem = this.filteredOpenTabs_[this.selectedIndex_];
this.apiProxy_.switchToTab({tabId: selectedItem.tabId}); this.apiProxy_.switchToTab({tabId : selectedItem.tabId},
!!this.searchText_);
break; break;
} }
} }
......
...@@ -7,6 +7,16 @@ import './tab_search.mojom-lite.js'; ...@@ -7,6 +7,16 @@ import './tab_search.mojom-lite.js';
import {addSingletonGetter} from 'chrome://resources/js/cr.m.js'; import {addSingletonGetter} from 'chrome://resources/js/cr.m.js';
/**
* These values are persisted to logs and should not be renumbered or re-used.
* See tools/metrics/histograms/enums.xml.
* @enum {number}
*/
export const TabSwitchAction = {
WITHOUT_SEARCH : 0,
WITH_SEARCH : 1,
};
/** @interface */ /** @interface */
export class TabSearchApiProxy { export class TabSearchApiProxy {
/** @param {number} tabId */ /** @param {number} tabId */
...@@ -15,8 +25,11 @@ export class TabSearchApiProxy { ...@@ -15,8 +25,11 @@ export class TabSearchApiProxy {
/** @return {Promise<{profileTabs: tabSearch.mojom.ProfileTabs}>} */ /** @return {Promise<{profileTabs: tabSearch.mojom.ProfileTabs}>} */
getProfileTabs() {} getProfileTabs() {}
/** @param {!tabSearch.mojom.SwitchToTabInfo} info */ /**
switchToTab(info) {} * @param {!tabSearch.mojom.SwitchToTabInfo} info
* @param {boolean} withSearch
*/
switchToTab(info, withSearch) {}
/** @return {!tabSearch.mojom.PageCallbackRouter} */ /** @return {!tabSearch.mojom.PageCallbackRouter} */
getCallbackRouter() {} getCallbackRouter() {}
...@@ -48,7 +61,12 @@ export class TabSearchApiProxyImpl { ...@@ -48,7 +61,12 @@ export class TabSearchApiProxyImpl {
} }
/** @override */ /** @override */
switchToTab(info) { switchToTab(info, withSearch) {
chrome.metricsPrivate.recordEnumerationValue(
'Tabs.TabSearch.WebUI.TabSwitchAction',
withSearch ? TabSwitchAction.WITH_SEARCH
: TabSwitchAction.WITHOUT_SEARCH,
Object.keys(TabSwitchAction).length);
this.handler.switchToTab(info); this.handler.switchToTab(info);
} }
......
...@@ -7,7 +7,7 @@ import {TabSearchAppElement} from 'chrome://tab-search/app.js'; ...@@ -7,7 +7,7 @@ import {TabSearchAppElement} from 'chrome://tab-search/app.js';
import {TabSearchSearchField} from 'chrome://tab-search/tab_search_search_field.js'; import {TabSearchSearchField} from 'chrome://tab-search/tab_search_search_field.js';
import {TabSearchApiProxy, TabSearchApiProxyImpl} from 'chrome://tab-search/tab_search_api_proxy.js' import {TabSearchApiProxy, TabSearchApiProxyImpl} from 'chrome://tab-search/tab_search_api_proxy.js'
import {assertEquals, assertTrue} from '../../chai_assert.js'; import {assertEquals, assertFalse, assertTrue} from '../../chai_assert.js';
import {flushTasks, waitAfterNextRender} from '../../test_util.m.js'; import {flushTasks, waitAfterNextRender} from '../../test_util.m.js';
import {TestTabSearchApiProxy} from './test_tab_search_api_proxy.js'; import {TestTabSearchApiProxy} from './test_tab_search_api_proxy.js';
...@@ -144,7 +144,7 @@ suite('TabSearchAppTest', () => { ...@@ -144,7 +144,7 @@ suite('TabSearchAppTest', () => {
const tabSearchItem = /** @type {!HTMLElement} */ const tabSearchItem = /** @type {!HTMLElement} */
(tabSearchApp.shadowRoot.querySelector('tab-search-item')); (tabSearchApp.shadowRoot.querySelector('tab-search-item'));
tabSearchItem.click(); tabSearchItem.click();
const tabInfo = await testProxy.whenCalled('switchToTab'); const [tabInfo] = await testProxy.whenCalled('switchToTab');
assertEquals(tabData.tabId, tabInfo.tabId); assertEquals(tabData.tabId, tabInfo.tabId);
const tabSearchItemCloseButton = /** @type {!HTMLElement} */ ( const tabSearchItemCloseButton = /** @type {!HTMLElement} */ (
...@@ -255,4 +255,44 @@ suite('TabSearchAppTest', () => { ...@@ -255,4 +255,44 @@ suite('TabSearchAppTest', () => {
// called once. // called once.
assertEquals(1, recordTimeCalled); assertEquals(1, recordTimeCalled);
}); });
test('Verify tab switch is logged correctly.', async () => {
await setupTest(sampleData());
// Make sure that tab data has been recieved.
verifyTabIds(queryRows(), [ 1, 5, 6, 2, 3, 4 ]);
// Click the first element with tabId 1.
let tabSearchItem = /** @type {!HTMLElement} */
(tabSearchApp.shadowRoot.querySelector('tab-search-item[id="1"]'));
tabSearchItem.click();
// Assert switchToTab() was called appropriately for an unfiltered tab list.
await testProxy.whenCalled('switchToTab')
.then(([ tabInfo, withSearch ]) => {
assertEquals(1, tabInfo.tabId);
assertFalse(withSearch);
});
// Force a change to filtered tab data that would result in a
// re-render.
const searchField = /** @type {!TabSearchSearchField} */
(tabSearchApp.shadowRoot.querySelector('#searchField'));
searchField.setValue('bing');
await flushTasks();
verifyTabIds(queryRows(), [ 2 ]);
testProxy.reset();
// Click the only remaining element with tabId 2.
tabSearchItem = /** @type {!HTMLElement} */
(tabSearchApp.shadowRoot.querySelector('tab-search-item[id="2"]'));
tabSearchItem.click();
// Assert switchToTab() was called appropriately for a tab list fitlered by
// the search query.
await testProxy.whenCalled('switchToTab')
.then(([ tabInfo, withSearch ]) => {
assertEquals(2, tabInfo.tabId);
assertTrue(withSearch);
});
});
}); });
...@@ -37,8 +37,8 @@ export class TestTabSearchApiProxy extends TestBrowserProxy { ...@@ -37,8 +37,8 @@ export class TestTabSearchApiProxy extends TestBrowserProxy {
} }
/** @override */ /** @override */
switchToTab(tabInfo) { switchToTab(tabInfo, withSearch) {
this.methodCalled('switchToTab', tabInfo); this.methodCalled('switchToTab', [ tabInfo, withSearch ]);
} }
/** @override */ /** @override */
......
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