85641 2012-05-04 10:40:43 -0700 KURL::isBlankURL would be a nicer idiom than KURL::protocolIs("about") 2012-05-06 15:56:52 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 webkit.review.bot abarth abarth darin mkwst oldest_to_newest 616391 0 webkit.review.bot 2012-05-04 10:40:43 -0700 KURL::isBlankURL would be a nicer idiom than KURL::protocolIs("about") Requested by abarth on #webkit. 616402 1 mkwst 2012-05-04 10:46:06 -0700 Seems like a GoodFirstBug way of dipping my toes into webkit innards. Mind if I grab this? 616410 2 abarth 2012-05-04 10:48:21 -0700 Go for it. 616868 3 140374 mkwst 2012-05-04 19:37:12 -0700 Created attachment 140374 Patch 616870 4 mkwst 2012-05-04 19:39:11 -0700 Adam, I'd like to add tests for this change but I'm not sure where. Based on some random grepping, I don't see any explicit KURL tests. Can you point me in the right direction? 617035 5 140374 abarth 2012-05-05 12:45:00 -0700 Comment on attachment 140374 Patch I'm surprised there aren't more of these. For example, I would have expected one in FrameLoader (or at least called from FrameLoader). As for testing, there's no way to test a change like this in WebKit. WebKit typically uses LayoutTests to test changes in web-visible behavior. In this case there isn't any external behavior change that we can test. Thanks for the patch! 617064 6 mkwst 2012-05-05 17:23:27 -0700 (In reply to comment #5) > For example, I would have expected one in FrameLoader (or > at least called from FrameLoader). I'm hopping on a plane back to Munich shortly, but I'll take a look at FrameLoader to see if I missed anything sometime tomorrow. Thanks for the pointer. > As for testing, there's no way to test a change like this in WebKit. WebKit > typically uses LayoutTests to test changes in web-visible behavior. In this > case there isn't any external behavior change that we can test. Ah. Good to know. > Thanks for the patch! I appreciate you throwing the patch into the queue, but it looks... unhappy (http://queues.webkit.org/patch/140374). I don't see anything in the patch that would be causing the errors being flagged up. Would you mind taking a look? 617065 7 darin 2012-05-05 17:27:42 -0700 I think the explanation for why there aren’t more protocolIs("about") calls is that there is a function called SchemeRegistry::shouldLoadURLSchemeAsEmptyDocument that is used in the loader for that check. We should probably think about whether any of the call sites that use protocolIs("about") should be using SchemeRegistry::shouldLoadURLSchemeAsEmptyDocument instead, and if so, whether we need a more efficient version of shouldLoadURLSchemeAsEmptyDocument that doesn’t require calling KURL::protocol and allocating a new string each time. Presumably that’s all beyond the scope of this patch, but worth thinking through. 617071 8 abarth 2012-05-05 19:09:28 -0700 > I appreciate you throwing the patch into the queue, but it looks... unhappy (http://queues.webkit.org/patch/140374). I don't see anything in the patch that would be causing the errors being flagged up. Would you mind taking a look? The build is just too red to land patches: http://build.webkit.org/builders/Chromium%20Linux%20Release%20%28Tests%29 Failed 30 failures The patch will land when once the gardener cleans up the tree. 617072 9 abarth 2012-05-05 19:10:42 -0700 > SchemeRegistry::shouldLoadURLSchemeAsEmptyDocument that is used in the loader for that check. Hum... I wonder what other schemes folks are registering. It might be worth looking a when that was added and why. 617073 10 darin 2012-05-05 19:20:37 -0700 The “load as empty document” is exposed only in Chromium WebKit and in WebKit2. Safari uses the WebKit2 hook for protocols that display special pages in Safari, specifically Top Sites and the bookmarks collection. Those are pages we don’t want to allow document.write into, so we I think we don’t want, say, the shouldInheritSecurityOriginFromOwner function in Document.cpp to treat those the same way as "about". 617075 11 abarth 2012-05-05 20:39:43 -0700 Interesting. That sounds like a similar goal to "display isolated", which is about preventing web sites from invoking these URLs. 617205 12 140374 webkit.review.bot 2012-05-06 15:56:46 -0700 Comment on attachment 140374 Patch Clearing flags on attachment: 140374 Committed r116248: <http://trac.webkit.org/changeset/116248> 617206 13 webkit.review.bot 2012-05-06 15:56:52 -0700 All reviewed patches have been landed. Closing bug. 140374 2012-05-04 19:37:12 -0700 2012-05-06 15:56:46 -0700 Patch bug-85641-20120505043710.patch text/plain 5484 mkwst U3VidmVyc2lvbiBSZXZpc2lvbjogMTE2MjAyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggZjY0MDQ2M2Y1NmU0MjFk NWZlYjk1YWI2NzY2YjQ3ZGM0ZTRkOTBhZi4uZTg4ZGNhYjc3M2FiNWZiMjg4MDk0NzZiNjU2ZDJk OTAyNGYwZmZhNCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI1IEBACisyMDEyLTA1LTA0ICBNaWtl IFdlc3QgIDxta3dzdEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgS1VSTDo6aXNCbGFua1VSTCB3 b3VsZCBiZSBhIG5pY2VyIGlkaW9tIHRoYW4gS1VSTDo6cHJvdG9jb2xJcygiYWJvdXQiKQorICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9ODU2NDEKKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGRvbS9Eb2N1bWVu dC5jcHA6CisgICAgICAgIChXZWJDb3JlOjpzaG91bGRJbmhlcml0U2VjdXJpdHlPcmlnaW5Gcm9t T3duZXIpOgorICAgICAgICAqIHBhZ2UvQ29udGVudFNlY3VyaXR5UG9saWN5LmNwcDoKKyAgICAg ICAgKFdlYkNvcmU6OkNTUERpcmVjdGl2ZUxpc3Q6OmFsbG93T2JqZWN0RnJvbVNvdXJjZSk6Cisg ICAgICAgIChXZWJDb3JlOjpDU1BEaXJlY3RpdmVMaXN0OjphbGxvd0NoaWxkRnJhbWVGcm9tU291 cmNlKToKKyAgICAgICAgKiBwYWdlL1BhZ2VTZXJpYWxpemVyLmNwcDoKKyAgICAgICAgKFdlYkNv cmU6OlNlcmlhbGl6ZXJNYXJrdXBBY2N1bXVsYXRvcjo6YXBwZW5kQ3VzdG9tQXR0cmlidXRlcyk6 CisgICAgICAgIChXZWJDb3JlOjpQYWdlU2VyaWFsaXplcjo6c2VyaWFsaXplRnJhbWUpOgorICAg ICAgICAqIHBsYXRmb3JtL0tVUkwuY3BwOgorICAgICAgICAoV2ViQ29yZTo6S1VSTDo6aXNCbGFu a1VSTCk6CisgICAgICAgIChXZWJDb3JlKToKKyAgICAgICAgKiBwbGF0Zm9ybS9LVVJMLmg6Cisg ICAgICAgIChLVVJMKToKKyAgICAgICAgKFdlYkNvcmUpOgorCiAyMDEyLTA1LTA0ICBSb2IgQnVp cyAgPHJidWlzQHJpbS5jb20+CiAKICAgICAgICAgUmVtb3ZlIElubGluZUJveDo6bmV4dCgpCmRp ZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9kb20vRG9jdW1lbnQuY3BwIGIvU291cmNlL1dlYkNv cmUvZG9tL0RvY3VtZW50LmNwcAppbmRleCAyN2M0YjE2NWNkZTFlZmM1ZWI3OWJiM2ZlYTg3Yjdj NDU2ZmZhOGFmLi5hNTBjYjk5MzQyY2Q0M2JjMzc1ZDQzZDIzMmI2ZmQzMjM4YjY4NWE5IDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9kb20vRG9jdW1lbnQuY3BwCisrKyBiL1NvdXJjZS9XZWJD b3JlL2RvbS9Eb2N1bWVudC5jcHAKQEAgLTMwMywxMCArMzAzLDEwIEBAIHN0YXRpYyBib29sIHNo b3VsZEluaGVyaXRTZWN1cml0eU9yaWdpbkZyb21Pd25lcihjb25zdCBLVVJMJiB1cmwpCiAgICAg Ly8gSWYgYSBEb2N1bWVudCBoYXMgdGhlIGFkZHJlc3MgImFib3V0OmJsYW5rIgogICAgIC8vICAg ICBUaGUgb3JpZ2luIG9mIHRoZSBEb2N1bWVudCBpcyB0aGUgb3JpZ2luIGl0IHdhcyBhc3NpZ25l ZCB3aGVuIGl0cyBicm93c2luZyBjb250ZXh0IHdhcyBjcmVhdGVkLgogICAgIC8vCi0gICAgLy8g Tm90ZTogV2UgZ2VuZXJhbGl6ZSB0aGlzIHRvIGFsbCAiYWJvdXQiIFVSTHMgYW5kIGludmFsaWQg VVJMcyBiZWNhdXNlIHdlCisgICAgLy8gTm90ZTogV2UgZ2VuZXJhbGl6ZSB0aGlzIHRvIGFsbCAi YmxhbmsiIFVSTHMgYW5kIGludmFsaWQgVVJMcyBiZWNhdXNlIHdlCiAgICAgLy8gdHJlYXQgYWxs IG9mIHRoZXNlIFVSTHMgYXMgYWJvdXQ6YmxhbmsuCiAgICAgLy8KLSAgICByZXR1cm4gIXVybC5p c1ZhbGlkKCkgfHwgdXJsLnByb3RvY29sSXMoImFib3V0Iik7CisgICAgcmV0dXJuICF1cmwuaXNW YWxpZCgpIHx8IHVybC5pc0JsYW5rVVJMKCk7CiB9CiAKIHN0YXRpYyBXaWRnZXQqIHdpZGdldEZv ck5vZGUoTm9kZSogZm9jdXNlZE5vZGUpCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wYWdl L0NvbnRlbnRTZWN1cml0eVBvbGljeS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wYWdlL0NvbnRlbnRT ZWN1cml0eVBvbGljeS5jcHAKaW5kZXggNjAzZTE4Zjg3Y2NkMDVjNjQ2ODFmOGI4NzE3ZWNiMTBh ZjY0ODAzYS4uODVlOTk4MTcxNWEzZjM4YmI4Y2U5ZmIzMzlmNDA1NGQwNDVkOTUyOSAxMDA2NDQK LS0tIGEvU291cmNlL1dlYkNvcmUvcGFnZS9Db250ZW50U2VjdXJpdHlQb2xpY3kuY3BwCisrKyBi L1NvdXJjZS9XZWJDb3JlL3BhZ2UvQ29udGVudFNlY3VyaXR5UG9saWN5LmNwcApAQCAtNjkxLDcg KzY5MSw3IEBAIGJvb2wgQ1NQRGlyZWN0aXZlTGlzdDo6YWxsb3dTY3JpcHRGcm9tU291cmNlKGNv bnN0IEtVUkwmIHVybCkgY29uc3QKIGJvb2wgQ1NQRGlyZWN0aXZlTGlzdDo6YWxsb3dPYmplY3RG cm9tU291cmNlKGNvbnN0IEtVUkwmIHVybCkgY29uc3QKIHsKICAgICBERUZJTkVfU1RBVElDX0xP Q0FMKFN0cmluZywgdHlwZSwgKCJvYmplY3QiKSk7Ci0gICAgaWYgKHVybC5wcm90b2NvbElzKCJh Ym91dCIpKQorICAgIGlmICh1cmwuaXNCbGFua1VSTCgpKQogICAgICAgICByZXR1cm4gdHJ1ZTsK ICAgICByZXR1cm4gY2hlY2tTb3VyY2VBbmRSZXBvcnRWaW9sYXRpb24ob3BlcmF0aXZlRGlyZWN0 aXZlKG1fb2JqZWN0U3JjLmdldCgpKSwgdXJsLCB0eXBlKTsKIH0KQEAgLTY5OSw3ICs2OTksNyBA QCBib29sIENTUERpcmVjdGl2ZUxpc3Q6OmFsbG93T2JqZWN0RnJvbVNvdXJjZShjb25zdCBLVVJM JiB1cmwpIGNvbnN0CiBib29sIENTUERpcmVjdGl2ZUxpc3Q6OmFsbG93Q2hpbGRGcmFtZUZyb21T b3VyY2UoY29uc3QgS1VSTCYgdXJsKSBjb25zdAogewogICAgIERFRklORV9TVEFUSUNfTE9DQUwo U3RyaW5nLCB0eXBlLCAoImZyYW1lIikpOwotICAgIGlmICh1cmwucHJvdG9jb2xJcygiYWJvdXQi KSkKKyAgICBpZiAodXJsLmlzQmxhbmtVUkwoKSkKICAgICAgICAgcmV0dXJuIHRydWU7CiAgICAg cmV0dXJuIGNoZWNrU291cmNlQW5kUmVwb3J0VmlvbGF0aW9uKG9wZXJhdGl2ZURpcmVjdGl2ZSht X2ZyYW1lU3JjLmdldCgpKSwgdXJsLCB0eXBlKTsKIH0KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJD b3JlL3BhZ2UvUGFnZVNlcmlhbGl6ZXIuY3BwIGIvU291cmNlL1dlYkNvcmUvcGFnZS9QYWdlU2Vy aWFsaXplci5jcHAKaW5kZXggZmRmODY5MDVjZGE3MmNhODI0ZGIwNWVmMmEwOTZkNjg1MWVlNTE4 Mi4uMzYzZWRkOTdkMjEwZDJhMzIyODNhNGQ4N2Y3NTRhZGRjYjAzYTVmMiAxMDA2NDQKLS0tIGEv U291cmNlL1dlYkNvcmUvcGFnZS9QYWdlU2VyaWFsaXplci5jcHAKKysrIGIvU291cmNlL1dlYkNv cmUvcGFnZS9QYWdlU2VyaWFsaXplci5jcHAKQEAgLTE1NCw3ICsxNTQsNyBAQCB2b2lkIFNlcmlh bGl6ZXJNYXJrdXBBY2N1bXVsYXRvcjo6YXBwZW5kQ3VzdG9tQXR0cmlidXRlcyhTdHJpbmdCdWls ZGVyJiBvdXQsIEVsZQogICAgICAgICByZXR1cm47CiAKICAgICBLVVJMIHVybCA9IGZyYW1lLT5k b2N1bWVudCgpLT51cmwoKTsKLSAgICBpZiAodXJsLmlzVmFsaWQoKSAmJiAhdXJsLnByb3RvY29s SXMoImFib3V0IikpCisgICAgaWYgKHVybC5pc1ZhbGlkKCkgJiYgIXVybC5pc0JsYW5rVVJMKCkp CiAgICAgICAgIHJldHVybjsKIAogICAgIC8vIFdlIG5lZWQgdG8gZ2l2ZSBhIGZha2UgbG9jYXRp b24gdG8gYmxhbmsgZnJhbWVzIHNvIHRoZXkgY2FuIGJlIHJlZmVyZW5jZWQgYnkgdGhlIHNlcmlh bGl6ZWQgZnJhbWUuCkBAIC0xOTQsNyArMTk0LDcgQEAgdm9pZCBQYWdlU2VyaWFsaXplcjo6c2Vy aWFsaXplRnJhbWUoRnJhbWUqIGZyYW1lKQogewogICAgIERvY3VtZW50KiBkb2N1bWVudCA9IGZy YW1lLT5kb2N1bWVudCgpOwogICAgIEtVUkwgdXJsID0gZG9jdW1lbnQtPnVybCgpOwotICAgIGlm ICghdXJsLmlzVmFsaWQoKSB8fCB1cmwucHJvdG9jb2xJcygiYWJvdXQiKSkgeworICAgIGlmICgh dXJsLmlzVmFsaWQoKSB8fCB1cmwuaXNCbGFua1VSTCgpKSB7CiAgICAgICAgIC8vIEZvciBibGFu ayBmcmFtZXMgd2UgZ2VuZXJhdGUgYSBmYWtlIFVSTCBzbyB0aGV5IGNhbiBiZSByZWZlcmVuY2Vk IGJ5IHRoZWlyIGNvbnRhaW5pbmcgZnJhbWUuCiAgICAgICAgIHVybCA9IHVybEZvckJsYW5rRnJh bWUoZnJhbWUpOwogICAgIH0KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL0tV UkwuY3BwIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vS1VSTC5jcHAKaW5kZXggMmI2MmVkNmZi YzY4N2U2MzJiMzc1ODdkNzM1N2Y4OTgwNGMxN2MyNC4uMGI0NmVjMGZjNzZmMGY3OWM4OTk0Y2Uz MzBiNjAxMzE1YTE0YTUxNSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vS1VS TC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vS1VSTC5jcHAKQEAgLTE3NzIsNiAr MTc3MiwxMSBAQCBjb25zdCBLVVJMJiBibGFua1VSTCgpCiAgICAgcmV0dXJuIHN0YXRpY0JsYW5r VVJMOwogfQogCitib29sIEtVUkw6OmlzQmxhbmtVUkwoKSBjb25zdAoreworICAgIHJldHVybiBw cm90b2NvbElzKCJhYm91dCIpOworfQorCiBib29sIGlzRGVmYXVsdFBvcnRGb3JQcm90b2NvbCh1 bnNpZ25lZCBzaG9ydCBwb3J0LCBjb25zdCBTdHJpbmcmIHByb3RvY29sKQogewogICAgIGlmIChw cm90b2NvbC5pc0VtcHR5KCkpCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9L VVJMLmggYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9LVVJMLmgKaW5kZXggM2NiYTA5MGFhYmM1 YWFiODVlZjU5NTY4MjNmMzgzZGYxOTMxZjZiNi4uZWU2N2UxNmZjOWYwNGMzNDQ2YTE3NmI5ZDRm NmQ5NWY0M2IxYTA2YSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vS1VSTC5o CisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL0tVUkwuaApAQCAtMTU1LDYgKzE1NSw3IEBA IHB1YmxpYzoKICAgICBib29sIHByb3RvY29sSXNEYXRhKCkgY29uc3QgeyByZXR1cm4gcHJvdG9j b2xJcygiZGF0YSIpOyB9CiAgICAgYm9vbCBwcm90b2NvbElzSW5IVFRQRmFtaWx5KCkgY29uc3Q7 CiAgICAgYm9vbCBpc0xvY2FsRmlsZSgpIGNvbnN0OworICAgIGJvb2wgaXNCbGFua1VSTCgpIGNv bnN0OwogCiAgICAgYm9vbCBzZXRQcm90b2NvbChjb25zdCBTdHJpbmcmKTsKICAgICB2b2lkIHNl dEhvc3QoY29uc3QgU3RyaW5nJik7CkBAIC0yNzMsNyArMjc0LDcgQEAgYm9vbCBvcGVyYXRvciE9 KGNvbnN0IFN0cmluZyYsIGNvbnN0IEtVUkwmKTsKIAogYm9vbCBlcXVhbElnbm9yaW5nRnJhZ21l bnRJZGVudGlmaWVyKGNvbnN0IEtVUkwmLCBjb25zdCBLVVJMJik7CiBib29sIHByb3RvY29sSG9z dEFuZFBvcnRBcmVFcXVhbChjb25zdCBLVVJMJiwgY29uc3QgS1VSTCYpOwotICAgIAorCiBjb25z dCBLVVJMJiBibGFua1VSTCgpOwogCiAvLyBGdW5jdGlvbnMgdG8gZG8gVVJMIG9wZXJhdGlvbnMg b24gc3RyaW5ncy4K