Reland "Change DragDropController::StartDragAndDrop return type"

This is a reland of f062e10

Original change's description:
> Change DragDropController::StartDragAndDrop return type
>
> This CL changes the return type of
> ash::DragDropController::StartDragAndDrop from integer to DragOperation,
> to improve type safety.
>
> Bug: 1093536
> Change-Id: I4099dd80cf07a07e481358a96ee6a12c0060e5c5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2764362
> Commit-Queue: Henrique Ferreiro <hferreiro@igalia.com>
> Reviewed-by: Nick Yamane <nickdiego@igalia.com>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Reviewed-by: Mitsuru Oshima (Slow - Gardening) <oshima@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#871936}

Bug: 1093536
Change-Id: I58789f4a32a81dfbe29e1b059b45a03f2c36fa61
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2825429
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Nick Yamane <nickdiego@igalia.com>
Reviewed-by: Mitsuru Oshima (Slow - Gardening) <oshima@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Commit-Queue: Henrique Ferreiro <hferreiro@igalia.com>
Cr-Commit-Position: refs/heads/master@{#873425}

ash/drag_drop/drag_drop_controller.cc

@@ -26,6 +26,7 @@
 #include "ui/base/data_transfer_policy/data_transfer_endpoint.h"
 #include "ui/base/data_transfer_policy/data_transfer_policy_controller.h"
 #include "ui/base/dragdrop/drag_drop_types.h"
+#include "ui/base/dragdrop/mojom/drag_drop_types.mojom.h"
 #include "ui/base/dragdrop/os_exchange_data.h"
 #include "ui/base/dragdrop/os_exchange_data_provider.h"
 #include "ui/base/hit_test.h"
@@ -43,6 +44,8 @@
 namespace ash {
 namespace {
 
+using ::ui::mojom::DragOperation;
+
 // The duration of the drag cancel animation in millisecond.
 constexpr base::TimeDelta kCancelAnimationDuration =
     base::TimeDelta::FromMilliseconds(250);
@@ -158,22 +161,23 @@ DragDropController::~DragDropController() {
   drag_image_widget_.reset();
 }
 
-int DragDropController::StartDragAndDrop(
+DragOperation DragDropController::StartDragAndDrop(
     std::unique_ptr<ui::OSExchangeData> data,
     aura::Window* root_window,
     aura::Window* source_window,
     const gfx::Point& screen_location,
-    int operation,
+    int allowed_operations,
     ui::mojom::DragEventSource source) {
   if (!enabled_ || IsDragDropInProgress())
-    return 0;
+    return DragOperation::kNone;
 
   const ui::OSExchangeDataProvider* provider = &data->provider();
   // We do not support touch drag/drop without a drag image.
   if (source == ui::mojom::DragEventSource::kTouch &&
       provider->GetDragImage().size().IsEmpty())
-    return 0;
+    return DragOperation::kNone;
 
+  operation_ = DragOperation::kNone;
   current_drag_event_source_ = source;
   DragDropTracker* tracker =
       new DragDropTracker(root_window, drag_drop_window_delegate_.get());
@@ -197,7 +201,7 @@ int DragDropController::StartDragAndDrop(
   pending_long_tap_.reset();
 
   drag_data_ = std::move(data);
-  drag_operation_ = operation;
+  allowed_operations_ = allowed_operations;
   current_drag_info_ = aura::client::DragUpdateInfo();
 
   start_location_ = screen_location;
@@ -242,7 +246,7 @@ int DragDropController::StartDragAndDrop(
     drag_source_window_ = nullptr;
   }
 
-  return drag_operation_;
+  return operation_;
 }
 
 void DragDropController::SetDragImage(const gfx::ImageSkia& image,
@@ -446,7 +450,7 @@ gfx::LinearAnimation* DragDropController::CreateCancelAnimation(
 void DragDropController::DragUpdate(aura::Window* target,
                                     const ui::LocatedEvent& event) {
   ui::DropTargetEvent e(*drag_data_.get(), event.location_f(),
-                        event.root_location_f(), drag_operation_);
+                        event.root_location_f(), allowed_operations_);
   e.set_flags(event.flags());
   ui::Event::DispatcherApi(&e).set_target(target);
 
@@ -542,24 +546,23 @@ void DragDropController::Drop(aura::Window* target,
       aura::client::GetDragDropDelegate(target);
   if (delegate) {
     ui::DropTargetEvent e(*drag_data_.get(), event.location_f(),
-                          event.root_location_f(), drag_operation_);
+                          event.root_location_f(), allowed_operations_);
     e.set_flags(event.flags());
     ui::Event::DispatcherApi(&e).set_target(target);
 
     ui::OSExchangeData copied_data(drag_data_->provider().Clone());
-    drag_operation_ =
-        static_cast<int>(delegate->OnPerformDrop(e, std::move(drag_data_)));
-    if (drag_operation_ == 0 && tab_drag_drop_delegate_) {
+    operation_ = delegate->OnPerformDrop(e, std::move(drag_data_));
+    if (operation_ == DragOperation::kNone && tab_drag_drop_delegate_) {
       gfx::Point location_in_screen = event.root_location();
       ::wm::ConvertPointToScreen(target->GetRootWindow(), &location_in_screen);
       tab_drag_drop_delegate_->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
       // visually return the tab to its original position. (crbug.com/1081905)
-      drag_operation_ = ui::DragDropTypes::DragOperation::DRAG_MOVE;
+      operation_ = DragOperation::kMove;
       StartCanceledAnimation(kCancelAnimationDuration);
-    } else if (drag_operation_ == 0) {
+    } else if (operation_ == DragOperation::kNone) {
       StartCanceledAnimation(kCancelAnimationDuration);
     } else {
       drag_image_widget_.reset();
@@ -568,10 +571,8 @@ void DragDropController::Drop(aura::Window* target,
     drag_image_widget_.reset();
   }
 
-  if (toplevel_window_drag_delegate_) {
-    drag_operation_ =
-        toplevel_window_drag_delegate_->OnToplevelWindowDragDropped();
-  }
+  if (toplevel_window_drag_delegate_)
+    operation_ = toplevel_window_drag_delegate_->OnToplevelWindowDragDropped();
 
   Cleanup();
   if (should_block_during_drag_drop_)
@@ -618,7 +619,6 @@ void DragDropController::DoDragCancel(
     toplevel_window_drag_delegate_->OnToplevelWindowDragCancelled();
 
   Cleanup();
-  drag_operation_ = 0;
   StartCanceledAnimation(drag_cancel_animation_duration);
   if (should_block_during_drag_drop_)
     std::move(quit_closure_).Run();
@@ -678,7 +678,7 @@ void DragDropController::Cleanup() {
     drag_window_->RemoveObserver(this);
   drag_window_ = nullptr;
   drag_data_.reset();
-
+  allowed_operations_ = 0;
   tab_drag_drop_delegate_.reset();
 
   // Cleanup can be called again while deleting DragDropTracker, so delete

ash/drag_drop/drag_drop_controller.h

@@ -60,12 +60,13 @@ class ASH_EXPORT DragDropController : public aura::client::DragDropClient,
   }
 
   // Overridden from aura::client::DragDropClient:
-  int StartDragAndDrop(std::unique_ptr<ui::OSExchangeData> data,
-                       aura::Window* root_window,
-                       aura::Window* source_window,
-                       const gfx::Point& screen_location,
-                       int operation,
-                       ui::mojom::DragEventSource source) override;
+  ui::mojom::DragOperation StartDragAndDrop(
+      std::unique_ptr<ui::OSExchangeData> data,
+      aura::Window* root_window,
+      aura::Window* source_window,
+      const gfx::Point& screen_location,
+      int allowed_operations,
+      ui::mojom::DragEventSource source) override;
   void DragCancel() override;
   bool IsDragDropInProgress() override;
   void AddObserver(aura::client::DragDropClientObserver* observer) override;
@@ -124,7 +125,8 @@ class ASH_EXPORT DragDropController : public aura::client::DragDropClient,
   views::UniqueWidgetPtr drag_image_widget_;
   gfx::Vector2d drag_image_offset_;
   std::unique_ptr<ui::OSExchangeData> drag_data_;
-  int drag_operation_ = 0;
+  int allowed_operations_ = 0;
+  ui::mojom::DragOperation operation_ = ui::mojom::DragOperation::kNone;
   aura::client::DragUpdateInfo current_drag_info_;
 
   // Used when processing a Chrome tab drag from a WebUI tab strip.

ash/drag_drop/drag_drop_controller_unittest.cc

@@ -166,17 +166,17 @@ class TestDragDropController : public DragDropController {
     drag_string_.clear();
   }
 
-  int StartDragAndDrop(std::unique_ptr<ui::OSExchangeData> data,
-                       aura::Window* root_window,
-                       aura::Window* source_window,
-                       const gfx::Point& location,
-                       int operation,
-                       ui::mojom::DragEventSource source) override {
+  DragOperation StartDragAndDrop(std::unique_ptr<ui::OSExchangeData> data,
+                                 aura::Window* root_window,
+                                 aura::Window* source_window,
+                                 const gfx::Point& location,
+                                 int allowed_operations,
+                                 ui::mojom::DragEventSource source) override {
     drag_start_received_ = true;
     data->GetString(&drag_string_);
     return DragDropController::StartDragAndDrop(std::move(data), root_window,
                                                 source_window, location,
-                                                operation, source);
+                                                allowed_operations, source);
   }
 
   void DragUpdate(aura::Window* target,
@@ -367,10 +367,10 @@ class TestToplevelWindowDragDelegate : public ToplevelWindowDragDelegate {
     source_ = source;
   }
 
-  int OnToplevelWindowDragDropped() override {
+  DragOperation OnToplevelWindowDragDropped() override {
     EXPECT_EQ(State::kDragStartedInvoked, state_);
     state_ = State::kDragDroppedInvoked;
-    return ui::DragDropTypes::DRAG_MOVE;
+    return DragOperation::kMove;
   }
 
   void OnToplevelWindowDragCancelled() override {
@@ -1421,12 +1421,12 @@ TEST_F(DragDropControllerTest, DragTabChangesDragOperationToMove) {
                                 base::Unretained(&generator)));
 
   drag_drop_controller_->set_should_block_during_drag_drop(true);
-  int operation = drag_drop_controller_->StartDragAndDrop(
+  DragOperation operation = drag_drop_controller_->StartDragAndDrop(
       std::make_unique<ui::OSExchangeData>(), window->GetRootWindow(), window,
       gfx::Point(5, 5), ui::DragDropTypes::DRAG_NONE,
       ui::mojom::DragEventSource::kMouse);
 
-  EXPECT_EQ(operation, ui::DragDropTypes::DRAG_MOVE);
+  EXPECT_EQ(operation, DragOperation::kMove);
 }
 
 TEST_F(DragDropControllerTest, ToplevelWindowDragDelegate) {

ash/drag_drop/toplevel_window_drag_delegate.h

@@ -5,7 +5,7 @@
 #ifndef ASH_DRAG_DROP_TOPLEVEL_WINDOW_DRAG_DELEGATE_H_
 #define ASH_DRAG_DROP_TOPLEVEL_WINDOW_DRAG_DELEGATE_H_
 
-#include "ui/base/dragdrop/mojom/drag_drop_types.mojom-shared.h"
+#include "ui/base/dragdrop/mojom/drag_drop_types.mojom-forward.h"
 
 namespace gfx {
 class PointF;
@@ -25,7 +25,7 @@ class ToplevelWindowDragDelegate {
       const gfx::PointF& start_location,
       ui::mojom::DragEventSource source) = 0;
 
-  virtual int OnToplevelWindowDragDropped() = 0;
+  virtual ui::mojom::DragOperation OnToplevelWindowDragDropped() = 0;
 
   virtual void OnToplevelWindowDragCancelled() = 0;
 

chrome/browser/ui/views/drag_and_drop_interactive_uitest.cc

@@ -55,15 +55,17 @@
 #include "ui/aura/window.h"
 #include "ui/base/dragdrop/drag_drop_types.h"
 #include "ui/base/dragdrop/drop_target_event.h"
+#include "ui/base/dragdrop/mojom/drag_drop_types.mojom.h"
 #include "ui/base/dragdrop/os_exchange_data.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/rect.h"
 #include "url/gurl.h"
 
 namespace chrome {
-
 namespace {
 
+using ::ui::mojom::DragOperation;
+
 // TODO(lukasza): Support testing on non-Aura platforms (i.e. Android + Mac?).
 //
 // Notes for the TODO above:
@@ -274,12 +276,12 @@ class DragStartWaiter : public aura::client::DragDropClient {
   }
 
   // aura::client::DragDropClient overrides:
-  int StartDragAndDrop(std::unique_ptr<ui::OSExchangeData> data,
-                       aura::Window* root_window,
-                       aura::Window* source_window,
-                       const gfx::Point& screen_location,
-                       int operation,
-                       ui::mojom::DragEventSource source) override {
+  DragOperation StartDragAndDrop(std::unique_ptr<ui::OSExchangeData> data,
+                                 aura::Window* root_window,
+                                 aura::Window* source_window,
+                                 const gfx::Point& screen_location,
+                                 int allowed_operations,
+                                 ui::mojom::DragEventSource source) override {
     DCHECK(!drag_started_);
     if (!drag_started_) {
       drag_started_ = true;
@@ -303,7 +305,7 @@ class DragStartWaiter : public aura::client::DragDropClient {
       location_inside_web_contents_ =
           screen_location - gfx::Vector2d(bounds.x(), bounds.y());
 
-      operation_ = operation;
+      operation_ = allowed_operations;
     }
 
     if (!callback_to_run_inside_drag_and_drop_message_loop_.is_null()) {
@@ -314,12 +316,12 @@ class DragStartWaiter : public aura::client::DragDropClient {
     }
 
     if (suppress_passing_of_start_drag_further_)
-      return 0;
+      return DragOperation::kNone;
 
     // Start a nested drag-and-drop loop (might not return for a long time).
     return old_client_->StartDragAndDrop(std::move(data), root_window,
                                          source_window, screen_location,
-                                         operation, source);
+                                         allowed_operations, source);
   }
 
   void DragCancel() override {

chrome/test/BUILD.gn

@@ -6754,6 +6754,7 @@ if (!is_android) {
       deps += [
         "//ui/aura:aura_interactive_ui_tests",
         "//ui/base/dragdrop:types",
+        "//ui/base/dragdrop/mojom",
       ]
     } else {
       sources -= [

components/exo/drag_drop_operation.cc

@@ -21,6 +21,7 @@
 #include "ui/base/clipboard/file_info.h"
 #include "ui/base/data_transfer_policy/data_transfer_endpoint.h"
 #include "ui/base/dragdrop/drag_drop_types.h"
+#include "ui/base/dragdrop/mojom/drag_drop_types.mojom.h"
 #include "ui/base/dragdrop/os_exchange_data.h"
 #include "ui/display/display.h"
 #include "ui/display/screen.h"
@@ -37,9 +38,10 @@
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)
 
 namespace exo {
-
 namespace {
 
+using ::ui::mojom::DragOperation;
+
 uint32_t DndActionsToDragOperations(const base::flat_set<DndAction>& actions) {
   uint32_t dnd_operations = 0;
   for (const DndAction action : actions) {
@@ -72,13 +74,13 @@ DndAction DragOperationsToPreferredDndAction(int op) {
 }
 #endif
 
-DndAction DragOperationToDndAction(int op) {
+DndAction DragOperationToDndAction(DragOperation op) {
   switch (op) {
-    case ui::DragDropTypes::DragOperation::DRAG_NONE:
+    case DragOperation::kNone:
       return DndAction::kNone;
-    case ui::DragDropTypes::DragOperation::DRAG_MOVE:
+    case DragOperation::kMove:
       return DndAction::kMove;
-    case ui::DragDropTypes::DragOperation::DRAG_COPY:
+    case DragOperation::kCopy:
       return DndAction::kCopy;
     default:
       NOTREACHED();
@@ -318,7 +320,7 @@ void DragDropOperation::StartDragDropOperation() {
 
   // This triggers a nested run loop that terminates when the drag and drop
   // operation is completed.
-  int op = drag_drop_controller_->StartDragAndDrop(
+  DragOperation op = drag_drop_controller_->StartDragAndDrop(
       std::move(os_exchange_data_), origin_->get()->window()->GetRootWindow(),
       origin_->get()->window(), drag_start_point, dnd_operations,
       event_source_);
@@ -327,7 +329,7 @@ void DragDropOperation::StartDragDropOperation() {
   if (!weak_ptr)
     return;
 
-  if (op) {
+  if (op != DragOperation::kNone) {
     // Success
 
     // TODO(crbug.com/994065) This is currently not the actual mime type used by

components/exo/extended_drag_source.cc

@@ -33,6 +33,8 @@
 
 namespace exo {
 
+using ::ui::mojom::DragOperation;
+
 // static
 ExtendedDragSource* ExtendedDragSource::instance_ = nullptr;
 
@@ -177,11 +179,11 @@ void ExtendedDragSource::OnToplevelWindowDragStarted(
     StartDrag(dragged_window_holder_->toplevel_window(), start_location);
 }
 
-int ExtendedDragSource::OnToplevelWindowDragDropped() {
+DragOperation ExtendedDragSource::OnToplevelWindowDragDropped() {
   DVLOG(1) << "OnDragDropped()";
   Cleanup();
-  return delegate_->ShouldAllowDropAnywhere() ? ui::DragDropTypes::DRAG_MOVE
-                                              : ui::DragDropTypes::DRAG_NONE;
+  return delegate_->ShouldAllowDropAnywhere() ? DragOperation::kMove
+                                              : DragOperation::kNone;
 }
 
 void ExtendedDragSource::OnToplevelWindowDragCancelled() {

components/exo/extended_drag_source.h

@@ -76,7 +76,7 @@ class ExtendedDragSource : public DataSourceObserver,
   // ash::ToplevelWindowDragDelegate:
   void OnToplevelWindowDragStarted(const gfx::PointF& start_location,
                                    ui::mojom::DragEventSource source) override;
-  int OnToplevelWindowDragDropped() override;
+  ui::mojom::DragOperation OnToplevelWindowDragDropped() override;
   void OnToplevelWindowDragCancelled() override;
   void OnToplevelWindowDragEvent(ui::LocatedEvent* event) override;