Notify drag start/end from WorkspaceWindowResizer

They were inconsistently sent from different places,
which led to this bug because DragResizer isn't used in multi window resizer.
They should be sent from single soruce.

BUG=b/72402303
TEST=Covered by unit tests. Also tested manually. See bug for repro step.

Change-Id: If9e24b7b62d879a83607ee99634c2670645c7bb6
Reviewed-on: https://chromium-review.googlesource.com/885081
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Dominik Laskowski <domlaskowski@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532263}

ash/wm/drag_window_resizer.cc

@@ -62,16 +62,15 @@ void DragWindowResizer::Drag(const gfx::Point& location, int event_flags) {
 }
 
 void DragWindowResizer::CompleteDrag() {
-  gfx::Point last_mouse_location_in_screen = last_mouse_location_;
-  ::wm::ConvertPointToScreen(GetTarget()->parent(),
-                             &last_mouse_location_in_screen);
-  window_state_->OnCompleteDrag(last_mouse_location_in_screen);
   next_window_resizer_->CompleteDrag();
 
   GetTarget()->layer()->SetOpacity(details().initial_opacity);
   drag_window_controller_.reset();
 
   // Check if the destination is another display.
+  gfx::Point last_mouse_location_in_screen = last_mouse_location_;
+  ::wm::ConvertPointToScreen(GetTarget()->parent(),
+                             &last_mouse_location_in_screen);
   display::Screen* screen = display::Screen::GetScreen();
   const display::Display dst_display =
       screen->GetDisplayNearestPoint(last_mouse_location_in_screen);
@@ -109,10 +108,6 @@ void DragWindowResizer::CompleteDrag() {
 }
 
 void DragWindowResizer::RevertDrag() {
-  gfx::Point last_mouse_location_in_screen = last_mouse_location_;
-  ::wm::ConvertPointToScreen(GetTarget()->parent(),
-                             &last_mouse_location_in_screen);
-  window_state_->OnRevertDrag(last_mouse_location_in_screen);
   next_window_resizer_->RevertDrag();
 
   drag_window_controller_.reset();

ash/wm/window_state.cc

@@ -435,13 +435,21 @@ void WindowState::set_bounds_changed_by_user(bool bounds_changed_by_user) {
   }
 }
 
+void WindowState::OnDragStarted(int window_component) {
+  DCHECK(drag_details_);
+  if (delegate_)
+    delegate_->OnDragStarted(window_component);
+}
+
 void WindowState::OnCompleteDrag(const gfx::Point& location) {
-  if (drag_details_ && delegate_)
+  DCHECK(drag_details_);
+  if (delegate_)
     delegate_->OnDragFinished(/*canceled=*/false, location);
 }
 
 void WindowState::OnRevertDrag(const gfx::Point& location) {
-  if (drag_details_ && delegate_)
+  DCHECK(drag_details_);
+  if (delegate_)
     delegate_->OnDragFinished(/*canceled=*/true, location);
 }
 
@@ -450,8 +458,6 @@ void WindowState::CreateDragDetails(const gfx::Point& point_in_parent,
                                     ::wm::WindowMoveSource source) {
   drag_details_ = std::make_unique<DragDetails>(window_, point_in_parent,
                                                 window_component, source);
-  if (drag_details_ && delegate_)
-    delegate_->OnDragStarted(window_component);
 }
 
 void WindowState::DeleteDragDetails() {

ash/wm/window_state.h

@@ -322,6 +322,9 @@ class ASH_EXPORT WindowState : public aura::WindowObserver {
   // Sets the currently stored restore bounds and clears the restore bounds.
   void SetAndClearRestoreBounds();
 
+  // Notifies that the drag operation has been started.
+  void OnDragStarted(int window_component);
+
   // Notifies that the drag operation has been either completed or reverted.
   // |location| is the last position of the pointer device used to drag.
   void OnCompleteDrag(const gfx::Point& location);

ash/wm/workspace/multi_window_resize_controller_unittest.cc

@@ -10,6 +10,7 @@
 #include "ash/shell_test_api.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/wm/window_state.h"
+#include "ash/wm/window_state_delegate.h"
 #include "ash/wm/window_util.h"
 #include "ash/wm/wm_event.h"
 #include "ash/wm/workspace/workspace_event_handler_test_helper.h"
@@ -350,6 +351,39 @@ TEST_F(MultiWindowResizeControllerTest, ClickOutside) {
   EXPECT_FALSE(IsShowing());
 }
 
+namespace {
+
+class TestWindowStateDelegate : public wm::WindowStateDelegate {
+ public:
+  TestWindowStateDelegate() = default;
+  ~TestWindowStateDelegate() override = default;
+
+  // wm::WindowStateDelegate:
+  void OnDragStarted(int component) override { component_ = component; }
+  void OnDragFinished(bool cancel, const gfx::Point& location) override {
+    location_ = location;
+  }
+
+  int GetComponentAndReset() {
+    int result = component_;
+    component_ = -1;
+    return result;
+  }
+
+  gfx::Point GetLocationAndReset() {
+    gfx::Point p = location_;
+    location_.SetPoint(0, 0);
+    return p;
+  }
+
+ private:
+  gfx::Point location_;
+  int component_ = -1;
+  DISALLOW_COPY_AND_ASSIGN(TestWindowStateDelegate);
+};
+
+}  // namespace
+
 // Tests dragging to resize two snapped windows.
 TEST_F(MultiWindowResizeControllerTest, TwoSnappedWindows) {
   UpdateDisplay("400x300");
@@ -382,6 +416,10 @@ TEST_F(MultiWindowResizeControllerTest, TwoSnappedWindows) {
   EXPECT_FALSE(HasPendingShow());
   EXPECT_TRUE(IsShowing());
 
+  // Setup delegates
+  auto* window_state_delegate1 = new TestWindowStateDelegate();
+  w1_state->SetDelegate(base::WrapUnique(window_state_delegate1));
+
   // Move the mouse over the resize widget.
   ASSERT_TRUE(resize_widget());
   gfx::Rect bounds(resize_widget()->GetWindowBoundsInScreen());
@@ -403,6 +441,11 @@ TEST_F(MultiWindowResizeControllerTest, TwoSnappedWindows) {
   EXPECT_EQ(gfx::Rect(300, 0, 100, 252), w2->bounds());
   EXPECT_EQ(0.75f, *w1_state->snapped_width_ratio());
   EXPECT_EQ(0.25f, *w2_state->snapped_width_ratio());
+
+  // Dragging should call the WindowStateDelegate.
+  EXPECT_EQ(HTRIGHT, window_state_delegate1->GetComponentAndReset());
+  EXPECT_EQ(gfx::Point(300, 173),
+            window_state_delegate1->GetLocationAndReset());
 }
 
 }  // namespace ash

ash/wm/workspace/workspace_window_resizer.cc

@@ -382,6 +382,11 @@ void WorkspaceWindowResizer::Drag(const gfx::Point& location_in_parent,
 }
 
 void WorkspaceWindowResizer::CompleteDrag() {
+  gfx::Point last_mouse_location_in_screen = last_mouse_location_;
+  ::wm::ConvertPointToScreen(GetTarget()->parent(),
+                             &last_mouse_location_in_screen);
+  window_state()->OnCompleteDrag(last_mouse_location_in_screen);
+
   if (!did_move_or_resize_)
     return;
 
@@ -440,6 +445,10 @@ void WorkspaceWindowResizer::CompleteDrag() {
 }
 
 void WorkspaceWindowResizer::RevertDrag() {
+  gfx::Point last_mouse_location_in_screen = last_mouse_location_;
+  ::wm::ConvertPointToScreen(GetTarget()->parent(),
+                             &last_mouse_location_in_screen);
+  window_state()->OnRevertDrag(last_mouse_location_in_screen);
   window_state()->set_bounds_changed_by_user(initial_bounds_changed_by_user_);
   snap_phantom_window_controller_.reset();
 
@@ -520,6 +529,8 @@ WorkspaceWindowResizer::WorkspaceWindowResizer(
     total_available += std::max(min_size, initial_size) - min_size;
   }
   instance = this;
+
+  window_state->OnDragStarted(details().window_component);
 }
 
 void WorkspaceWindowResizer::LayoutAttachedWindows(gfx::Rect* bounds) {