| Summary: | Add logging of the source of a response used to respond a fetch event | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||
| Component: | Service Workers | Assignee: | 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
youenn fablet
2020-01-17 09:48:46 PST
Created attachment 388052 [details]
Patch
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. Created attachment 388297 [details]
Patch for landing
> 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.
Created attachment 388416 [details]
Patch for landing
Comment on attachment 388416 [details] Patch for landing Clearing flags on attachment: 388416 Committed r254919: <https://trac.webkit.org/changeset/254919> All reviewed patches have been landed. Closing bug. |