WebUI Tab Strip: Separate keyboard and mouse focus

This CL moves away from listening on the generic window onfocus event
and moves towards sending a custom WebUI event when the AccessiblePane
receives focus. This resolves 2 main issues:
(1) Visual focus indicators being visible on focus caused by touch.
(2) The tab strip scrolls to the very first tab on tap because the
first tab immediately becomes focused on window focus.

Bug: 1017472
Change-Id: I8565b2e244a205791ba81eca23ba5a475c2f441c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1929447
Commit-Queue: John Lee <johntlee@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Collin Baker <collinbaker@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718769}

chrome/browser/resources/tab_strip/BUILD.gn

@@ -44,6 +44,9 @@ js_library("tab") {
 }
 
 js_library("tab_list") {
+  deps = [
+    "//ui/webui/resources/js/cr/ui:focus_outline_manager.m",
+  ]
 }
 
 js_library("tab_strip_embedder_proxy") {

chrome/browser/resources/tab_strip/tab.html

@@ -26,15 +26,18 @@
   }
 
   #tab:focus {
-    box-shadow: 0 0 0 2px var(--tabstrip-focus-outline-color);
     outline: none;
   }
 
+  :host-context(.focus-outline-visible) #tab:focus {
+    box-shadow: 0 0 0 2px var(--tabstrip-focus-outline-color);
+  }
+
   :host([active]) #tab {
     box-shadow: 0 0 0 2px var(--tabstrip-tab-active-border-color);
   }
 
-  :host([active]) #tab:focus {
+  :host-context(.focus-outline-visible):host([active]) #tab:focus {
     box-shadow: 0 0 0 4px var(--tabstrip-focus-outline-color),
                 0 0 0 2px var(--tabstrip-tab-active-border-color);
   }
@@ -221,7 +224,7 @@
     width: 18px;
   }
 
-  #close:focus #closeIconFocus {
+  :host-context(.focus-outline-visible) #close:focus #closeIconFocus {
     background: currentColor;
     border-radius: 50%;
     height: 24px;

chrome/browser/resources/tab_strip/tab_list.js

@@ -7,6 +7,7 @@ import './tab.js';
 
 import {assert} from 'chrome://resources/js/assert.m.js';
 import {addWebUIListener} from 'chrome://resources/js/cr.m.js';
+import {FocusOutlineManager} from 'chrome://resources/js/cr/ui/focus_outline_manager.m.js';
 import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 
 import {CustomElement} from './custom_element.js';
