Fix mixed usage of DragOperation/DragOperationsMask

Because both types are enums, they are used in many places
interchangeably. This CL changes incorrect uses to use the appropriate
type.

Bug: 1093536
Change-Id: I7b4609f70ac0803b78c84354dd51364207dd1e11
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2587741
Commit-Queue: Henrique Ferreiro <hferreiro@igalia.com>
Reviewed-by: Darwin Huang <huangdarwin@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#839086}

content/browser/web_contents/web_contents_view_aura.cc

@@ -356,7 +356,7 @@ void PrepareDropData(DropData* drop_data, const ui::OSExchangeData& data) {
 
 // Utilities to convert between blink::DragOperationsMask and
 // ui::DragDropTypes.
-int ConvertFromWeb(blink::DragOperationsMask ops) {
+int ConvertFromDragOperationsMask(blink::DragOperationsMask ops) {
   int drag_op = ui::DragDropTypes::DRAG_NONE;
   if (ops & blink::kDragOperationCopy)
     drag_op |= ui::DragDropTypes::DRAG_COPY;
@@ -367,7 +367,7 @@ int ConvertFromWeb(blink::DragOperationsMask ops) {
   return drag_op;
 }
 
-blink::DragOperationsMask ConvertToWeb(int drag_op) {
+blink::DragOperationsMask ConvertToDragOperationsMask(int drag_op) {
   int web_drag_op = blink::kDragOperationNone;
   if (drag_op & ui::DragDropTypes::DRAG_COPY)
     web_drag_op |= blink::kDragOperationCopy;
@@ -378,6 +378,15 @@ blink::DragOperationsMask ConvertToWeb(int drag_op) {
   return static_cast<blink::DragOperationsMask>(web_drag_op);
 }
 
+blink::DragOperation ConvertToDragOperation(int drag_ops) {
+  int op_mask = ConvertToDragOperationsMask(drag_ops);
+  DCHECK(op_mask == blink::kDragOperationNone ||
+         op_mask == blink::kDragOperationCopy ||
+         op_mask == blink::kDragOperationLink ||
+         op_mask == blink::kDragOperationMove);
+  return static_cast<blink::DragOperation>(op_mask);
+}
+
 GlobalRoutingID GetRenderViewHostID(RenderViewHost* rvh) {
   return GlobalRoutingID(rvh->GetProcess()->GetID(), rvh->GetRoutingID());
 }
@@ -716,7 +725,7 @@ WebContentsViewAura::~WebContentsViewAura() {
 
 void WebContentsViewAura::EndDrag(
     base::WeakPtr<RenderWidgetHostImpl> source_rwh_weak_ptr,
-    blink::DragOperationsMask ops) {
+    blink::DragOperation op) {
   drag_start_process_id_ = ChildProcessHost::kInvalidUniqueID;
   drag_start_view_id_ = GlobalRoutingID(ChildProcessHost::kInvalidUniqueID,
                                         MSG_ROUTING_NONE);
@@ -749,7 +758,7 @@ void WebContentsViewAura::EndDrag(
   }
 
   web_contents_->DragSourceEndedAt(transformed_point.x(), transformed_point.y(),
-                                   screen_loc.x(), screen_loc.y(), ops,
+                                   screen_loc.x(), screen_loc.y(), op,
                                    source_rwh);
 
   web_contents_->SystemDragEnded(source_rwh);
@@ -1055,7 +1064,8 @@ void WebContentsViewAura::StartDragging(
         aura::client::GetDragDropClient(root_window)
             ->StartDragAndDrop(std::move(data), root_window,
                                content_native_view, event_info.location,
-                               ConvertFromWeb(operations), event_info.source);
+                               ConvertFromDragOperationsMask(operations),
+                               event_info.source);
   }
 
   // Bail out immediately if the contents view window is gone. Note that it is
@@ -1072,11 +1082,11 @@ void WebContentsViewAura::StartDragging(
   // callback yet. So we have to make sure to delay calling EndDrag until drop
   // is done.
   if (!drag_in_progress_) {
-    EndDrag(std::move(source_rwh_weak_ptr), ConvertToWeb(result_op));
+    EndDrag(std::move(source_rwh_weak_ptr), ConvertToDragOperation(result_op));
   } else {
     end_drag_runner_.ReplaceClosure(base::BindOnce(
         &WebContentsViewAura::EndDrag, weak_ptr_factory_.GetWeakPtr(),
-        std::move(source_rwh_weak_ptr), ConvertToWeb(result_op)));
+        std::move(source_rwh_weak_ptr), ConvertToDragOperation(result_op)));
   }
 }
 
@@ -1237,12 +1247,13 @@ void WebContentsViewAura::DragEnteredCallback(
   current_drop_data_.reset(drop_data.release());
   current_rwh_for_drag_->FilterDropData(current_drop_data_.get());
 
-  blink::DragOperationsMask op = ConvertToWeb(event.source_operations());
+  blink::DragOperationsMask op_mask =
+      ConvertToDragOperationsMask(event.source_operations());
 
   // Give the delegate an opportunity to cancel the drag.
   if (web_contents_->GetDelegate() &&
       !web_contents_->GetDelegate()->CanDragEnter(
-          web_contents_, *current_drop_data_.get(), op)) {
+          web_contents_, *current_drop_data_.get(), op_mask)) {
     current_drop_data_.reset(nullptr);
     return;
   }
@@ -1250,7 +1261,7 @@ void WebContentsViewAura::DragEnteredCallback(
   DCHECK(transformed_pt.has_value());
   gfx::PointF screen_pt(display::Screen::GetScreen()->GetCursorScreenPoint());
   current_rwh_for_drag_->DragTargetDragEnter(
-      *current_drop_data_, transformed_pt.value(), screen_pt, op,
+      *current_drop_data_, transformed_pt.value(), screen_pt, op_mask,
       ui::EventFlagsToWebEventModifiers(event.flags()));
 
   if (drag_dest_delegate_) {
@@ -1331,9 +1342,10 @@ void WebContentsViewAura::DragUpdatedCallback(
   }
 
   DCHECK(transformed_pt.has_value());
-  blink::DragOperationsMask op = ConvertToWeb(event.source_operations());
+  blink::DragOperationsMask op_mask =
+      ConvertToDragOperationsMask(event.source_operations());
   target_rwh->DragTargetDragOver(
-      transformed_pt.value(), screen_pt, op,
+      transformed_pt.value(), screen_pt, op_mask,
       ui::EventFlagsToWebEventModifiers(event.flags()));
 
   if (drag_dest_delegate_)
@@ -1355,7 +1367,8 @@ int WebContentsViewAura::OnDragUpdated(const ui::DropTargetEvent& event) {
           base::BindOnce(&WebContentsViewAura::DragUpdatedCallback,
                          weak_ptr_factory_.GetWeakPtr(), event,
                          std::move(drop_data)));
-  return ConvertFromWeb(current_drag_op_);
+  return ConvertFromDragOperationsMask(
+      static_cast<blink::DragOperationsMask>(current_drag_op_));
 }
 
 void WebContentsViewAura::OnDragExited() {
@@ -1503,7 +1516,8 @@ int WebContentsViewAura::OnPerformDrop(
           base::BindOnce(&WebContentsViewAura::PerformDropCallback,
                          weak_ptr_factory_.GetWeakPtr(), event,
                          std::move(data)));
-  return ConvertFromWeb(current_drag_op_);
+  return ConvertFromDragOperationsMask(
+      static_cast<blink::DragOperationsMask>(current_drag_op_));
 }
 
 void WebContentsViewAura::CompleteDrop(RenderWidgetHostImpl* target_rwh,

content/browser/web_contents/web_contents_view_aura.h

@@ -110,7 +110,7 @@ class CONTENT_EXPORT WebContentsViewAura
   ~WebContentsViewAura() override;
 
   void EndDrag(base::WeakPtr<RenderWidgetHostImpl> source_rwh_weak_ptr,
-               blink::DragOperationsMask ops);
+               blink::DragOperation op);
 
   void InstallOverscrollControllerDelegate(RenderWidgetHostViewAura* view);
 
@@ -291,7 +291,7 @@ class CONTENT_EXPORT WebContentsViewAura
 
   std::unique_ptr<WebContentsViewDelegate> delegate_;
 
-  blink::DragOperationsMask current_drag_op_;
+  blink::DragOperation current_drag_op_;
 
   std::unique_ptr<DropData> current_drop_data_;
 

third_party/blink/renderer/core/clipboard/data_transfer.cc

@@ -158,9 +158,7 @@ class DraggedNodeImageBuilder {
 #endif
 };
 
-}  // namespace
-
-static base::Optional<DragOperation> ConvertEffectAllowedToDragOperation(
+base::Optional<DragOperationsMask> ConvertEffectAllowedToDragOperationsMask(
     const String& op) {
   // Values specified in
   // https://html.spec.whatwg.org/multipage/dnd.html#dom-datatransfer-effectallowed
@@ -174,18 +172,24 @@ static base::Optional<DragOperation> ConvertEffectAllowedToDragOperation(
     return kDragOperationLink;
   if (op == "move")
     return kDragOperationMove;
-  if (op == "copyLink")
-    return static_cast<DragOperation>(kDragOperationCopy | kDragOperationLink);
-  if (op == "copyMove")
-    return static_cast<DragOperation>(kDragOperationCopy | kDragOperationMove);
-  if (op == "linkMove")
-    return static_cast<DragOperation>(kDragOperationLink | kDragOperationMove);
+  if (op == "copyLink") {
+    return static_cast<DragOperationsMask>(kDragOperationCopy |
+                                           kDragOperationLink);
+  }
+  if (op == "copyMove") {
+    return static_cast<DragOperationsMask>(kDragOperationCopy |
+                                           kDragOperationMove);
+  }
+  if (op == "linkMove") {
+    return static_cast<DragOperationsMask>(kDragOperationLink |
+                                           kDragOperationMove);
+  }
   if (op == "all")
     return kDragOperationEvery;
   return base::nullopt;
 }
 
-static String ConvertDragOperationToEffectAllowed(DragOperation op) {
+String ConvertDragOperationsMaskToEffectAllowed(DragOperationsMask op) {
   if (((op & kDragOperationMove) && (op & kDragOperationCopy) &&
        (op & kDragOperationLink)) ||
       (op == kDragOperationEvery))
@@ -208,8 +212,7 @@ static String ConvertDragOperationToEffectAllowed(DragOperation op) {
 // We provide the IE clipboard types (URL and Text), and the clipboard types
 // specified in the HTML spec. See
 // https://html.spec.whatwg.org/multipage/dnd.html#the-datatransfer-interface
-static String NormalizeType(const String& type,
-                            bool* convert_to_url = nullptr) {
+String NormalizeType(const String& type, bool* convert_to_url = nullptr) {
   String clean_type = type.StripWhiteSpace().LowerASCII();
   if (clean_type == kMimeTypeText ||
       clean_type.StartsWith(kMimeTypeTextPlainEtc))
@@ -222,6 +225,8 @@ static String NormalizeType(const String& type,
   return clean_type;
 }
 
+}  // namespace
+
 // static
 DataTransfer* DataTransfer::Create() {
   DataTransfer* data = Create(
@@ -259,7 +264,7 @@ void DataTransfer::setEffectAllowed(const String& effect) {
   if (!IsForDragAndDrop())
     return;
 
-  if (!ConvertEffectAllowedToDragOperation(effect)) {
+  if (!ConvertEffectAllowedToDragOperationsMask(effect)) {
     // This means that there was no conversion, and the effectAllowed that
     // we are passed isn't a valid effectAllowed, so we should ignore it,
     // and not set |effect_allowed_|.
@@ -556,30 +561,31 @@ bool DataTransfer::CanSetDragImage() const {
          policy_ == DataTransferAccessPolicy::kWritable;
 }
 
-DragOperation DataTransfer::SourceOperation() const {
-  base::Optional<DragOperation> op =
-      ConvertEffectAllowedToDragOperation(effect_allowed_);
+DragOperationsMask DataTransfer::SourceOperation() const {
+  base::Optional<DragOperationsMask> op =
+      ConvertEffectAllowedToDragOperationsMask(effect_allowed_);
   DCHECK(op);
   return *op;
 }
 
 DragOperation DataTransfer::DestinationOperation() const {
   DCHECK(DropEffectIsInitialized());
-  base::Optional<DragOperation> op =
-      ConvertEffectAllowedToDragOperation(drop_effect_);
+  base::Optional<DragOperationsMask> op =
+      ConvertEffectAllowedToDragOperationsMask(drop_effect_);
   DCHECK(op == kDragOperationCopy || op == kDragOperationNone ||
          op == kDragOperationLink || op == kDragOperationMove);
-  return *op;
+  return static_cast<DragOperation>(*op);
 }
 
-void DataTransfer::SetSourceOperation(DragOperation op) {
-  effect_allowed_ = ConvertDragOperationToEffectAllowed(op);
+void DataTransfer::SetSourceOperation(DragOperationsMask op) {
+  effect_allowed_ = ConvertDragOperationsMaskToEffectAllowed(op);
 }
 
 void DataTransfer::SetDestinationOperation(DragOperation op) {
   DCHECK(op == kDragOperationCopy || op == kDragOperationNone ||
          op == kDragOperationLink || op == kDragOperationMove);
-  drop_effect_ = ConvertDragOperationToEffectAllowed(op);
+  drop_effect_ = ConvertDragOperationsMaskToEffectAllowed(
+      static_cast<DragOperationsMask>(op));
 }
 
 DataTransferItemList* DataTransfer::items() {

third_party/blink/renderer/core/clipboard/data_transfer.h

@@ -122,9 +122,9 @@ class CORE_EXPORT DataTransfer final : public ScriptWrappable,
   // anyway.
   bool CanSetDragImage() const;
 
-  DragOperation SourceOperation() const;
+  DragOperationsMask SourceOperation() const;
   DragOperation DestinationOperation() const;
-  void SetSourceOperation(DragOperation);
+  void SetSourceOperation(DragOperationsMask);
   void SetDestinationOperation(DragOperation);
 
   DataTransferItemList* items();

third_party/blink/renderer/core/frame/web_frame_widget_impl.h

@@ -847,7 +847,7 @@ class CORE_EXPORT WebFrameWidgetImpl
   Member<DataObject> current_drag_data_;
 
   // The available drag operations (copy, move link...) allowed by the source.
-  DragOperation operations_allowed_ = kDragOperationNone;
+  DragOperationsMask operations_allowed_ = kDragOperationNone;
 
   // The current drag operation as negotiated by the source and destination.
   // When not equal to DragOperationNone, the drag data can be dropped onto the

third_party/blink/renderer/core/page/drag_controller.cc

@@ -754,7 +754,7 @@ bool DragController::CanProcessDrag(DragData* drag_data,
   return true;
 }
 
-static DragOperation DefaultOperationForDrag(DragOperation src_op_mask) {
+static DragOperation DefaultOperationForDrag(DragOperationsMask src_op_mask) {
   // This is designed to match IE's operation fallback for the case where
   // the page calls preventDefault() in a drag event but doesn't set dropEffect.
   if (src_op_mask == kDragOperationEvery)
@@ -781,7 +781,7 @@ bool DragController::TryDHTMLDrag(DragData* drag_data,
 
   DataTransferAccessPolicy policy = DataTransferAccessPolicy::kTypesReadable;
   DataTransfer* data_transfer = CreateDraggingDataTransfer(policy, drag_data);
-  DragOperation src_op_mask = drag_data->DraggingSourceOperationMask();
+  DragOperationsMask src_op_mask = drag_data->DraggingSourceOperationMask();
   data_transfer->SetSourceOperation(src_op_mask);
 
   WebMouseEvent event = CreateMouseEvent(drag_data);

third_party/blink/renderer/core/page/drag_controller_test.cc

@@ -101,8 +101,8 @@ TEST_F(DragControllerSimTest, DropURLOnNonNavigatingClearsState) {
   object->SetURLAndTitle("https://www.example.com/index.html", "index");
   DragData data(
       object, FloatPoint(10, 10), FloatPoint(10, 10),
-      static_cast<DragOperation>(kDragOperationCopy | kDragOperationLink |
-                                 kDragOperationMove));
+      static_cast<DragOperationsMask>(kDragOperationCopy | kDragOperationLink |
+                                      kDragOperationMove));
 
   WebView().GetPage()->GetDragController().DragEnteredOrUpdated(
       &data, *GetDocument().GetFrame());
@@ -141,8 +141,8 @@ TEST_F(DragControllerSimTest, ThrottledDocumentHandled) {
   object->SetURLAndTitle("https://www.example.com/index.html", "index");
   DragData data(
       object, FloatPoint(10, 10), FloatPoint(10, 10),
-      static_cast<DragOperation>(kDragOperationCopy | kDragOperationLink |
-                                 kDragOperationMove));
+      static_cast<DragOperationsMask>(kDragOperationCopy | kDragOperationLink |
+                                      kDragOperationMove));
 
   WebView().GetPage()->GetDragController().DragEnteredOrUpdated(
       &data, *GetDocument().GetFrame());

third_party/blink/renderer/core/page/drag_data.cc

@@ -41,7 +41,7 @@ namespace blink {
 DragData::DragData(DataObject* data,
                    const FloatPoint& client_position,
                    const FloatPoint& global_position,
-                   DragOperation source_operation_mask)
+                   DragOperationsMask source_operation_mask)
     : client_position_(client_position),
       global_position_(global_position),
       platform_drag_data_(data),

third_party/blink/renderer/core/page/drag_data.h

@@ -52,11 +52,11 @@ class CORE_EXPORT DragData {
   DragData(DataObject*,
            const FloatPoint& client_position,
            const FloatPoint& global_position,
-           DragOperation);
+           DragOperationsMask);
   const FloatPoint& ClientPosition() const { return client_position_; }
   const FloatPoint& GlobalPosition() const { return global_position_; }
   DataObject* PlatformData() const { return platform_drag_data_; }
-  DragOperation DraggingSourceOperationMask() const {
+  DragOperationsMask DraggingSourceOperationMask() const {
     return dragging_source_operation_mask_;
   }
   bool ContainsURL(
@@ -79,7 +79,7 @@ class CORE_EXPORT DragData {
   const FloatPoint client_position_;
   const FloatPoint global_position_;
   DataObject* const platform_drag_data_;
-  const DragOperation dragging_source_operation_mask_;
+  const DragOperationsMask dragging_source_operation_mask_;
 
   bool ContainsHTML() const;
 };