Bug 209853 – Remove FrameLoader::addExtraFieldsToMainResourceRequest

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Rob Buis]

Remove FrameLoader::addExtraFieldsToMainResourceRequest since the call is
not needed in DocumentLoader and can be inlined in FrameLoader. The call
in DocumentLoader is no longer needed since adding the User-Agent header
is decoupled from addExtraFields functionality and the User-Agent header
will be added in CachedResourceLoader after any custom setting of the
user agent (setCustomUserAgent API).

[Rob Buis]

Created attachment 395158 [details]
Patch

[Rob Buis]

Created attachment 395159 [details]
Patch

[Rob Buis]

Created attachment 395164 [details]
Patch

[Darin Adler]

Comment on attachment 395164 [details]
Patch

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

Code is fine, comments not so good

> Source/WebCore/loader/DocumentLoader.cpp:1822
>      // Make sure we re-apply the user agent to the Document's ResourceRequest upon reload in case the embedding
>      // application has changed it.

The code below *clears* the user agent. The comment says "make sure we re-apply the user agent". Could you reword it to clarify what it means or remove it? Why is it valuable to clear the user agent here? If CachedResourceLoader is going to add it, why do we need to touch it at all here? Do we need to clear it because CachedResourceLoader only overwrites it and doesn’t clear it? Is it valuable to clear it to save memory or something?

> Source/WebCore/loader/FrameLoader.cpp:1525
> +    // FIXME: Using m_loadType seems wrong for some callers.
> +    // If we are only preparing to load the main resource, that is previous load's load type!

The wording of this comment no longer makes sense. It said "some callers" and that meant "some callers of the addExtraFieldsToMainResourceRequest function". Need to rewrite the comment to make logical sense. I like keeping it, but needs rewording.

[Rob Buis]

Created attachment 395182 [details]
Patch

[EWS]

Committed r259379: <https://trac.webkit.org/changeset/259379>

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

[Radar WebKit Bug Importer]

<rdar://problem/61195938>