Handle immersive mode in WebUI tab strip's autoclose behavior

In immersive mode, the toolbar floats on top of the tab content. Their
bounds intersect.

The WebUI tab strip would autoclose whenever an event occured inside the
tab content's bounds. However, this didn't consider if the event would
ultimately be delivered to browser UI instead.

This CL lets events whose locations are in the toolbar pass through. It
also adds test coverage for these behaviors.

A more robust solution would consider the event's target window rather
than just its location, but this is more complex. For now these bounds
checks should be sufficient.

Bug: 1112028
Change-Id: Id290bacb973bad1db535c7f69122c1d4019571e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2339591
Commit-Queue: Collin Baker <collinbaker@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795659}

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

@@ -36,6 +36,8 @@
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/chrome_view_class_properties.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/frame/immersive_mode_controller.h"
+#include "chrome/browser/ui/views/frame/top_container_view.h"
 #include "chrome/browser/ui/views/in_product_help/feature_promo_bubble_params.h"
 #include "chrome/browser/ui/views/in_product_help/feature_promo_bubble_view.h"
 #include "chrome/browser/ui/views/in_product_help/feature_promo_colors.h"
@@ -132,13 +134,17 @@ class WebUITabStripContainerView::AutoCloser : public ui::EventHandler,
   using CloseCallback = base::RepeatingCallback<void(TabStripUICloseAction)>;
 
   AutoCloser(CloseCallback close_callback,
+             views::View* top_container,
              views::View* content_area,
              views::View* omnibox)
       : close_callback_(std::move(close_callback)),
+        top_container_(top_container),
         content_area_(content_area),
         omnibox_(omnibox) {
+    DCHECK(top_container_);
     DCHECK(content_area_);
     DCHECK(omnibox_);
+    view_observer_.Add(top_container_);
     view_observer_.Add(content_area_);
     view_observer_.Add(omnibox_);
 
@@ -181,6 +187,14 @@ class WebUITabStripContainerView::AutoCloser : public ui::EventHandler,
     if (!content_area_->GetBoundsInScreen().Contains(event_location_in_screen))
       return;
 
+    // The event may intersect both the content area's bounds and the
+    // top container's bounds. In this case, the top container is
+    // occluding the web content so we shouldn't close. This happens in
+    // immersive mode while the top container is revealed. For more info see
+    // https://crbug.com/1112028
+    if (top_container_->GetBoundsInScreen().Contains(event_location_in_screen))
+      return;
+
     located_event->StopPropagation();
     close_callback_.Run(TabStripUICloseAction::kTapInTabContent);
   }
@@ -201,6 +215,8 @@ class WebUITabStripContainerView::AutoCloser : public ui::EventHandler,
       content_area_ = nullptr;
     else if (observed_view == omnibox_)
       omnibox_ = nullptr;
+    else if (observed_view == top_container_)
+      top_container_ = nullptr;
     else
       NOTREACHED();
   }
@@ -228,6 +244,7 @@ class WebUITabStripContainerView::AutoCloser : public ui::EventHandler,
 
  private:
   CloseCallback close_callback_;
+  views::View* top_container_;
   views::View* content_area_;
   views::View* omnibox_;
 
