Bug 106476 – Replace unnecessary null-checks with an assert in MainResourceLoader::continueAfterNavigationPolicy

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Nate Chapin]

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.

[Nate Chapin]

Created attachment 181957 [details]
patch

[Alexey Proskuryakov]

Comment on attachment 181957 [details]
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.

[Nate Chapin]

(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.

[Nate Chapin]

Created attachment 182177 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 182177 [details]
Patch for landing

Clearing flags on attachment: 182177

Committed r139350: <http://trac.webkit.org/changeset/139350>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.