106476 2013-01-09 11:18:03 -0800 Replace unnecessary null-checks with an assert in MainResourceLoader::continueAfterNavigationPolicy 2013-01-10 12:23:01 -0800 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 japhet japhet ap webkit.review.bot oldest_to_newest 803515 0 japhet 2013-01-09 11:18:03 -0800 This was requested in https://bugs.webkit.org/show_bug.cgi?id=106123#c20 I've convinced myself that resourceLoader will be non-null so long as continueAfterNavigationPolicy() isn't called twice with valid SubstituteData. Given that continueAfterNavigationPolicy() is only called for redirects and SubstituteData doesn't support redirects, I believe this assumptions always hold. 803525 1 181957 japhet 2013-01-09 11:28:49 -0800 Created attachment 181957 patch 803627 2 181957 ap 2013-01-09 12:53:32 -0800 Comment on attachment 181957 patch View in context: https://bugs.webkit.org/attachment.cgi?id=181957&action=review > Source/WebCore/loader/MainResourceLoader.cpp:192 > + ASSERT(resourceLoader && resourceLoader->shouldSendResourceLoadCallbacks()); Please break this into two lines, to make it easier to know which part failed. I'm still unconvinced that we need to assert that resourceLoader is not null though. 804559 3 japhet 2013-01-10 11:12:09 -0800 (In reply to comment #2) > (From update of attachment 181957 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=181957&action=review > > > Source/WebCore/loader/MainResourceLoader.cpp:192 > > + ASSERT(resourceLoader && resourceLoader->shouldSendResourceLoadCallbacks()); > > Please break this into two lines, to make it easier to know which part failed. > > I'm still unconvinced that we need to assert that resourceLoader is not null though. The more I think about it, the less convinced I am, too. Will land without the resourceLoader non-null assert. 804577 4 182177 japhet 2013-01-10 11:23:55 -0800 Created attachment 182177 Patch for landing 804631 5 182177 webkit.review.bot 2013-01-10 12:22:58 -0800 Comment on attachment 182177 Patch for landing Clearing flags on attachment: 182177 Committed r139350: <http://trac.webkit.org/changeset/139350> 804632 6 webkit.review.bot 2013-01-10 12:23:01 -0800 All reviewed patches have been landed. Closing bug. 181957 2013-01-09 11:28:49 -0800 2013-01-10 11:23:53 -0800 patch rl.txt text/plain 2326 japhet SW5kZXg6IFNvdXJjZS9XZWJDb3JlL2xvYWRlci9NYWluUmVzb3VyY2VMb2FkZXIuY3BwCj09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0KLS0tIFNvdXJjZS9XZWJDb3JlL2xvYWRlci9NYWluUmVzb3VyY2VMb2FkZXIuY3BwCShy ZXZpc2lvbiAxMzkxOTYpCisrKyBTb3VyY2UvV2ViQ29yZS9sb2FkZXIvTWFpblJlc291cmNlTG9h ZGVyLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTg5LDEyICsxODksMTAgQEAKICAgICAgICAgLy8g SG93ZXZlciwgZnJvbSBhbiBBUEkgcGVyc3BlY3RpdmUsIHRoaXMgaXNuJ3QgYSBjYW5jZWxsYXRp b24uIFRoZXJlZm9yZSwgc2V2ZXIgb3VyIHJlbGF0aW9uc2hpcCB3aXRoIHRoZSBuZXR3b3JrIGxv YWQgdmlhIGNsZWFyUmVzb3VyY2UoKSwKICAgICAgICAgLy8gYnV0IHByZXZlbnQgdGhlIFJlc291 cmNlTG9hZGVyIGZyb20gc2VuZGluZyBSZXNvdXJjZUxvYWROb3RpZmllciBjYWxsYmFja3MuCiAg ICAgICAgIFJlZlB0cjxSZXNvdXJjZUxvYWRlcj4gcmVzb3VyY2VMb2FkZXIgPSBsb2FkZXIoKTsK LSAgICAgICAgQVNTRVJUKCFyZXNvdXJjZUxvYWRlciB8fCByZXNvdXJjZUxvYWRlci0+c2hvdWxk U2VuZFJlc291cmNlTG9hZENhbGxiYWNrcygpKTsKLSAgICAgICAgaWYgKHJlc291cmNlTG9hZGVy KQotICAgICAgICAgICAgcmVzb3VyY2VMb2FkZXItPnNldFNlbmRDYWxsYmFja1BvbGljeShEb05v dFNlbmRDYWxsYmFja3MpOworICAgICAgICBBU1NFUlQocmVzb3VyY2VMb2FkZXIgJiYgcmVzb3Vy Y2VMb2FkZXItPnNob3VsZFNlbmRSZXNvdXJjZUxvYWRDYWxsYmFja3MoKSk7CisgICAgICAgIHJl c291cmNlTG9hZGVyLT5zZXRTZW5kQ2FsbGJhY2tQb2xpY3koRG9Ob3RTZW5kQ2FsbGJhY2tzKTsK ICAgICAgICAgY2xlYXJSZXNvdXJjZSgpOwotICAgICAgICBpZiAocmVzb3VyY2VMb2FkZXIpCi0g ICAgICAgICAgICByZXNvdXJjZUxvYWRlci0+c2V0U2VuZENhbGxiYWNrUG9saWN5KFNlbmRDYWxs YmFja3MpOworICAgICAgICByZXNvdXJjZUxvYWRlci0+c2V0U2VuZENhbGxiYWNrUG9saWN5KFNl bmRDYWxsYmFja3MpOwogICAgICAgICBoYW5kbGVTdWJzdGl0dXRlRGF0YUxvYWRTb29uKHJlcXVl c3QpOwogICAgIH0KIApJbmRleDogU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0K LS0tIFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTM5MjEzKQorKysgU291cmNl L1dlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTMt MDEtMDkgIE5hdGUgQ2hhcGluICA8amFwaGV0QGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXBs YWNlIHVubmVjZXNzYXJ5IG51bGwtY2hlY2tzIHdpdGggYW4gYXNzZXJ0IGluIE1haW5SZXNvdXJj ZUxvYWRlcjo6Y29udGludWVBZnRlck5hdmlnYXRpb25Qb2xpY3kuCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMDY0NzYKKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIGxvYWRlci9NYWluUmVzb3VyY2VMb2Fk ZXIuY3BwOgorICAgICAgICAoV2ViQ29yZTo6TWFpblJlc291cmNlTG9hZGVyOjpjb250aW51ZUFm dGVyTmF2aWdhdGlvblBvbGljeSk6IE51bGwtY2hlY2tzIGZvciByZXNvdXJjZUxvYWRlciB3ZXJl CisgICAgICAgICAgICBhZGRlZCBpbiBhbiBhYnVuZGFuY2Ugb2YgY2F1dGlvbiBpbiByMTM5MTUw LiBHaXZlbiB0aGF0IHdlIGtub3cgdGhlIGxvYWQgaGFzbid0IGJlZW4gY2FuY2VsbGVkLAorICAg ICAgICAgICAgcmVzb3VyY2VMb2FkZXIgY2FuIG9ubHkgYmUgbnVsbCBpZiBjb250aW51ZUFmdGVy TmF2aWdhdGlvblBvbGljeSgpIGhhcyBhbHJlYWR5IGJlZW4gY2FsbGVkCisgICAgICAgICAgICBi ZWZvcmUgd2hlbiBtX3N1YnN0aXR1dGVEYXRhIGlzIHZhbGlkLiBjb250aW51ZUFmdGVyTmF2aWdh dGlvblBvbGljeSgpIGlzIG9ubHkgY2FsbGVkIGZvciByZWRpcmVjdHMsCisgICAgICAgICAgICBh bmQgU3Vic3RpdHV0ZURhdGEgZG9lc24ndCBzdXBwb3J0IHJlZGlyZWN0cywgc28gaXQgaXMgY29y cmVjdCB0byBhc3NlcnQgdGhhdCByZXNvdXJjZUxvYWRlciBpcyBub24tbnVsbC4KKwogMjAxMy0w MS0wOSAgQWJoaXNoZWsgQXJ5YSAgPGluZmVybm9AY2hyb21pdW0ub3JnPgogCiAgICAgICAgIE1p dGlnYXRlIG91dC1vZi1ib3VuZHMgYWNjZXNzIGluIElubGluZUl0ZXJhdG9yCg== 182177 2013-01-10 11:23:55 -0800 2013-01-10 12:22:57 -0800 Patch for landing bug-106476-20130110112058.patch text/plain 2350 japhet SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDEzOTI0OSkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE3IEBACisyMDEzLTAxLTA5ICBOYXRlIENo YXBpbiAgPGphcGhldEBjaHJvbWl1bS5vcmc+CisKKyAgICAgICAgUmVwbGFjZSB1bm5lY2Vzc2Fy eSBudWxsLWNoZWNrcyB3aXRoIGFuIGFzc2VydCBpbiBNYWluUmVzb3VyY2VMb2FkZXI6OmNvbnRp bnVlQWZ0ZXJOYXZpZ2F0aW9uUG9saWN5LgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9y Zy9zaG93X2J1Zy5jZ2k/aWQ9MTA2NDc2CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgQWxleGV5IFBy b3NrdXJ5YWtvdi4KKworICAgICAgICAqIGxvYWRlci9NYWluUmVzb3VyY2VMb2FkZXIuY3BwOgor ICAgICAgICAoV2ViQ29yZTo6TWFpblJlc291cmNlTG9hZGVyOjpjb250aW51ZUFmdGVyTmF2aWdh dGlvblBvbGljeSk6IE51bGwtY2hlY2tzIGZvciByZXNvdXJjZUxvYWRlciB3ZXJlCisgICAgICAg ICAgICBhZGRlZCBpbiBhbiBhYnVuZGFuY2Ugb2YgY2F1dGlvbiBpbiByMTM5MTUwLiBHaXZlbiB0 aGF0IHdlIGtub3cgdGhlIGxvYWQgaGFzbid0IGJlZW4gY2FuY2VsbGVkLAorICAgICAgICAgICAg cmVzb3VyY2VMb2FkZXIgY2FuIG9ubHkgYmUgbnVsbCBpZiBjb250aW51ZUFmdGVyTmF2aWdhdGlv blBvbGljeSgpIGhhcyBhbHJlYWR5IGJlZW4gY2FsbGVkCisgICAgICAgICAgICBiZWZvcmUgd2hl biBtX3N1YnN0aXR1dGVEYXRhIGlzIHZhbGlkLiBjb250aW51ZUFmdGVyTmF2aWdhdGlvblBvbGlj eSgpIGlzIG9ubHkgY2FsbGVkIGZvciByZWRpcmVjdHMsCisgICAgICAgICAgICBhbmQgU3Vic3Rp dHV0ZURhdGEgZG9lc24ndCBzdXBwb3J0IHJlZGlyZWN0cywgc28gaXQgaXMgY29ycmVjdCB0byBh c3NlcnQgdGhhdCByZXNvdXJjZUxvYWRlciBpcyBub24tbnVsbC4KKwogMjAxMy0wMS0wOSAgVG9u eSBHZW50aWxjb3JlICA8dG9ueWdAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJlbW92ZSB1bnVz ZWQgaW5jbHVkZXMgZnJvbSBIVE1MVHJlZUJ1aWxkZXIKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL2xv YWRlci9NYWluUmVzb3VyY2VMb2FkZXIuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJDb3Jl L2xvYWRlci9NYWluUmVzb3VyY2VMb2FkZXIuY3BwCShyZXZpc2lvbiAxMzkyNDkpCisrKyBTb3Vy Y2UvV2ViQ29yZS9sb2FkZXIvTWFpblJlc291cmNlTG9hZGVyLmNwcAkod29ya2luZyBjb3B5KQpA QCAtMTg5LDEyICsxODksMTAgQEAgdm9pZCBNYWluUmVzb3VyY2VMb2FkZXI6OmNvbnRpbnVlQWZ0 ZXJOYQogICAgICAgICAvLyBIb3dldmVyLCBmcm9tIGFuIEFQSSBwZXJzcGVjdGl2ZSwgdGhpcyBp c24ndCBhIGNhbmNlbGxhdGlvbi4gVGhlcmVmb3JlLCBzZXZlciBvdXIgcmVsYXRpb25zaGlwIHdp dGggdGhlIG5ldHdvcmsgbG9hZCB2aWEgY2xlYXJSZXNvdXJjZSgpLAogICAgICAgICAvLyBidXQg cHJldmVudCB0aGUgUmVzb3VyY2VMb2FkZXIgZnJvbSBzZW5kaW5nIFJlc291cmNlTG9hZE5vdGlm aWVyIGNhbGxiYWNrcy4KICAgICAgICAgUmVmUHRyPFJlc291cmNlTG9hZGVyPiByZXNvdXJjZUxv YWRlciA9IGxvYWRlcigpOwotICAgICAgICBBU1NFUlQoIXJlc291cmNlTG9hZGVyIHx8IHJlc291 cmNlTG9hZGVyLT5zaG91bGRTZW5kUmVzb3VyY2VMb2FkQ2FsbGJhY2tzKCkpOwotICAgICAgICBp ZiAocmVzb3VyY2VMb2FkZXIpCi0gICAgICAgICAgICByZXNvdXJjZUxvYWRlci0+c2V0U2VuZENh bGxiYWNrUG9saWN5KERvTm90U2VuZENhbGxiYWNrcyk7CisgICAgICAgIEFTU0VSVChyZXNvdXJj ZUxvYWRlci0+c2hvdWxkU2VuZFJlc291cmNlTG9hZENhbGxiYWNrcygpKTsKKyAgICAgICAgcmVz b3VyY2VMb2FkZXItPnNldFNlbmRDYWxsYmFja1BvbGljeShEb05vdFNlbmRDYWxsYmFja3MpOwog ICAgICAgICBjbGVhclJlc291cmNlKCk7Ci0gICAgICAgIGlmIChyZXNvdXJjZUxvYWRlcikKLSAg ICAgICAgICAgIHJlc291cmNlTG9hZGVyLT5zZXRTZW5kQ2FsbGJhY2tQb2xpY3koU2VuZENhbGxi YWNrcyk7CisgICAgICAgIHJlc291cmNlTG9hZGVyLT5zZXRTZW5kQ2FsbGJhY2tQb2xpY3koU2Vu ZENhbGxiYWNrcyk7CiAgICAgICAgIGhhbmRsZVN1YnN0aXR1dGVEYXRhTG9hZFNvb24ocmVxdWVz dCk7CiAgICAgfQogCg==