In tab dragging, the window size should be larger than its minimum size.

In tablet mode, when doing a tab dragging, the dragged window's size
is 1/4 of the work area's size. It worked well for big screens, but on
smaller screens, the size might be smaller than the window's minimum
size. We should take the minimum size into consideration as well.

Bug: 1002728
Change-Id: I6a834150015890f7e9eb949b8d724c064474a84f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1799693
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697759}

ash/wm/tablet_mode/tablet_mode_window_drag_delegate.cc

@@ -294,12 +294,14 @@ void TabletModeWindowDragDelegate::EndWindowDrag(
 }
 
 void TabletModeWindowDragDelegate::FlingOrSwipe(ui::GestureEvent* event) {
-  if (ShouldFlingIntoOverview(event)) {
-    DCHECK(Shell::Get()->overview_controller()->InOverviewSession());
-    Shell::Get()->overview_controller()->overview_session()->AddItem(
-        dragged_window_, /*reposition=*/true, /*animate=*/false);
+  if (event->type() == ui::ET_SCROLL_FLING_START) {
+    if (ShouldFlingIntoOverview(event)) {
+      DCHECK(Shell::Get()->overview_controller()->InOverviewSession());
+      Shell::Get()->overview_controller()->overview_session()->AddItem(
+          dragged_window_, /*reposition=*/true, /*animate=*/false);
+    }
+    StartFling(event);
   }
-  StartFling(event);
   EndWindowDrag(ToplevelWindowEventHandler::DragResult::SUCCESS,
                 GetEventLocationInScreen(event));
 }
@@ -453,9 +455,6 @@ bool TabletModeWindowDragDelegate::ShouldDropWindowIntoOverview(
 
 bool TabletModeWindowDragDelegate::ShouldFlingIntoOverview(
     const ui::GestureEvent* event) const {
-  if (event->type() != ui::ET_SCROLL_FLING_START)
-    return false;
-
   // Only fling into overview if overview is currently open. In some case,
   // overview is not opened when drag starts (if it's tab-dragging and the
   // dragged window is not the same with the source window), we should not fling

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

@@ -63,6 +63,8 @@ bool ConvertedHitTest(views::View* src, views::View* dst, gfx::Point* point) {
 
 }  // namespace
 
+constexpr int BrowserViewLayout::kMainBrowserContentsMinimumWidth;
+
 class BrowserViewLayout::WebContentsModalDialogHostViews
     : public WebContentsModalDialogHost {
  public:
@@ -169,13 +171,8 @@ gfx::Size BrowserViewLayout::GetMinimumSize(const views::View* host) const {
   // 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);
+  // The minimum height for the normal (tabbed) browser window's contents area.
+  constexpr int kMainBrowserContentsMinimumHeight = 1;
 
   const bool has_tabstrip =
       browser()->SupportsWindowFeature(Browser::FEATURE_TABSTRIP);
@@ -202,7 +199,8 @@ gfx::Size BrowserViewLayout::GetMinimumSize(const views::View* host) const {
 
   gfx::Size contents_size(contents_container_->GetMinimumSize());
   contents_size.SetToMax(browser()->is_type_normal()
-                             ? kMainBrowserContentsMinimumSize
+                             ? gfx::Size(kMainBrowserContentsMinimumWidth,
+                                         kMainBrowserContentsMinimumHeight)
                              : kContentsMinimumSize);
 
   const int min_height =

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

@@ -11,6 +11,7 @@
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "ui/gfx/geometry/rect.h"
+#include "ui/gfx/geometry/size.h"
 #include "ui/views/layout/layout_manager.h"
 
 class BookmarkBarView;
@@ -22,14 +23,13 @@ class TabStrip;
 
 namespace gfx {
 class Point;
-class Size;
-}
+}  // namespace gfx
 
 namespace views {
 class ClientView;
 class View;
 class Widget;
-}
+}  // namespace views
 
 namespace web_modal {
 class WebContentsModalDialogHost;
@@ -38,6 +38,15 @@ class WebContentsModalDialogHost;
 // The layout manager used in chrome browser.
 class BrowserViewLayout : public views::LayoutManager {
  public:
+  // The minimum width for the normal (tabbed) browser window's contents area.
+  // 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.
+  static constexpr int kMainBrowserContentsMinimumWidth = 500;
+
   // |browser_view| may be null in tests.
   BrowserViewLayout(std::unique_ptr<BrowserViewLayoutDelegate> delegate,
                     Browser* browser,
@@ -55,9 +64,7 @@ class BrowserViewLayout : public views::LayoutManager {
   ~BrowserViewLayout() override;
 
   // Sets or updates views that are not available when |this| is initialized.
-  void set_tab_strip(TabStrip* tab_strip) {
-    tab_strip_ = tab_strip;
-  }
+  void set_tab_strip(TabStrip* tab_strip) { tab_strip_ = tab_strip; }
   void set_bookmark_bar(BookmarkBarView* bookmark_bar) {
     bookmark_bar_ = bookmark_bar;
   }

chrome/browser/ui/views/tabs/tab_drag_controller.cc

@@ -50,8 +50,9 @@
 #if defined(OS_CHROMEOS)
 #include "ash/public/cpp/ash_features.h"
 #include "ash/public/cpp/tablet_mode.h"
-#include "ash/public/cpp/window_properties.h"               // nogncheck
-#include "ash/public/cpp/window_state_type.h"               // nogncheck
+#include "ash/public/cpp/window_properties.h"  // nogncheck
+#include "ash/public/cpp/window_state_type.h"  // nogncheck
+#include "ui/aura/window_delegate.h"
 #include "ui/wm/core/coordinate_conversion.h"
 #endif
 
@@ -102,13 +103,18 @@ bool IsSnapped(const TabDragContext* context) {
 }
 
 // In Chrome OS tablet mode, when dragging a tab/tabs around, the desired
-// browser bounds during dragging is one-fourth of the workspace bounds.
+// browser size during dragging is one-fourth of the workspace size or the
+// window's minimum size.
 gfx::Rect GetDraggedBrowserBoundsInTabletMode(aura::Window* window) {
   const gfx::Rect work_area =
       display::Screen::GetScreen()->GetDisplayNearestWindow(window).work_area();
+  gfx::Size mininum_size;
+  if (window->delegate())
+    mininum_size = window->delegate()->GetMinimumSize();
+
   gfx::Rect bounds(window->GetBoundsInScreen());
-  bounds.set_width(work_area.width() / 2);
-  bounds.set_height(work_area.height() / 2);
+  bounds.set_width(std::max(work_area.width() / 2, mininum_size.width()));
+  bounds.set_height(std::max(work_area.height() / 2, mininum_size.height()));
   return bounds;
 }
 

chrome/browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc

@@ -78,6 +78,7 @@
 #include "ash/wm/splitview/split_view_controller.h"
 #include "ash/wm/window_state.h"
 #include "base/test/simple_test_tick_clock.h"
+#include "chrome/browser/ui/views/frame/browser_view_layout.h"
 #include "chrome/browser/ui/views/frame/immersive_mode_controller.h"
 #include "chrome/browser/ui/views/frame/immersive_mode_controller_ash.h"
 #include "content/public/common/service_manager_connection.h"
@@ -87,6 +88,7 @@
 #include "ui/aura/client/screen_position_client.h"
 #include "ui/aura/window_event_dispatcher.h"
 #include "ui/display/manager/display_manager.h"
+#include "ui/display/test/display_manager_test_api.h"
 #include "ui/events/base_event_utils.h"
 #include "ui/events/gesture_detection/gesture_configuration.h"
 #endif
@@ -2495,6 +2497,56 @@ IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
   EXPECT_EQ(browser_list->size(), 1u);
 }
 
+namespace {
+
+void WindowSizeDuringDraggingTestStep2(
+    DetachToBrowserTabDragControllerTest* test,
+    TabStrip* tab_strip) {
+  // There should be two browser windows, including the newly created one for
+  // the dragged tab.
+  EXPECT_EQ(2u, test->browser_list->size());
+
+  // Get this new created window for the dragged tab.
+  Browser* new_browser = test->browser_list->get(1);
+  aura::Window* window = new_browser->window()->GetNativeWindow();
+  const gfx::Size work_area = display::Screen::GetScreen()
+                                  ->GetDisplayNearestWindow(window)
+                                  .work_area()
+                                  .size();
+  const gfx::Size minimum_size = window->delegate()->GetMinimumSize();
+  const gfx::Size window_size = window->bounds().size();
+  EXPECT_GE(minimum_size.width(),
+            BrowserViewLayout::kMainBrowserContentsMinimumWidth);
+  EXPECT_GE(window_size.width(), minimum_size.width());
+  EXPECT_GE(window_size.height(), minimum_size.height());
+  EXPECT_EQ(window_size,
+            gfx::Size(std::max(work_area.width() / 2, minimum_size.width()),
+                      std::max(work_area.height() / 2, minimum_size.height())));
+
+  ASSERT_TRUE(test->ReleaseInput());
+}
+
+}  // namespace
+
+// Tests that when drgging a tab out of a browser window, the dragged window's
+// size should be equal or larger than its minimum size.
+IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
+                       WindowSizeDuringDraggingTest) {
+  // Set the display size small enough.
+  display::test::DisplayManagerTestApi(ash::Shell::Get()->display_manager())
+      .UpdateDisplay(base::NumberToString(
+                         BrowserViewLayout::kMainBrowserContentsMinimumWidth) +
+                     "x400");
+  ash::ShellTestApi().SetTabletModeEnabledForTest(true);
+
+  TabStrip* tab_strip = GetTabStripForBrowser(browser());
+  AddTabsAndResetBrowser(browser(), 1);
+
+  // Drag it far enough that the first tab detaches.
+  DragTabAndNotify(tab_strip, base::BindOnce(&WindowSizeDuringDraggingTestStep2,
+                                             this, tab_strip));
+}
+
 // Subclass of DetachToBrowserTabDragControllerTest that
 // creates multiple displays.
 class DetachToBrowserInSeparateDisplayTabDragControllerTest