Implement in-WebProcess cookie cache to avoid sync IPC for document.cookie in most cases.
<rdar://problem/56027027>
Created attachment 390450 [details] WIP Patch
Created attachment 390462 [details] WIP Patch
Created attachment 390533 [details] WIP Patch
Created attachment 390536 [details] WIP Patch
Created attachment 390539 [details] WIP Patch
Created attachment 390549 [details] WIP Patch Now passing layout tests locally.
Created attachment 390561 [details] WIP Patch
Created attachment 390567 [details] WIP Patch With more layout tests.
Created attachment 390575 [details] Patch
Comment on attachment 390575 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=390575&action=review > Source/WTF/wtf/PlatformEnable.h:802 > +#if !defined(ENABLE_COOKIE_CACHE) && PLATFORM(COCOA) && HAVE(CFNETWORK_COOKIE_CHANGE_LISTENER_API) > +#define ENABLE_COOKIE_CACHE 1 > +#endif Couldn't this just be a runtime no-op without CFNETWORK_COOKIE_CHANGE_LISTENER_API and avoid most #ifs? > Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm:617 > + ASSERT(observers.add(&observer).isNewEntry); Why is observer only added in debug build? > Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp:131 > + if (!m_hostsWithCookieListeners.isEmpty()) { > + auto hostsWithCookieListeners = copyToVector(m_hostsWithCookieListeners); > + unsubscribeFromCookieChangeNotifications(hostsWithCookieListeners); > + } I think it would be nicer to have a separate function or just loop over m_hostsWithCookieListeners here (avoiding the vector copy). It is just call to stopListeningForCookieChangeNotifications for each. > Source/WebKit/WebProcess/WebPage/WebCookieCache.h:44 > + bool isFunctional(); isEnabled/isSupported?
Comment on attachment 390575 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=390575&action=review >> Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm:617 >> + ASSERT(observers.add(&observer).isNewEntry); > > Why is observer only added in debug build? ouch, last minute clean up I made :/ Thanks for catching.
> ouch, last minute clean up I made :/ Thanks for catching. Observers don't seem to have much test coverage.
(In reply to Antti Koivisto from comment #13) > > ouch, last minute clean up I made :/ Thanks for catching. > > Observers don't seem to have much test coverage. EWS does not have the CFNetwork SPI needed and thus does not cover my change. Actual layout test bots will run the tests with the cookie cache enabled (at least some of the bots will).
Created attachment 390687 [details] Patch
Created attachment 390688 [details] Patch
Created attachment 390691 [details] Patch
Created attachment 390695 [details] Patch
Created attachment 390925 [details] Patch
Created attachment 390926 [details] Patch
Created attachment 390988 [details] Patch
Created attachment 391004 [details] Patch
Comment on attachment 391004 [details] Patch Clearing flags on attachment: 391004 Committed r256820: <https://trac.webkit.org/changeset/256820>
All reviewed patches have been landed. Closing bug.
The new test http/tests/cookies/document-cookie-after-showModalDialog.html is a flaky failure. tracking in https://bugs.webkit.org/show_bug.cgi?id=208165