Title: [238174] trunk
- Revision
- 238174
- Author
- cdu...@apple.com
- Date
- 2018-11-14 07:50:05 -0800 (Wed, 14 Nov 2018)
Log Message
WebKit.WKHTTPCookieStoreWithoutProcessPool API test is failing with process prewarming is enabled
https://bugs.webkit.org/show_bug.cgi?id=191624
Reviewed by Alex Christensen.
Source/WebKit:
Stop setting the WebProcessPool's primary data store (m_websiteDataStore) to the default one in
WebProcessPool::prewarmProcess(). We did not really need to, we can pass the default data store
to the new WebPageProxy without having to set m_websiteDataStore. m_websiteDataStore only gets
set upon constructor if thr default data store already exists or later on when creating a WebPage
that uses the default data store.
In the case of the API test, the following was happening:
1. Create an ephemeral data store EDS
2. Create a WebView V1 using datastore EDS
3. Do a load in V1
4. Process prewarming would kick in and wrongly associated V1's process pool PP1 with the default data store
5. Create/Get the default datastore and set a few cookies on it
6. Create a WebView V2 using default datastore and a fresh new process pool PP2
7. Do a load in V2 and expect the cookies to be there
In HTTPCookieStore::setCookie() that is called at step 5, we call:
m_owningDataStore->processPoolForCookieStorageOperations()
In this case, m_owningDataStore is the default datastore and this call would previously return null because
there is no WebProcessPool yet associated with the default datastore. However, with the process prewarming
bug at step 4, the process pool PP1 would be returned since it was wrongly associated with the default
data store. As a result, we would call setCookie() on PP1's WebCookieManagerProxy and this would fail
because PP1's network process knows nothing about this session ID (it was only ever used with an ephemeral
session).
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::prewarmProcess):
Tools:
Add API test coverage.
* TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:
(runWKHTTPCookieStoreWithoutProcessPool):
(TEST):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (238173 => 238174)
--- trunk/Source/WebKit/ChangeLog 2018-11-14 15:31:17 UTC (rev 238173)
+++ trunk/Source/WebKit/ChangeLog 2018-11-14 15:50:05 UTC (rev 238174)
@@ -1,3 +1,38 @@
+2018-11-14 Chris Dumez <cdu...@apple.com>
+
+ WebKit.WKHTTPCookieStoreWithoutProcessPool API test is failing with process prewarming is enabled
+ https://bugs.webkit.org/show_bug.cgi?id=191624
+
+ Reviewed by Alex Christensen.
+
+ Stop setting the WebProcessPool's primary data store (m_websiteDataStore) to the default one in
+ WebProcessPool::prewarmProcess(). We did not really need to, we can pass the default data store
+ to the new WebPageProxy without having to set m_websiteDataStore. m_websiteDataStore only gets
+ set upon constructor if thr default data store already exists or later on when creating a WebPage
+ that uses the default data store.
+
+ In the case of the API test, the following was happening:
+ 1. Create an ephemeral data store EDS
+ 2. Create a WebView V1 using datastore EDS
+ 3. Do a load in V1
+ 4. Process prewarming would kick in and wrongly associated V1's process pool PP1 with the default data store
+ 5. Create/Get the default datastore and set a few cookies on it
+ 6. Create a WebView V2 using default datastore and a fresh new process pool PP2
+ 7. Do a load in V2 and expect the cookies to be there
+
+ In HTTPCookieStore::setCookie() that is called at step 5, we call:
+ m_owningDataStore->processPoolForCookieStorageOperations()
+
+ In this case, m_owningDataStore is the default datastore and this call would previously return null because
+ there is no WebProcessPool yet associated with the default datastore. However, with the process prewarming
+ bug at step 4, the process pool PP1 would be returned since it was wrongly associated with the default
+ data store. As a result, we would call setCookie() on PP1's WebCookieManagerProxy and this would fail
+ because PP1's network process knows nothing about this session ID (it was only ever used with an ephemeral
+ session).
+
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::prewarmProcess):
+
2018-11-13 Jiewen Tan <jiewen_...@apple.com>
[WebAuthN] Support CTAP HID authenticators on macOS
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (238173 => 238174)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2018-11-14 15:31:17 UTC (rev 238173)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2018-11-14 15:50:05 UTC (rev 238174)
@@ -955,11 +955,12 @@
if (m_prewarmedProcess)
return;
- if (!m_websiteDataStore)
- m_websiteDataStore = API::WebsiteDataStore::defaultDataStore().ptr();
+ auto* websiteDataStore = m_websiteDataStore.get();
+ if (!websiteDataStore)
+ websiteDataStore = API::WebsiteDataStore::defaultDataStore().ptr();
RELEASE_LOG(PerformanceLogging, "Prewarming a WebProcess for performance");
- createNewWebProcess(m_websiteDataStore->websiteDataStore(), WebProcessProxy::IsPrewarmed::Yes);
+ createNewWebProcess(websiteDataStore->websiteDataStore(), WebProcessProxy::IsPrewarmed::Yes);
}
void WebProcessPool::enableProcessTermination()
Modified: trunk/Tools/ChangeLog (238173 => 238174)
--- trunk/Tools/ChangeLog 2018-11-14 15:31:17 UTC (rev 238173)
+++ trunk/Tools/ChangeLog 2018-11-14 15:50:05 UTC (rev 238174)
@@ -1,3 +1,16 @@
+2018-11-14 Chris Dumez <cdu...@apple.com>
+
+ WebKit.WKHTTPCookieStoreWithoutProcessPool API test is failing with process prewarming is enabled
+ https://bugs.webkit.org/show_bug.cgi?id=191624
+
+ Reviewed by Alex Christensen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:
+ (runWKHTTPCookieStoreWithoutProcessPool):
+ (TEST):
+
2018-11-13 Jiewen Tan <jiewen_...@apple.com>
[WebAuthN] Support CTAP HID authenticators on macOS
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm (238173 => 238174)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm 2018-11-14 15:31:17 UTC (rev 238173)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm 2018-11-14 15:50:05 UTC (rev 238174)
@@ -31,6 +31,7 @@
#import <WebKit/WKHTTPCookieStore.h>
#import <WebKit/WKProcessPoolPrivate.h>
#import <WebKit/WKWebsiteDataStorePrivate.h>
+#import <WebKit/_WKProcessPoolConfiguration.h>
#import <WebKit/_WKWebsiteDataStoreConfiguration.h>
#import <wtf/ProcessPrivilege.h>
#import <wtf/RetainPtr.h>
@@ -478,7 +479,8 @@
// FIXME: on iOS, UI process should be using the same cookie file as the network process for default session.
#if PLATFORM(MAC)
-TEST(WebKit, WKHTTPCookieStoreWithoutProcessPool)
+enum class ShouldEnableProcessPrewarming { No, Yes };
+void runWKHTTPCookieStoreWithoutProcessPool(ShouldEnableProcessPrewarming shouldEnableProcessPrewarming)
{
RetainPtr<NSHTTPCookie> sessionCookie = [NSHTTPCookie cookieWithProperties:@{
NSHTTPCookiePath: @"/",
@@ -516,10 +518,15 @@
}];
}];
TestWebKitAPI::Util::run(&finished);
+ finished = false;
- finished = false;
+ auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+ processPoolConfiguration.get().prewarmsProcessesAutomatically = shouldEnableProcessPrewarming == ShouldEnableProcessPrewarming::Yes;
+ auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
configuration.get().websiteDataStore = ephemeralStoreWithCookies.get();
+ [configuration setProcessPool:processPool.get()];
auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
auto delegate = adoptNS([[CookieUIDelegate alloc] init]);
webView.get().UIDelegate = delegate.get();
@@ -569,5 +576,16 @@
[webView loadHTMLString:alertCookieHTML baseURL:[NSURL URLWithString:@"http://127.0.0.1"]];
TestWebKitAPI::Util::run(&finished);
}
+
+TEST(WebKit, WKHTTPCookieStoreWithoutProcessPoolWithoutPrewarming)
+{
+ runWKHTTPCookieStoreWithoutProcessPool(ShouldEnableProcessPrewarming::No);
+}
+
+TEST(WebKit, WKHTTPCookieStoreWithoutProcessPoolWithPrewarming)
+{
+ runWKHTTPCookieStoreWithoutProcessPool(ShouldEnableProcessPrewarming::Yes);
+}
+
#endif // PLATFORM(MAC)
#endif
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
