Bug 125582

Created attachment 218973 [details] Patch

Comment on attachment 218973 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218973&action=review I don't understand why a page ID is needed by custom protocol handlers. Isn't is a layering violation for protocol handlers to have any idea of "web page"? > Source/WebKit2/ChangeLog:3 > + [SOUP] Implement RemoteNetworkingContext::initiatingPageID() This patch changes cross-platform code, so it should not have a [SOUP] prefix.

(In reply to comment #2) > (From update of attachment 218973 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=218973&action=review Thanks for looking at the patch. > I don't understand why a page ID is needed by custom protocol handlers. Isn't is a layering violation for protocol handlers to have any idea of "web page"? It allows to identify the web page that initiated the load, since custom protocols are handled globally by the context, it's not possible to know which web page is associated to the custom protocol load. This is the same concept that the initiating page of a download operation. I don't think there's any layering violation, it's just an id. > > Source/WebKit2/ChangeLog:3 > > + [SOUP] Implement RemoteNetworkingContext::initiatingPageID() > > This patch changes cross-platform code, so it should not have a [SOUP] prefix. Yes, actually it's soup specific change, because it's the only port having a initiatingPageID (qt used to have it too), but changed the cross-platform constructor instead of adding a new one for soup, to avoid more ifdefs in other files. Will change the bug title in any case.

(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 218973 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=218973&action=review > > Thanks for looking at the patch. > > > I don't understand why a page ID is needed by custom protocol handlers. Isn't is a layering violation for protocol handlers to have any idea of "web page"? > > It allows to identify the web page that initiated the load, since custom protocols are handled globally by the context, it's not possible to know which web page is associated to the custom protocol load. This is the same concept that the initiating page of a download operation. I don't think there's any layering violation, it's just an id. > This still sounds like a layering violation. I don't think protocols should have any knowledge of the web page period.

(In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > (From update of attachment 218973 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=218973&action=review > > > > Thanks for looking at the patch. > > > > > I don't understand why a page ID is needed by custom protocol handlers. Isn't is a layering violation for protocol handlers to have any idea of "web page"? > > > > It allows to identify the web page that initiated the load, since custom protocols are handled globally by the context, it's not possible to know which web page is associated to the custom protocol load. This is the same concept that the initiating page of a download operation. I don't think there's any layering violation, it's just an id. > > > > This still sounds like a layering violation. I don't think protocols should have any knowledge of the web page period. It's not actually the protocol but the request. We associate the web page id (just as an identifier, like random metadata) to any request (SoupRequest) to be able to know which page made the request. It's specially useful when the request is not handled by WebPageProxy (because in that case it's obvious) but by WebContext like downloads and custom protocol handlers. To do that association we use the networking context, so that the same way the networking context in the web process is created for a frame, in the network process it could be created for a frame as well. The networking context of the web process is passed to the network process serialized in NetworkResourceLoadParameters after all so the webpage - network request association is there already. In our API a WebKitWebView starts a load of a custom protocol, then in the network/web process a new custom protocol load starts and it's handled by the WebContext. The WebKitWebContext is asked to provide the data for the load (response, data, error, finish, etc.) and the user is asked to provide the actual data, but it's impossible to know which WebKitWebView (tab, window, whatever) that custom protocol request belongs to. In our API we have webkit_uri_scheme_request_get_web_view(). This is something we don't need for any other request, because the loader client is already associated to a web page. This could be used for many things, but it depends on every custom protocol implementation, in the case of the downloads, for example, we use the initiating page to know which browser window to send the notifications about a download when it has been started by a web page.

Ok, I'm going to rework the patches to not depend on the initiating page id for now, but we need to figure out a way to implement it, because that's part of our public API and already used by applications like midori, see: http://bazaar.launchpad.net/~midori/midori/trunk/view/head:/midori/midori-view.c#L777

Comment on attachment 218973 [details] Patch This is no longer needed.