Bug 164364 - Downloads started by context menu actions should also have a web view associated
Summary: Downloads started by context menu actions should also have a web view associated
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2016-11-03 05:08 PDT by Carlos Garcia Campos
Modified: 2016-11-17 23:51 PST (History)
9 users (show)

See Also:

Attachments
Patch (17.49 KB, patch)
2016-11-03 05:14 PDT, Carlos Garcia Campos 		mcatanzaro: review+
Details | Formatted Diff | Diff
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 Carlos Garcia Campos 2016-11-03 05:08:31 PDT 
When a request is converted to a download WebPageProxy calls handleDownloadRequest() and clients handle that to associate the web view to the download. When a download is started by a context menu action, WebPageProxy calls WebProcessPool::download() with this as initiatingPage parameter, but clients are not notified in this case.
Comment 1 Carlos Garcia Campos 2016-11-03 05:14:21 PDT 
Created attachment 293758 [details]
Patch
Comment 2 WebKit Commit Bot 2016-11-03 05:15:02 PDT 
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 3 Michael Catanzaro 2016-11-12 18:33:33 PST 
Comment on attachment 293758 [details]
Patch

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

> Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:1237
> +    return download.leakRef();

Where is this ref adopted? Normally we leave a comment in situations like this.

> Source/WebKit2/UIProcess/WebProcessPool.cpp:816
> +    if (initiatingPage)
> +        initiatingPage->handleDownloadRequest(downloadProxy);

We need an owner to approve this part. Alex?
Comment 4 Carlos Garcia Campos 2016-11-17 23:23:16 PST 
Ping Owners?
Comment 5 Alex Christensen 2016-11-17 23:29:20 PST 
Comment on attachment 293758 [details]
Patch

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

>> Source/WebKit2/UIProcess/WebProcessPool.cpp:816
>> +        initiatingPage->handleDownloadRequest(downloadProxy);
> 
> We need an owner to approve this part. Alex?

I approve this part.
Comment 6 Carlos Garcia Campos 2016-11-17 23:50:29 PST 
(In reply to comment #3)
> Comment on attachment 293758 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=293758&action=review
> 
> > Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:1237
> > +    return download.leakRef();
> 
> Where is this ref adopted? Normally we leave a comment in situations like
> this.

This is returned by public API in transfer-full methods. To clarify it, instead of adding a comment, I've changed it to leak the ref from webkit_web_context_download_uri() and webkit_web_view_download_uri().

> > Source/WebKit2/UIProcess/WebProcessPool.cpp:816
> > +    if (initiatingPage)
> > +        initiatingPage->handleDownloadRequest(downloadProxy);
> 
> We need an owner to approve this part. Alex?
Comment 7 Carlos Garcia Campos 2016-11-17 23:51:12 PST 
Committed r208882: <http://trac.webkit.org/changeset/208882>