Bug 206426

Summary: Add logging of the source of a response used to respond a fetch event
Product: WebKit Reporter: youenn fablet <youennf>
Component: Service WorkersAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, commit-queue, darin, dbates, ews-watchlist, hi, japhet, joepeck, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing
none
Patch for landing none

youenn fablet
Reported 2020-01-17 09:48:46 PST
Add logging of the source of a response used to respond a fetch event
Attachments
Patch (10.44 KB, patch)
2020-01-17 09:54 PST, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch for landing (11.33 KB, patch)
2020-01-21 07:24 PST, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch for landing (11.40 KB, patch)
2020-01-22 05:05 PST, youenn fablet 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
youenn fablet
Comment 1 2020-01-17 09:54:38 PST
Created attachment 388052 [details] Patch
Darin Adler
Comment 2 2020-01-20 12:35:17 PST
Comment on attachment 388052 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=388052&action=review > Source/WebCore/Modules/cache/DOMCache.cpp:578 > + auto resourceResponse = WTFMove(record.response); > + resourceResponse.setSource(ResourceResponse::Source::DOMCache); We do not need to move this into a local variable to modify it. We can just write: record.response.setSource(ResourceResponse::Source::DOMCache); and not make the other changes. Basically, an rvalue reference gives us the permission to modify and/or take the value of something. No reason we need to move it to modify it. If we were copying rather than moving, then yes, it’s important to copy before modifying something. But that’s not the case here. > Source/WebCore/Modules/cache/DOMCache.cpp:591 > + auto resourceResponse = WTFMove(record.response); > + resourceResponse.setSource(ResourceResponse::Source::DOMCache); Ditto.
youenn fablet
Comment 3 2020-01-21 07:24:41 PST
Created attachment 388297 [details] Patch for landing
youenn fablet
Comment 4 2020-01-21 07:25:33 PST
> We do not need to move this into a local variable to modify it. We can just > write: > > record.response.setSource(ResourceResponse::Source::DOMCache); Right, fixed in latest patch. I also added a small internals test to verify the response source is correctly set if coming from DOM cache.
youenn fablet
Comment 5 2020-01-22 05:05:41 PST
Created attachment 388416 [details] Patch for landing
WebKit Commit Bot
Comment 6 2020-01-22 06:26:15 PST
Comment on attachment 388416 [details] Patch for landing Clearing flags on attachment: 388416 Committed r254919: <https://trac.webkit.org/changeset/254919>
WebKit Commit Bot
Comment 7 2020-01-22 06:26:17 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2020-01-22 06:27:15 PST
<rdar://problem/58795155>
Note You need to log in before you can comment on or make changes to this bug.