Bug 86670

Summary: [chromium] plumb the frame for which a drag was initiated to the WebViewClient
Product: WebKit Reporter: jochen
Component: New BugsAssignee: jochen
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dglazkov, fishd, jamesr, tkent, tkent+wkapi, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

jochen
Reported 2012-05-16 13:32:44 PDT
[chromium] plumb the frame for which a drag was initiated to the WebViewClient
Attachments
Patch (4.37 KB, patch)
2012-05-16 13:33 PDT, jochen 		no flags
DetailsFormatted DiffDiff
Patch (6.96 KB, patch)
2012-05-16 13:47 PDT, jochen 		no flags
DetailsFormatted DiffDiff
Patch (6.96 KB, patch)
2012-05-18 00:18 PDT, jochen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
jochen
Comment 1 2012-05-16 13:33:02 PDT
Created attachment 142334 [details] Patch
WebKit Review Bot
Comment 2 2012-05-16 13:35:15 PDT
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
jochen
Comment 3 2012-05-16 13:37:03 PDT
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
jochen
Comment 4 2012-05-16 13:47:26 PDT
Created attachment 142337 [details] Patch
James Robinson
Comment 5 2012-05-16 14:44:30 PDT
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.
jochen
Comment 6 2012-05-18 00:18:26 PDT
Created attachment 142651 [details] Patch
jochen
Comment 7 2012-05-18 00:21:05 PDT
(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 8 2012-05-18 00:33:23 PDT
Comment on attachment 142651 [details] Patch looks good.
WebKit Review Bot
Comment 9 2012-05-18 01:11:19 PDT
Comment on attachment 142651 [details] Patch Clearing flags on attachment: 142651 Committed r117560: <http://trac.webkit.org/changeset/117560>
WebKit Review Bot
Comment 10 2012-05-18 01:11:24 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.