Bug 86670 – [chromium] plumb the frame for which a drag was initiated to the WebViewClient

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[chromium] plumb the frame for which a drag was initiated to the WebViewClient

Created attachment 142334 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

I intend to use the Frame to get its document's referrer policy, so it can be used for when the drag results in a download

Created attachment 142337 [details]
Patch

[James Robinson]

Comment on attachment 142337 [details]
Patch

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

Left some comments on how to stage this, but I don't have any insight about the change itself.

> Source/WebKit/chromium/public/WebViewClient.h:254
> +    // FIXME: Remove once the WebKit side has landed.

I'm confused by this comment - this patch is the WebKit side, isn't it?  Did you mean chromium side?

One way to do this without waiting for rolls is to guard both sides of the change in an #ifdef like WEBVIEWCLIENT_STARTDRAGGING_HAS_FRAME and #define that in this header, then remove the #ifdefs/#defines after everything lands and rolls.

Created attachment 142651 [details]
Patch

(In reply to comment #5)
> (From update of attachment 142337 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=142337&action=review
> 
> Left some comments on how to stage this, but I don't have any insight about the change itself.
> 
> > Source/WebKit/chromium/public/WebViewClient.h:254
> > +    // FIXME: Remove once the WebKit side has landed.
> 
> I'm confused by this comment - this patch is the WebKit side, isn't it?  Did you mean chromium side?

Yes

> One way to do this without waiting for rolls is to guard both sides of the change in an #ifdef like WEBVIEWCLIENT_STARTDRAGGING_HAS_FRAME and #define that in this header, then remove the #ifdefs/#defines after everything lands and rolls.

Your approach requires 4 patches, mine only three (or 6 vs 5 if you include rolls)

The advantage of your approach is that you can do breaking changes, e.g. introduce a new enum value. But for just changing a signature or similar, my approach seems to be the more common one

[Kent Tamura]

Comment on attachment 142651 [details]
Patch

looks good.

[WebKit Review Bot]

Comment on attachment 142651 [details]
Patch

Clearing flags on attachment: 142651

Committed r117560: <http://trac.webkit.org/changeset/117560>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.