Re-focus View on tab strip close

When the WebUI tab strip's WebContents gains focus, the focused
TextInputClient changes while the focused View does not. This causes
text input to stop working in the omnibox despite it having Views-focus.

This blurs and re-focuses the currently focused View. For the Omnibox,
this makes it re-install itself as the focused TextInputClient.

This is a workaround and should be removed once bug 994350 is fixed.

Bug: 1027375, 994350
Change-Id: I2be055110535dc6164a7d31a64a3cf503fae7a7b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1935729Reviewed-by: Dana Fried <dfried@chromium.org>
Commit-Queue: Dana Fried <dfried@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719473}

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

@@ -198,6 +198,13 @@ void WebUITabStripContainerView::UpdateButtons() {
   }
 }
 
+void WebUITabStripContainerView::SetVisibleForTesting(bool visible) {
+  SetContainerTargetVisibility(visible);
+  animation_.SetCurrentValue(visible ? 1.0 : 0.0);
+  animation_.End();
+  PreferredSizeChanged();
+}
+
 const ui::AcceleratorProvider*
 WebUITabStripContainerView::GetAcceleratorProvider() const {
   return BrowserView::GetBrowserViewForBrowser(browser_);
@@ -218,6 +225,24 @@ void WebUITabStripContainerView::SetContainerTargetVisibility(
     animation_.SetSlideDuration(base::TimeDelta::FromMilliseconds(200));
     animation_.Hide();
     web_view_->SetFocusBehavior(FocusBehavior::NEVER);
+
+    // Tapping in the WebUI tab strip gives keyboard focus to the
+    // WebContents's native window. While this doesn't take away View
+    // focus, it will change the focused TextInputClient; see
+    // |ui::InputMethod::SetFocusedTextInputClient()|. The Omnibox is a
+    // TextInputClient, and it installs itself as the focused
+    // TextInputClient when it receives Views-focus. So, tapping in the
+    // tab strip while the Omnibox has focus will mean text cannot be
+    // entered until it is blurred and re-focused. This caused
+    // crbug.com/1027375.
+    //
+    // TODO(crbug.com/994350): stop WebUI tab strip from taking focus on
+    // tap and remove this workaround.
+    views::FocusManager* const focus_manager = GetFocusManager();
+    if (focus_manager) {
+      focus_manager->StoreFocusedView(true /* clear_native_focus */);
+      focus_manager->RestoreFocusedView();
+    }
   }
   auto_closer_->set_enabled(target_visible);
 }

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

@@ -52,6 +52,13 @@ class WebUITabStripContainerView : public TabStripUI::Embedder,
 
   void UpdateButtons();
 
+  // Clicking the tab counter button opens and closes the container with
+  // an animation, so it is unsuitable for an interactive test. This
+  // should be called instead. View::SetVisible() isn't sufficient since
+  // the container's preferred size will change.
+  void SetVisibleForTesting(bool visible);
+  views::WebView* web_view_for_testing() const { return web_view_; }
+
  private:
   class AutoCloser;
 

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

+// 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.
+
+#include "base/strings/utf_string_conversions.h"
+#include "base/test/scoped_feature_list.h"
+#include "chrome/browser/ui/ui_features.h"
+#include "chrome/browser/ui/view_ids.h"
+#include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/frame/webui_tab_strip_container_view.h"
+#include "chrome/browser/ui/views/location_bar/location_bar_view.h"
+#include "chrome/browser/ui/views/omnibox/omnibox_view_views.h"
+#include "chrome/browser/ui/views/toolbar/toolbar_view.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/interactive_test_utils.h"
+#include "ui/base/ui_base_switches.h"
+#include "ui/views/controls/webview/webview.h"
+
+class WebUITabStripInteractiveTest : public InProcessBrowserTest {
+ public:
+  WebUITabStripInteractiveTest() {
+    base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+        switches::kTopChromeTouchUi, switches::kTopChromeTouchUiEnabled);
+    base::CommandLine::ForCurrentProcess()->AppendSwitch(
+        switches::kWebUITabStrip);
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_override_;
+};
+
+// Regression test for crbug.com/1027375.
+IN_PROC_BROWSER_TEST_F(WebUITabStripInteractiveTest,
+                       CanTypeInOmniboxAfterTabStripClose) {
+  BrowserView* const browser_view =
+      BrowserView::GetBrowserViewForBrowser(browser());
+  WebUITabStripContainerView* const container = browser_view->webui_tab_strip();
+
+  // Open the tab strip.
+  container->SetVisibleForTesting(true);
+  browser_view->Layout();
+
+  // Make sure the tab strip's contents are fully loaded.
+  views::WebView* const container_web_view = container->web_view_for_testing();
+  ASSERT_TRUE(WaitForLoadStop(container_web_view->GetWebContents()));
+
+  ui_test_utils::FocusView(browser(), VIEW_ID_OMNIBOX);
+
+  OmniboxViewViews* const omnibox =
+      browser_view->toolbar()->location_bar()->omnibox_view();
+  omnibox->SetUserText(base::ASCIIToUTF16(""));
+
+  // Click in tab strip, then close it.
+  base::RunLoop click_loop;
+  ui_test_utils::MoveMouseToCenterAndPress(
+      container_web_view, ui_controls::LEFT,
+      ui_controls::DOWN | ui_controls::UP, click_loop.QuitClosure());
+  click_loop.Run();
+  container->SetVisibleForTesting(false);
+
+  // The omnibox should still be focused and should accept keyboard input.
+  EXPECT_TRUE(ui_test_utils::IsViewFocused(browser(), VIEW_ID_OMNIBOX));
+  EXPECT_TRUE(ui_test_utils::SendKeyPressSync(browser(), ui::VKEY_A, false,
+                                              false, false, false));
+  EXPECT_EQ(base::ASCIIToUTF16("a"), omnibox->GetText());
+}

chrome/browser/ui/views/toolbar/webui_tab_counter_button.cc

@@ -6,6 +6,7 @@
 
 #include "base/i18n/message_formatter.h"
 #include "base/i18n/number_formatting.h"
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/ui/layout_constants.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -75,9 +76,13 @@ std::unique_ptr<views::View> CreateWebUITabCounterButton(
     TabStripModel* tab_strip_model) {
   auto tab_counter = std::make_unique<WebUITabCounterButton>(listener);
 
+  tab_counter->SetID(VIEW_ID_WEBUI_TAB_STRIP_TAB_COUNTER);
+  // TODO(crbug.com/1028827): replace this with production string and provide
+  // tab count.
+  tab_counter->SetAccessibleName(base::ASCIIToUTF16("Open tab strip"));
+
   // TODO(999557): Create a custom text style to get the correct size/weight.
   // TODO(999557): Figure out how to get the right font.
-  tab_counter->SetID(VIEW_ID_WEBUI_TAB_STRIP_TAB_COUNTER);
   tab_counter->SetProperty(views::kFlexBehaviorKey,
                            views::FlexSpecification::ForSizeRule(
                                views::MinimumFlexSizeRule::kScaleToMinimum,

chrome/test/BUILD.gn

@@ -5829,6 +5829,11 @@ if (!is_android) {
     if (is_linux && !is_chromeos) {
       deps += [ "//ui/base/ime/linux" ]
     }
+
+    if (enable_webui_tab_strip) {
+      sources +=
+          [ "../browser/ui/views/frame/webui_tab_strip_interactive_uitest.cc" ]
+    }
   }
 }