[webkit-reviews] review denied: [Bug 196807] Use normal loading path for ping loads : [Attachment 368138] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Apr 25 22:35:31 PDT 2019
Alex Christensen <achristensen at apple.com> has denied youenn fablet
<youennf at gmail.com>'s request for review:
Bug 196807: Use normal loading path for ping loads
https://bugs.webkit.org/show_bug.cgi?id=196807
Attachment 368138: Patch
https://bugs.webkit.org/attachment.cgi?id=368138&action=review
--- Comment #23 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 368138
--> https://bugs.webkit.org/attachment.cgi?id=368138
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=368138&action=review
I like this. I would like it even more if we could completely remove PingLoad,
but I guess that's coming. This makes our loading code more like the current
specification.
> Source/WebCore/loader/cache/CachedResourceLoader.cpp:775
> case CachedResource::Type::Beacon:
> + case CachedResource::Type::Ping:
Do we want these to be distinct types? Will we add another type for fetch
keep-alive?
> Source/WebKit/NetworkProcess/NetworkSession.h:109
> + HashSet<Ref<NetworkResourceLoader>> m_keptAliveLoads;
I think it would be better if this were on the NetworkSession. That would make
cleanup a lot more intuitive when we remove a private browsing session, at
which point I assume we would want to cancel all the outstanding pings,
beacons, and keep-alive fetches, right?
> Source/WebKit/WebProcess/Network/WebLoaderStrategy.cpp:577
> + return
!RuntimeEnabledFeatures::sharedFeatures().fetchAPIKeepAliveEnabled();
Won't we soon never want to use ping load in modern WebKit?
More information about the webkit-reviews
mailing list