Set lower bound on contents pane size.

We have had several recent problems with the lower bound on browser
width being driven by the toolbar, which can have its minimum size
change as buttons are hidden or shown, or as elements in the omnibox are
hidden or shown or change size.

The change to the new FlexLayout for the toolbar has made the browser
much more responsive to the toolbar's minimum size, and has resulted in
an overall smaller minimum width for the browser window. This has
created additional problems, such as WebUI pages which have important
content or controls cut off due to not having enough room to render.

This patch kills two birds with one stone - it restores the minimum
width required for our WebUI pages, while also making the contents view
the long tent pole for browser minimum width rather than the toolbar,
which should also solve a number of minor regressions caused by the
switch to declarative layout for the toolbar that were the result of
its minimum size being able to change.

For this CL the value of 500 DIPs was chosen. This is not enough to
display chrome://settings without a horizontal scroll bar, but none of
the UI elements are completely hidden either, and it is also enough to
display the content in WebUI dialogs. The choice of 500 (rather than,
say, 550, which would have accommodated the entire width of WebUI) was
to maintain the tradeoff betwqeen usability and privacy; some users
like to browser certain content in a very small window even on large
screens to prevent others from seeing what they are doing.

Bug: 927708, 927846
Change-Id: Ie30e6dd31d26e01900b4f5ee0a1209164e151e59
Reviewed-on: https://chromium-review.googlesource.com/c/1450647
Commit-Queue: Dana Fried <dfried@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629491}
diff --git a/chrome/browser/ui/views/frame/browser_frame_ash_browsertest.cc b/chrome/browser/ui/views/frame/browser_frame_ash_browsertest.cc
index d337f67..b72a47c 100644
--- a/chrome/browser/ui/views/frame/browser_frame_ash_browsertest.cc
+++ b/chrome/browser/ui/views/frame/browser_frame_ash_browsertest.cc
@@ -77,7 +77,7 @@
                         "test_browser_app", true /* trusted_source */,
                         gfx::Rect(), browser()->profile(), true)
                   : Browser::CreateParams(browser()->profile(), true);
-  gfx::Rect original_bounds(gfx::Rect(150, 250, 400, 100));
+  gfx::Rect original_bounds(gfx::Rect(150, 250, 510, 150));
   params.initial_show_state = ui::SHOW_STATE_NORMAL;
   params.initial_bounds = original_bounds;
   Browser* browser = new Browser(params);
diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc
index 5f549cdf..6def11e 100644
--- a/chrome/browser/ui/views/frame/browser_view.cc
+++ b/chrome/browser/ui/views/frame/browser_view.cc
@@ -2554,7 +2554,6 @@
                             toolbar_,
                             infobar_container_,
                             contents_container_,
-                            GetContentsLayoutManager(),
                             immersive_mode_controller_.get());
   SetLayoutManager(std::move(browser_view_layout));
 
diff --git a/chrome/browser/ui/views/frame/browser_view_layout.cc b/chrome/browser/ui/views/frame/browser_view_layout.cc
index 3df7db89..9dfc067 100644
--- a/chrome/browser/ui/views/frame/browser_view_layout.cc
+++ b/chrome/browser/ui/views/frame/browser_view_layout.cc
@@ -133,7 +133,6 @@
       bookmark_bar_(nullptr),
       infobar_container_(nullptr),
       contents_container_(nullptr),
-      contents_layout_manager_(nullptr),
       download_shelf_(nullptr),
       immersive_mode_controller_(nullptr),
       dialog_host_(new WebContentsModalDialogHostViews(this)),
@@ -151,7 +150,6 @@
     views::View* toolbar,
     InfoBarContainerView* infobar_container,
     views::View* contents_container,
