Commit 06aec628 authored by John Lee's avatar John Lee Committed by Commit Bot

[Reland] WebUI Tab Strip: Only track thumbnails for visible tabs

- Track thumbnails only if they are currently in view or are
just a standard finger swipe away.
- Do not track thumbnails for pinned tabs.
- Do not track thumbnails when the tab strip is closed.

Note that this CL does not implement actually untracking thumbnails.
It merely calls the method in the C++ side that will eventually do it.

Bug: 1015132
Change-Id: I16cce4a71ad3e6e887555be27e1525cdef9beb33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1894245Reviewed-by: default avatarDemetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: John Lee <johntlee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711609}
parent aa89874f
......@@ -137,10 +137,6 @@ export class TabElement extends CustomElement {
this.toggleAttribute('has-alert-states_', alertIndicatorsCount > 0);
});
if (!this.tab_ || this.tab_.id !== tab.id) {
this.tabsApi_.trackThumbnailForTab(tab.id);
}
this.tab_ = Object.freeze(tab);
}
......
......@@ -54,6 +54,24 @@ class TabListElement extends CustomElement {
*/
this.draggedItem_;
/**
* An intersection observer is needed to observe which TabElements are
* currently in view or close to being in view, which will help determine
* which thumbnails need to be tracked to stay fresh and which can be
* untracked until they become visible.
* @private {!IntersectionObserver}
*/
this.intersectionObserver_ = new IntersectionObserver(entries => {
for (const entry of entries) {
this.tabsApi_.setThumbnailTracked(
entry.target.tab.id, entry.isIntersecting);
}
}, {
// The horizontal root margin is set to 100% to also track thumbnails that
// are one standard finger swipe away.
rootMargin: '0% 100%',
});
/** @private {!Element} */
this.pinnedTabsContainerElement_ =
/** @type {!Element} */ (
......@@ -88,7 +106,7 @@ class TabListElement extends CustomElement {
this.addEventListener(
'dragover', (e) => this.onDragOver_(/** @type {!DragEvent} */ (e)));
document.addEventListener(
'visibilitychange', () => this.moveOrScrollToActiveTab_());
'visibilitychange', () => this.onVisibilityChange_());
if (loadTimeData.getBoolean('showDemoOptions')) {
this.shadowRoot.querySelector('#demoOptions').style.display = 'block';
......@@ -187,6 +205,8 @@ class TabListElement extends CustomElement {
* @private
*/
insertTabOrMoveTo_(tabElement, index) {
const isInserting = !tabElement.isConnected;
// Remove the tabElement if it already exists in the DOM
tabElement.remove();
......@@ -201,6 +221,10 @@ class TabListElement extends CustomElement {
this.tabsContainerElement_.insertBefore(
tabElement, this.tabsContainerElement_.childNodes[offsetIndex]);
}
if (isInserting) {
this.updateThumbnailTrackStatus_(tabElement);
}
}
/** @private */
......@@ -319,7 +343,6 @@ class TabListElement extends CustomElement {
*/
onTabCreated_(tab) {
const tabElement = this.createTabElement_(tab);
if (tabStripOptions.mruEnabled && tab.active && !tab.pinned &&
tab.index !== this.pinnedTabsContainerElement_.childElementCount) {
// Newly created active tabs should first be moved to the very beginning
......@@ -383,9 +406,18 @@ class TabListElement extends CustomElement {
if (tab.active) {
this.scrollToTab_(tabElement);
}
this.updateThumbnailTrackStatus_(tabElement);
}
}
/** @private */
onVisibilityChange_() {
this.moveOrScrollToActiveTab_();
Array.from(this.tabsContainerElement_.children)
.forEach((tabElement) => this.updateThumbnailTrackStatus_(tabElement));
}
/**
* @param {!TabElement} tabElement
* @private
......@@ -422,6 +454,24 @@ class TabListElement extends CustomElement {
tab.updateThumbnail(imgData);
}
}
/**
* @param {!TabElement} tabElement
* @private
*/
updateThumbnailTrackStatus_(tabElement) {
if (this.tabStripEmbedderProxy_.isVisible() && !tabElement.tab.pinned) {
// If the tab strip is visible and the tab is not pinned, let the
// IntersectionObserver start observing the TabElement to automatically
// determine if the tab's thumbnail should be tracked.
this.intersectionObserver_.observe(tabElement);
} else {
// If the tab strip is not visible or the tab is pinned, the tab does not
// need to show or update any thumbnails.
this.intersectionObserver_.unobserve(tabElement);
this.tabsApi_.setThumbnailTracked(tabElement.tab.id, false);
}
}
}
customElements.define('tabstrip-tab-list', TabListElement);
......@@ -100,9 +100,10 @@ export class TabsApiProxy {
/**
* @param {number} tabId
* @param {boolean} thumbnailTracked
*/
trackThumbnailForTab(tabId) {
chrome.send('addTrackedTab', [tabId]);
setThumbnailTracked(tabId, thumbnailTracked) {
chrome.send('setThumbnailTracked', [tabId, thumbnailTracked]);
}
}
......
......@@ -217,10 +217,8 @@ class TabStripUIHandler : public content::WebUIMessageHandler,
"getThemeColors", base::Bind(&TabStripUIHandler::HandleGetThemeColors,
base::Unretained(this)));
web_ui()->RegisterMessageCallback(
"addTrackedTab",
base::Bind(&TabStripUIHandler::AddTrackedTab, base::Unretained(this)));
web_ui()->RegisterMessageCallback(
"removeTrackedTab", base::Bind(&TabStripUIHandler::RemoveTrackedTab,
"setThumbnailTracked",
base::Bind(&TabStripUIHandler::HandleSetThumbnailTracked,
base::Unretained(this)));
web_ui()->RegisterMessageCallback(
"closeContainer", base::Bind(&TabStripUIHandler::HandleCloseContainer,
......@@ -377,13 +375,15 @@ class TabStripUIHandler : public content::WebUIMessageHandler,
ResolveJavascriptCallback(callback_id, layout);
}
void AddTrackedTab(const base::ListValue* args) {
void HandleSetThumbnailTracked(const base::ListValue* args) {
AllowJavascript();
int tab_id = 0;
if (!args->GetInteger(0, &tab_id))
return;
const bool thumbnail_tracked = args->GetList()[1].GetBool();
content::WebContents* tab = nullptr;
if (!extensions::ExtensionTabUtil::GetTabById(tab_id, browser_->profile(),
true, &tab)) {
......@@ -391,17 +391,13 @@ class TabStripUIHandler : public content::WebUIMessageHandler,
DVLOG(1) << "Invalid tab ID";
return;
}
thumbnail_tracker_.WatchTab(tab);
}
void RemoveTrackedTab(const base::ListValue* args) {
AllowJavascript();
int tab_id = 0;
if (!args->GetInteger(0, &tab_id))
return;
if (thumbnail_tracked) {
thumbnail_tracker_.WatchTab(tab);
} else {
// TODO(crbug.com/991393): un-watch tabs when we are done.
}
}
// Callback passed to |thumbnail_tracker_|. Called when a tab's thumbnail
// changes, or when we start watching the tab.
......
......@@ -61,6 +61,7 @@ suite('TabList', () => {
alertStates: [],
id: 0,
index: 0,
pinned: false,
title: 'Tab 1',
},
{
......@@ -68,6 +69,7 @@ suite('TabList', () => {
alertStates: [],
id: 1,
index: 1,
pinned: false,
title: 'Tab 2',
},
{
......@@ -75,6 +77,7 @@ suite('TabList', () => {
alertStates: [],
id: 2,
index: 2,
pinned: false,
title: 'Tab 3',
},
];
......@@ -85,6 +88,12 @@ suite('TabList', () => {
webUIListenerCallback('tab-updated', updatedTab);
}
function unpinTabAt(tab, index) {
const changeInfo = {index: index, pinned: false};
const updatedTab = Object.assign({}, tab, changeInfo);
webUIListenerCallback('tab-updated', updatedTab);
}
function getUnpinnedTabs() {
return tabList.shadowRoot.querySelectorAll('#tabsContainer tabstrip-tab');
}
......@@ -111,6 +120,7 @@ suite('TabList', () => {
'--height': '100px',
'--width': '150px',
});
testTabStripEmbedderProxy.setVisible(true);
TabStripEmbedderProxy.instance_ = testTabStripEmbedderProxy;
tabList = document.createElement('tabstrip-tab-list');
......@@ -404,4 +414,104 @@ suite('TabList', () => {
assertEquals(moveId, tabs[1].id);
assertEquals(newIndex, 0);
});
test('tracks and untracks thumbnails based on viewport', async () => {
// Wait for slideIn animations to complete updating widths and reset
// resolvers to track new calls.
await tabList.animationPromises;
testTabsApiProxy.reset();
const tabElements = getUnpinnedTabs();
// Update width such that at most one tab can fit in the viewport at once.
tabList.style.setProperty('--tabstrip-tab-width', `${window.innerWidth}px`);
// At this point, the only visible tab should be the first tab. The second
// tab should fit within the rootMargin of the IntersectionObserver. The
// third tab should not be intersecting.
let [tabId, thumbnailTracked] =
await testTabsApiProxy.whenCalled('setThumbnailTracked');
assertEquals(tabId, tabElements[2].tab.id);
assertEquals(thumbnailTracked, false);
assertEquals(testTabsApiProxy.getCallCount('setThumbnailTracked'), 1);
testTabsApiProxy.reset();
// Scroll such that the second tab is now the only visible tab. At this
// point, all 3 tabs should fit within the root and rootMargin of the
// IntersectionObserver. Since the 3rd tab was not being tracked before,
// it should be the only tab to become tracked.
document.documentElement.scrollLeft = tabElements[1].offsetLeft;
[tabId, thumbnailTracked] =
await testTabsApiProxy.whenCalled('setThumbnailTracked');
assertEquals(tabId, tabElements[2].tab.id);
assertEquals(thumbnailTracked, true);
assertEquals(testTabsApiProxy.getCallCount('setThumbnailTracked'), 1);
testTabsApiProxy.reset();
// Scroll such that the third tab is now the only visible tab. At this
// point, the first tab should be outside of the rootMargin of the
// IntersectionObserver.
document.documentElement.scrollLeft = tabElements[2].offsetLeft;
[tabId, thumbnailTracked] =
await testTabsApiProxy.whenCalled('setThumbnailTracked');
assertEquals(tabId, tabElements[0].tab.id);
assertEquals(thumbnailTracked, false);
assertEquals(testTabsApiProxy.getCallCount('setThumbnailTracked'), 1);
});
test('tracks and untracks thumbnails based on pinned state', async () => {
await tabList.animationPromises;
testTabsApiProxy.reset();
// Update width such that at all tabs can fit and do not fire the
// IntersectionObserver based on intersection alone.
tabList.style.setProperty(
'--tabstrip-tab-width', `${window.innerWidth / tabs.length}px`);
// Pinning the third tab should untrack thumbnails for the tab
pinTabAt(tabs[2], 0);
let [tabId, thumbnailTracked] =
await testTabsApiProxy.whenCalled('setThumbnailTracked');
assertEquals(tabId, tabs[2].id);
assertEquals(thumbnailTracked, false);
testTabsApiProxy.reset();
// Unpinning the tab should re-track the thumbnails
unpinTabAt(tabs[2], 0);
[tabId, thumbnailTracked] =
await testTabsApiProxy.whenCalled('setThumbnailTracked');
// TODO(johntlee): Remove debug logs if tests are no longer flaky.
console.log(`Window width is ${window.innerWidth}px`);
for (const tabElement of getUnpinnedTabs()) {
console.log(`Tab ${tabElement.tab.id} is at ${tabElement.offsetLeft}`);
}
assertEquals(tabId, tabs[2].id);
assertEquals(thumbnailTracked, true);
});
test('should update thumbnail track status on visibilitychange', async () => {
await tabList.animationPromises;
testTabsApiProxy.reset();
testTabStripEmbedderProxy.setVisible(false);
document.dispatchEvent(new Event('visibilitychange'));
// The tab strip should force untrack thumbnails for all tabs.
await testTabsApiProxy.whenCalled('setThumbnailTracked');
assertEquals(
testTabsApiProxy.getCallCount('setThumbnailTracked'), tabs.length);
testTabsApiProxy.reset();
// Update width such that at all tabs can fit
tabList.style.setProperty(
'--tabstrip-tab-width', `${window.innerWidth / tabs.length}px`);
testTabStripEmbedderProxy.setVisible(true);
document.dispatchEvent(new Event('visibilitychange'));
await testTabsApiProxy.whenCalled('setThumbnailTracked');
assertEquals(
testTabsApiProxy.getCallCount('setThumbnailTracked'), tabs.length);
});
});
......@@ -252,11 +252,7 @@ suite('Tab', function() {
assertEquals(window.getComputedStyle(thumbnailImage).display, 'none');
});
test('tracks and updates the thumbnail source', async () => {
const requestedTabId =
await testTabsApiProxy.whenCalled('trackThumbnailForTab');
assertEquals(requestedTabId, tab.id);
test('updates the thumbnail source', async () => {
const thumbnailSource = 'data:mock-thumbnail-source';
tabElement.updateThumbnail(thumbnailSource);
assertEquals(
......
......@@ -11,7 +11,7 @@ export class TestTabsApiProxy extends TestBrowserProxy {
'closeTab',
'getTabs',
'moveTab',
'trackThumbnailForTab',
'setThumbnailTracked',
]);
this.tabs_;
......@@ -41,7 +41,7 @@ export class TestTabsApiProxy extends TestBrowserProxy {
this.tabs_ = tabs;
}
trackThumbnailForTab(tabId) {
this.methodCalled('trackThumbnailForTab', tabId);
setThumbnailTracked(tabId, thumbnailTracked) {
this.methodCalled('setThumbnailTracked', [tabId, thumbnailTracked]);
}
}
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