Bug 173832

Summary:

Refactor drag start codepaths to plumb a DragItem to client layers

Product:

WebKit

Reporter:

Wenson Hsieh <wenson_hsieh>

Component:

WebCore Misc.

Assignee:

Wenson Hsieh <wenson_hsieh>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bdakin, buildbot, cdumez, dbates, japhet, megan_gardner, rniwa, thorton

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
First pass	none
Attempt to fix Windows build	rniwa: review+
Patch for landing	none

Wenson Hsieh

Reported 2017-06-26 07:52:29 PDT

Work towards <rdar://problem/32236827>

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 attachment proposed patch, testcase, etc.

Wenson Hsieh

Comment 1 2017-06-26 08:32:09 PDT

Created attachment 313841 [details] First pass

Wenson Hsieh

Comment 2 2017-06-26 11:56:34 PDT

Created attachment 313858 [details] Attempt to fix Windows build

Ryosuke Niwa

Comment 3 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.

Ryosuke Niwa

Comment 4 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.

Wenson Hsieh

Comment 5 2017-06-27 10:26:33 PDT

Created attachment 313925 [details] Patch for landing

Wenson Hsieh

Comment 6 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!

Wenson Hsieh

Comment 7 2017-06-27 12:11:55 PDT

Comment on attachment 313925 [details] Patch for landing Committed r218837: <http://trac.webkit.org/changeset/218837>

Note You need to log in before you can comment on or make changes to this bug.