97076 2012-09-18 23:38:08 -0700 [BlackBerry] Empty URL gets through to acceptNavigationRequest() 2012-09-27 23:09:52 -0700 1 1 1 Unclassified WebKit WebKit BlackBerry 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 charles.wei charles.wei mifenton rwlbuis staikos tonikitoo webkit.review.bot oldest_to_newest 723321 0 charles.wei 2012-09-18 23:38:08 -0700 Internal PR: 205008 invalid URL will result in an empty URL in platform request, we should stop loading of an empty URL. 723333 1 164674 charles.wei 2012-09-19 00:03:19 -0700 Created attachment 164674 Patch 728217 2 164674 staikos 2012-09-25 23:01:09 -0700 Comment on attachment 164674 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164674&action=review r- to make the diff smaller. > Source/WebKit/blackberry/WebCoreSupport/FrameLoaderClientBlackBerry.cpp:221 > + if (!platformRequest.getUrlRef().empty()) { // Some invalid URLs will result in empty URL in platformRequest Let's make this an early return instead? 728272 3 164674 charles.wei 2012-09-26 00:30:20 -0700 Comment on attachment 164674 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=164674&action=review >> Source/WebKit/blackberry/WebCoreSupport/FrameLoaderClientBlackBerry.cpp:221 >> + if (!platformRequest.getUrlRef().empty()) { // Some invalid URLs will result in empty URL in platformRequest > > Let's make this an early return instead? We should not early return. look at the last several lines of this block, we should go there to call : dispatchDidCancelClientRedirect(), policyChecker()->*function)(decision), didDecidePolicyForNavigationAction(action, request) to do some clean up work no matter what the decision is. So I don't think we should do an early return here. 728275 4 164674 charles.wei 2012-09-26 00:31:11 -0700 Comment on attachment 164674 Patch Re-raise the review flag since I don't agree with the reviewer's comments, which seems wrong. 730245 5 164674 staikos 2012-09-27 22:22:04 -0700 Comment on attachment 164674 Patch right. 730267 6 164674 webkit.review.bot 2012-09-27 23:09:48 -0700 Comment on attachment 164674 Patch Clearing flags on attachment: 164674 Committed r129853: <http://trac.webkit.org/changeset/129853> 730268 7 webkit.review.bot 2012-09-27 23:09:52 -0700 All reviewed patches have been landed. Closing bug. 164674 2012-09-19 00:03:19 -0700 2012-09-27 23:09:48 -0700 Patch bug-97076-20120919150244.patch text/plain 3682 charles.wei U3VidmVyc2lvbiBSZXZpc2lvbjogMTI4OTYwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L2Js YWNrYmVycnkvQ2hhbmdlTG9nIGIvU291cmNlL1dlYktpdC9ibGFja2JlcnJ5L0NoYW5nZUxvZwpp bmRleCA3NjJkNzY5OGVlNmUwNGNlYzQ2Mjg2ZTEyNmQ0MTA4MTgzZjcxN2EyLi43NGYzNWMwY2Uy ZTgyNDQxODRkYjhjMGFlMDRhYWYyNWY1YTlkYTA2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 L2JsYWNrYmVycnkvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJLaXQvYmxhY2tiZXJyeS9DaGFu Z2VMb2cKQEAgLTEsMyArMSwxNiBAQAorMjAxMi0wOS0xOCAgQ2hhcmxlcyBXZWkgIDxjaGFybGVz LndlaUB0b3JjaG1vYmlsZS5jb20uY24+CisKKyAgICAgICAgRW1wdHkgVVJMIGdldHMgdGhyb3Vn aCB0byBhY2NlcHROYXZpZ2F0aW9uUmVxdWVzdCgpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD05NzA3NgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIEludmFsaWQgVVJMcyBpbiBpZnJhbWUgc3JjIHJlc3VsdHMg aW4gYW4gZW1wdHkgVVJMIGluIHBsYXRmb3JtIHJlcXVlc3QsIHdlIHNob3VsZCByZWplY3QKKyAg ICAgICAgc3VjaCBVUkwgYXMgZWFybHkgYXMgd2UgaWRlbnRpZnkgaXQuCisKKyAgICAgICAgKiBX ZWJDb3JlU3VwcG9ydC9GcmFtZUxvYWRlckNsaWVudEJsYWNrQmVycnkuY3BwOgorICAgICAgICAo V2ViQ29yZTo6RnJhbWVMb2FkZXJDbGllbnRCbGFja0JlcnJ5OjpkaXNwYXRjaERlY2lkZVBvbGlj eUZvck5hdmlnYXRpb25BY3Rpb24pOgorCiAyMDEyLTA5LTE4ICBOaW1hIEdoYW5hdmF0aWFuICA8 bmdoYW5hdmF0aWFuQHJpbS5jb20+CiAKICAgICAgICAgW0JsYWNrQmVycnldIFVzZSBkaWRDYW5j ZWwgYW5kIGRpZFN1Y2NlZWQgaW5zdGVhZCBvZiBkaWRDaGVja0NhbmNlbCBhbmQgZGlkQ2hlY2tT dWNjZWVkCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L2JsYWNrYmVycnkvV2ViQ29yZVN1cHBv cnQvRnJhbWVMb2FkZXJDbGllbnRCbGFja0JlcnJ5LmNwcCBiL1NvdXJjZS9XZWJLaXQvYmxhY2ti ZXJyeS9XZWJDb3JlU3VwcG9ydC9GcmFtZUxvYWRlckNsaWVudEJsYWNrQmVycnkuY3BwCmluZGV4 IDhkMWY2NDAwMDg1ZThiZDUzMzAzMGNmNjM3N2JjMTM4YThjMThmMzAuLjQ0ZmI5MWMyZGZhMjNi NmUzMjVlMTI2NzY2OTQwYmZiMGQ0YWQ1MzkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvYmxh Y2tiZXJyeS9XZWJDb3JlU3VwcG9ydC9GcmFtZUxvYWRlckNsaWVudEJsYWNrQmVycnkuY3BwCisr KyBiL1NvdXJjZS9XZWJLaXQvYmxhY2tiZXJyeS9XZWJDb3JlU3VwcG9ydC9GcmFtZUxvYWRlckNs aWVudEJsYWNrQmVycnkuY3BwCkBAIC0yMTgsMTggKzIxOCwyMCBAQCB2b2lkIEZyYW1lTG9hZGVy Q2xpZW50QmxhY2tCZXJyeTo6ZGlzcGF0Y2hEZWNpZGVQb2xpY3lGb3JOYXZpZ2F0aW9uQWN0aW9u KEZyYW1lUAogICAgICAgICAvLyBMZXQgdGhlIGNsaWVudCBoYXZlIGEgY2hhbmNlIHRvIHNheSB3 aGV0aGVyIHRoaXMgbmF2aWdhdGlvbiBzaG91bGQgYmUgaWdub3JlZCBvciBub3QuCiAgICAgICAg IE5ldHdvcmtSZXF1ZXN0IHBsYXRmb3JtUmVxdWVzdDsKICAgICAgICAgcmVxdWVzdC5pbml0aWFs aXplUGxhdGZvcm1SZXF1ZXN0KHBsYXRmb3JtUmVxdWVzdCwgY29va2llc0VuYWJsZWQoKSk7Ci0g ICAgICAgIGlmIChwbGF0Zm9ybVJlcXVlc3QuZ2V0VGFyZ2V0VHlwZSgpID09IE5ldHdvcmtSZXF1 ZXN0OjpUYXJnZXRJc1Vua25vd24pCi0gICAgICAgICAgICBwbGF0Zm9ybVJlcXVlc3Quc2V0VGFy Z2V0VHlwZShpc01haW5GcmFtZSgpID8gTmV0d29ya1JlcXVlc3Q6OlRhcmdldElzTWFpbkZyYW1l IDogTmV0d29ya1JlcXVlc3Q6OlRhcmdldElzU3ViZnJhbWUpOwotCi0gICAgICAgIGlmICghbV93 ZWJQYWdlUHJpdmF0ZS0+bV9jbGllbnQtPmFjY2VwdE5hdmlnYXRpb25SZXF1ZXN0KHBsYXRmb3Jt UmVxdWVzdCwgQmxhY2tCZXJyeTo6UGxhdGZvcm06Ok5hdmlnYXRpb25UeXBlKGFjdGlvbi50eXBl KCkpKSkgewotICAgICAgICAgICAgZGVjaXNpb24gPSBQb2xpY3lJZ25vcmU7Ci0gICAgICAgICAg ICBpZiAoaXNNYWluRnJhbWUoKSkgewotICAgICAgICAgICAgICAgIGlmIChhY3Rpb24udHlwZSgp ID09IE5hdmlnYXRpb25UeXBlRm9ybVN1Ym1pdHRlZCB8fCBhY3Rpb24udHlwZSgpID09IE5hdmln YXRpb25UeXBlRm9ybVJlc3VibWl0dGVkKQotICAgICAgICAgICAgICAgICAgICBtX2ZyYW1lLT5s b2FkZXIoKS0+cmVzZXRNdWx0aXBsZUZvcm1TdWJtaXNzaW9uUHJvdGVjdGlvbigpOwotCi0gICAg ICAgICAgICAgICAgaWYgKGFjdGlvbi50eXBlKCkgPT0gTmF2aWdhdGlvblR5cGVMaW5rQ2xpY2tl ZCAmJiB1cmwuaGFzRnJhZ21lbnRJZGVudGlmaWVyKCkpIHsKLSAgICAgICAgICAgICAgICAgICAg UmVzb3VyY2VSZXF1ZXN0IGVtcHR5UmVxdWVzdDsKLSAgICAgICAgICAgICAgICAgICAgbV9mcmFt ZS0+bG9hZGVyKCktPmFjdGl2ZURvY3VtZW50TG9hZGVyKCktPnNldExhc3RDaGVja2VkUmVxdWVz dChlbXB0eVJlcXVlc3QpOworICAgICAgICBpZiAoIXBsYXRmb3JtUmVxdWVzdC5nZXRVcmxSZWYo KS5lbXB0eSgpKSB7IC8vIFNvbWUgaW52YWxpZCBVUkxzIHdpbGwgcmVzdWx0IGluIGVtcHR5IFVS TCBpbiBwbGF0Zm9ybVJlcXVlc3QKKyAgICAgICAgICAgIGlmIChwbGF0Zm9ybVJlcXVlc3QuZ2V0 VGFyZ2V0VHlwZSgpID09IE5ldHdvcmtSZXF1ZXN0OjpUYXJnZXRJc1Vua25vd24pCisgICAgICAg ICAgICAgICAgcGxhdGZvcm1SZXF1ZXN0LnNldFRhcmdldFR5cGUoaXNNYWluRnJhbWUoKSA/IE5l dHdvcmtSZXF1ZXN0OjpUYXJnZXRJc01haW5GcmFtZSA6IE5ldHdvcmtSZXF1ZXN0OjpUYXJnZXRJ c1N1YmZyYW1lKTsKKworICAgICAgICAgICAgaWYgKCFtX3dlYlBhZ2VQcml2YXRlLT5tX2NsaWVu dC0+YWNjZXB0TmF2aWdhdGlvblJlcXVlc3QocGxhdGZvcm1SZXF1ZXN0LCBCbGFja0JlcnJ5OjpQ bGF0Zm9ybTo6TmF2aWdhdGlvblR5cGUoYWN0aW9uLnR5cGUoKSkpKSB7CisgICAgICAgICAgICAg ICAgZGVjaXNpb24gPSBQb2xpY3lJZ25vcmU7CisgICAgICAgICAgICAgICAgaWYgKGlzTWFpbkZy YW1lKCkpIHsKKyAgICAgICAgICAgICAgICAgICAgaWYgKGFjdGlvbi50eXBlKCkgPT0gTmF2aWdh dGlvblR5cGVGb3JtU3VibWl0dGVkIHx8IGFjdGlvbi50eXBlKCkgPT0gTmF2aWdhdGlvblR5cGVG b3JtUmVzdWJtaXR0ZWQpCisgICAgICAgICAgICAgICAgICAgICAgICBtX2ZyYW1lLT5sb2FkZXIo KS0+cmVzZXRNdWx0aXBsZUZvcm1TdWJtaXNzaW9uUHJvdGVjdGlvbigpOworCisgICAgICAgICAg ICAgICAgICAgIGlmIChhY3Rpb24udHlwZSgpID09IE5hdmlnYXRpb25UeXBlTGlua0NsaWNrZWQg JiYgdXJsLmhhc0ZyYWdtZW50SWRlbnRpZmllcigpKSB7CisgICAgICAgICAgICAgICAgICAgICAg ICBSZXNvdXJjZVJlcXVlc3QgZW1wdHlSZXF1ZXN0OworICAgICAgICAgICAgICAgICAgICAgICAg bV9mcmFtZS0+bG9hZGVyKCktPmFjdGl2ZURvY3VtZW50TG9hZGVyKCktPnNldExhc3RDaGVja2Vk UmVxdWVzdChlbXB0eVJlcXVlc3QpOworICAgICAgICAgICAgICAgICAgICB9CiAgICAgICAgICAg ICAgICAgfQogICAgICAgICAgICAgfQogICAgICAgICB9Cg==