-    ContentsLayoutManager* contents_layout_manager,
     ImmersiveModeController* immersive_mode_controller) {
   delegate_.reset(delegate);
   browser_ = browser;
@@ -161,7 +159,6 @@
   toolbar_ = toolbar;
   infobar_container_ = infobar_container;
   contents_container_ = contents_container;
-  contents_layout_manager_ = contents_layout_manager;
   immersive_mode_controller_ = immersive_mode_controller;
 }
 
@@ -171,17 +168,38 @@
 }
 
 gfx::Size BrowserViewLayout::GetMinimumSize(const views::View* host) const {
-  gfx::Size tabstrip_size(
-      browser()->SupportsWindowFeature(Browser::FEATURE_TABSTRIP) ?
-      tab_strip_->GetMinimumSize() : gfx::Size());
-  gfx::Size toolbar_size(
-      (browser()->SupportsWindowFeature(Browser::FEATURE_TOOLBAR) ||
-       browser()->SupportsWindowFeature(Browser::FEATURE_LOCATIONBAR)) ?
-           toolbar_->GetMinimumSize() : gfx::Size());
+  // Prevent having a 0x0 sized-contents as this can allow the window to be
+  // resized down such that it's invisible and can no longer accept events.
+  // Use a very small 1x1 size to allow the user and the web contents to be able
+  // to resize the window as small as possible without introducing bugs.
+  // https://crbug.com/847179.
+  constexpr gfx::Size kContentsMinimumSize(1, 1);
+
+  // This should be wide enough that WebUI pages (e.g. chrome://settings) and
+  // the various associated WebUI dialogs (e.g. Import Bookmarks) can still be
+  // functional. This value provides a trade-off between browser usability and
+  // privacy - specifically, the ability to browse in a very small window, even
+  // on large monitors (which is why a minimum height is not specified). This
+  // value is used for the main browser window only, not for popups.
+  constexpr gfx::Size kMainBrowserContentsMinimumSize(500, 1);
+
+  const bool has_tabstrip =
+      browser()->SupportsWindowFeature(Browser::FEATURE_TABSTRIP);
+  const bool has_toolbar =
+      browser()->SupportsWindowFeature(Browser::FEATURE_TOOLBAR);
+  const bool has_location_bar =
+      browser()->SupportsWindowFeature(Browser::FEATURE_LOCATIONBAR);
+  const bool has_bookmarks_bar =
+      bookmark_bar_ && bookmark_bar_->visible() &&
+      browser()->SupportsWindowFeature(Browser::FEATURE_BOOKMARKBAR);
+
+  gfx::Size tabstrip_size(has_tabstrip ? tab_strip_->GetMinimumSize()
+                                       : gfx::Size());
+  gfx::Size toolbar_size((has_toolbar || has_location_bar)
+                             ? toolbar_->GetMinimumSize()
+                             : gfx::Size());
   gfx::Size bookmark_bar_size;
-  if (bookmark_bar_ &&
-      bookmark_bar_->visible() &&
-      browser()->SupportsWindowFeature(Browser::FEATURE_BOOKMARKBAR)) {
+  if (has_bookmarks_bar) {
     bookmark_bar_size = bookmark_bar_->GetMinimumSize();
     bookmark_bar_size.Enlarge(0, -bookmark_bar_->GetToolbarOverlap());
   }
@@ -189,24 +207,19 @@
   // TODO(pkotwicz): Adjust the minimum height for the find bar.
 
   gfx::Size contents_size(contents_container_->GetMinimumSize());
-  // Prevent having a 0x0 sized-contents as this can allow the window to be
-  // resized down such that it's invisible and can no longer accept events.
-  // Use a very small 1x1 size to allow the user and the web contents to be able
-  // to resize the window as small as possible without introducing bugs.
-  // https://crbug.com/847179.
-  contents_size.SetToMax(gfx::Size(1, 1));
+  contents_size.SetToMax(browser()->is_type_popup()
+                             ? kContentsMinimumSize
+                             : kMainBrowserContentsMinimumSize);
 
-  int min_height = delegate_->GetTopInsetInBrowserView() +
-      tabstrip_size.height() + toolbar_size.height() +
-      bookmark_bar_size.height() + infobar_container_size.height() +
-      contents_size.height();
-  int widths[] = {
-        tabstrip_size.width(),
-        toolbar_size.width(),
-        bookmark_bar_size.width(),
-        infobar_container_size.width(),
-        contents_size.width() };
-  int min_width = *std::max_element(&widths[0], &widths[base::size(widths)]);
+  const int min_height =
+      delegate_->GetTopInsetInBrowserView() + tabstrip_size.height() +
+      toolbar_size.height() + bookmark_bar_size.height() +
+      infobar_container_size.height() + contents_size.height();
+
+  const int min_width = std::max(
+      {tabstrip_size.width(), toolbar_size.width(), bookmark_bar_size.width(),
+       infobar_container_size.width(), contents_size.width()});
+
   return gfx::Size(min_width, min_height);
 }
 
diff --git a/chrome/browser/ui/views/frame/browser_view_layout.h b/chrome/browser/ui/views/frame/browser_view_layout.h
index 42792f7..5c81be2 100644
--- a/chrome/browser/ui/views/frame/browser_view_layout.h
+++ b/chrome/browser/ui/views/frame/browser_view_layout.h
@@ -16,7 +16,6 @@
 class BookmarkBarView;
 class Browser;
 class BrowserViewLayoutDelegate;
-class ContentsLayoutManager;
 class ImmersiveModeController;
 class InfoBarContainerView;
 class TabStrip;
@@ -51,7 +50,6 @@
             views::View* toolbar,
             InfoBarContainerView* infobar_container,
             views::View* contents_container,
-            ContentsLayoutManager* contents_layout_manager,
             ImmersiveModeController* immersive_mode_controller);
 
   // Sets or updates views that are not available when |this| is initialized.
