Tab Strip WebUI: Use network state to display loading spinner

This CL also adds differentiation between a loading and a waiting state,
each displaying a spinner in their own colors and directions, similar
to how the native tab strip works.

This CL also updates types to allow for the new data.

Bug: 1004985
Change-Id: I1b9361b3796e369d683b8f63c6a2026364d439f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1848573Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: John Lee <johntlee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704743}

chrome/browser/resources/tab_strip/BUILD.gn

@@ -33,14 +33,9 @@ js_library("tab") {
   deps = [
     "//ui/webui/resources/js:icon.m",
   ]
-  externs_list = [ "$externs_path/chrome.js" ]
 }
 
 js_library("tab_list") {
-  deps = [
-    ":types",
-  ]
-  externs_list = [ "$externs_path/chrome.js" ]
 }
 
 js_library("tab_strip_view_proxy") {
@@ -49,9 +44,6 @@ js_library("tab_strip_view_proxy") {
   ]
 }
 
-js_library("types") {
-}
-
 group("tab_strip_modules") {
   deps = [
     ":alert_indicator_module",

chrome/browser/resources/tab_strip/tab.html

@@ -47,7 +47,7 @@
     width: var(--favicon-size);
   }
 
-  #loading,
+  #progressSpinner,
   #favicon,
   #crashedIcon {
     height: var(--favicon-size);
@@ -58,11 +58,10 @@
     width: var(--favicon-size);
   }
 
-  #loading {
+  #progressSpinner {
     -webkit-mask:
         url(chrome://resources/images/throbber_small.svg)
         center/contain no-repeat;
-    background-color: var(--tabstrip-tab-loading-spinning-color);
     display: none;
   }
 
@@ -93,8 +92,8 @@
     width: 6px;
   }
 
-  :host([loading]) #loading,
-  :host([loading]) #favicon {
+  :host([waiting]) #progressSpinner,
+  :host([loading]) #progressSpinner {
     display: block;
   }
 
@@ -105,6 +104,20 @@
     width: calc(var(--favicon-size) - 6px);
   }
 
+  :host([waiting]) #progressSpinner {
+    background-color: var(--tabstrip-tab-waiting-spinning-color);
+    transform: /* Center first, then flip horizontally. */
+              translate(-50%, -50%) scaleX(-1);
+  }
+
+  :host([waiting]) #favicon {
+    display: none;
+  }
+
+  :host([loading]) #progressSpinner {
+    background-color: var(--tabstrip-tab-loading-spinning-color);
+  }
+
   :host([crashed]) #favicon {
     opacity: 0;
     transform: translate(-50%, 100%);
@@ -222,7 +235,7 @@
 <div id="dragImage">
   <header id="title">
     <div id="faviconContainer">
-      <div id="loading"></div>
+      <div id="progressSpinner"></div>
       <div id="favicon"></div>
       <div id="crashedIcon"></div>
       <div id="blocked"></div>

chrome/browser/resources/tab_strip/tab.js

@@ -5,16 +5,10 @@
 import {getFavicon, getFaviconForPageURL} from 'chrome://resources/js/icon.m.js';
 
 import {CustomElement} from './custom_element.js';
-import {TabsApiProxy} from './tabs_api_proxy.js';
+import {TabData, TabNetworkState, TabsApiProxy} from './tabs_api_proxy.js';
 
 export const DEFAULT_ANIMATION_DURATION = 125;
 
