[webkit-reviews] review granted: [Bug 233490] Reuse navigation preload if service worker is fetching the corresponding navigation request : [Attachment 445405] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Nov 30 07:58:11 PST 2021


Chris Dumez <cdumez at apple.com> has granted youenn fablet <youennf at gmail.com>'s
request for review:
Bug 233490: Reuse navigation preload if service worker is fetching the
corresponding navigation request
https://bugs.webkit.org/show_bug.cgi?id=233490

Attachment 445405: Patch

https://bugs.webkit.org/attachment.cgi?id=445405&action=review




--- Comment #4 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 445405
  --> https://bugs.webkit.org/attachment.cgi?id=445405
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.


More information about the webkit-reviews mailing list