By default, WebKit would create a new WKProcessPool for each WKWebView, and all WKWebViews share one DefaultDataStore. As each processPool has its own network process and storage process, data corruption may happen when multiple network/storage processes use same data store at the same time.
Created attachment 343933 [details] Patch
(In reply to Sihui Liu from comment #1) > Created attachment 343933 [details] > Patch Initial patch for checking the test results.
Attachment 343933 [details] did not pass style-queue: ERROR: Source/WebKit/UIProcess/API/Cocoa/WKWebViewConfiguration.mm:418: More than one command on the same line [whitespace/newline] [4] Total errors found: 1 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 343933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343933&action=review > Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.h:42 > @interface WKProcessPool : NSObject <NSSecureCoding> > ++ (WKProcessPool *)defaultProcessPool; > @end > I think we should put this in WKProcessPoolPrivate.h for now. We can offer it as public API after we've settled the design a little bit more. One question we should figure out before we offer this as public API is whether we're going to make the change to only ever have one network process per UI process. > Source/WebKit/UIProcess/WebProcessPool.cpp:131 > +static WebProcessPool* globalDefaultProcessPool; You can declare this static inside WebProcessPool::defaultProcessPool() to encapsulate it a little better. > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1359 > + if (processPools.isEmpty()) > + processPools.add(WebProcessPool::defaultProcessPool().ptr()); This doesn't seem right to me. This function returns all the process pools that this website data store has been used with. But not all website data stores are necessarily used with the default process pool. So why is it correct to return the default process pool? And if it's correct, why do we return it only if we don't have another process pool?
Comment on attachment 343933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343933&action=review > Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:45 > + if (!pool) > registerForNewProcessPoolNotifications(); This patch makes it so that pool is never null. If we keep that behavior, we should remove this check for null, and even change processPoolForCookieStorageOperations() to return a reference rather than a pointer. But we might need to keep that old null behavior, at least for now -- depending on what we decide to do in WebsiteDataStore::processPools().
Comment on attachment 343933 [details] Patch Attachment 343933 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8387054 New failing tests: http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-audio.html
Created attachment 343946 [details] Archive of layout-test-results from ews205 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews205 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment on attachment 343933 [details] Patch I think we want to do a few things: (1) Network process and storage process should be singletons, rather than data members per process pool. We believe that network process and storage process know how to handle multiple sessions with multiple website data stores, so there's no need to launch a new network process or storage process when you use a new website data store. (2) When you want to set a cookie, you should not set a pending cookie. Instead, you should launch the singleton network process if needed, and set the cookie by messaging the singleton network process. (3) WebProcessPool and ProcessPoolConfiguration should not hold any paths. Only WebsiteDataStore should determine which paths we use when accessing storage on disk. A website data store determines its paths when it registers its session with the networking process or storage process.
We've agreed to not use defaultProcessPool as a temporary fix as it may cause other problems.
Comment on attachment 343933 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343933&action=review >> Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1359 >> + processPools.add(WebProcessPool::defaultProcessPool().ptr()); > > This doesn't seem right to me. This function returns all the process pools that this website data store has been used with. But not all website data stores are necessarily used with the default process pool. So why is it correct to return the default process pool? And if it's correct, why do we return it only if we don't have another process pool? We looked at this again today. Adding the idea of a default process pool for API clients could work. (SPI clients still have deeper problems.) If WebsiteDataStore::processPoolForCookieStorageOperations() returned the default process pool when no other process pool was available, that would be correct for API clients because API clients always use the default process pool. This solution is still not complete because it doesn't address SPI clients, and it doesn't provide a path for API clients to eventually be able to use more than one process pool.