That will help speed up time and ensure we only get a sandbox extension when actually needed.
Created attachment 341702 [details] Patch
<rdar://problem/40452995>
Created attachment 341762 [details] Patch
Comment on attachment 341762 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341762&action=review Is it common to load the first page without initializing a cache storage engine at all? > Source/WebKit/NetworkProcess/NetworkProcess.cpp:855 > +void NetworkProcess::cacheStorageParameters(PAL::SessionID sessionID, CacheStorageParametersCallback&& callback) This seems to ask the UIProcess every time this is called unless it is called more than once before we get a response. Do we store the result anywhere? A cacheStorageParameters cache if you will... if (we already have parameters) call completion handler immediately else retrieve cache storage parameters Then store them on NetworkProcess in NetworkProcess::setCacheStorageParameters > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:582 > + m_websiteDataStores.set(store->sessionID(), WTFMove(store)); Parameter evaluation order is undefined. We need to get the sessionID before moving from store. > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:590 > + if (!sessionID.isEphemeral()) > + m_websiteDataStores.remove(sessionID); Don't we want to remove ephemeral sessions, too?
> View in context: > https://bugs.webkit.org/attachment.cgi?id=341762&action=review > > Is it common to load the first page without initializing a cache storage > engine at all? Yes, it is common, you typically do not use a cache storage engine unless your page is served by a service worker. In such a case, we would spin NetworkProcess, spin WebProcess, spin StorageProcess, spin another WebProcess, instantiate service worker in this process, execute some JavaScript and then grab the sandbox extension for cache storage API. > > > Source/WebKit/NetworkProcess/NetworkProcess.cpp:855 > > +void NetworkProcess::cacheStorageParameters(PAL::SessionID sessionID, CacheStorageParametersCallback&& callback) > > This seems to ask the UIProcess every time this is called unless it is > called more than once before we get a response. Do we store the result > anywhere? A cacheStorageParameters cache if you will... It does ask UIProcess each time. This is fine as we create one CacheStorageEngine per session ID and this is the one that will cache the information. The CacheStorageEngine stays alive as long as the network process is not notified that the sessionID is no longer valid. > > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:582 > > + m_websiteDataStores.set(store->sessionID(), WTFMove(store)); > > Parameter evaluation order is undefined. We need to get the sessionID > before moving from store. Ah, that is right, since we are creating a RefPtr from a Ref. I'll fix it. > > Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp:590 > > + if (!sessionID.isEphemeral()) > > + m_websiteDataStores.remove(sessionID); > > Don't we want to remove ephemeral sessions, too? There should be no need since we are not adding them to the map anyway.
Created attachment 342777 [details] Patch
Comment on attachment 342777 [details] Patch Clearing flags on attachment: 342777 Committed r232863: <https://trac.webkit.org/changeset/232863>
All reviewed patches have been landed. Closing bug.
Comment on attachment 342777 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=342777&action=review > Source/WebKit/NetworkProcess/NetworkProcess.cpp:138 > + for (auto& callbacks : m_cacheStorageParametersCallbacks.values()) { > + for (auto& callback : callbacks) > + callback(String { }, 0); > + } See bug #193702.