@@ -58,6 +59,9 @@ class TabListElement extends CustomElement {
      */
     this.draggedItem_;
 
+    /** @private @const {!FocusOutlineManager} */
+    this.focusOutlineManager_ = FocusOutlineManager.forDocument(document);
+
     /**
      * An intersection observer is needed to observe which TabElements are
      * currently in view or close to being in view, which will help determine
@@ -98,9 +102,6 @@ class TabListElement extends CustomElement {
     /** @private {!Function} */
     this.windowBlurListener_ = () => this.onWindowBlur_();
 
-    /** @private {!Function} */
-    this.windowFocusListener_ = () => this.onWindowFocus_();
-
     addWebUIListener(
         'layout-changed', layout => this.applyCSSDictionary_(layout));
     addWebUIListener('theme-changed', () => this.fetchAndUpdateColors_());
@@ -118,8 +119,9 @@ class TabListElement extends CustomElement {
 
     document.addEventListener(
         'visibilitychange', this.documentVisibilityChangeListener_);
+    addWebUIListener(
+        'received-keyboard-focus', () => this.onReceivedKeyboardFocus_());
     window.addEventListener('blur', this.windowBlurListener_);
-    window.addEventListener('focus', this.windowFocusListener_);
 
     if (loadTimeData.getBoolean('showDemoOptions')) {
       this.shadowRoot.querySelector('#demoOptions').style.display = 'block';
@@ -182,7 +184,6 @@ class TabListElement extends CustomElement {
     document.removeEventListener(
         'visibilitychange', this.documentVisibilityChangeListener_);
     window.removeEventListener('blur', this.windowBlurListener_);
-    window.removeEventListener('focus', this.windowFocusListener_);
   }
 
   /**
@@ -341,6 +342,15 @@ class TabListElement extends CustomElement {
         event.pageY - this.draggedItem_.offsetTop);
   }
 
+  /** @private */
+  onReceivedKeyboardFocus_() {
+    // FocusOutlineManager relies on the most recent event fired on the
+    // document. When the tab strip first gains keyboard focus, no such event
+    // exists yet, so the outline needs to be explicitly set to visible.
+    this.focusOutlineManager_.visible = true;
+    this.shadowRoot.querySelector('tabstrip-tab').focus();
+  }
+
   /**
    * @param {number} tabId
    * @private
@@ -465,11 +475,6 @@ class TabListElement extends CustomElement {
     }
   }
 
-  /** @private */
-  onWindowFocus_() {
-    this.shadowRoot.querySelector('tabstrip-tab').focus();
-  }
-
   /**
    * @param {!TabElement} tabElement
    * @private

chrome/browser/ui/views/frame/webui_tab_strip_container_view.cc

@@ -298,7 +298,7 @@ void WebUITabStripContainerView::ButtonPressed(views::Button* sender,
     if (GetVisible() && sender->HasFocus()) {
       // Automatically move focus to the tab strip WebUI if the focus is
       // currently on the toggle button.
-      SetPaneFocus(web_view_);
+      SetPaneFocusAndFocusDefault();
     }
   } else if (sender->GetID() == VIEW_ID_WEBUI_TAB_STRIP_NEW_TAB_BUTTON) {
     chrome::ExecuteCommand(browser_, IDC_NEW_TAB);
@@ -330,3 +330,15 @@ void WebUITabStripContainerView::OnViewIsDeleting(View* observed_view) {
   else if (observed_view == tab_counter_)
     tab_counter_ = nullptr;
 }
+
+bool WebUITabStripContainerView::SetPaneFocusAndFocusDefault() {
+  // Make sure the pane first receives focus, then send a WebUI event to the
+  // front-end so the correct HTML element receives focus.
+  bool received_focus = AccessiblePaneView::SetPaneFocusAndFocusDefault();
+  if (received_focus) {
+    TabStripUI* const tab_strip_ui = static_cast<TabStripUI*>(
+        web_view_->GetWebContents()->GetWebUI()->GetController());
+    tab_strip_ui->ReceivedKeyboardFocus();
+  }
+  return received_focus;
+}

chrome/browser/ui/views/frame/webui_tab_strip_container_view.h

@@ -86,6 +86,9 @@ class WebUITabStripContainerView : public TabStripUI::Embedder,
   void OnViewBoundsChanged(View* observed_view) override;
   void OnViewIsDeleting(View* observed_view) override;
 
+  // views::AccessiblePaneView
+  bool SetPaneFocusAndFocusDefault() override;
+
   Browser* const browser_;
   views::WebView* const web_view_;
   views::View* const tab_contents_container_;

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

@@ -197,6 +197,12 @@ class TabStripUIHandler : public content::WebUIMessageHandler,
     FireWebUIListener("layout-changed", embedder_->GetLayout().AsDictionary());
   }
 
+  void NotifyReceivedKeyboardFocus() {
+    if (!IsJavascriptAllowed())
+      return;
+    FireWebUIListener("received-keyboard-focus");
+  }
+
   // TabStripModelObserver:
   void OnTabStripModelChanged(
       TabStripModel* tab_strip_model,
@@ -575,3 +581,7 @@ void TabStripUI::Initialize(Browser* browser, Embedder* embedder) {
 void TabStripUI::LayoutChanged() {
   handler_->NotifyLayoutChanged();
 }
+
+void TabStripUI::ReceivedKeyboardFocus() {
+  handler_->NotifyReceivedKeyboardFocus();
+}

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

@@ -58,6 +58,9 @@ class TabStripUI : public content::WebUIController {
   // Embedder::GetLayout() changes.
   void LayoutChanged();
 
+  // The embedder should call this whenever the tab strip gains keyboard focus.
+  void ReceivedKeyboardFocus();
+
  private:
   void HandleThumbnailUpdate(int extension_tab_id,
                              ThumbnailTracker::CompressedThumbnailData image);

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

@@ -5,6 +5,7 @@
 import 'chrome://tab-strip/tab_list.js';
 
 import {webUIListenerCallback} from 'chrome://resources/js/cr.m.js';
+import {FocusOutlineManager} from 'chrome://resources/js/cr/ui/focus_outline_manager.m.js';
 import {TabStripEmbedderProxy} from 'chrome://tab-strip/tab_strip_embedder_proxy.js';
 import {TabsApiProxy} from 'chrome://tab-strip/tabs_api_proxy.js';
 
@@ -513,16 +514,23 @@ suite('TabList', () => {
   });
 
   test(
-      'focusing and blurring the window focuses and blurs the first tab',
+      'focusing on tab strip with the keyboard adds a class and focuses ' +
+          'the first tab',
       () => {
-        window.dispatchEvent(new Event('focus'));
+        webUIListenerCallback('received-keyboard-focus');
         assertEquals(document.activeElement, tabList);
         assertEquals(tabList.shadowRoot.activeElement, getUnpinnedTabs()[0]);
-
-        window.dispatchEvent(new Event('blur'));
-        assertEquals(tabList.shadowRoot.activeElement, null);
+        assertTrue(FocusOutlineManager.forDocument(document).visible);
       });
 
+  test('blurring the tab strip blurs the active element', () => {
+    // First, make sure tab strip has keyboard focus.
+    webUIListenerCallback('received-keyboard-focus');
+
+    window.dispatchEvent(new Event('blur'));
+    assertEquals(tabList.shadowRoot.activeElement, null);
+  });
+
   test('should update the ID when a tab is replaced', () => {
     assertEquals(getUnpinnedTabs()[0].tab.id, 0);
     webUIListenerCallback('tab-replaced', tabs[0].id, 1000);