@@ -137,7 +135,6 @@
   BookmarkBarView* bookmark_bar_;
   InfoBarContainerView* infobar_container_;
   views::View* contents_container_;
-  ContentsLayoutManager* contents_layout_manager_;
   views::View* download_shelf_;
 
   ImmersiveModeController* immersive_mode_controller_;
diff --git a/chrome/browser/ui/views/frame/browser_view_layout_unittest.cc b/chrome/browser/ui/views/frame/browser_view_layout_unittest.cc
index 1025477..40637a4 100644
--- a/chrome/browser/ui/views/frame/browser_view_layout_unittest.cc
+++ b/chrome/browser/ui/views/frame/browser_view_layout_unittest.cc
@@ -166,10 +166,9 @@
     contents_container_ = CreateFixedSizeView(gfx::Size(800, 600));
     contents_container_->AddChildView(devtools_web_view_);
     contents_container_->AddChildView(contents_web_view_);
-    ContentsLayoutManager* contents_layout_manager =
-        contents_container_->SetLayoutManager(
-            std::make_unique<ContentsLayoutManager>(devtools_web_view_,
-                                                    contents_web_view_));
+    contents_container_->SetLayoutManager(
+        std::make_unique<ContentsLayoutManager>(devtools_web_view_,
+                                                contents_web_view_));
 
     root_view_->AddChildView(contents_container_);
 
@@ -184,7 +183,6 @@
                   toolbar_,
                   infobar_container_,
                   contents_container_,
-                  contents_layout_manager,
                   immersive_mode_controller_.get());
   }
 
diff --git a/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc b/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
index 3d7653e..b7b8eab0 100644
--- a/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
+++ b/chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc
@@ -7,7 +7,10 @@
 #include <stddef.h>
 
 #include <algorithm>
+#include <limits>
 #include <memory>
+#include <set>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ -55,7 +58,6 @@
 
 #if defined(USE_AURA)
 #include "ui/aura/client/aura_constants.h"
-#include "ui/aura/env.h"
 #include "ui/aura/test/test_window_delegate.h"
 #include "ui/aura/test/test_windows.h"
 #include "ui/aura/window_targeter.h"
