Bug 209853

Summary: Remove FrameLoader::addExtraFieldsToMainResourceRequest
Product: WebKit Reporter: Rob Buis <rbuis>
Component: New BugsAssignee: Rob Buis <rbuis>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, darin, ews-watchlist, japhet, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Rob Buis
Reported 2020-04-01 04:49:16 PDT
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).
Attachments
Patch (4.33 KB, patch)
2020-04-01 04:58 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (4.23 KB, patch)
2020-04-01 05:35 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (4.22 KB, patch)
2020-04-01 07:25 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (4.35 KB, patch)
2020-04-01 10:09 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Rob Buis
Comment 1 2020-04-01 04:58:36 PDT
Created attachment 395158 [details] Patch
Rob Buis
Comment 2 2020-04-01 05:35:58 PDT
Created attachment 395159 [details] Patch
Rob Buis
Comment 3 2020-04-01 07:25:43 PDT
Created attachment 395164 [details] Patch
Darin Adler
Comment 4 2020-04-01 09:54:10 PDT
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
Comment 5 2020-04-01 10:09:26 PDT
Created attachment 395182 [details] Patch
EWS
Comment 6 2020-04-02 00:43:19 PDT
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
Comment 7 2020-04-02 00:44:14 PDT
<rdar://problem/61195938>
Note You need to log in before you can comment on or make changes to this bug.