Tab dragging: Merge the dragged tabs back to the source window conditionally.

After the drag ends, the dragged tab window might need to merge back into the
source window if 1) the dragged window or the source window is not added to
overview and 2) the dragged window has dragged farther than half of the screen
height and 3) the dragged window is not in snap preview area and 4) the dragged
window is not dragged to other side of the splitscreen if source window is
showing in splitscreen.

Bug: 887739
Change-Id: I21bab4ef5fbfc4c9c4294a6028798be3793b8b5c
Reviewed-on: https://chromium-review.googlesource.com/c/1259468
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#597002}

ash/wm/splitview/split_view_controller_unittest.cc

@@ -2894,6 +2894,120 @@ TEST_F(SplitViewTabDraggingTest, OverviewEndedOnWindowDrag) {
   EXPECT_TRUE(window1_layer > window2_layer);
 }
 
+// When tab dragging a window, the dragged window might need to merge back into
+// the source window when the drag ends. Tests the related functionalities.
+TEST_F(SplitViewTabDraggingTest, MergeBackToSourceWindow) {
+  UpdateDisplay("600x600");
+  const gfx::Rect bounds(0, 0, 400, 400);
+  std::unique_ptr<aura::Window> dragged_window(
+      CreateWindowWithType(bounds, AppType::BROWSER));
+  std::unique_ptr<aura::Window> source_window(
+      CreateWindowWithType(bounds, AppType::BROWSER));
+
+  // 1. If splitview is not active and the dragged window is not the source
+  // window.
+  // a. Drag the window to less than half of the display height, and not in the
+  // snap preview area.
+  std::unique_ptr<WindowResizer> resizer =
+      StartDrag(dragged_window.get(), source_window.get());
+  ASSERT_TRUE(resizer.get());
+  EXPECT_FALSE(
+      source_window->GetProperty(ash::kIsDeferredTabDraggingTargetWindowKey));
+  DragWindowTo(resizer.get(), gfx::Point(300, 200));
+  CompleteDrag(std::move(resizer));
+  EXPECT_TRUE(
+      source_window->GetProperty(ash::kIsDeferredTabDraggingTargetWindowKey));
+  source_window->ClearProperty(ash::kIsDeferredTabDraggingTargetWindowKey);
+
+  // b. Drag the window to more than half of the display height and not in the
+  // snap preview area.
+  resizer = StartDrag(dragged_window.get(), source_window.get());
+  ASSERT_TRUE(resizer.get());
+  EXPECT_FALSE(
+      source_window->GetProperty(ash::kIsDeferredTabDraggingTargetWindowKey));
+  DragWindowTo(resizer.get(), gfx::Point(300, 500));
+  CompleteDrag(std::move(resizer));
+  EXPECT_FALSE(
+      source_window->GetProperty(ash::kIsDeferredTabDraggingTargetWindowKey));
+
+  // c. Drag the window to the snap preview area.
+  resizer = StartDrag(dragged_window.get(), source_window.get());
+  ASSERT_TRUE(resizer.get());
+  EXPECT_FALSE(
+      source_window->GetProperty(ash::kIsDeferredTabDraggingTargetWindowKey));
+  DragWindowTo(resizer.get(), gfx::Point(0, 200));
+  CompleteDrag(std::move(resizer));
+  EXPECT_FALSE(
+      source_window->GetProperty(ash::kIsDeferredTabDraggingTargetWindowKey));
+  EndSplitView();
+
+  // d. The dragged window is already added into overview before drag ends:
+  resizer = StartDrag(dragged_window.get(), source_window.get());
+  ASSERT_TRUE(resizer.get());
+  EXPECT_FALSE(
+      source_window->GetProperty(ash::kIsDeferredTabDraggingTargetWindowKey));
+  DragWindowTo(resizer.get(), gfx::Point(300, 200));
+  dragged_window->SetProperty(ash::kIsShowingInOverviewKey, true);
+  CompleteDrag(std::move(resizer));
+  EXPECT_FALSE(
+      source_window->GetProperty(ash::kIsDeferredTabDraggingTargetWindowKey));
+  dragged_window->ClearProperty(ash::kIsShowingInOverviewKey);
+
+  // 2. If splitview is active and the dragged window is not the source window.
+  // a. Drag the window to less than half of the display height, in the same
+  // split of the source window, and not in the snap preview area.
+  split_view_controller()->SnapWindow(source_window.get(),
+                                      SplitViewController::LEFT);
+  resizer = StartDrag(dragged_window.get(), source_window.get());
+  EXPECT_FALSE(
+      source_window->GetProperty(ash::kIsDeferredTabDraggingTargetWindowKey));
+  DragWindowTo(resizer.get(), gfx::Point(0, 200));
+  CompleteDrag(std::move(resizer));
+  EXPECT_TRUE(
+      source_window->GetProperty(ash::kIsDeferredTabDraggingTargetWindowKey));
+  EndSplitView();
+  source_window->ClearProperty(ash::kIsDeferredTabDraggingTargetWindowKey);
+
+  // b. Drag the window to less than half of the display height, in the
+  // different split of the source window, and not in the snap preview area.
+  split_view_controller()->SnapWindow(source_window.get(),
+                                      SplitViewController::LEFT);
+  resizer = StartDrag(dragged_window.get(), source_window.get());
+  EXPECT_FALSE(
+      source_window->GetProperty(ash::kIsDeferredTabDraggingTargetWindowKey));
+  DragWindowTo(resizer.get(), gfx::Point(500, 200));
+  CompleteDrag(std::move(resizer));
+  EXPECT_FALSE(
+      source_window->GetProperty(ash::kIsDeferredTabDraggingTargetWindowKey));
+  EndSplitView();
+
+  // c. Drag the window to move a small distance, but is still in the different
+  // split of the source window, and not in the snap preview area.
+  split_view_controller()->SnapWindow(source_window.get(),
+                                      SplitViewController::LEFT);
+  resizer = StartDrag(dragged_window.get(), source_window.get());
+  EXPECT_FALSE(
+      source_window->GetProperty(ash::kIsDeferredTabDraggingTargetWindowKey));
+  DragWindowTo(resizer.get(), gfx::Point(500, 20));
+  CompleteDrag(std::move(resizer));
+  EXPECT_FALSE(
+      source_window->GetProperty(ash::kIsDeferredTabDraggingTargetWindowKey));
+  EndSplitView();
+
+  // d. The dragged window was added to overview before the drag ends.
+  split_view_controller()->SnapWindow(source_window.get(),
+                                      SplitViewController::LEFT);
+  resizer = StartDrag(dragged_window.get(), source_window.get());
+  EXPECT_FALSE(
+      source_window->GetProperty(ash::kIsDeferredTabDraggingTargetWindowKey));
+  DragWindowTo(resizer.get(), gfx::Point(0, 200));
+  dragged_window->SetProperty(ash::kIsShowingInOverviewKey, true);
+  CompleteDrag(std::move(resizer));
+  EXPECT_FALSE(
+      source_window->GetProperty(ash::kIsDeferredTabDraggingTargetWindowKey));
+  dragged_window->ClearProperty(ash::kIsShowingInOverviewKey);
+}
+
 class TestWindowDelegateWithWidget : public views::WidgetDelegate {
  public:
   TestWindowDelegateWithWidget(bool can_activate)

ash/wm/tablet_mode/tablet_mode_app_window_drag_controller.cc

@@ -35,17 +35,17 @@ class TabletModeAppWindowDragDelegate : public TabletModeWindowDragDelegate {
 
  private:
   // TabletModeWindowDragDelegate:
-  void PrepareForDraggedWindow(const gfx::Point& location_in_screen) override {
+  void PrepareWindowDrag(const gfx::Point& location_in_screen) override {
     wm::GetWindowState(dragged_window_)
         ->CreateDragDetails(location_in_screen, HTCLIENT,
                             ::wm::WINDOW_MOVE_SOURCE_TOUCH);
   }
-  void UpdateForDraggedWindow(const gfx::Point& location_in_screen) override {}
-  void EndingForDraggedWindow(
-      wm::WmToplevelWindowEventHandler::DragResult result,
-      const gfx::Point& location_in_screen) override {
+  void UpdateWindowDrag(const gfx::Point& location_in_screen) override {}
+  void EndingWindowDrag(wm::WmToplevelWindowEventHandler::DragResult result,
+                        const gfx::Point& location_in_screen) override {
     wm::GetWindowState(dragged_window_)->DeleteDragDetails();
   }
+  void EndedWindowDrag(const gfx::Point& location_in_screen) override {}
 
   DISALLOW_COPY_AND_ASSIGN(TabletModeAppWindowDragDelegate);
 };

ash/wm/tablet_mode/tablet_mode_browser_window_drag_delegate.cc

@@ -17,6 +17,7 @@
 #include "ash/wm/splitview/split_view_constants.h"
 #include "ash/wm/tablet_mode/tablet_mode_browser_window_drag_controller.h"
 #include "ash/wm/tablet_mode/tablet_mode_window_state.h"
+#include "ash/wm/window_util.h"
 #include "ui/aura/window.h"
 #include "ui/compositor/layer_animation_observer.h"
 #include "ui/compositor/scoped_layer_animation_settings.h"
@@ -34,14 +35,6 @@ namespace {
 // threshold.
 constexpr float kSourceWindowScale = 0.85;
 
-// Returns the window selector if overview mode is active, otherwise returns
-// nullptr.
-WindowSelector* GetWindowSelector() {
-  return Shell::Get()->window_selector_controller()->IsSelecting()
-             ? Shell::Get()->window_selector_controller()->window_selector()
-             : nullptr;
-}
-
 // The class to observe the source window's bounds change animation. It's used
 // to prevent the dragged window to merge back into the source window during
 // dragging. Only when the source window restores to its maximized window size,
@@ -247,7 +240,7 @@ TabletModeBrowserWindowDragDelegate::TabletModeBrowserWindowDragDelegate() =
 TabletModeBrowserWindowDragDelegate::~TabletModeBrowserWindowDragDelegate() =
     default;
 
-void TabletModeBrowserWindowDragDelegate::PrepareForDraggedWindow(
+void TabletModeBrowserWindowDragDelegate::PrepareWindowDrag(
     const gfx::Point& location_in_screen) {
   DCHECK(dragged_window_);
 
@@ -255,7 +248,7 @@ void TabletModeBrowserWindowDragDelegate::PrepareForDraggedWindow(
   window_state->OnDragStarted(window_state->drag_details()->window_component);
 }
 
-void TabletModeBrowserWindowDragDelegate::UpdateForDraggedWindow(
+void TabletModeBrowserWindowDragDelegate::UpdateWindowDrag(
     const gfx::Point& location_in_screen) {
   DCHECK(dragged_window_);
 
@@ -263,7 +256,7 @@ void TabletModeBrowserWindowDragDelegate::UpdateForDraggedWindow(
   UpdateSourceWindow(location_in_screen);
 }
 
-void TabletModeBrowserWindowDragDelegate::EndingForDraggedWindow(
+void TabletModeBrowserWindowDragDelegate::EndingWindowDrag(
     wm::WmToplevelWindowEventHandler::DragResult result,
     const gfx::Point& location_in_screen) {
   if (result == wm::WmToplevelWindowEventHandler::DragResult::SUCCESS)
@@ -282,6 +275,11 @@ void TabletModeBrowserWindowDragDelegate::EndingForDraggedWindow(
   windows_hider_.reset();
 }
 
+void TabletModeBrowserWindowDragDelegate::EndedWindowDrag(
+    const gfx::Point& location_in_screen) {
+  MergeBackToSourceWindowIfApplicable(location_in_screen);
+}
+
 bool TabletModeBrowserWindowDragDelegate::ShouldOpenOverviewWhenDragStarts() {
   DCHECK(dragged_window_);
   aura::Window* source_window =
@@ -299,8 +297,7 @@ void TabletModeBrowserWindowDragDelegate::UpdateSourceWindow(
   if (!source_window || source_window == dragged_window_ ||
       source_window == split_view_controller_->left_window() ||
       source_window == split_view_controller_->right_window() ||
-      (GetWindowSelector() &&
-       GetWindowSelector()->IsWindowInOverview(source_window))) {
+      source_window->GetProperty(ash::kIsShowingInOverviewKey)) {
     return;
   }
 
@@ -347,4 +344,55 @@ void TabletModeBrowserWindowDragDelegate::UpdateSourceWindow(
   }
 }
 
+void TabletModeBrowserWindowDragDelegate::MergeBackToSourceWindowIfApplicable(
+    const gfx::Point& location_in_screen) {
+  // No need to merge back if we're not in tab dragging process.
+  if (!wm::IsDraggingTabs(dragged_window_))
+    return;
+
+  aura::Window* source_window =
+      dragged_window_->GetProperty(ash::kTabDraggingSourceWindowKey);
+  // Do not merge back if there is no source window or the source window or
+  // the dragged window is currently in overview.
+  if (!source_window ||
+      source_window->GetProperty(ash::kIsShowingInOverviewKey) ||
+      dragged_window_->GetProperty(ash::kIsShowingInOverviewKey)) {
+    return;
+  }
+
+  // Do not merge back if the window has dragged farther than half of the screen
+  // height.
+  const gfx::Rect work_area_bounds =
+      display::Screen::GetScreen()
+          ->GetDisplayNearestWindow(dragged_window_)
+          .work_area();
+  if (location_in_screen.y() >= work_area_bounds.CenterPoint().y())
+    return;
+
+  SplitViewController::SnapPosition desired_snap_position =
+      GetSnapPosition(location_in_screen);
+  // If splitscreen is not active, do not merge back if the dragged window is
+  // in the drag-to-snap preview area.
+  if (!split_view_controller_->IsSplitViewModeActive() &&
+      desired_snap_position != SplitViewController::NONE) {
+    return;
+  }
+
+  // If source window is currently showing in splitscreen, do not merge back if
+  // the dragged window has been dragged to the other side of the split.
+  if (split_view_controller_->IsSplitViewModeActive() &&
+      wm::GetWindowState(source_window)->IsSnapped()) {
+    if ((source_window == split_view_controller_->left_window() &&
+         desired_snap_position == SplitViewController::RIGHT) ||
+        (source_window == split_view_controller_->right_window() &&
+         desired_snap_position == SplitViewController::LEFT)) {
+      return;
+    }
+  }
+
+  // Arriving here we know the dragged window should merge back into its source
+  // window.
+  source_window->SetProperty(ash::kIsDeferredTabDraggingTargetWindowKey, true);
+}
+
 }  // namespace ash

ash/wm/tablet_mode/tablet_mode_browser_window_drag_delegate.h

@@ -24,11 +24,11 @@ class TabletModeBrowserWindowDragDelegate
   class WindowsHider;
 
   // TabletModeWindowDragDelegate:
-  void PrepareForDraggedWindow(const gfx::Point& location_in_screen) override;
-  void UpdateForDraggedWindow(const gfx::Point& location_in_screen) override;
-  void EndingForDraggedWindow(
-      wm::WmToplevelWindowEventHandler::DragResult result,
-      const gfx::Point& location_in_screen) override;
+  void PrepareWindowDrag(const gfx::Point& location_in_screen) override;
+  void UpdateWindowDrag(const gfx::Point& location_in_screen) override;
+  void EndingWindowDrag(wm::WmToplevelWindowEventHandler::DragResult result,
+                        const gfx::Point& location_in_screen) override;
+  void EndedWindowDrag(const gfx::Point& location_in_screen) override;
   bool ShouldOpenOverviewWhenDragStarts() override;
 
   // Scales down the source window if the dragged window is dragged past the
@@ -37,6 +37,14 @@ class TabletModeBrowserWindowDragDelegate
   // current drag location for the dragged window.
   void UpdateSourceWindow(const gfx::Point& location_in_screen);
 
+  // After drag ends, the dragged window might need to merge back into the
+  // source window if 1) the dragged window or the source window is not added to
+  // overview and 2) the dragged window has dragged farther than half of the
+  // screen height and 3) the dragged window is not in snap preview area and 4)
+  // the dragged window is not dragged to the other side of the split screen.
+  void MergeBackToSourceWindowIfApplicable(
+      const gfx::Point& location_in_screen);
+
   // It's used to hide all visible windows if the source window needs to be
   // scaled up/down during dragging a tab out of the source window. It also
   // hides the home launcher if home launcher is enabled, blurs and darkens the

ash/wm/tablet_mode/tablet_mode_window_drag_delegate.cc

@@ -78,7 +78,7 @@ void TabletModeWindowDragDelegate::StartWindowDrag(
   dragged_window_ = dragged_window;
   initial_location_in_screen_ = location_in_screen;
 
-  PrepareForDraggedWindow(location_in_screen);
+  PrepareWindowDrag(location_in_screen);
 
   // Update the shelf's visibility to keep shelf visible during drag.
   RootWindowController::ForWindow(dragged_window_)
@@ -151,7 +151,7 @@ void TabletModeWindowDragDelegate::ContinueWindowDrag(
   }
 
   // For child classes to do their special handling if any.
-  UpdateForDraggedWindow(location_in_screen);
+  UpdateWindowDrag(location_in_screen);
 
   // Update drag indicators and preview window if necessary.
   IndicatorState indicator_state = GetIndicatorState(location_in_screen);
@@ -185,7 +185,7 @@ bool TabletModeWindowDragDelegate::ShouldDropWindowIntoOverviewOnDragPosition(
 void TabletModeWindowDragDelegate::EndWindowDrag(
     wm::WmToplevelWindowEventHandler::DragResult result,
     const gfx::Point& location_in_screen) {
-  EndingForDraggedWindow(result, location_in_screen);
+  EndingWindowDrag(result, location_in_screen);
 
   dragged_window_->SetProperty(kBackdropWindowMode, original_backdrop_mode_);
   SplitViewController::SnapPosition snap_position = SplitViewController::NONE;
@@ -210,6 +210,8 @@ void TabletModeWindowDragDelegate::EndWindowDrag(
   // Reset the dragged window's window shadow elevation.
   ::wm::SetShadowElevation(dragged_window_, original_shadow_elevation_);
 
+  // For child class to do its special handling if any.
+  EndedWindowDrag(location_in_screen);
   dragged_window_ = nullptr;
   did_move_ = false;
 }
@@ -262,26 +264,14 @@ int TabletModeWindowDragDelegate::GetIndicatorsVerticalThreshold(
 
 SplitViewController::SnapPosition TabletModeWindowDragDelegate::GetSnapPosition(
     const gfx::Point& location_in_screen) const {
-  gfx::Rect work_area_bounds = display::Screen::GetScreen()
-                                   ->GetDisplayNearestWindow(dragged_window_)
-                                   .work_area();
-
-  // The user has to drag pass the indicator vertical threshold to snap the
-  // window.
-  if (!did_move_ && location_in_screen.y() <
-                        GetIndicatorsVerticalThreshold(work_area_bounds)) {
-    return SplitViewController::NONE;
-  }
-
-  const bool is_landscape =
-      split_view_controller_->IsCurrentScreenOrientationLandscape();
-  const bool is_primary =
-      split_view_controller_->IsCurrentScreenOrientationPrimary();
-
   // If split view mode is active during dragging, the dragged window will be
   // either snapped left or right (if it's not merged into overview window),
   // depending on the relative position of |location_in_screen| and the current
   // divider position.
+  const bool is_landscape =
+      split_view_controller_->IsCurrentScreenOrientationLandscape();
+  const bool is_primary =
+      split_view_controller_->IsCurrentScreenOrientationPrimary();
   if (split_view_controller_->IsSplitViewModeActive()) {
     const int position =
         is_landscape ? location_in_screen.x() : location_in_screen.y();
@@ -294,8 +284,18 @@ SplitViewController::SnapPosition TabletModeWindowDragDelegate::GetSnapPosition(
     }
   }
 
-  // Otherwise, check to see if the current event location |location_in_screen|
-  // is within the drag indicators bounds.
+  // Otherwise, the user has to drag pass the indicator vertical threshold to
+  // snap the window.
+  gfx::Rect work_area_bounds = display::Screen::GetScreen()
+                                   ->GetDisplayNearestWindow(dragged_window_)
+                                   .work_area();
+  if (!did_move_ && location_in_screen.y() <
+                        GetIndicatorsVerticalThreshold(work_area_bounds)) {
+    return SplitViewController::NONE;
+  }
+
+  // Check to see if the current event location |location_in_screen|is within
+  // the drag indicators bounds.
   if (is_landscape) {
     const int screen_edge_inset =
         work_area_bounds.width() * kHighlightScreenPrimaryAxisRatio +

ash/wm/tablet_mode/tablet_mode_window_drag_delegate.h

@@ -64,14 +64,14 @@ class TabletModeWindowDragDelegate {
   }
 
  protected:
-  // These three methods are used by its child class to do its special handling
+  // These four methods are used by its child class to do its special handling
   // before/during/after dragging.
-  virtual void PrepareForDraggedWindow(
-      const gfx::Point& location_in_screen) = 0;
-  virtual void UpdateForDraggedWindow(const gfx::Point& location_in_screen) = 0;
-  virtual void EndingForDraggedWindow(
+  virtual void PrepareWindowDrag(const gfx::Point& location_in_screen) = 0;
+  virtual void UpdateWindowDrag(const gfx::Point& location_in_screen) = 0;
+  virtual void EndingWindowDrag(
       wm::WmToplevelWindowEventHandler::DragResult result,
       const gfx::Point& location_in_screen) = 0;
+  virtual void EndedWindowDrag(const gfx::Point& location_in_screen) = 0;
 
   // Returns true if we should open overview behind the dragged window when drag
   // starts.

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

@@ -568,6 +568,18 @@ void TabDragController::EndDrag(EndDragReason reason) {
   // dragged tabs to it first.
   if (reason == END_DRAG_COMPLETE && deferred_target_tabstrip_observer_)
     PerformDeferredAttach();
+
+  // It's also possible that we need to merge the dragged tabs back into the
+  // source window even if the dragged tabs is dragged away from the source
+  // window.
+  // TODO(xdai/mukai): Move ClearTabDraggingInfo() to a later point and let
+  // RevertDrag() handle this case.
+  if (source_tabstrip_ &&
+      GetWindowForTabDraggingProperties(source_tabstrip_)
+          ->GetProperty(ash::kIsDeferredTabDraggingTargetWindowKey)) {
+    SetDeferredTargetTabstrip(source_tabstrip_);
+    PerformDeferredAttach();
+  }
 #endif
 
   EndDragImpl(reason != END_DRAG_COMPLETE && source_tabstrip_ ?