@@ -2123,7 +2125,7 @@
     DetachToBrowserTabDragControllerTest::SetUpCommandLine(command_line);
     // Make screens sufficiently wide to host 2 browsers side by side.
     command_line->AppendSwitchASCII("ash-host-window-bounds",
-                                    "0+0-600x600,600+0-600x600");
+                                    "0+0-800x600,800+0-800x600");
   }
 
  private:
@@ -2158,8 +2160,14 @@
 
   // Move to the first tab and drag it enough so that it detaches.
   // Then drag it to the final destination on the second screen.
-  const gfx::Point target = GetCenterInScreenCoordinates(tab_strip->tab_at(0)) +
-                            gfx::Vector2d(600, GetDetachY(tab_strip));
+  display::Screen* const screen = display::Screen::GetScreen();
+  display::Display second_display = ui_test_utils::GetSecondaryDisplay(screen);
+  const gfx::Point start = GetCenterInScreenCoordinates(tab_strip->tab_at(0));
+  ASSERT_FALSE(second_display.bounds().Contains(start));
+  const gfx::Point target(second_display.bounds().x() + 1,
+                          start.y() + GetDetachY(tab_strip));
+  ASSERT_TRUE(second_display.bounds().Contains(target));
+
   DragTabAndNotify(
       tab_strip,
       base::BindOnce(&DragSingleTabToSeparateWindowInSecondDisplayStep2, this,
@@ -2180,7 +2188,6 @@
   // With the touch input the browser cannot be dragged from one screen
   // to another and the window stays on the first screen.
   if (input_source() == INPUT_SOURCE_MOUSE) {
-    display::Screen* screen = display::Screen::GetScreen();
     EXPECT_EQ(
         ui_test_utils::GetSecondaryDisplay(screen).id(),
         screen
@@ -2351,7 +2358,11 @@
   gfx::Rect work_area = second_display.work_area();
   work_area.set_width(work_area.width() / 2);
   browser()->window()->SetBounds(work_area);
-  work_area.set_x(work_area.right());
+  // It's possible the window will not fit in half the screen, in which case we
+  // will position the windows as well as we can.
+  work_area.set_x(browser()->window()->GetBounds().right());
+  // Sanity check: second browser should still be on the second display.
+  ASSERT_LT(work_area.x(), second_display.work_area().right());
   browser2->window()->SetBounds(work_area);
   // Wait for the display changes. See the earlier comments for the details.
   aura::test::WaitForAllChangesToComplete();
@@ -2364,6 +2375,11 @@
       screen->GetDisplayNearestWindow(browser2->window()->GetNativeWindow())
           .id());
 
+  // Sanity check: make sure the target position is also within in the screen
+  // bounds:
+  ASSERT_LT(GetCenterInScreenCoordinates(tab_strip2->tab_at(0)).x(),
+            second_display.work_area().right());
+
   // Move to the first tab and drag it enough so that it detaches, but not
   // enough that it attaches to browser2.
   DragTabAndNotify(tab_strip,
@@ -2543,7 +2559,7 @@
   void SetUpCommandLine(base::CommandLine* command_line) override {
     DetachToBrowserTabDragControllerTest::SetUpCommandLine(command_line);
     command_line->AppendSwitchASCII("ash-host-window-bounds",
-                                    "400x400,400+0-800x800*2");
+                                    "800x600,800+0-800x600*2");
   }
 
   float GetCursorDeviceScaleFactor() const {
@@ -2635,7 +2651,7 @@
   void SetUpCommandLine(base::CommandLine* command_line) override {
     DetachToBrowserTabDragControllerTest::SetUpCommandLine(command_line);
     command_line->AppendSwitchASCII("ash-host-window-bounds",
-                                    "0+0-250x250,250+0-250x250");
+                                    "0+0-800x600,800+0-800x600");
   }
 
  private: