Bug 173832 - Refactor drag start codepaths to plumb a DragItem to client layers
Summary: Refactor drag start codepaths to plumb a DragItem to client layers
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-26 07:52 PDT by Wenson Hsieh
Modified: 2017-06-27 12:12 PDT (History)
8 users (show)

See Also:

Attachments
First pass (49.15 KB, patch)
2017-06-26 08:32 PDT, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Attempt to fix Windows build (50.95 KB, patch)
2017-06-26 11:56 PDT, Wenson Hsieh 		rniwa: review+
Details | Formatted Diff | Diff
Patch for landing (48.25 KB, patch)
2017-06-27 10:26 PDT, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2017-06-26 07:52:29 PDT 
Work towards <rdar://problem/32236827>
Comment 1 Wenson Hsieh 2017-06-26 08:32:09 PDT 
Created attachment 313841 [details]
First pass
Comment 2 Wenson Hsieh 2017-06-26 11:56:34 PDT 
Created attachment 313858 [details]
Attempt to fix Windows build
Comment 3 Ryosuke Niwa 2017-06-27 00:08:26 PDT 
Comment on attachment 313858 [details]
Attempt to fix Windows build

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

It's a bit scary to expose DragItem like this to WebKit/WebKit2 but I guess there isn't a good alternative here.
r=me.

> Source/WebKit2/Shared/WebCoreArgumentCoders.h:541
> +#if ENABLE(DRAG_SUPPORT)
> +template<> struct ArgumentCoder<WebCore::DragItem> {

Modern coding style is to implement encode/decode in WebCore right next to the actual data type.
Comment 4 Ryosuke Niwa 2017-06-27 00:19:10 PDT 
Comment on attachment 313858 [details]
Attempt to fix Windows build

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

> Source/WebKit2/Shared/WebCoreArgumentCoders.cpp:2235
> +    encoder << hasIndicatorData;
> +    if (hasIndicatorData)
> +        encoder << item.image.indicatorData().value();

It's a bit strange to not include the image data but only the indicator data.
We should probably add a FIXME here.
Comment 5 Wenson Hsieh 2017-06-27 10:26:33 PDT 
Created attachment 313925 [details]
Patch for landing
Comment 6 Wenson Hsieh 2017-06-27 10:28:25 PDT 
Comment on attachment 313858 [details]
Attempt to fix Windows build

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

>> Source/WebKit2/Shared/WebCoreArgumentCoders.cpp:2235
>> +        encoder << item.image.indicatorData().value();
> 
> It's a bit strange to not include the image data but only the indicator data.
> We should probably add a FIXME here.

I added a FIXME earlier in this function that references wkbug.com/173815, which tracks the rest of the DragItems refactoring.

>> Source/WebKit2/Shared/WebCoreArgumentCoders.h:541
>> +template<> struct ArgumentCoder<WebCore::DragItem> {
> 
> Modern coding style is to implement encode/decode in WebCore right next to the actual data type.

Got it -- fixed!
Comment 7 Wenson Hsieh 2017-06-27 12:11:55 PDT 
Comment on attachment 313925 [details]
Patch for landing

Committed r218837: <http://trac.webkit.org/changeset/218837>