63591 2011-06-28 18:15:33 -0700 [Chromium] isRedirect is incorrectly determined for decidePolicyForNavigation 2011-09-06 20:00:06 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 0 binji webkit-unassigned abarth webkit.review.bot oldest_to_newest 429339 0 99017 binji 2011-06-28 18:15:33 -0700 Created attachment 99017 Patch to fix this bug. In FrameLoaderClientImpl::dispatchDecidePolicyForNavigationAction() isRedirect is currently determined by hasRedirectChain(), which checks for an non-empty redirect chain. Instead, it should check for a redirect chain with more than one element. All valid redirects will have at least the source URL and the destination URL in the redirect chain. 429343 1 binji 2011-06-28 18:17:30 -0700 I don't believe a LayoutTest is necessary for this patch, because isRedirect is only used in chromium (for determining whether to fork a process), and has no user-visible effect in the browser. 429752 2 99017 abarth 2011-06-29 11:48:05 -0700 Comment on attachment 99017 Patch to fix this bug. All patch require a ChangeLog: http://www.webkit.org/coding/contributing.html 429753 3 99017 abarth 2011-06-29 11:51:07 -0700 Comment on attachment 99017 Patch to fix this bug. Is this testable with a Chromium WebKit API test? Generally, these sorts of subtle changes are easy to regress if we don't have any test coverage. If this is used for forking processes in the embedder, perhaps we can write a test at that layer? (Marking r- because of the lack of changelog.) 429770 4 binji 2011-06-29 12:10:28 -0700 Yes, I believe it would be possible to test at that level. Is it WebKit/Source/WebKit/chromium/tests/*? 459277 5 105710 binji 2011-08-30 15:40:22 -0700 Created attachment 105710 Patch 459935 6 binji 2011-08-31 14:36:29 -0700 It's been a while since I've had time to work on this one... :) Adam, can you review my new patch? I've included a test in WebFrameTest that exhibits the bug (when the fix in FrameLoaderClientImpl.cpp is removed). 459940 7 105710 abarth 2011-08-31 14:44:06 -0700 Comment on attachment 105710 Patch Looks great. Thanks for adding the ChangeLog and test. 463055 8 binji 2011-09-06 18:47:10 -0700 Adam, is there something else I need to do to commit this patch? It seems that it is in an infinite commit-queue. :) 463089 9 105710 abarth 2011-09-06 19:33:53 -0700 Comment on attachment 105710 Patch We just need to approve it for the queue! 463102 10 105710 webkit.review.bot 2011-09-06 20:00:01 -0700 Comment on attachment 105710 Patch Clearing flags on attachment: 105710 Committed r94630: <http://trac.webkit.org/changeset/94630> 463103 11 webkit.review.bot 2011-09-06 20:00:06 -0700 All reviewed patches have been landed. Closing bug. 99017 2011-06-28 18:15:33 -0700 2011-08-30 15:40:19 -0700 Patch to fix this bug. issue73104.patch text/plain 1328 binji ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vc3JjL0ZyYW1lTG9hZGVyQ2xpZW50 SW1wbC5jcHAgYi9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL3NyYy9GcmFtZUxvYWRlckNsaWVudElt cGwuY3BwCmluZGV4IDBlMDljNTIuLjhiZmVlMjYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQv Y2hyb21pdW0vc3JjL0ZyYW1lTG9hZGVyQ2xpZW50SW1wbC5jcHAKKysrIGIvU291cmNlL1dlYktp dC9jaHJvbWl1bS9zcmMvRnJhbWVMb2FkZXJDbGllbnRJbXBsLmNwcApAQCAtOTU2LDcgKzk1Niw3 IEBAIHZvaWQgRnJhbWVMb2FkZXJDbGllbnRJbXBsOjpkaXNwYXRjaERlY2lkZVBvbGljeUZvck5h dmlnYXRpb25BY3Rpb24oCiAgICAgICAgICAgICBLVVJMIHVybCA9IGRzLT5yZXF1ZXN0KCkudXJs KCk7CiAgICAgICAgICAgICBBU1NFUlQoIXVybC5wcm90b2NvbElzKGJhY2tGb3J3YXJkTmF2aWdh dGlvblNjaGVtZSkpOwogCi0gICAgICAgICAgICBib29sIGlzUmVkaXJlY3QgPSBkcy0+aGFzUmVk aXJlY3RDaGFpbigpOworICAgICAgICAgICAgYm9vbCBpc1JlZGlyZWN0ID0gZHMtPmlzUmVkaXJl Y3QoKTsKIAogICAgICAgICAgICAgV2ViTmF2aWdhdGlvblR5cGUgd2VibmF2VHlwZSA9CiAgICAg ICAgICAgICAgICAgV2ViRGF0YVNvdXJjZUltcGw6OnRvV2ViTmF2aWdhdGlvblR5cGUoYWN0aW9u LnR5cGUoKSk7CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL3NyYy9XZWJEYXRh U291cmNlSW1wbC5oIGIvU291cmNlL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViRGF0YVNvdXJjZUlt cGwuaAppbmRleCBiYzQ1ODM5Li44Y2MyYzgwIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L2No cm9taXVtL3NyYy9XZWJEYXRhU291cmNlSW1wbC5oCisrKyBiL1NvdXJjZS9XZWJLaXQvY2hyb21p dW0vc3JjL1dlYkRhdGFTb3VyY2VJbXBsLmgKQEAgLTc0LDYgKzc0LDcgQEAgcHVibGljOgogICAg IHN0YXRpYyBXZWJOYXZpZ2F0aW9uVHlwZSB0b1dlYk5hdmlnYXRpb25UeXBlKFdlYkNvcmU6Ok5h dmlnYXRpb25UeXBlIHR5cGUpOwogCiAgICAgYm9vbCBoYXNSZWRpcmVjdENoYWluKCkgY29uc3Qg eyByZXR1cm4gIW1fcmVkaXJlY3RDaGFpbi5pc0VtcHR5KCk7IH0KKyAgICBib29sIGlzUmVkaXJl Y3QoKSBjb25zdCB7IHJldHVybiBtX3JlZGlyZWN0Q2hhaW4uc2l6ZSgpID4gMTsgfQogICAgIGNv bnN0IFdlYkNvcmU6OktVUkwmIGVuZE9mUmVkaXJlY3RDaGFpbigpIGNvbnN0OwogICAgIHZvaWQg Y2xlYXJSZWRpcmVjdENoYWluKCk7CiAgICAgdm9pZCBhcHBlbmRSZWRpcmVjdChjb25zdCBXZWJD b3JlOjpLVVJMJiB1cmwpOwo= 105710 2011-08-30 15:40:22 -0700 2011-09-06 20:00:01 -0700 Patch bug-63591-20110830154021.patch text/plain 4679 binji U3VidmVyc2lvbiBSZXZpc2lvbjogOTQxMTYKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvY2hy b21pdW0vQ2hhbmdlTG9nIGIvU291cmNlL1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cKaW5kZXgg YTI5NjAwNTRlYzQ5YWM1ODE1YjJjMjFmMTU1Mzk2NTk4ZjVlN2M1OC4uZTI0YzVkMTFlNjQ2Y2Fj OTU4NTFjZjA0MDFkNmQ4NGNhOTQ5ZWIwZSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9jaHJv bWl1bS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdC9jaHJvbWl1bS9DaGFuZ2VMb2cKQEAg LTEsMyArMSwyMyBAQAorMjAxMS0wOC0zMCAgQmVuIFNtaXRoICA8YmluamlAY2hyb21pdW0ub3Jn PgorCisgICAgICAgIFtjaHJvbWl1bV0gRml4IFdlYkZyYW1lQ2xpZW50IHJlY2VpdmluZyBpc1Jl ZGlyZWN0IHdoZW4gcmVsb2FkaW5nCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD02MzU5MQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEp LgorCisgICAgICAgIEZpeCBidWcgd2hlbiBxdWlja2x5IHJlbG9hZGluZyBhIFdlYkZyYW1lIHdo ZXJlIHRoZQorICAgICAgICBXZWJGcmFtZUNsaWVudCB3b3VsZCBnZXQgY2FsbGVkIHdpdGggZGVj aWRlUG9saWN5Rm9yTmF2aWdhdGlvbgorICAgICAgICBpc1JlZGlyZWN0PXRydWUuCisKKyAgICAg ICAgKiBzcmMvRnJhbWVMb2FkZXJDbGllbnRJbXBsLmNwcDoKKyAgICAgICAgKFdlYktpdDo6RnJh bWVMb2FkZXJDbGllbnRJbXBsOjpkaXNwYXRjaERlY2lkZVBvbGljeUZvck5hdmlnYXRpb25BY3Rp b24pOgorICAgICAgICAqIHNyYy9XZWJEYXRhU291cmNlSW1wbC5oOgorICAgICAgICAoV2ViS2l0 OjpXZWJEYXRhU291cmNlSW1wbDo6aXNSZWRpcmVjdCk6CisgICAgICAgICogdGVzdHMvV2ViRnJh bWVUZXN0LmNwcDoKKyAgICAgICAgKFdlYktpdDo6VGVzdFJlbG9hZERvZXNudFJlZGlyZWN0V2Vi RnJhbWVDbGllbnQ6OmRlY2lkZVBvbGljeUZvck5hdmlnYXRpb24pOgorICAgICAgICAoV2ViS2l0 OjpUZXN0UmVsb2FkRG9lc250UmVkaXJlY3RXZWJGcmFtZUNsaWVudDo6Y2FuY2VsbGVkRXJyb3Ip OgorICAgICAgICAoV2ViS2l0OjpURVNUX0YpOgorCiAyMDExLTA4LTMwICBOYXQgRHVjYSAgPG5k dWNhQGNocm9taXVtLm9yZz4KIAogICAgICAgICBbY2hyb21pdW1dIEFsbG93IEdyYXBoaWNzQ29u dGV4dHMgdG8gYmUgY3JlYXRlZCBmcm9tIE1vY2tXZWJHcmFwaGljc0NvbnRleHRzCmRpZmYgLS1n aXQgYS9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL3NyYy9GcmFtZUxvYWRlckNsaWVudEltcGwuY3Bw IGIvU291cmNlL1dlYktpdC9jaHJvbWl1bS9zcmMvRnJhbWVMb2FkZXJDbGllbnRJbXBsLmNwcApp bmRleCBmOTQwYjRiOGI3ZTFlMzkyNzU1MTYzMGM4Y2Q4MTE5NzgzZjAzZWM3Li5jYmU0MTI5ZGU1 Y2YxYWM3NjY5YTk3YWRhM2VjZDQxNjYzMGVkOTE5IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 L2Nocm9taXVtL3NyYy9GcmFtZUxvYWRlckNsaWVudEltcGwuY3BwCisrKyBiL1NvdXJjZS9XZWJL aXQvY2hyb21pdW0vc3JjL0ZyYW1lTG9hZGVyQ2xpZW50SW1wbC5jcHAKQEAgLTk2MSw3ICs5NjEs NyBAQCB2b2lkIEZyYW1lTG9hZGVyQ2xpZW50SW1wbDo6ZGlzcGF0Y2hEZWNpZGVQb2xpY3lGb3JO YXZpZ2F0aW9uQWN0aW9uKAogICAgICAgICAgICAgS1VSTCB1cmwgPSBkcy0+cmVxdWVzdCgpLnVy bCgpOwogICAgICAgICAgICAgQVNTRVJUKCF1cmwucHJvdG9jb2xJcyhiYWNrRm9yd2FyZE5hdmln YXRpb25TY2hlbWUpKTsKIAotICAgICAgICAgICAgYm9vbCBpc1JlZGlyZWN0ID0gZHMtPmhhc1Jl ZGlyZWN0Q2hhaW4oKTsKKyAgICAgICAgICAgIGJvb2wgaXNSZWRpcmVjdCA9IGRzLT5pc1JlZGly ZWN0KCk7CiAKICAgICAgICAgICAgIFdlYk5hdmlnYXRpb25UeXBlIHdlYm5hdlR5cGUgPQogICAg ICAgICAgICAgICAgIFdlYkRhdGFTb3VyY2VJbXBsOjp0b1dlYk5hdmlnYXRpb25UeXBlKGFjdGlv bi50eXBlKCkpOwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViRGF0 YVNvdXJjZUltcGwuaCBiL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vc3JjL1dlYkRhdGFTb3VyY2VJ bXBsLmgKaW5kZXggYmM0NTgzOTc2NjdhYzgzNzZlODEyNzJiYmIxYzBiOGIzMTg5MmRlNi4uOGNj MmM4MGM2MDE0MjMwNGRmOWNhYzgzZDFiMTQxZTlmODIwZjNlNiAxMDA2NDQKLS0tIGEvU291cmNl L1dlYktpdC9jaHJvbWl1bS9zcmMvV2ViRGF0YVNvdXJjZUltcGwuaAorKysgYi9Tb3VyY2UvV2Vi S2l0L2Nocm9taXVtL3NyYy9XZWJEYXRhU291cmNlSW1wbC5oCkBAIC03NCw2ICs3NCw3IEBAIHB1 YmxpYzoKICAgICBzdGF0aWMgV2ViTmF2aWdhdGlvblR5cGUgdG9XZWJOYXZpZ2F0aW9uVHlwZShX ZWJDb3JlOjpOYXZpZ2F0aW9uVHlwZSB0eXBlKTsKIAogICAgIGJvb2wgaGFzUmVkaXJlY3RDaGFp bigpIGNvbnN0IHsgcmV0dXJuICFtX3JlZGlyZWN0Q2hhaW4uaXNFbXB0eSgpOyB9CisgICAgYm9v bCBpc1JlZGlyZWN0KCkgY29uc3QgeyByZXR1cm4gbV9yZWRpcmVjdENoYWluLnNpemUoKSA+IDE7 IH0KICAgICBjb25zdCBXZWJDb3JlOjpLVVJMJiBlbmRPZlJlZGlyZWN0Q2hhaW4oKSBjb25zdDsK ICAgICB2b2lkIGNsZWFyUmVkaXJlY3RDaGFpbigpOwogICAgIHZvaWQgYXBwZW5kUmVkaXJlY3Qo Y29uc3QgV2ViQ29yZTo6S1VSTCYgdXJsKTsKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvY2hy b21pdW0vdGVzdHMvV2ViRnJhbWVUZXN0LmNwcCBiL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vdGVz dHMvV2ViRnJhbWVUZXN0LmNwcAppbmRleCBkN2VlYzhhMGM0NTY2MTcwMjI0NTM3MDI1YWQ2NTI4 MjVlZWQ3NDhiLi43NzQzNjFmZmU3YTc0OTUyMDMyMjhjMzc0ZWUyOTY0N2ZhYWZmYTY2IDEwMDY0 NAotLS0gYS9Tb3VyY2UvV2ViS2l0L2Nocm9taXVtL3Rlc3RzL1dlYkZyYW1lVGVzdC5jcHAKKysr IGIvU291cmNlL1dlYktpdC9jaHJvbWl1bS90ZXN0cy9XZWJGcmFtZVRlc3QuY3BwCkBAIC0zMCw2 ICszMCw3IEBACiAKICNpbmNsdWRlICJjb25maWcuaCIKIAorI2luY2x1ZGUgIlJlc291cmNlRXJy b3IuaCIKICNpbmNsdWRlICJXZWJEb2N1bWVudC5oIgogI2luY2x1ZGUgIldlYkZvcm1FbGVtZW50 LmgiCiAjaW5jbHVkZSAiV2ViRnJhbWUuaCIKQEAgLTIxNSw0ICsyMTYsNDMgQEAgVEVTVF9GKFdl YkZyYW1lVGVzdCwgQ2hyb21lUGFnZU5vSmF2YXNjcmlwdCkKICAgICBFWFBFQ1RfRVEoc3RkOjpz dHJpbmc6Om5wb3MsIGNvbnRlbnQuZmluZCgiQ2xvYmJlcmVkIikpOwogfQogCitjbGFzcyBUZXN0 UmVsb2FkRG9lc250UmVkaXJlY3RXZWJGcmFtZUNsaWVudCA6IHB1YmxpYyBXZWJGcmFtZUNsaWVu dCB7CitwdWJsaWM6CisgICAgdmlydHVhbCBXZWJOYXZpZ2F0aW9uUG9saWN5IGRlY2lkZVBvbGlj eUZvck5hdmlnYXRpb24oCisgICAgICAgIFdlYkZyYW1lKiwgY29uc3QgV2ViVVJMUmVxdWVzdCYs IFdlYk5hdmlnYXRpb25UeXBlLAorICAgICAgICBjb25zdCBXZWJOb2RlJiBvcmlnaW5hdGluZ05v ZGUsCisgICAgICAgIFdlYk5hdmlnYXRpb25Qb2xpY3kgZGVmYXVsdFBvbGljeSwgYm9vbCBpc1Jl ZGlyZWN0KQorICAgIHsKKyAgICAgICAgRVhQRUNUX0VRKGZhbHNlLCBpc1JlZGlyZWN0KTsKKyAg ICAgICAgcmV0dXJuIFdlYk5hdmlnYXRpb25Qb2xpY3lDdXJyZW50VGFiOworICAgIH0KKworICAg IHZpcnR1YWwgV2ViVVJMRXJyb3IgY2FuY2VsbGVkRXJyb3IoV2ViRnJhbWUqLCBjb25zdCBXZWJV UkxSZXF1ZXN0JiByZXF1ZXN0KQorICAgIHsKKyAgICAgICAgLy8gUmV0dXJuIGEgZHVtbXkgZXJy b3Igc28gdGhlIERvY3VtZW50TG9hZGVyIGRvZXNuJ3QgYXNzZXJ0IHdoZW4KKyAgICAgICAgLy8g dGhlIHJlbG9hZCBjYW5jZWxzIGl0LgorICAgICAgICByZXR1cm4gV2ViVVJMRXJyb3IoV2ViQ29y ZTo6UmVzb3VyY2VFcnJvcigiIiwgMSwgIiIsICJjYW5jZWxsZWQiKSk7CisgICAgfQorfTsKKwor VEVTVF9GKFdlYkZyYW1lVGVzdCwgUmVsb2FkRG9lc250U2V0UmVkaXJlY3QpCit7CisgICAgLy8g VGVzdCBmb3IgY2FzZSBpbiBodHRwOi8vY3JidWcuY29tLzczMTA0LiBSZWxvYWRpbmcgYSBmcmFt ZSB2ZXJ5IHF1aWNrbHkKKyAgICAvLyB3b3VsZCBzb21ldGltZXMgY2FsbCBkZWNpZGVQb2xpY3lG b3JOYXZpZ2F0aW9uIHdpdGggaXNSZWRpcmVjdD10cnVlCisgICAgcmVnaXN0ZXJNb2NrZWRIdHRw VVJMTG9hZCgiZm9ybS5odG1sIik7CisKKyAgICBUZXN0UmVsb2FkRG9lc250UmVkaXJlY3RXZWJG cmFtZUNsaWVudCB3ZWJGcmFtZUNsaWVudDsKKyAgICBXZWJWaWV3KiB3ZWJWaWV3ID0gV2ViVmll dzo6Y3JlYXRlKDApOworICAgIHdlYlZpZXctPmluaXRpYWxpemVNYWluRnJhbWUoJndlYkZyYW1l Q2xpZW50KTsKKworICAgIGxvYWRIdHRwRnJhbWUod2ViVmlldy0+bWFpbkZyYW1lKCksICJmb3Jt Lmh0bWwiKTsKKyAgICBzZXJ2ZVJlcXVlc3RzKCk7CisgICAgLy8gRnJhbWUgaXMgbG9hZGVkLgor CisgICAgd2ViVmlldy0+bWFpbkZyYW1lKCktPnJlbG9hZCh0cnVlKTsKKyAgICAvLyBzdGFydCBy ZWxvYWQgYmVmb3JlIHJlcXVlc3QgaXMgZGVsaXZlcmVkLgorICAgIHdlYlZpZXctPm1haW5GcmFt ZSgpLT5yZWxvYWQodHJ1ZSk7CisgICAgc2VydmVSZXF1ZXN0cygpOworfQorCiB9IC8vIG5hbWVz cGFjZQo=