125582 2013-12-11 09:20:22 -0800 Implement RemoteNetworkingContext::initiatingPageID() for Soup 2014-01-27 23:48:42 -0800 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) Unspecified Unspecified RESOLVED INVALID Soup P2 Normal --- 108832 1 cgarcia webkit-unassigned andersca ap beidson oldest_to_newest 958757 0 cgarcia 2013-12-11 09:20:22 -0800 It's currently unimplemented and required by custom protocol handlers 958759 1 218973 cgarcia 2013-12-11 09:23:19 -0800 Created attachment 218973 Patch 965963 2 218973 ap 2014-01-09 10:21:19 -0800 Comment on attachment 218973 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. 965991 3 cgarcia 2014-01-09 11:09:27 -0800 (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. 966047 4 andersca 2014-01-09 13:33:15 -0800 (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. 966270 5 cgarcia 2014-01-10 00:19:49 -0800 (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. 968647 6 cgarcia 2014-01-16 01:12:14 -0800 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 972807 7 218973 cgarcia 2014-01-27 23:48:23 -0800 Comment on attachment 218973 Patch This is no longer needed. 218973 2013-12-11 09:23:19 -0800 2014-01-27 23:48:23 -0800 Patch wk2-remote-networking-page-id.diff text/plain 4628 cgarcia ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCA4Y2ZmNDkyLi5mZTQ3NDZhIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMjAg QEAKIDIwMTMtMTItMTEgIENhcmxvcyBHYXJjaWEgQ2FtcG9zICA8Y2dhcmNpYUBpZ2FsaWEuY29t PgogCisgICAgICAgIFtTT1VQXSBJbXBsZW1lbnQgUmVtb3RlTmV0d29ya2luZ0NvbnRleHQ6Omlu aXRpYXRpbmdQYWdlSUQoKQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9MTI1NTgyCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisK KyAgICAgICAgKiBOZXR3b3JrUHJvY2Vzcy9OZXR3b3JrUmVzb3VyY2VMb2FkZXIuY3BwOgorICAg ICAgICAoV2ViS2l0OjpOZXR3b3JrUmVzb3VyY2VMb2FkZXI6OnN0YXJ0KTogUGFzcyB0aGUgd2Vi IHBhZ2UgSUQgdG8KKyAgICAgICAgdGhlIFJlbW90ZU5ldHdvcmtpbmdDb250ZXh0IGNvbnN0cnVj dG9yLgorICAgICAgICAqIE5ldHdvcmtQcm9jZXNzL1JlbW90ZU5ldHdvcmtpbmdDb250ZXh0Lmg6 CisgICAgICAgICogTmV0d29ya1Byb2Nlc3Mvc291cC9SZW1vdGVOZXR3b3JraW5nQ29udGV4dFNv dXAuY3BwOgorICAgICAgICAoV2ViS2l0OjpSZW1vdGVOZXR3b3JraW5nQ29udGV4dDo6aW5pdGlh dGluZ1BhZ2VJRCk6IFJldHVybiB0aGUKKyAgICAgICAgaW5pdGlhdGluZyBwYWdlIGlkLgorCisy MDEzLTEyLTExICBDYXJsb3MgR2FyY2lhIENhbXBvcyAgPGNnYXJjaWFAaWdhbGlhLmNvbT4KKwog ICAgICAgICBNb3ZlIENoaWxkUHJvY2Vzc1Byb3h5IGZyb20gU2hhcmVkIHRvIFVJUHJvY2Vzcwog ICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTI1NTcwCiAK ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL05ldHdvcmtQcm9jZXNzL05ldHdvcmtSZXNvdXJj ZUxvYWRlci5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9OZXR3b3JrUHJvY2Vzcy9OZXR3b3JrUmVzb3Vy Y2VMb2FkZXIuY3BwCmluZGV4IDFiMGQ1NmMuLjNlYWNmM2QgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9X ZWJLaXQyL05ldHdvcmtQcm9jZXNzL05ldHdvcmtSZXNvdXJjZUxvYWRlci5jcHAKKysrIGIvU291 cmNlL1dlYktpdDIvTmV0d29ya1Byb2Nlc3MvTmV0d29ya1Jlc291cmNlTG9hZGVyLmNwcApAQCAt MTI3LDcgKzEyNyw3IEBAIHZvaWQgTmV0d29ya1Jlc291cmNlTG9hZGVyOjpzdGFydCgpCiAgICAg cmVmKCk7CiAKICAgICAvLyBGSVhNRSAoTmV0d29ya1Byb2Nlc3MpOiBTZXQgcGxhdGZvcm0gc3Bl Y2lmaWMgc2V0dGluZ3MuCi0gICAgbV9uZXR3b3JraW5nQ29udGV4dCA9IFJlbW90ZU5ldHdvcmtp bmdDb250ZXh0OjpjcmVhdGUobV9pblByaXZhdGVCcm93c2luZ01vZGUsIG1fc2hvdWxkQ2xlYXJS ZWZlcnJlck9uSFRUUFNUb0hUVFBSZWRpcmVjdCk7CisgICAgbV9uZXR3b3JraW5nQ29udGV4dCA9 IFJlbW90ZU5ldHdvcmtpbmdDb250ZXh0OjpjcmVhdGUobV9pblByaXZhdGVCcm93c2luZ01vZGUs IG1fc2hvdWxkQ2xlYXJSZWZlcnJlck9uSFRUUFNUb0hUVFBSZWRpcmVjdCwgbV93ZWJQYWdlSUQp OwogCiAgICAgY29uc3VtZVNhbmRib3hFeHRlbnNpb25zKCk7CiAKZGlmZiAtLWdpdCBhL1NvdXJj ZS9XZWJLaXQyL05ldHdvcmtQcm9jZXNzL1JlbW90ZU5ldHdvcmtpbmdDb250ZXh0LmggYi9Tb3Vy Y2UvV2ViS2l0Mi9OZXR3b3JrUHJvY2Vzcy9SZW1vdGVOZXR3b3JraW5nQ29udGV4dC5oCmluZGV4 IGRmZjdiYTIuLjRhYTQxMjUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL05ldHdvcmtQcm9j ZXNzL1JlbW90ZU5ldHdvcmtpbmdDb250ZXh0LmgKKysrIGIvU291cmNlL1dlYktpdDIvTmV0d29y a1Byb2Nlc3MvUmVtb3RlTmV0d29ya2luZ0NvbnRleHQuaApAQCAtMzMsOSArMzMsOSBAQCBuYW1l c3BhY2UgV2ViS2l0IHsKIAogY2xhc3MgUmVtb3RlTmV0d29ya2luZ0NvbnRleHQgRklOQUwgOiBw dWJsaWMgV2ViQ29yZTo6TmV0d29ya2luZ0NvbnRleHQgewogcHVibGljOgotICAgIHN0YXRpYyBQ YXNzUmVmUHRyPFJlbW90ZU5ldHdvcmtpbmdDb250ZXh0PiBjcmVhdGUoYm9vbCBwcml2YXRlQnJv d3NpbmdFbmFibGVkLCBib29sIHNob3VsZENsZWFyUmVmZXJyZXJPbkhUVFBTVG9IVFRQUmVkaXJl Y3QpCisgICAgc3RhdGljIFBhc3NSZWZQdHI8UmVtb3RlTmV0d29ya2luZ0NvbnRleHQ+IGNyZWF0 ZShib29sIHByaXZhdGVCcm93c2luZ0VuYWJsZWQsIGJvb2wgc2hvdWxkQ2xlYXJSZWZlcnJlck9u SFRUUFNUb0hUVFBSZWRpcmVjdCwgdWludDY0X3Qgd2ViUGFnZUlEKQogICAgIHsKLSAgICAgICAg cmV0dXJuIGFkb3B0UmVmKG5ldyBSZW1vdGVOZXR3b3JraW5nQ29udGV4dChwcml2YXRlQnJvd3Np bmdFbmFibGVkLCBzaG91bGRDbGVhclJlZmVycmVyT25IVFRQU1RvSFRUUFJlZGlyZWN0KSk7Cisg ICAgICAgIHJldHVybiBhZG9wdFJlZihuZXcgUmVtb3RlTmV0d29ya2luZ0NvbnRleHQocHJpdmF0 ZUJyb3dzaW5nRW5hYmxlZCwgc2hvdWxkQ2xlYXJSZWZlcnJlck9uSFRUUFNUb0hUVFBSZWRpcmVj dCwgd2ViUGFnZUlEKSk7CiAgICAgfQogICAgIHZpcnR1YWwgflJlbW90ZU5ldHdvcmtpbmdDb250 ZXh0KCk7CiAKQEAgLTQ4LDE0ICs0OCwyMCBAQCBwdWJsaWM6CiAgICAgdmlydHVhbCBib29sIHNo b3VsZENsZWFyUmVmZXJyZXJPbkhUVFBTVG9IVFRQUmVkaXJlY3QoKSBjb25zdCBPVkVSUklERSB7 IHJldHVybiBtX3Nob3VsZENsZWFyUmVmZXJyZXJPbkhUVFBTVG9IVFRQUmVkaXJlY3Q7IH0KIAog cHJpdmF0ZToKLSAgICBSZW1vdGVOZXR3b3JraW5nQ29udGV4dChib29sIHByaXZhdGVCcm93c2lu Z0VuYWJsZWQsIGJvb2wgc2hvdWxkQ2xlYXJSZWZlcnJlck9uSFRUUFNUb0hUVFBSZWRpcmVjdCkK KyAgICBSZW1vdGVOZXR3b3JraW5nQ29udGV4dChib29sIHByaXZhdGVCcm93c2luZ0VuYWJsZWQs IGJvb2wgc2hvdWxkQ2xlYXJSZWZlcnJlck9uSFRUUFNUb0hUVFBSZWRpcmVjdCwgdWludDY0X3Qg d2ViUGFnZUlEKQogICAgICAgICA6IG1fcHJpdmF0ZUJyb3dzaW5nRW5hYmxlZChwcml2YXRlQnJv d3NpbmdFbmFibGVkKQogICAgICAgICAsIG1fc2hvdWxkQ2xlYXJSZWZlcnJlck9uSFRUUFNUb0hU VFBSZWRpcmVjdChzaG91bGRDbGVhclJlZmVycmVyT25IVFRQU1RvSFRUUFJlZGlyZWN0KQogI2lm IFBMQVRGT1JNKE1BQykKICAgICAgICAgLCBtX25lZWRzU2l0ZVNwZWNpZmljUXVpcmtzKGZhbHNl KQogICAgICAgICAsIG1fbG9jYWxGaWxlQ29udGVudFNuaWZmaW5nRW5hYmxlZChmYWxzZSkKICNl bmRpZgotICAgIHsgfQorICAgIHsKKyNpZiBVU0UoU09VUCkKKyAgICAgICAgbV9pbml0aWF0aW5n UGFnZUlEID0gd2ViUGFnZUlEOworI2Vsc2UKKyAgICAgICAgVU5VU0VEX1BBUkFNKHdlYlBhZ2VJ RCk7CisjZW5kaWYKKyAgICB9CiAKICAgICB2aXJ0dWFsIGJvb2wgaXNWYWxpZCgpIGNvbnN0IE9W RVJSSURFOwogICAgIHZpcnR1YWwgV2ViQ29yZTo6TmV0d29ya1N0b3JhZ2VTZXNzaW9uJiBzdG9y YWdlU2Vzc2lvbigpIGNvbnN0IE9WRVJSSURFOwpAQCAtNzMsNyArNzksNyBAQCBwcml2YXRlOgog I2VuZGlmCiAKICNpZiBVU0UoU09VUCkKLSAgICB2aXJ0dWFsIHVpbnQ2NF90IGluaXRpYXRpbmdQ YWdlSUQoKSBjb25zdDsKKyAgICB2aXJ0dWFsIHVpbnQ2NF90IGluaXRpYXRpbmdQYWdlSUQoKSBj b25zdCBPVkVSUklERTsKICNlbmRpZgogCiAgICAgYm9vbCBtX3ByaXZhdGVCcm93c2luZ0VuYWJs ZWQ7CkBAIC04Myw2ICs4OSw5IEBAIHByaXZhdGU6CiAgICAgYm9vbCBtX25lZWRzU2l0ZVNwZWNp ZmljUXVpcmtzOwogICAgIGJvb2wgbV9sb2NhbEZpbGVDb250ZW50U25pZmZpbmdFbmFibGVkOwog I2VuZGlmCisjaWYgVVNFKFNPVVApCisgICAgdWludDY0X3QgbV9pbml0aWF0aW5nUGFnZUlEOwor I2VuZGlmCiB9OwogCiB9CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9OZXR3b3JrUHJvY2Vz cy9zb3VwL1JlbW90ZU5ldHdvcmtpbmdDb250ZXh0U291cC5jcHAgYi9Tb3VyY2UvV2ViS2l0Mi9O ZXR3b3JrUHJvY2Vzcy9zb3VwL1JlbW90ZU5ldHdvcmtpbmdDb250ZXh0U291cC5jcHAKaW5kZXgg M2JlZjU2NC4uODBkMzM3OSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvTmV0d29ya1Byb2Nl c3Mvc291cC9SZW1vdGVOZXR3b3JraW5nQ29udGV4dFNvdXAuY3BwCisrKyBiL1NvdXJjZS9XZWJL aXQyL05ldHdvcmtQcm9jZXNzL3NvdXAvUmVtb3RlTmV0d29ya2luZ0NvbnRleHRTb3VwLmNwcApA QCAtNDgsOCArNDgsNyBAQCBib29sIFJlbW90ZU5ldHdvcmtpbmdDb250ZXh0Ojppc1ZhbGlkKCkg Y29uc3QKIAogdWludDY0X3QgUmVtb3RlTmV0d29ya2luZ0NvbnRleHQ6OmluaXRpYXRpbmdQYWdl SUQoKSBjb25zdAogewotICAgIG5vdEltcGxlbWVudGVkKCk7Ci0gICAgcmV0dXJuIDA7CisgICAg cmV0dXJuIG1faW5pdGlhdGluZ1BhZ2VJRDsKIH0KIAogdm9pZCBSZW1vdGVOZXR3b3JraW5nQ29u dGV4dDo6c2V0UHJpdmF0ZUJyb3dzaW5nU3RvcmFnZVNlc3Npb25JZGVudGlmaWVyQmFzZShjb25z dCBTdHJpbmcmKQo=