[webkit-reviews] review granted: [Bug 212507] Don't use casts to convert between WebCore types and {Web, WK}DragDestinationActionMask/NSDragOperation : [Attachment 400642] Patch v7

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted David Kilzer (:ddkilzer)
<ddkilzer at webkit.org>'s request for review:
Bug 212507: Don't use casts to convert between WebCore types and
{Web,WK}DragDestinationActionMask/NSDragOperation
https://bugs.webkit.org/show_bug.cgi?id=212507

Attachment 400642: Patch v7

https://bugs.webkit.org/attachment.cgi?id=400642&action=review




--- Comment #19 from Darin Adler <darin at apple.com> ---
Comment on attachment 400642
  --> https://bugs.webkit.org/attachment.cgi?id=400642
Patch v7

View in context: https://bugs.webkit.org/attachment.cgi?id=400642&action=review

> Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:7323
> +    WebCore::DragOperation dragOperationMask =
coreDragOperationMask(session.allowsMoveOperation ? WebCore::DragOperationEvery
: (WebCore::DragOperationEvery & ~WebCore::DragOperationMove));

WebCore::DragOperationEvery and WebCore::DragOperationMove are already a
WebCore::DragOperation. We should not call coreDragOperationMask. This only
works because the values are the same, but this is the whole point of the patch
really.

> Source/WebKitLegacy/mac/WebView/WebView.mm:601
> +#if !USE(APPKIT)
> +#define NSDragOperationCopy 1
> +#define NSDragOperationLink 2
> +#define NSDragOperationGeneric 4
> +#define NSDragOperationPrivate 8
> +#define NSDragOperationMove 16
> +#define NSDragOperationDelete 32
> +#endif

I suggest constants instead of macros. Like:

    constexpr uint64_t	NSDragOperationCopy = 1;

Then you don’t have to do all the #undef.

Further, might also be good if this could go into an SPI header instead of
being here, but maybe not a rush to do that right now.