Bug 211544 – Add some missing null checks for DocumentLoader

• Component: Page Loading
• Status: RESOLVED FIXED

[Darin Adler]

Add some missing null checks for DocumentLoader

[Darin Adler]

Created attachment 398696 [details]
Patch

[Darin Adler]

rdar://62843516

[Anders Carlsson]

Comment on attachment 398696 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=398696&action=review

> Source/WebCore/loader/FrameLoader.cpp:2173
> +    auto dl = m_documentLoader;

I would have renamed this to documentLoader.

> Source/WebCore/loader/FrameLoader.cpp:2226
> +        m_documentLoader->writer().setMIMEType(dl->responseMIMEType());

Should this use dl instead of m_documentLoader? (Maybe there's a chance that m_documentLoader is replaced mid-function?)

> Source/WebCore/loader/HistoryController.cpp:417
> +    const URL& historyURL = m_frame.loader().documentLoader() ? m_frame.loader().documentLoader()->urlForHistory() : URL { };

I'd change this to no longer hold a const reference, since urlForHistory() returns a const&, it will always be copied anyway AFAICT.

[Darin Adler]

Comment on attachment 398696 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=398696&action=review

>> Source/WebCore/loader/FrameLoader.cpp:2173
>> +    auto dl = m_documentLoader;
> 
> I would have renamed this to documentLoader.

OK, I’ll do that.

>> Source/WebCore/loader/FrameLoader.cpp:2226
>> +        m_documentLoader->writer().setMIMEType(dl->responseMIMEType());
> 
> Should this use dl instead of m_documentLoader? (Maybe there's a chance that m_documentLoader is replaced mid-function?)

I thought the point was to set the MIME type on the new writer with the possibly-old loader’s MIME type. Not really sure.

>> Source/WebCore/loader/HistoryController.cpp:417
>> +    const URL& historyURL = m_frame.loader().documentLoader() ? m_frame.loader().documentLoader()->urlForHistory() : URL { };
> 
> I'd change this to no longer hold a const reference, since urlForHistory() returns a const&, it will always be copied anyway AFAICT.

Will do.

[Darin Adler]

Created attachment 398751 [details]
Patch

[Darin Adler]

Comment on attachment 398751 [details]
Patch

I plan to land this later with the commit queue.

[Darin Adler]

Created attachment 398752 [details]
Patch

[EWS]

Committed r261323: <https://trac.webkit.org/changeset/261323>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 398752 [details].

[Radar WebKit Bug Importer]

<rdar://problem/62986860>