Layout top container if preferred size changes during immersive mode

In normal mode, BrowserView's layout is invalidated if the top
container's preferred size changes. In immersive mode, this wasn't
happening since the top container is held under a sibling view.

To ensure BrowserView's layout logic is used in this case, the overlay
view it provides is changed to propagate preferred size changes to
BrowserView.

Fixed: 1096569
Change-Id: Ibf4166422f178b28a652a5f9956eaf5bd913a8ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2265412Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Collin Baker <collinbaker@chromium.org>
Cr-Commit-Position: refs/heads/master@{#783097}

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

@@ -19,6 +19,7 @@
 #include "base/i18n/rtl.h"
 #include "base/location.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics.h"
 #include "base/single_thread_task_runner.h"
@@ -349,6 +350,32 @@ bool IsShowingWebContentsModalDialog(content::WebContents* web_contents) {
   return manager && manager->IsDialogActive();
 }
 
+// Overlay view that owns TopContainerView in some cases (such as during
+// immersive fullscreen reveal).
+class TopContainerOverlayView : public views::View {
+ public:
+  explicit TopContainerOverlayView(base::WeakPtr<BrowserView> browser_view)
+      : browser_view_(std::move(browser_view)) {}
+  ~TopContainerOverlayView() override = default;
+
+  void ChildPreferredSizeChanged(views::View* child) override {
+    // When a child of BrowserView changes its preferred size, it
+    // invalidates the BrowserView's layout as well. When a child is
+    // reparented under this overlay view, this doesn't happen since the
+    // overlay view is owned by NonClientView.
+    //
+    // BrowserView's layout logic still applies in this case. To ensure
+    // it is used, we must invalidate BrowserView's layout.
+    if (browser_view_)
+      browser_view_->InvalidateLayout();
+  }
+
+ private:
+  // The BrowserView this overlay is created for. WeakPtr is used since
+  // this view is held in a different hierarchy.
+  base::WeakPtr<BrowserView> browser_view_;
+};
+
 // A view targeter for the overlay view, which makes sure the overlay view
 // itself is never a target for events, but its children (i.e. top_container)
 // may be.
@@ -2396,7 +2423,7 @@ views::ClientView* BrowserView::CreateClientView(views::Widget* widget) {
 }
 
 views::View* BrowserView::CreateOverlayView() {
-  overlay_view_ = new views::View();
+  overlay_view_ = new TopContainerOverlayView(weak_ptr_factory_.GetWeakPtr());
   overlay_view_->SetVisible(false);
   overlay_view_targeter_ = std::make_unique<OverlayViewTargeterDelegate>();
   overlay_view_->SetEventTargeter(

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

@@ -12,19 +12,23 @@
 #include "ash/test/ash_test_base.h"
 #include "base/command_line.h"
 #include "base/macros.h"
+#include "base/test/scoped_feature_list.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/exclusive_access/exclusive_access_test.h"
 #include "chrome/browser/ui/exclusive_access/fullscreen_controller.h"
+#include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/views/frame/browser_non_client_frame_view.h"
 #include "chrome/browser/ui/views/frame/browser_non_client_frame_view_ash.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/frame/test_with_browser_view.h"
 #include "chrome/browser/ui/views/frame/top_container_view.h"
+#include "chrome/browser/ui/views/frame/webui_tab_strip_container_view.h"
 #include "chrome/browser/ui/views/fullscreen_control/fullscreen_control_host.h"
 #include "chrome/browser/ui/views/tabs/tab_strip.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "ui/aura/window.h"
+#include "ui/base/pointer/touch_ui_controller.h"
 #include "ui/events/event.h"
 #include "ui/views/controls/native/native_view_host.h"
 #include "ui/views/controls/webview/webview.h"
@@ -241,3 +245,33 @@ TEST_F(ImmersiveModeControllerAshTest, LayeredSpinners) {
   ToggleFullscreen();
   EXPECT_TRUE(tabstrip->CanPaintThrobberToLayer());
 }
+
+// Ensures the WebUI tab strip can be opened during immersive reveal.
+// Regression test for crbug.com/1096569 where it couldn't be opened.
+TEST_F(ImmersiveModeControllerAshTest, WebUITabStripCanOpen) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(features::kWebUITabStrip);
+
+  AddTab(browser(), GURL("about:blank"));
+
+  // The WebUI tab strip is only used in touch mode.
+  ui::TouchUiController::TouchUiScoperForTesting touch_mode_override(true);
+
+  WebUITabStripContainerView* const webui_tab_strip =
+      browser_view()->webui_tab_strip();
+  ASSERT_TRUE(webui_tab_strip);
+  EXPECT_FALSE(webui_tab_strip->GetVisible());
+
+  ToggleFullscreen();
+  EXPECT_FALSE(webui_tab_strip->GetVisible());
+
+  AttemptReveal();
+  EXPECT_FALSE(webui_tab_strip->GetVisible());
+
+  webui_tab_strip->SetVisibleForTesting(true);
+
+  // The WebUITabStrip should be layed out.
+  browser_view()->GetWidget()->LayoutRootViewIfNecessary();
+  EXPECT_TRUE(webui_tab_strip->GetVisible());
+  EXPECT_FALSE(webui_tab_strip->size().IsEmpty());
+}