Reland "Change the ownership of ash::TabDragDropDelegate"

This is a reland of 1654d66

Same as original; The reverted CL was not the one causing the failure.
Details in: [1].

[1] https://bugs.chromium.org/p/chromium/issues/detail?id=1259127#c2

Original change's description:
> Change the ownership of ash::TabDragDropDelegate
>
> In the current implementation, TabDragDropDelegate runs all its methods
> synchronously and the deletion model is simple: object is deleted
> when it goes out of scope.
> However, to support Lacros' WebUI tab drop, TabDragDropDelegate::Drop()
> becomes naturally asynchronous, since its closure routine needs to
> be called by Lacros.
> Hence, the current ownership model does not work well for lacros,
> since one can not predict when the Drop() closure routine is called.
>
> This CL fixes this by changing the ownership model of
> ash::TabDragDropDelegate to self delete after the Drop()'s closure
> routine is called.
>
> This is a preparation email for fully support WebUI tab drop.
>
> BUG=1236708
> R=oshima@chromium.org
>
> Change-Id: I330f35f9697a27fc3a35454f059cb3377378a718
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3203250
> Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
> Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#930161}

Bug: 1236708
Change-Id: I4d09f225c7e9c8d76613ff693ac03152f2c7b27d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3220950
Auto-Submit: Antonio Gomes <tonikitoo@igalia.com>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/main@{#931022}

ash/drag_drop/drag_drop_controller.cc

@@ -720,7 +720,9 @@ void DragDropController::PerformDrop(
 
   if (operation_ == DragOperation::kNone && tab_drag_drop_delegate) {
     DCHECK(drag_image_widget_);
-    tab_drag_drop_delegate->Drop(drop_location_in_screen, copied_data);
+    // Release the ownership of object so that it can delete itself.
+    tab_drag_drop_delegate.release()->DropAndDeleteSelf(drop_location_in_screen,
+                                                        copied_data);
     // Override the drag event's drop effect as a move to inform the front-end
     // that the tab or group was moved. Otherwise, the WebUI tab strip does
     // not know that a drop resulted in a tab being moved and will temporarily

ash/drag_drop/tab_drag_drop_delegate.cc

@@ -130,12 +130,13 @@ void TabDragDropDelegate::DragUpdate(const gfx::Point& location_in_screen) {
   tab_dragging_recorder_->RequestNext();
 }
 
-void TabDragDropDelegate::Drop(const gfx::Point& location_in_screen,
-                               const ui::OSExchangeData& drop_data) {
+void TabDragDropDelegate::DropAndDeleteSelf(
+    const gfx::Point& location_in_screen,
+    const ui::OSExchangeData& drop_data) {
   tab_dragging_recorder_.reset();
 
   auto closure = base::BindOnce(&TabDragDropDelegate::OnNewBrowserWindowCreated,
-                                base::Unretained(this), location_in_screen);
+                                base::Owned(this), location_in_screen);
   NewWindowDelegate::GetPrimary()->NewWindowForWebUITabDrop(
       source_window_, drop_data, std::move(closure));
 }

ash/drag_drop/tab_drag_drop_delegate.h

@@ -56,8 +56,8 @@ class ASH_EXPORT TabDragDropDelegate {
 
   // Must be called on drop if it was not accepted by the drop target. After
   // calling this, this delegate must not be used.
-  void Drop(const gfx::Point& location_in_screen,
-            const ui::OSExchangeData& drop_data);
+  void DropAndDeleteSelf(const gfx::Point& location_in_screen,
+                         const ui::OSExchangeData& drop_data);
 
  private:
   // Scales or transforms the source window if appropriate for this drag.
@@ -70,8 +70,8 @@ class ASH_EXPORT TabDragDropDelegate {
   // Puts the source window back into its original position.
   void RestoreSourceWindowBounds();
 
-  // Effectively handles the new window creation in Drop(). This method can be
-  // called asynchronously in case of Lacros.
+  // Effectively handles the new window creation in DropAndDeleteSelf(). This
+  // method can be called asynchronously in case of Lacros.
   void OnNewBrowserWindowCreated(const gfx::Point& location_in_screen,
                                  aura::Window* new_window);
 

ash/drag_drop/tab_drag_drop_delegate_unittest.cc

@@ -159,11 +159,11 @@ TEST_F(TabDragDropDelegateTest, DragToNewWindow) {
 
   const gfx::Point drag_start_location = source_window->bounds().CenterPoint();
   // Emulate a drag session ending in a drop to a new window.
-  TabDragDropDelegate delegate(Shell::GetPrimaryRootWindow(),
-                               source_window.get(), drag_start_location);
-  delegate.DragUpdate(drag_start_location);
-  delegate.DragUpdate(drag_start_location + gfx::Vector2d(1, 0));
-  delegate.DragUpdate(drag_start_location + gfx::Vector2d(2, 0));
+  auto delegate = std::make_unique<TabDragDropDelegate>(
+      Shell::GetPrimaryRootWindow(), source_window.get(), drag_start_location);
+  delegate->DragUpdate(drag_start_location);
+  delegate->DragUpdate(drag_start_location + gfx::Vector2d(1, 0));
+  delegate->DragUpdate(drag_start_location + gfx::Vector2d(2, 0));
 
   // Check that a new window is requested. Assume the correct drop data
   // is passed. Return the new window.
@@ -173,8 +173,8 @@ TEST_F(TabDragDropDelegateTest, DragToNewWindow) {
       .Times(1)
       .WillOnce(RunOnceCallback<2>(new_window.get()));
 
-  delegate.Drop(drag_start_location + gfx::Vector2d(2, 0),
-                ui::OSExchangeData());
+  delegate.release()->DropAndDeleteSelf(
+      drag_start_location + gfx::Vector2d(2, 0), ui::OSExchangeData());
 
   EXPECT_FALSE(
       SplitViewController::Get(source_window.get())->InTabletSplitViewMode());
@@ -197,18 +197,19 @@ TEST_F(TabDragDropDelegateTest, DropOnEdgeEntersSplitView) {
           source_window.get())
           .right_center();
 
-  TabDragDropDelegate delegate(Shell::GetPrimaryRootWindow(),
-                               source_window.get(), drag_start_location);
-  delegate.DragUpdate(drag_start_location);
-  delegate.DragUpdate(drag_end_location);
+  auto delegate = std::make_unique<TabDragDropDelegate>(
+      Shell::GetPrimaryRootWindow(), source_window.get(), drag_start_location);
+  delegate->DragUpdate(drag_start_location);
+  delegate->DragUpdate(drag_end_location);
 
   new_window = CreateToplevelTestWindow();
   EXPECT_CALL(*mock_new_window_delegate(),
               NewWindowForWebUITabDrop(source_window.get(), _, _))
       .Times(1)
       .WillOnce(RunOnceCallback<2>(new_window.get()));
 
-  delegate.Drop(drag_end_location, ui::OSExchangeData());
+  delegate.release()->DropAndDeleteSelf(drag_end_location,
+                                        ui::OSExchangeData());
 
   SplitViewController* const split_view_controller =
       SplitViewController::Get(source_window.get());
@@ -236,16 +237,17 @@ TEST_F(TabDragDropDelegateTest, DropTabInSplitViewMode) {
   // Emulate a drag to the right side of the screen.
   // |new_window1| should snap to the right split view.
   gfx::Point drag_end_location_right(area.width() * 0.8, area.height() * 0.5);
-  TabDragDropDelegate delegate1(Shell::GetPrimaryRootWindow(),
-                                source_window.get(), drag_start_location);
-  delegate1.DragUpdate(drag_start_location);
-  delegate1.DragUpdate(drag_end_location_right);
+  auto delegate1 = std::make_unique<TabDragDropDelegate>(
+      Shell::GetPrimaryRootWindow(), source_window.get(), drag_start_location);
+  delegate1->DragUpdate(drag_start_location);
+  delegate1->DragUpdate(drag_end_location_right);
   std::unique_ptr<aura::Window> new_window1 = CreateToplevelTestWindow();
   EXPECT_CALL(*mock_new_window_delegate(),
               NewWindowForWebUITabDrop(source_window.get(), _, _))
       .Times(1)
       .WillOnce(RunOnceCallback<2>(new_window1.get()));
-  delegate1.Drop(drag_end_location_right, ui::OSExchangeData());
+  delegate1.release()->DropAndDeleteSelf(drag_end_location_right,
+                                         ui::OSExchangeData());
 
   EXPECT_TRUE(split_view_controller->InTabletSplitViewMode());
   EXPECT_EQ(new_window1.get(), split_view_controller->GetSnappedWindow(
@@ -258,16 +260,17 @@ TEST_F(TabDragDropDelegateTest, DropTabInSplitViewMode) {
   // |new_window2| should snap to the left split view.
   // |source_window| should go into overview mode.
   gfx::Point drag_end_location_left(area.width() * 0.2, area.height() * 0.5);
-  TabDragDropDelegate delegate2(Shell::GetPrimaryRootWindow(),
-                                source_window.get(), drag_start_location);
-  delegate2.DragUpdate(drag_start_location);
-  delegate2.DragUpdate(drag_end_location_left);
+  auto delegate2 = std::make_unique<TabDragDropDelegate>(
+      Shell::GetPrimaryRootWindow(), source_window.get(), drag_start_location);
+  delegate2->DragUpdate(drag_start_location);
+  delegate2->DragUpdate(drag_end_location_left);
   std::unique_ptr<aura::Window> new_window2 = CreateToplevelTestWindow();
   EXPECT_CALL(*mock_new_window_delegate(),
               NewWindowForWebUITabDrop(source_window.get(), _, _))
       .Times(1)
       .WillOnce(RunOnceCallback<2>(new_window2.get()));
-  delegate2.Drop(drag_end_location_left, ui::OSExchangeData());
+  delegate2.release()->DropAndDeleteSelf(drag_end_location_left,
+                                         ui::OSExchangeData());
 
   EXPECT_TRUE(split_view_controller->InTabletSplitViewMode());
   EXPECT_EQ(nullptr, split_view_controller->GetSnappedWindow(
@@ -375,9 +378,9 @@ TEST_F(TabDragDropDelegateTest, TabDraggingHistogram) {
 
   // Emulate a drag session ending in a drop to a new window. This should
   // generate a histogram.
-  TabDragDropDelegate delegate(Shell::GetPrimaryRootWindow(),
-                               source_window.get(), drag_start_location);
-  delegate.DragUpdate(drag_start_location + gfx::Vector2d(1, 0));
+  auto delegate = std::make_unique<TabDragDropDelegate>(
+      Shell::GetPrimaryRootWindow(), source_window.get(), drag_start_location);
+  delegate->DragUpdate(drag_start_location + gfx::Vector2d(1, 0));
   EXPECT_TRUE(ui::WaitForNextFrameToBePresented(
       source_window->layer()->GetCompositor()));
 
@@ -388,8 +391,8 @@ TEST_F(TabDragDropDelegateTest, TabDraggingHistogram) {
               NewWindowForWebUITabDrop(source_window.get(), _, _))
       .Times(1)
       .WillOnce(RunOnceCallback<2>(new_window.get()));
-  delegate.Drop(drag_start_location + gfx::Vector2d(1, 0),
-                ui::OSExchangeData());
+  delegate.release()->DropAndDeleteSelf(
+      drag_start_location + gfx::Vector2d(1, 0), ui::OSExchangeData());
   EXPECT_TRUE(ui::WaitForNextFrameToBePresented(
       source_window->layer()->GetCompositor()));