Summary: | Reuse navigation preload if service worker is fetching the corresponding navigation request | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||||||
Component: | Service Workers | Assignee: | youenn fablet <youennf> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | cdumez, ews-watchlist, japhet, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 233523 | ||||||||||||
Attachments: |
|
Description
youenn fablet
2021-11-25 03:25:35 PST
Created attachment 445131 [details]
Patch
Created attachment 445201 [details]
Patch
Created attachment 445405 [details]
Patch
Comment on attachment 445405 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445405&action=review r=me with comments. > Source/WebCore/loader/ResourceLoaderOptions.h:226 > + FetchIdentifier navigationPreloadIdentifier; We probably want to move this data member before the bitfield? > Source/WebKit/NetworkProcess/NetworkSession.h:178 > + ServiceWorkerFetchTask* navigationPreloaderTaskFromFetchIdentifier(WebCore::FetchIdentifier); Shouldn't this be const? > Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp:83 > + , m_timeoutTimer(makeUnique<Timer>(*this, &ServiceWorkerFetchTask::timeoutTimerFired)) I don't understand this change. You made it a pointer and you unconditionally initialize it. Yet, you never seem to null the pointer out. I don't understand the purpose of making it a pointer and null checking it every time then. Comment on attachment 445405 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445405&action=review >> Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp:83 >> + , m_timeoutTimer(makeUnique<Timer>(*this, &ServiceWorkerFetchTask::timeoutTimerFired)) > > I don't understand this change. You made it a pointer and you unconditionally initialize it. Yet, you never seem to null the pointer out. I don't understand the purpose of making it a pointer and null checking it every time then. m_timeoutTimer is only created in one of the two ServiceWorkerFetchTask constructor. (In reply to youenn fablet from comment #5) > Comment on attachment 445405 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=445405&action=review > > >> Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp:83 > >> + , m_timeoutTimer(makeUnique<Timer>(*this, &ServiceWorkerFetchTask::timeoutTimerFired)) > > > > I don't understand this change. You made it a pointer and you unconditionally initialize it. Yet, you never seem to null the pointer out. I don't understand the purpose of making it a pointer and null checking it every time then. > > m_timeoutTimer is only created in one of the two ServiceWorkerFetchTask > constructor. Oh, I missed that. All good then. Created attachment 445546 [details]
Patch for landing
Committed r286361 (244720@main): <https://commits.webkit.org/244720@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 445546 [details]. |