Chrome OS tablet mode: Allow window dragging for non-resizable windows

CreateWindowResizer was unintentionally aborting early, before calling
CreateWindowResizerForTabletMode. Thus it applied the resizable check
to windows that shouldn't have gotten it, such as non-resizable ARC
windows in tablet mode.

Bug: 947769
Change-Id: If0863936c8bdb43513fd4b3d8ba39d597275d604
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1565355
Commit-Queue: Evan Stade <estade@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#650524}

ash/wm/wm_toplevel_window_event_handler_unittest.cc

@@ -30,7 +30,6 @@ class WmToplevelWindowEventHandlerTest : public AshTestBase {
 
   void SetUp() override {
     AshTestBase::SetUp();
-    TabletModeControllerTestApi().EnterTabletMode();
 
     dragged_window_ = CreateTestWindow();
     non_dragged_window_ = CreateTestWindow();
@@ -68,7 +67,7 @@ class WmToplevelWindowEventHandlerTest : public AshTestBase {
 // the overview mode.
 TEST_F(WmToplevelWindowEventHandlerTest,
        TapWindowInOverviewGridDuringWindowDrag) {
-  ASSERT_TRUE(TabletModeControllerTestApi().IsTabletModeStarted());
+  TabletModeControllerTestApi().EnterTabletMode();
 
   SendGestureEvent(gfx::Point(0, 0), 0, 5, ui::ET_GESTURE_SCROLL_BEGIN);
   // Drag the window to the right corner to avoid overlap with
@@ -96,8 +95,48 @@ TEST_F(WmToplevelWindowEventHandlerTest,
   EXPECT_FALSE(overview_controller->overview_session());
 }
 
+// In tablet mode, the window's resizability shouldn't be taken into account
+// when dragging from the top. Regression test for https://crbug.com/1444132
+TEST_F(WmToplevelWindowEventHandlerTest,
+       NonResizableWindowsCanBeDraggedInTabletMode) {
+  TabletModeControllerTestApi().EnterTabletMode();
+
+  dragged_window_->SetProperty(aura::client::kResizeBehaviorKey,
+                               ws::mojom::kResizeBehaviorNone);
+
+  SendGestureEvent(gfx::Point(0, 0), 0, 5, ui::ET_GESTURE_SCROLL_BEGIN);
+  SendGestureEvent(gfx::Point(700, 500), 700, 500,
+                   ui::ET_GESTURE_SCROLL_UPDATE);
+  EXPECT_TRUE(wm::GetWindowState(dragged_window_.get())->is_dragged());
+
+  OverviewController* overview_controller = Shell::Get()->overview_controller();
+  EXPECT_TRUE(overview_controller->IsSelecting());
+  EXPECT_TRUE(overview_controller->overview_session()->IsWindowInOverview(
+      non_dragged_window_.get()));
+}
+
+// Contrary to tablet mode, in non-tablet mode, non resizable windows cannot be
+// dragged.
+TEST_F(WmToplevelWindowEventHandlerTest,
+       NonResizableWindowsCannotBeDraggedInClamshellMode) {
+  ASSERT_FALSE(TabletModeControllerTestApi().IsTabletModeStarted());
+
+  dragged_window_->SetProperty(aura::client::kResizeBehaviorKey,
+                               ws::mojom::kResizeBehaviorNone);
+
+  SendGestureEvent(gfx::Point(0, 0), 0, 5, ui::ET_GESTURE_SCROLL_BEGIN);
+  SendGestureEvent(gfx::Point(700, 500), 700, 500,
+                   ui::ET_GESTURE_SCROLL_UPDATE);
+  EXPECT_FALSE(wm::GetWindowState(dragged_window_.get())->is_dragged());
+
+  OverviewController* overview_controller = Shell::Get()->overview_controller();
+  EXPECT_FALSE(overview_controller->IsSelecting());
+}
+
 // Tests that the window drag will be reverted if the screen is being rotated.
 TEST_F(WmToplevelWindowEventHandlerTest, DisplayConfigurationChangeTest) {
+  TabletModeControllerTestApi().EnterTabletMode();
+
   int64_t display_id = display::Screen::GetScreen()->GetPrimaryDisplay().id();
   display::DisplayManager* display_manager = Shell::Get()->display_manager();
   display::test::ScopedSetInternalDisplayId set_internal(display_manager,

ash/wm/workspace/workspace_window_resizer.cc

@@ -97,9 +97,8 @@ std::unique_ptr<WindowResizer> CreateWindowResizerForTabletMode(
     std::unique_ptr<WindowResizer> window_resizer =
         std::make_unique<TabletModeWindowDragController>(
             window_state, std::make_unique<TabletModeAppWindowDragDelegate>());
-    window_resizer = std::make_unique<DragWindowResizer>(
-        std::move(window_resizer), window_state);
-    return window_resizer;
+    return std::make_unique<DragWindowResizer>(std::move(window_resizer),
+                                               window_state);
   }
 
   // Only allow drag that happens on caption or top area. Note: for a maxmized
@@ -125,9 +124,8 @@ std::unique_ptr<WindowResizer> CreateWindowResizerForTabletMode(
       std::make_unique<TabletModeWindowDragController>(
           window_state,
           std::make_unique<TabletModeBrowserWindowDragDelegate>());
-  window_resizer = std::make_unique<DragWindowResizer>(
-      std::move(window_resizer), window_state);
-  return window_resizer;
+  return std::make_unique<DragWindowResizer>(std::move(window_resizer),
+                                             window_state);
 }
 
 }  // namespace
@@ -140,28 +138,14 @@ std::unique_ptr<WindowResizer> CreateWindowResizer(
   DCHECK(window);
 
   wm::WindowState* window_state = wm::GetWindowState(window);
-  // No need to return a resizer when the window cannot get resized or when a
-  // resizer already exists for this window.
-  if ((!window_state->CanResize() && window_component != HTCAPTION) ||
-      window_state->drag_details()) {
-    return nullptr;
-  }
 
-  // TODO(varkha): The chaining of window resizers causes some of the logic
-  // to be repeated and the logic flow difficult to control. With some windows
-  // classes using reparenting during drag operations it becomes challenging to
-  // implement proper transition from one resizer to another during or at the
-  // end of the drag. This also causes http://crbug.com/247085.
-  // We should have a better way of doing this, perhaps by having a way of
-  // observing drags or having a generic drag window wrapper which informs a
-  // layout manager that a drag has started or stopped. It may be possible to
-  // refactor and eliminate chaining.
-  std::unique_ptr<WindowResizer> window_resizer;
+  // A resizer already exists; don't create a new one.
+  if (window_state->drag_details())
+    return nullptr;
 
   if (window_state->IsPip()) {
     window_state->CreateDragDetails(point_in_parent, window_component, source);
-    window_resizer = std::make_unique<PipWindowResizer>(window_state);
-    return window_resizer;
+    return std::make_unique<PipWindowResizer>(window_state);
   }
 
   if (Shell::Get()
@@ -171,6 +155,10 @@ std::unique_ptr<WindowResizer> CreateWindowResizer(
                                             window_component, source);
   }
 
+  // No need to return a resizer when the window cannot get resized.
+  if (!window_state->CanResize() && window_component != HTCAPTION)
+    return nullptr;
+
   if (!window_state->IsNormalOrSnapped())
     return nullptr;
 
@@ -182,6 +170,17 @@ std::unique_ptr<WindowResizer> CreateWindowResizer(
   window_state->CreateDragDetails(point_in_parent, window_component, source);
   const int parent_shell_window_id =
       window->parent() ? window->parent()->id() : -1;
+
+  // TODO(varkha): The chaining of window resizers causes some of the logic
+  // to be repeated and the logic flow difficult to control. With some windows
+  // classes using reparenting during drag operations it becomes challenging to
+  // implement proper transition from one resizer to another during or at the
+  // end of the drag. This also causes http://crbug.com/247085.
+  // We should have a better way of doing this, perhaps by having a way of
+  // observing drags or having a generic drag window wrapper which informs a
+  // layout manager that a drag has started or stopped. It may be possible to
+  // refactor and eliminate chaining.
+  std::unique_ptr<WindowResizer> window_resizer;
   if (window->parent() &&
       // TODO(afakhry): Maybe use switchable containers?
       (desks_util::IsDeskContainerId(parent_shell_window_id) ||
@@ -191,9 +190,8 @@ std::unique_ptr<WindowResizer> CreateWindowResizer(
   } else {
     window_resizer.reset(DefaultWindowResizer::Create(window_state));
   }
-  window_resizer = std::make_unique<DragWindowResizer>(
-      std::move(window_resizer), window_state);
-  return window_resizer;
+  return std::make_unique<DragWindowResizer>(std::move(window_resizer),
+                                             window_state);
 }
 
 namespace {