-/** @const @enum {string} */
-const STATUS = {
-  LOADING: 'loading',
-  COMPLETE: 'complete',
-};
-
 export class TabElement extends CustomElement {
   static get template() {
     return `{__html_template__}`;
@@ -45,7 +39,7 @@ export class TabElement extends CustomElement {
     this.thumbnail_ =
         /** @type {!Image} */ (this.shadowRoot.querySelector('#thumbnailImg'));
 
-    /** @private {!Tab} */
+    /** @private {!TabData} */
     this.tab_;
 
     /** @private {!TabsApiProxy} */
@@ -60,17 +54,22 @@ export class TabElement extends CustomElement {
     this.closeButtonEl_.addEventListener('click', this.onClose_.bind(this));
   }
 
-  /** @return {!Tab} */
+  /** @return {!TabData} */
   get tab() {
     return this.tab_;
   }
 
-  /** @param {!Tab} tab */
+  /** @param {!TabData} tab */
   set tab(tab) {
     this.toggleAttribute('active', tab.active);
-    // TODO(johntlee): Update loading status to also be based on whether
-    // the tab is navigating to a new document.
-    this.toggleAttribute('loading', tab.status === STATUS.LOADING);
+    this.toggleAttribute(
+        'waiting',
+        !tab.shouldHideThrobber &&
+            tab.networkState === TabNetworkState.WAITING);
+    this.toggleAttribute(
+        'loading',
+        !tab.shouldHideThrobber &&
+            tab.networkState === TabNetworkState.LOADING);
     this.toggleAttribute('pinned', tab.pinned);
     this.setAttribute('draggable', tab.pinned);
 
@@ -83,7 +82,8 @@ export class TabElement extends CustomElement {
         this.faviconEl_.style.backgroundImage = getFavicon(tab.favIconUrl);
       }
     } else {
-      if (tab.status === STATUS.COMPLETE) {
+      if (tab.networkState === TabNetworkState.NONE ||
+          tab.networkState === TabNetworkState.ERROR) {
         // If the tab has finished loading and there still is no favicon,
         // fallback to a favicon generated by the page URL. This guarantees
         // there is always some favicon, even if it's the default icon.

chrome/browser/resources/tab_strip/tab_list.js

@@ -10,7 +10,7 @@ import {addWebUIListener} from 'chrome://resources/js/cr.m.js';
 import {CustomElement} from './custom_element.js';
 import {TabElement} from './tab.js';
 import {TabStripViewProxy} from './tab_strip_view_proxy.js';
-import {TabsApiProxy} from './tabs_api_proxy.js';
+import {TabData, TabsApiProxy} from './tabs_api_proxy.js';
 
 /**
  * The amount of padding to leave between the edge of the screen and the active
@@ -111,7 +111,7 @@ class TabListElement extends CustomElement {
   }
 
   /**
-   * @param {!Tab} tab
+   * @param {!TabData} tab
    * @return {!TabElement}
    * @private
    */
@@ -248,20 +248,20 @@ class TabListElement extends CustomElement {
   onTabActivated_(tabId) {
     const previouslyActiveTab = this.getActiveTab_();
     if (previouslyActiveTab) {
-      previouslyActiveTab.tab = /** @type {!Tab} */ (
+      previouslyActiveTab.tab = /** @type {!TabData} */ (
           Object.assign({}, previouslyActiveTab.tab, {active: false}));
     }
 
     const newlyActiveTab = this.findTabElement_(tabId);
     if (newlyActiveTab) {
-      newlyActiveTab.tab = /** @type {!Tab} */ (
+      newlyActiveTab.tab = /** @type {!TabData} */ (
           Object.assign({}, newlyActiveTab.tab, {active: true}));
       this.moveOrScrollToActiveTab_();
     }
   }
 
   /**
-   * @param {!Tab} tab
+   * @param {!TabData} tab
    * @private
    */
   onTabCreated_(tab) {
@@ -311,7 +311,7 @@ class TabListElement extends CustomElement {
   }
 
   /**
-   * @param {!Tab} tab
+   * @param {!TabData} tab
    * @private
    */
   onTabUpdated_(tab) {

chrome/browser/resources/tab_strip/tabs_api_proxy.js

@@ -4,10 +4,40 @@
 
 import {addSingletonGetter, sendWithPromise} from 'chrome://resources/js/cr.m.js';
 
+/**
+ * Must be kept in sync with TabNetworkState from
+ * //chrome/browser/ui/tabs/tab_network_state.h.
+ * @enum {number}
+ */
+export const TabNetworkState = {
+  NONE: 0,
+  WAITING: 1,
+  LOADING: 2,
+  ERROR: 3,
+};
+
+/**
+ * @typedef {{
+ *    active: boolean,
+ *    favIconUrl: string,
+ *    id: number,
+ *    index: number,
+ *    networkState: !TabNetworkState,
+ *    pinned: boolean,
+ *    shouldHideThrobber: boolean,
+ *    title: string,
+ *    url: string,
+ * }}
+ */
+export let TabData;
+
+/** @typedef {!Tab} */
+let ExtensionsApiTab;
+
 export class TabsApiProxy {
   /**
    * @param {number} tabId
-   * @return {!Promise<!Tab>}
+   * @return {!Promise<!ExtensionsApiTab>}
    */
   activateTab(tabId) {
     return new Promise(resolve => {
@@ -16,7 +46,7 @@ export class TabsApiProxy {
   }
 
   /**
-   * @return {!Promise<!Array<!Tab>>}
+   * @return {!Promise<!Array<!TabData>>}
    */
   getTabs() {
     return sendWithPromise('getTabs');
@@ -35,7 +65,7 @@ export class TabsApiProxy {
   /**
    * @param {number} tabId
    * @param {number} newIndex
-   * @return {!Promise<!Tab>}
+   * @return {!Promise<!ExtensionsApiTab>}
    */
   moveTab(tabId, newIndex) {
     return new Promise(resolve => {

chrome/browser/resources/tab_strip/types.js

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-/**
- * @fileoverview Closure typedefs for Tab Strip.
- */
-
-/**
- * @typedef {{
- *    tabId: number,
- *    windowId: number,
- * }}
- */
-let TabActivatedInfo;
-
-/**
- * @typedef {{
- *    newPosition: number,
- *    newWindowId: number,
- * }}
- */
-let TabAttachedInfo;
-
-/**
- * @typedef {{
- *    oldPosition: number,
- *    oldWindowId: number,
- * }}
- */
-let TabDetachedInfo;
-
-/**
- * @typedef {{
- *    fromIndex: number,
- *    toIndex: number,
- *    windowId: number,
- * }}
- */
-let TabMovedInfo;
-
-/**
- * @typedef {{
- *    isWindowClosing: boolean,
- *    windowId: number,
- * }}
- */
-let WindowRemoveInfo;

chrome/browser/ui/webui/tab_strip/tab_strip_ui.cc

@@ -194,8 +194,6 @@ class TabStripUIHandler : public content::WebUIMessageHandler,
                         browser_->tab_strip_model()->active_index() == index);
     tab_data.SetInteger("id", extensions::ExtensionTabUtil::GetTabId(contents));
     tab_data.SetInteger("index", index);
-    tab_data.SetString("status", extensions::ExtensionTabUtil::GetTabStatusText(
-                                     contents->IsLoading()));
 
     // TODO(johntlee): Replace with favicon from TabRendererData
     content::NavigationEntry* visible_entry =
@@ -209,6 +207,10 @@ class TabStripUIHandler : public content::WebUIMessageHandler,
     tab_data.SetBoolean("pinned", tab_renderer_data.pinned);
     tab_data.SetString("title", tab_renderer_data.title);
     tab_data.SetString("url", tab_renderer_data.visible_url.GetContent());
+    tab_data.SetInteger("networkState",
+                        static_cast<int>(tab_renderer_data.network_state));
+    tab_data.SetBoolean("shouldHideThrobber",
+                        tab_renderer_data.should_hide_throbber);
     // TODO(johntlee): Add the rest of TabRendererData
 
     return tab_data;
@@ -253,6 +255,9 @@ class TabStripUIHandler : public content::WebUIMessageHandler,
     colors.SetString("--tabstrip-tab-loading-spinning-color",
                      color_utils::SkColorToRgbaString(tp.GetColor(
                          ThemeProperties::COLOR_TAB_THROBBER_SPINNING)));
+    colors.SetString("--tabstrip-tab-waiting-spinning-color",
+                     color_utils::SkColorToRgbaString(tp.GetColor(
+                         ThemeProperties::COLOR_TAB_THROBBER_WAITING)));
     colors.SetString("--tabstrip-indicator-recording-color",
                      color_utils::SkColorToRgbaString(tp.GetColor(
                          ThemeProperties::COLOR_TAB_ALERT_RECORDING)));

chrome/test/data/webui/tab_strip/tab_test.js

@@ -5,7 +5,8 @@
 import 'chrome://tab-strip/tab.js';
 
 import {getFavicon, getFaviconForPageURL} from 'chrome://resources/js/icon.m.js';
-import {TabsApiProxy} from 'chrome://tab-strip/tabs_api_proxy.js';
+import {TabNetworkState, TabsApiProxy} from 'chrome://tab-strip/tabs_api_proxy.js';
+
 import {TestTabsApiProxy} from './test_tabs_api_proxy.js';
 
 suite('Tab', function() {
@@ -14,7 +15,7 @@ suite('Tab', function() {
 
   const tab = {
     id: 1001,
-    status: 'complete',
+    networkState: TabNetworkState.NONE,
     title: 'My title',
   };
 
@@ -65,12 +66,23 @@ suite('Tab', function() {
   });
 
   test('toggles a [loading] attribute when loading', () => {
-    tabElement.tab = Object.assign({}, tab, {status: 'loading'});
+    tabElement.tab =
+        Object.assign({}, tab, {networkState: TabNetworkState.LOADING});
     assertTrue(tabElement.hasAttribute('loading'));
-    tabElement.tab = Object.assign({}, tab, {status: 'complete'});
+    tabElement.tab =
+        Object.assign({}, tab, {networkState: TabNetworkState.NONE});
     assertFalse(tabElement.hasAttribute('loading'));
   });
 
+  test('toggles a [waiting] attribute when waiting', () => {
+    tabElement.tab =
+        Object.assign({}, tab, {networkState: TabNetworkState.WAITING});
+    assertTrue(tabElement.hasAttribute('waiting'));
+    tabElement.tab =
+        Object.assign({}, tab, {networkState: TabNetworkState.NONE});
+    assertFalse(tabElement.hasAttribute('waiting'));
+  });
+
   test('clicking on the element activates the tab', () => {
     tabElement.click();
     return testTabsApiProxy.whenCalled('activateTab', tabId => {
@@ -116,7 +128,8 @@ suite('Tab', function() {
       'removes the favicon if the tab is loading and there is no favicon URL',
       () => {
         delete tab.favIconUrl;
-        tabElement.tab = Object.assign({}, tab, {status: 'loading'});
+        tabElement.tab =
+            Object.assign({}, tab, {networkState: TabNetworkState.LOADING});
         const faviconElement = tabElement.shadowRoot.querySelector('#favicon');
         assertEquals(faviconElement.style.backgroundImage, 'none');
       });