[Read Anything] Set maximum side panel width during resizing.

Current side panel resizing rules:
1. The minimum browser window width is 500
   (kMainBrowserContentsMinimumWidth).
2. The minimum side panel width is 320.
3. The side panel can be widened up until it leaves 500 - 320 = 180
   in the main web contents.

Proposed side panel resizing rules:
1. The minimum window width remains at its current 500.
2. The minimum main web contents width remains at its current 180.
3. At 500px, the side panel is 320 and the main web contents is
   180px, as before the proposal.
4. Between window width = 500 to 820, the side panel remains at
   320px and the main contents increases in width.
5. At window width > 820, side panel can now be widened. It cannot
   go wider than a width that would keep the main contents at least
   at 500.

The motivation for this change is that the current behavior, which
allows side panel to be resized up to leaving the main web contents
at 180, even when the browser window size is very large:
https://screenshot.googleplex.com/9Fr9axFE4hfJWFE. For Read Anything
we would like to remove this possibility.

For more context and alternatives, see
https://docs.google.com/document/d/1qfuhKHWrveRc5UQ37KdFGMDvt2JypvyU_4h8ejNfips/edit#

Bug: 1329585, 1266555
Change-Id: I166760e168809db6400e12c5c359ef681c7879d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4000980
Reviewed-by: Peter Boström <pbos@chromium.org>
Commit-Queue: Abigail Klein <abigailbklein@google.com>
Cr-Commit-Position: refs/heads/main@{#1076212}

chrome/browser/ui/views/side_panel/side_panel.cc

@@ -213,9 +213,8 @@ bool SidePanel::IsRightAligned() {
 }
 
 gfx::Size SidePanel::GetMinimumSize() const {
-  const int min_side_panel_contents_width = 320;
   const int min_height = 0;
-  return gfx::Size(min_side_panel_contents_width + kBorderInsets.width(),
+  return gfx::Size(min_side_panel_contents_width_ + kBorderInsets.width(),
                    min_height);
 }
 
@@ -253,8 +252,21 @@ void SidePanel::OnResize(int resize_amount, bool done_resizing) {
     starting_width_on_resize_ = -1;
   }
   const int minimum_width = GetMinimumSize().width();
-  if (proposed_width < minimum_width) {
+  // The side panel may be resized up to leaving the main contents at
+  // kMainBrowserContentsMinimumWidth.
+  DCHECK_EQ(browser_view_->GetMinimumSize().width(),
+            BrowserViewLayout::kMainBrowserContentsMinimumWidth);
+  const int maximum_width = width() +
+                            browser_view_->contents_web_view()->width() -
+                            BrowserViewLayout::kMainBrowserContentsMinimumWidth;
+  // Side panel must stay at minimum width if either:
+  // a) The proposed width is less than the minimum.
+  // b) There is not enough room for the side panel to be wider than the
+  //    minimum.
+  if (proposed_width < minimum_width || maximum_width < minimum_width) {
     proposed_width = minimum_width;
+  } else if (proposed_width > maximum_width) {
+    proposed_width = maximum_width;
   }
   if (width() != proposed_width) {
     SetPanelWidth(proposed_width);

chrome/browser/ui/views/side_panel/side_panel.h

@@ -37,6 +37,9 @@ class SidePanel : public views::AccessiblePaneView,
   HorizontalAlignment GetHorizontalAlignment();
   bool IsRightAligned();
   gfx::Size GetMinimumSize() const override;
+  void SetMinimumSidePanelContentsWidthForTesting(int width) {
+    min_side_panel_contents_width_ = width;
+  }
 
   // views::ResizeAreaDelegate:
   void OnResize(int resize_amount, bool done_resizing) override;
@@ -72,6 +75,8 @@ class SidePanel : public views::AccessiblePaneView,
 
   // Observes and listens to side panel alignment changes.
   PrefChangeRegistrar pref_change_registrar_;
+
+  int min_side_panel_contents_width_ = 320;
 };
 
 #endif  // CHROME_BROWSER_UI_VIEWS_SIDE_PANEL_SIDE_PANEL_H_

chrome/browser/ui/views/side_panel/side_panel_coordinator_unittest.cc

@@ -119,6 +119,19 @@ class SidePanelCoordinatorTest : public TestWithBrowserView {
     return coordinator_->header_combobox_ != nullptr;
   }
 
+  void SetBrowserViewWidth(const int width) {
+    // If browser window is maximized then explicitly restore it, as otherwise
+    // the SetBounds call would be a no-op.
+    // TODO(crbug.com/1393153): Fix this on lacros builds and remove buildflag.
+#if !BUILDFLAG(IS_CHROMEOS_LACROS)
+    if (browser_view()->IsMaximized())
+      browser_view()->Restore();
+#endif
+
+    browser_view()->SetBounds(
+        gfx::Rect(width, browser_view()->GetBounds().height()));
+  }
+
  protected:
   raw_ptr<SidePanelCoordinator> coordinator_;
   raw_ptr<SidePanelRegistry> global_registry_;
@@ -142,8 +155,22 @@ TEST_F(SidePanelCoordinatorTest, ChangeSidePanelWidth) {
   // Set side panel to right-aligned
   browser_view()->GetProfile()->GetPrefs()->SetBoolean(
       prefs::kSidePanelHorizontalAlignment, true);
+
+  // The side panel can only be widened when the browser main web contents width
+  // is greater than |BrowserViewLayout::kMainBrowserContentsMinimumWidth|,
+  // which is 500. Therefore the side panel can only be widened when the total
+  // browser view width is greater than the minimum side panel contents width +
+  // 500. The browser view maximum width is constrained by the display maximum,
+  // which is 800 on win32 and ChromeOS.
+  // Therefore, we set the browser view width to be 800 and we reduce the side
+  // panel width to 100 so that it can be widened and shrunk.
+  SetBrowserViewWidth(800);
+  browser_view()
+      ->unified_side_panel()
+      ->SetMinimumSidePanelContentsWidthForTesting(100);
+
   coordinator_->Toggle();
-  const int starting_width = 500;
+  const int starting_width = 200;
   browser_view()->unified_side_panel()->SetPanelWidth(starting_width);
   views::test::RunScheduledLayout(browser_view());
   EXPECT_EQ(browser_view()->unified_side_panel()->width(), starting_width);
@@ -168,8 +195,21 @@ TEST_F(SidePanelCoordinatorTest, ChangeSidePanelWidth) {
 }
 
 TEST_F(SidePanelCoordinatorTest, ChangeSidePanelWidthMaxMin) {
+  // The side panel can only be widened when the browser main web contents width
+  // is greater than |BrowserViewLayout::kMainBrowserContentsMinimumWidth|,
+  // which is 500. Therefore the side panel can only be widened when the total
+  // browser view width is greater than the minimum side panel contents width +
+  // 500. The browser view maximum width is constrained by the display maximum,
+  // which is 800 on win32 and ChromeOS.
+  // Therefore, we set the browser view width to be 800 and we reduce the side
+  // panel width to 100 so that it can be widened and shrunk.
+  SetBrowserViewWidth(800);
+  browser_view()
+      ->unified_side_panel()
+      ->SetMinimumSidePanelContentsWidthForTesting(100);
+
   coordinator_->Toggle();
-  const int starting_width = 500;
+  const int starting_width = 200;
   browser_view()->unified_side_panel()->SetPanelWidth(starting_width);
   views::test::RunScheduledLayout(browser_view());
   EXPECT_EQ(browser_view()->unified_side_panel()->width(), starting_width);
@@ -184,22 +224,32 @@ TEST_F(SidePanelCoordinatorTest, ChangeSidePanelWidthMaxMin) {
 
   browser_view()->unified_side_panel()->OnResize(-large_increment, true);
   views::test::RunScheduledLayout(browser_view());
-  BrowserViewLayout* layout_manager =
-      static_cast<BrowserViewLayout*>(browser_view()->GetLayoutManager());
-  const int min_web_contents_width =
-      layout_manager->GetMinWebContentsWidthForTesting();
+  EXPECT_GT(browser_view()->unified_side_panel()->width(), starting_width);
   EXPECT_EQ(browser_view()->contents_web_view()->width(),
-            min_web_contents_width);
+            BrowserViewLayout::kMainBrowserContentsMinimumWidth);
 }
 
 TEST_F(SidePanelCoordinatorTest, ChangeSidePanelWidthRTL) {
+  // The side panel can only be widened when the browser main web contents width
+  // is greater than |BrowserViewLayout::kMainBrowserContentsMinimumWidth|,
+  // which is 500. Therefore the side panel can only be widened when the total
+  // browser view width is greater than the minimum side panel contents width +
+  // 500. The browser view maximum width is constrained by the display maximum,
+  // which is 800 on win32 and ChromeOS.
+  // Therefore, we set the browser view width to be 800 and we reduce the side
+  // panel width to 100 so that it can be widened and shrunk.
+  SetBrowserViewWidth(800);
+  browser_view()
+      ->unified_side_panel()
+      ->SetMinimumSidePanelContentsWidthForTesting(100);
+
   // Set side panel to right-aligned
   browser_view()->GetProfile()->GetPrefs()->SetBoolean(
       prefs::kSidePanelHorizontalAlignment, true);
   // Set UI direction to LTR
   base::i18n::SetRTLForTesting(false);
   coordinator_->Toggle();
-  const int starting_width = 500;
+  const int starting_width = 200;
   browser_view()->unified_side_panel()->SetPanelWidth(starting_width);
   views::test::RunScheduledLayout(browser_view());
   EXPECT_EQ(browser_view()->unified_side_panel()->width(), starting_width);
@@ -223,25 +273,31 @@ TEST_F(SidePanelCoordinatorTest, ChangeSidePanelWidthRTL) {
 }
 
 TEST_F(SidePanelCoordinatorTest, ChangeSidePanelWidthWindowResize) {
+  // The side panel can only be widened when the browser main web contents width
+  // is greater than |BrowserViewLayout::kMainBrowserContentsMinimumWidth|,
+  // which is 500. Therefore the side panel can only be widened when the total
+  // browser view width is greater than the minimum side panel contents width +
+  // 500. The browser view maximum width is constrained by the display maximum,
+  // which is 800 on win32 and ChromeOS.
+  // Therefore, we set the browser view width to be 800 and we reduce the side
+  // panel width to 100 so that it can be widened and shrunk.
+  SetBrowserViewWidth(800);
+  browser_view()
+      ->unified_side_panel()
+      ->SetMinimumSidePanelContentsWidthForTesting(100);
+
   coordinator_->Toggle();
-  const int starting_width = 500;
+  const int starting_width = 200;
   browser_view()->unified_side_panel()->SetPanelWidth(starting_width);
   views::test::RunScheduledLayout(browser_view());
   EXPECT_EQ(browser_view()->unified_side_panel()->width(), starting_width);
 
   // Shrink browser window enough that side panel should also shrink in
   // observance of web contents minimum width.
-  gfx::Rect original_bounds(browser_view()->GetBounds());
-  gfx::Size new_size(starting_width, starting_width);
-  gfx::Rect new_bounds(original_bounds);
-  new_bounds.set_size(new_size);
-  // Explicitly restore the browser window on ChromeOS, as it would otherwise
-  // be maximized and the SetBounds call would be a no-op.
-#if BUILDFLAG(IS_CHROMEOS_ASH)
-  browser_view()->Restore();
-#endif
-  browser_view()->SetBounds(new_bounds);
-  EXPECT_LT(browser_view()->unified_side_panel()->width(), starting_width);
+  const int original_width = browser_view()->GetBounds().width();
+  SetBrowserViewWidth(starting_width);
+  EXPECT_EQ(browser_view()->unified_side_panel()->width(),
+            browser_view()->unified_side_panel()->GetMinimumSize().width());
   BrowserViewLayout* layout_manager =
       static_cast<BrowserViewLayout*>(browser_view()->GetLayoutManager());
   const int min_web_contents_width =
@@ -251,10 +307,43 @@ TEST_F(SidePanelCoordinatorTest, ChangeSidePanelWidthWindowResize) {
 
   // Return browser window to original size, side panel should also return to
   // size prior to window resize.
-  browser_view()->SetBounds(original_bounds);
+  SetBrowserViewWidth(original_width);
   EXPECT_EQ(browser_view()->unified_side_panel()->width(), starting_width);
 }
 
+TEST_F(SidePanelCoordinatorTest, ChangeSidePanelWidthSmallWindow) {
+  SetBrowserViewWidth(500);
+  browser_view()
+      ->unified_side_panel()
+      ->SetMinimumSidePanelContentsWidthForTesting(100);
+  coordinator_->Toggle();
+  const int starting_width = 200;
+  const int min_side_panel_width =
+      browser_view()->unified_side_panel()->GetMinimumSize().width();
+  browser_view()->unified_side_panel()->SetPanelWidth(starting_width);
+  views::test::RunScheduledLayout(browser_view());
+  EXPECT_EQ(browser_view()->unified_side_panel()->width(),
+            min_side_panel_width);
+
+  // Attempt to resize the side panel, side panel should not be able to resized.
+  const int increment = 50;
+  browser_view()->unified_side_panel()->OnResize(increment, true);
+  views::test::RunScheduledLayout(browser_view());
+  EXPECT_EQ(browser_view()->unified_side_panel()->width(),
+            min_side_panel_width);
+
+  browser_view()->unified_side_panel()->OnResize(-increment, true);
+  views::test::RunScheduledLayout(browser_view());
+  EXPECT_EQ(browser_view()->unified_side_panel()->width(),
+            min_side_panel_width);
+  BrowserViewLayout* layout_manager =
+      static_cast<BrowserViewLayout*>(browser_view()->GetLayoutManager());
+  const int min_web_contents_width =
+      layout_manager->GetMinWebContentsWidthForTesting();
+  EXPECT_EQ(browser_view()->contents_web_view()->width(),
+            min_web_contents_width);
+}
+
 TEST_F(SidePanelCoordinatorTest, ChangeSidePanelAlignment) {
   browser_view()->GetProfile()->GetPrefs()->SetBoolean(
       prefs::kSidePanelHorizontalAlignment, true);