@@ -404,6 +421,7 @@ WebUITabStripContainerView::WebUITabStripContainerView(
       auto_closer_(std::make_unique<AutoCloser>(
           base::Bind(&WebUITabStripContainerView::CloseForEventOutsideTabStrip,
                      base::Unretained(this)),
+          browser_view->top_container(),
           tab_contents_container,
           omnibox)),
       drag_to_open_handler_(
@@ -521,7 +539,14 @@ std::unique_ptr<views::View> WebUITabStripContainerView::CreateTabCounter() {
 
 void WebUITabStripContainerView::SetVisibleForTesting(bool visible) {
   SetContainerTargetVisibility(visible);
-  animation_.SetCurrentValue(visible ? 1.0 : 0.0);
+  FinishAnimationForTesting();
+}
+
+void WebUITabStripContainerView::FinishAnimationForTesting() {
+  if (!animation_.is_animating())
+    return;
+  const bool target = animation_.IsShowing();
+  animation_.SetCurrentValue(target ? 1.0 : 0.0);
   animation_.End();
   PreferredSizeChanged();
 }
@@ -585,6 +610,11 @@ void WebUITabStripContainerView::EndDragToOpen(
 void WebUITabStripContainerView::SetContainerTargetVisibility(
     bool target_visible) {
   if (target_visible) {
+    immersive_revealed_lock_.reset(
+        BrowserView::GetBrowserViewForBrowser(browser_)
+            ->immersive_mode_controller()
+            ->GetRevealedLock(ImmersiveModeController::ANIMATE_REVEAL_YES));
+
     SetVisible(true);
     PreferredSizeChanged();
     if (animation_.GetCurrentValue() < 1.0) {
@@ -618,6 +648,8 @@ void WebUITabStripContainerView::SetContainerTargetVisibility(
     }
 
     web_view_->SetFocusBehavior(FocusBehavior::NEVER);
+
+    immersive_revealed_lock_.reset();
   }
   auto_closer_->set_enabled(target_visible);
 }

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

@@ -39,6 +39,7 @@ class WebView;
 
 class Browser;
 class BrowserView;
+class ImmersiveRevealedLock;
 
 class WebUITabStripContainerView : public TabStripUIEmbedder,
                                    public gfx::AnimationDelegate,
@@ -75,6 +76,9 @@ class WebUITabStripContainerView : public TabStripUIEmbedder,
   views::WebView* web_view_for_testing() const { return web_view_; }
   views::View* tab_counter_for_testing() const { return tab_counter_; }
 
+  // Finish the open or close animation if it's active.
+  void FinishAnimationForTesting();
+
  private:
   class AutoCloser;
   class DragToOpenHandler;
@@ -142,6 +146,9 @@ class WebUITabStripContainerView : public TabStripUIEmbedder,
   // long the tab strip is kept open.
   base::Optional<base::TimeTicks> time_at_open_;
 
+  // Used to keep the toolbar revealed while the tab strip is open.
+  std::unique_ptr<ImmersiveRevealedLock> immersive_revealed_lock_;
+
   gfx::SlideAnimation animation_{this};
 
   std::unique_ptr<AutoCloser> auto_closer_;

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

@@ -4,12 +4,15 @@
 
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_feature_list.h"
+#include "build/build_config.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/immersive_mode_controller.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/reload_button.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/interactive_test_utils.h"
@@ -17,6 +20,11 @@
 #include "ui/base/pointer/touch_ui_controller.h"
 #include "ui/views/controls/webview/webview.h"
 
+#if defined(OS_CHROMEOS)
+#include "ash/public/cpp/immersive/immersive_fullscreen_controller.h"
+#include "ash/public/cpp/immersive/immersive_fullscreen_controller_test_api.h"
+#endif  // defined(OS_CHROMEOS)
+
 class WebUITabStripInteractiveTest : public InProcessBrowserTest {
  public:
   WebUITabStripInteractiveTest() {
@@ -73,3 +81,106 @@ IN_PROC_BROWSER_TEST_F(WebUITabStripInteractiveTest,
                                               false, false, false));
   EXPECT_EQ(base::ASCIIToUTF16("a"), omnibox->GetText());
 }
+
+IN_PROC_BROWSER_TEST_F(WebUITabStripInteractiveTest,
+                       EventInTabContentClosesContainer) {
+  BrowserView* const browser_view =
+      BrowserView::GetBrowserViewForBrowser(browser());
+
+  WebUITabStripContainerView* const container = browser_view->webui_tab_strip();
+  ASSERT_NE(nullptr, container);
+
+  // Open the tab strip
+  container->SetVisibleForTesting(true);
+  browser_view->Layout();
+
+  base::RunLoop click_loop;
+  ui_test_utils::MoveMouseToCenterAndPress(
+      browser_view->contents_web_view(), ui_controls::LEFT,
+      ui_controls::DOWN | ui_controls::UP, click_loop.QuitClosure());
+  click_loop.Run();
+
+  // Make sure it's closed (after the close animation).
+  container->FinishAnimationForTesting();
+  EXPECT_FALSE(container->GetVisible());
+}
+
+IN_PROC_BROWSER_TEST_F(WebUITabStripInteractiveTest,
+                       EventInContainerDoesNotClose) {
+  BrowserView* const browser_view =
+      BrowserView::GetBrowserViewForBrowser(browser());
+
+  WebUITabStripContainerView* const container = browser_view->webui_tab_strip();
+  ASSERT_NE(nullptr, container);
+
+  // Open the tab strip
+  container->SetVisibleForTesting(true);
+  browser_view->Layout();
+
+  base::RunLoop click_loop;
+  ui_test_utils::MoveMouseToCenterAndPress(container, ui_controls::LEFT,
+                                           ui_controls::DOWN | ui_controls::UP,
+                                           click_loop.QuitClosure());
+  click_loop.Run();
+
+  // Make sure it stays open. The FinishAnimationForTesting() call
+  // should be a no-op.
+  container->FinishAnimationForTesting();
+  EXPECT_TRUE(container->GetVisible());
+  EXPECT_FALSE(container->bounds().IsEmpty());
+}
+
+#if defined(OS_CHROMEOS)
+
+// Regression test for crbug.com/1112028
+IN_PROC_BROWSER_TEST_F(WebUITabStripInteractiveTest, CanUseInImmersiveMode) {
+  BrowserView* const browser_view =
+      BrowserView::GetBrowserViewForBrowser(browser());
+
+  ash::ImmersiveFullscreenControllerTestApi immersive_test_api(
+      ash::ImmersiveFullscreenController::Get(browser_view->GetWidget()));
+  immersive_test_api.SetupForTest();
+
+  ImmersiveModeController* const immersive_mode_controller =
+      browser_view->immersive_mode_controller();
+  immersive_mode_controller->SetEnabled(true);
+
+  WebUITabStripContainerView* const container = browser_view->webui_tab_strip();
+  ASSERT_NE(nullptr, container);
+
+  EXPECT_FALSE(immersive_mode_controller->IsRevealed());
+
+  // Try opening the tab strip.
+  container->SetVisibleForTesting(true);
+  browser_view->Layout();
+  EXPECT_TRUE(container->GetVisible());
+  EXPECT_FALSE(container->bounds().IsEmpty());
+  EXPECT_TRUE(immersive_mode_controller->IsRevealed());
+
+  // Tapping in the tab strip shouldn't hide the toolbar.
+  base::RunLoop click_loop_1;
+  ui_test_utils::MoveMouseToCenterAndPress(container, ui_controls::LEFT,
+                                           ui_controls::DOWN | ui_controls::UP,
+                                           click_loop_1.QuitClosure());
+  click_loop_1.Run();
+
+  // If the behavior is correct, this call will be a no-op.
+  container->FinishAnimationForTesting();
+  EXPECT_TRUE(container->GetVisible());
+  EXPECT_FALSE(container->bounds().IsEmpty());
+  EXPECT_TRUE(immersive_mode_controller->IsRevealed());
+
+  // Interacting with the toolbar should also not close the container.
+  base::RunLoop click_loop_2;
+  ui_test_utils::MoveMouseToCenterAndPress(
+      browser_view->toolbar()->reload_button(), ui_controls::LEFT,
+      ui_controls::DOWN | ui_controls::UP, click_loop_2.QuitClosure());
+  click_loop_2.Run();
+
+  container->FinishAnimationForTesting();
+  EXPECT_TRUE(container->GetVisible());
+  EXPECT_FALSE(container->bounds().IsEmpty());
+  EXPECT_TRUE(immersive_mode_controller->IsRevealed());
+}
+
+#endif  // defined(OS_CHROMEOS)