208893 2020-03-10 16:31:43 -0700 Some common domains should always be App-bound domains 2020-03-25 09:30:03 -0700 1 1 1 Unclassified WebKit WebKit Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 katherine_cheney katherine_cheney bfulgham cdumez commit-queue webkit-bug-importer wilander oldest_to_newest 1628426 0 katherine_cheney 2020-03-10 16:31:43 -0700 <rdar://problem/60280943> 1628447 1 393191 katherine_cheney 2020-03-10 17:36:25 -0700 Created attachment 393191 Patch 1628449 2 393191 bfulgham 2020-03-10 17:39:52 -0700 Comment on attachment 393191 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393191&action=review > Source/WebKit/UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:483 > + || startsWithLettersIgnoringASCIICase(domain.string(), "data:")); I don't think this will work, because RegistrableDomains are just things like "apple.com" or "webkit.org". I don't think we retain the protocol in the class. 1628451 3 393191 cdumez 2020-03-10 17:42:03 -0700 Comment on attachment 393191 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393191&action=review >> Source/WebKit/UIProcess/WebsiteData/Cocoa/WebsiteDataStoreCocoa.mm:483 >> + || startsWithLettersIgnoringASCIICase(domain.string(), "data:")); > > I don't think this will work, because RegistrableDomains are just things like "apple.com" or "webkit.org". I don't think we retain the protocol in the class. Correct. Can this be tested somehow? 1628456 4 393196 bfulgham 2020-03-10 18:10:32 -0700 Created attachment 393196 Patch 1628459 5 katherine_cheney 2020-03-10 18:14:29 -0700 (In reply to Brent Fulgham from comment #4) > Created attachment 393196 [details] > Patch Looks good, Unofficial r=me 1628477 6 393196 commit-queue 2020-03-10 19:00:21 -0700 Comment on attachment 393196 Patch Clearing flags on attachment: 393196 Committed r258247: <https://trac.webkit.org/changeset/258247> 1628478 7 commit-queue 2020-03-10 19:00:22 -0700 All reviewed patches have been landed. Closing bug. 1628647 8 393196 cdumez 2020-03-11 08:04:54 -0700 Comment on attachment 393196 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393196&action=review > Source/WebKit/UIProcess/WebPageProxy.cpp:3098 > + return requestURL.protocolIsAbout() || requestURL.protocolIsData() || requestURL.protocolIsBlob() || requestURL.isLocalFile(); Would !requestURL.protocolIsInHTTPFamily() have worked? Or do you expect there are other non HTTP-family protocols that should not be treated as AppBound? I am asking because !requestURL.protocolIsInHTTPFamily() would be more concise and more efficient. Technically, I think registrable domains only make sense for HTTP-Family protocol anyway. > Source/WebKit/UIProcess/WebPageProxy.cpp:3105 > + if (isNavigatingToAppBoundDomain == NavigatingToAppBoundDomain::No && shouldBeTreatedAsAppBound(requestURL)) This does not seems ideally to me. Am I right that for those special protocols you are still going to extract the registrable domain and check if it is part of app bounds domains, and then ignore the result of that work and simply set that value to false? I think a much better way would be to call setIsNavigatingToAppBoundDomain(NavigatingToAppBoundDomain::No) right away for non-http URLs instead of doing the whole registrable check logic unnecessarily. 1628649 9 393196 cdumez 2020-03-11 08:07:23 -0700 Comment on attachment 393196 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393196&action=review > Source/WebKit/ChangeLog:3 > + Some common domains should always be App-bound domains Also, the title talks about domains but this has nothing to do with domains, it is really about non-http protocols. > Source/WebKit/ChangeLog:9 > + Some domains, like about:blank and pages loaded from files should Same, this sentence does not make sense. 1633899 10 katherine_cheney 2020-03-25 09:30:03 -0700 (In reply to Chris Dumez from comment #8) > Comment on attachment 393196 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=393196&action=review > > > Source/WebKit/UIProcess/WebPageProxy.cpp:3098 > > + return requestURL.protocolIsAbout() || requestURL.protocolIsData() || requestURL.protocolIsBlob() || requestURL.isLocalFile(); > > Would !requestURL.protocolIsInHTTPFamily() have worked? Or do you expect > there are other non HTTP-family protocols that should not be treated as > AppBound? I am asking because !requestURL.protocolIsInHTTPFamily() would be > more concise and more efficient. > Technically, I think registrable domains only make sense for HTTP-Family > protocol anyway. > > > Source/WebKit/UIProcess/WebPageProxy.cpp:3105 > > + if (isNavigatingToAppBoundDomain == NavigatingToAppBoundDomain::No && shouldBeTreatedAsAppBound(requestURL)) > > This does not seems ideally to me. Am I right that for those special > protocols you are still going to extract the registrable domain and check if > it is part of app bounds domains, and then ignore the result of that work > and simply set that value to false? > I think a much better way would be to call > setIsNavigatingToAppBoundDomain(NavigatingToAppBoundDomain::No) right away > for non-http URLs instead of doing the whole registrable check logic > unnecessarily. Late but agree this should be changed. Tracking this in rdar://problem/60875376. 393191 2020-03-10 17:36:25 -0700 2020-03-10 18:10:30 -0700 Patch bug-208893-20200310173625.patch text/plain 2356 katherine_cheney U3VidmVyc2lvbiBSZXZpc2lvbjogMjU4MjI1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDBiZjNmMTkwMDhhNWZiZjc3 ZWZhNThlMjE3NWVmYTExMmIzYWZlYTYuLjk5NjYxMzliYmYyNmU1OWQxZTBiNmNjM2M0NDBkNTg4 MjVlN2ZkNTUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTggQEAKKzIwMjAtMDMtMTAgIEthdGUgQ2hl bmV5ICA8a2F0aGVyaW5lX2NoZW5leUBhcHBsZS5jb20+CisKKyAgICAgICAgU29tZSBjb21tb24g ZG9tYWlucyBzaG91bGQgYWx3YXlzIGJlIEFwcC1ib3VuZCBkb21haW5zCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMDg4OTMKKyAgICAgICAgPHJkYXI6 Ly9wcm9ibGVtLzYwMjgwOTQzPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIFNvbWUgZG9tYWlucywgbGlrZSBhYm91dDpibGFuayBhbmQgcGFnZXMgbG9h ZGVkIGZyb20gZmlsZXMgc2hvdWxkCisgICAgICAgIGFsd2F5cyBiZSBjb25zaWRlcmVkIGFwcC1i b3VuZC4KKworICAgICAgICAqIFVJUHJvY2Vzcy9XZWJzaXRlRGF0YS9Db2NvYS9XZWJzaXRlRGF0 YVN0b3JlQ29jb2EubW06CisgICAgICAgIChXZWJLaXQ6OmlzRGVmYXVsdEFwcEJvdW5kRG9tYWlu KToKKyAgICAgICAgKFdlYktpdDo6V2Vic2l0ZURhdGFTdG9yZTo6YmVnaW5BcHBCb3VuZERvbWFp bkNoZWNrKToKKwogMjAyMC0wMy0xMCAgTWVnYW4gR2FyZG5lciAgPG1lZ2FuX2dhcmRuZXJAYXBw bGUuY29tPgogCiAgICAgICAgIEZpbGUgdXBsb2FkIGNhbmNlbHMgYmVmb3JlIGZpbGVzIGNhbiBi ZSB1cGxvYWRlZC4KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL1dlYnNpdGVE YXRhL0NvY29hL1dlYnNpdGVEYXRhU3RvcmVDb2NvYS5tbSBiL1NvdXJjZS9XZWJLaXQvVUlQcm9j ZXNzL1dlYnNpdGVEYXRhL0NvY29hL1dlYnNpdGVEYXRhU3RvcmVDb2NvYS5tbQppbmRleCBlY2M3 N2UzZGNjZTIzMjA1NDM0YmIwOGFlNGYxMzFlN2Q2Y2ZiNDEyLi44YjQ5ZTJmNWJlNTAzNTMxMjk3 ZDRhYzFjNTFhOGNiNDQwMzExMWY0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vz cy9XZWJzaXRlRGF0YS9Db2NvYS9XZWJzaXRlRGF0YVN0b3JlQ29jb2EubW0KKysrIGIvU291cmNl L1dlYktpdC9VSVByb2Nlc3MvV2Vic2l0ZURhdGEvQ29jb2EvV2Vic2l0ZURhdGFTdG9yZUNvY29h Lm1tCkBAIC00NzYsNiArNDc2LDEzIEBAIHZvaWQgV2Vic2l0ZURhdGFTdG9yZTo6aW5pdGlhbGl6 ZUFwcEJvdW5kRG9tYWlucygpCiAgICAgfSk7CiB9CiAKK3N0YXRpYyBib29sIGlzRGVmYXVsdEFw cEJvdW5kRG9tYWluKFdlYkNvcmU6OlJlZ2lzdHJhYmxlRG9tYWluJiBkb21haW4pCit7CisgICAg cmV0dXJuIChkb21haW4gPT0gV2ViQ29yZTo6UmVnaXN0cmFibGVEb21haW46OnVuY2hlY2tlZENy ZWF0ZUZyb21Ib3N0KCJhYm91dDpibGFuayIpCisgICAgICAgIHx8IHN0YXJ0c1dpdGhMZXR0ZXJz SWdub3JpbmdBU0NJSUNhc2UoZG9tYWluLnN0cmluZygpLCAiZmlsZTovLyIpCisgICAgICAgIHx8 IHN0YXJ0c1dpdGhMZXR0ZXJzSWdub3JpbmdBU0NJSUNhc2UoZG9tYWluLnN0cmluZygpLCAiZGF0 YToiKSk7Cit9CisKIHZvaWQgV2Vic2l0ZURhdGFTdG9yZTo6ZW5zdXJlQXBwQm91bmREb21haW5z KENvbXBsZXRpb25IYW5kbGVyPHZvaWQoY29uc3QgSGFzaFNldDxXZWJDb3JlOjpSZWdpc3RyYWJs ZURvbWFpbj4mKT4mJiBjb21wbGV0aW9uSGFuZGxlcikgY29uc3QKIHsKICAgICBpZiAoaGFzSW5p dGlhbGl6ZWRBcHBCb3VuZERvbWFpbnMpIHsKQEAgLTQ5Niw3ICs1MDMsNyBAQCB2b2lkIFdlYnNp dGVEYXRhU3RvcmU6OmJlZ2luQXBwQm91bmREb21haW5DaGVjayhXZWJDb3JlOjpSZWdpc3RyYWJs ZURvbWFpbiYmIGRvbQogICAgIEFTU0VSVChSdW5Mb29wOjppc01haW4oKSk7CiAKICAgICBlbnN1 cmVBcHBCb3VuZERvbWFpbnMoW2RvbWFpbiA9IFdURk1vdmUoZG9tYWluKSwgbGlzdGVuZXIgPSBt YWtlUmVmKGxpc3RlbmVyKV0gKGF1dG8mIGRvbWFpbnMpIG11dGFibGUgewotICAgICAgICBsaXN0 ZW5lci0+ZGlkUmVjZWl2ZUFwcEJvdW5kRG9tYWluUmVzdWx0KGRvbWFpbnMuY29udGFpbnMoZG9t YWluKSk7CisgICAgICAgIGxpc3RlbmVyLT5kaWRSZWNlaXZlQXBwQm91bmREb21haW5SZXN1bHQo ZG9tYWlucy5jb250YWlucyhkb21haW4pIHx8IGlzRGVmYXVsdEFwcEJvdW5kRG9tYWluKGRvbWFp bikpOwogICAgIH0pOwogfQogCg== 393196 2020-03-10 18:10:32 -0700 2020-03-10 19:00:21 -0700 Patch bug-208893-20200310181030.patch text/plain 2264 bfulgham U3VidmVyc2lvbiBSZXZpc2lvbjogMjU4MjI2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDBiZjNmMTkwMDhhNWZiZjc3 ZWZhNThlMjE3NWVmYTExMmIzYWZlYTYuLjk4MzdkNTFiM2Q5OWYyM2ExMjcyODM5NzkxYWRiZWVm NmFlNGVkYTAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTggQEAKKzIwMjAtMDMtMTAgIEthdGUgQ2hl bmV5ICA8a2F0aGVyaW5lX2NoZW5leUBhcHBsZS5jb20+CisKKyAgICAgICAgU29tZSBjb21tb24g ZG9tYWlucyBzaG91bGQgYWx3YXlzIGJlIEFwcC1ib3VuZCBkb21haW5zCisgICAgICAgIGh0dHBz Oi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMDg4OTMKKyAgICAgICAgIDxyZGFy Oi8vcHJvYmxlbS82MDI4MDk0Mz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBTb21lIGRvbWFpbnMsIGxpa2UgYWJvdXQ6YmxhbmsgYW5kIHBhZ2VzIGxv YWRlZCBmcm9tIGZpbGVzIHNob3VsZAorICAgICAgICBhbHdheXMgYmUgY29uc2lkZXJlZCBhcHAt Ym91bmQuCisKKyAgICAgICAgKiBVSVByb2Nlc3MvV2ViUGFnZVByb3h5LmNwcDoKKyAgICAgICAg KFdlYktpdDo6c2hvdWxkQmVUcmVhdGVkQXNBcHBCb3VuZCk6CisgICAgICAgIChXZWJLaXQ6Oldl YlBhZ2VQcm94eTo6c2V0SXNOYXZpZ2F0aW5nVG9BcHBCb3VuZERvbWFpbik6CisKIDIwMjAtMDMt MTAgIE1lZ2FuIEdhcmRuZXIgIDxtZWdhbl9nYXJkbmVyQGFwcGxlLmNvbT4KIAogICAgICAgICBG aWxlIHVwbG9hZCBjYW5jZWxzIGJlZm9yZSBmaWxlcyBjYW4gYmUgdXBsb2FkZWQuCmRpZmYgLS1n aXQgYS9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9XZWJQYWdlUHJveHkuY3BwIGIvU291cmNlL1dl YktpdC9VSVByb2Nlc3MvV2ViUGFnZVByb3h5LmNwcAppbmRleCAyNmVmYmIzOTBiMzgxNjNlOThm Njg4MzkxY2Y3NDUzYzc5NzBhOTNlLi41MDlmYWEwM2IwYmYwYzc2ZWYwYjc0ZTdhNTAzYTNlNTM3 YzQyOTM3IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L1VJUHJvY2Vzcy9XZWJQYWdlUHJveHku Y3BwCisrKyBiL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL1dlYlBhZ2VQcm94eS5jcHAKQEAgLTMw OTMsMTAgKzMwOTMsMTggQEAgcHJpdmF0ZToKICAgICBQb2xpY3lDaGVja0lkZW50aWZpZXIgbV9p ZGVudGlmaWVyOwogfTsKIAorc3RhdGljIGJvb2wgc2hvdWxkQmVUcmVhdGVkQXNBcHBCb3VuZChj b25zdCBVUkwmIHJlcXVlc3RVUkwpCit7CisgICAgcmV0dXJuIHJlcXVlc3RVUkwucHJvdG9jb2xJ c0Fib3V0KCkgfHwgcmVxdWVzdFVSTC5wcm90b2NvbElzRGF0YSgpIHx8IHJlcXVlc3RVUkwucHJv dG9jb2xJc0Jsb2IoKSB8fCByZXF1ZXN0VVJMLmlzTG9jYWxGaWxlKCk7Cit9CisKIHZvaWQgV2Vi UGFnZVByb3h5OjpzZXRJc05hdmlnYXRpbmdUb0FwcEJvdW5kRG9tYWluKGJvb2wgaXNNYWluRnJh bWUsIGNvbnN0IFVSTCYgcmVxdWVzdFVSTCwgTmF2aWdhdGluZ1RvQXBwQm91bmREb21haW4gaXNO YXZpZ2F0aW5nVG9BcHBCb3VuZERvbWFpbikKIHsKICAgICBpZiAoaXNNYWluRnJhbWUgJiYgKG1f cHJlZmVyZW5jZXMtPmlzSW5BcHBCcm93c2VyUHJpdmFjeUVuYWJsZWQoKSB8fCBXRUJfUEFHRV9Q Uk9YWV9BRERJVElPTlNfU0VUSVNOQVZJR0FUSU5HVE9BUFBCT1VORERPTUFJTl8yKSkgewogICAg ICAgICBXRUJfUEFHRV9QUk9YWV9BRERJVElPTlNfU0VUSVNOQVZJR0FUSU5HVE9BUFBCT1VORERP TUFJTgorICAgICAgICBpZiAoaXNOYXZpZ2F0aW5nVG9BcHBCb3VuZERvbWFpbiA9PSBOYXZpZ2F0 aW5nVG9BcHBCb3VuZERvbWFpbjo6Tm8gJiYgc2hvdWxkQmVUcmVhdGVkQXNBcHBCb3VuZChyZXF1 ZXN0VVJMKSkKKyAgICAgICAgICAgIGlzTmF2aWdhdGluZ1RvQXBwQm91bmREb21haW4gPSBOYXZp Z2F0aW5nVG9BcHBCb3VuZERvbWFpbjo6WWVzOworCiAgICAgICAgIGlmIChpc05hdmlnYXRpbmdU b0FwcEJvdW5kRG9tYWluID09IE5hdmlnYXRpbmdUb0FwcEJvdW5kRG9tYWluOjpObykgewogICAg ICAgICAgICAgbV9jb25maWd1cmF0aW9uLT5zZXRXZWJWaWV3Q2F0ZWdvcnkoV2ViVmlld0NhdGVn b3J5OjpJbkFwcEJyb3dzZXIpOwogICAgICAgICAgICAgbV9pc05hdmlnYXRpbmdUb0FwcEJvdW5k RG9tYWluID0gTmF2aWdhdGluZ1RvQXBwQm91bmREb21haW46Ok5vOwo=