<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[238174] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/238174">238174</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2018-11-14 07:50:05 -0800 (Wed, 14 Nov 2018)</dd>
</dl>

<h3>Log Message</h3>
<pre>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):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKitChangeLog">trunk/Source/WebKit/ChangeLog</a></li>
<li><a href="#trunkSourceWebKitUIProcessWebProcessPoolcpp">trunk/Source/WebKit/UIProcess/WebProcessPool.cpp</a></li>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsTestWebKitAPITestsWebKitCocoaWKHTTPCookieStoremm">trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKitChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/ChangeLog (238173 => 238174)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,38 @@
</span><ins>+2018-11-14  Chris Dumez  <cdumez@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):
+
</ins><span class="cx"> 2018-11-13  Jiewen Tan  <jiewen_tan@apple.com>
</span><span class="cx"> 
</span><span class="cx">         [WebAuthN] Support CTAP HID authenticators on macOS
</span></span></pre></div>
<a id="trunkSourceWebKitUIProcessWebProcessPoolcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (238173 => 238174)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -955,11 +955,12 @@
</span><span class="cx">     if (m_prewarmedProcess)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    if (!m_websiteDataStore)
-        m_websiteDataStore = API::WebsiteDataStore::defaultDataStore().ptr();
</del><ins>+    auto* websiteDataStore = m_websiteDataStore.get();
+    if (!websiteDataStore)
+        websiteDataStore = API::WebsiteDataStore::defaultDataStore().ptr();
</ins><span class="cx"> 
</span><span class="cx">     RELEASE_LOG(PerformanceLogging, "Prewarming a WebProcess for performance");
</span><del>-    createNewWebProcess(m_websiteDataStore->websiteDataStore(), WebProcessProxy::IsPrewarmed::Yes);
</del><ins>+    createNewWebProcess(websiteDataStore->websiteDataStore(), WebProcessProxy::IsPrewarmed::Yes);
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void WebProcessPool::enableProcessTermination()
</span></span></pre></div>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (238173 => 238174)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog    2018-11-14 15:31:17 UTC (rev 238173)
+++ trunk/Tools/ChangeLog       2018-11-14 15:50:05 UTC (rev 238174)
</span><span class="lines">@@ -1,3 +1,16 @@
</span><ins>+2018-11-14  Chris Dumez  <cdumez@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):
+
</ins><span class="cx"> 2018-11-13  Jiewen Tan  <jiewen_tan@apple.com>
</span><span class="cx"> 
</span><span class="cx">         [WebAuthN] Support CTAP HID authenticators on macOS
</span></span></pre></div>
<a id="trunkToolsTestWebKitAPITestsWebKitCocoaWKHTTPCookieStoremm"></a>
<div class="modfile"><h4>Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm (238173 => 238174)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -31,6 +31,7 @@
</span><span class="cx"> #import <WebKit/WKHTTPCookieStore.h>
</span><span class="cx"> #import <WebKit/WKProcessPoolPrivate.h>
</span><span class="cx"> #import <WebKit/WKWebsiteDataStorePrivate.h>
</span><ins>+#import <WebKit/_WKProcessPoolConfiguration.h>
</ins><span class="cx"> #import <WebKit/_WKWebsiteDataStoreConfiguration.h>
</span><span class="cx"> #import <wtf/ProcessPrivilege.h>
</span><span class="cx"> #import <wtf/RetainPtr.h>
</span><span class="lines">@@ -478,7 +479,8 @@
</span><span class="cx"> 
</span><span class="cx"> // FIXME: on iOS, UI process should be using the same cookie file as the network process for default session.
</span><span class="cx"> #if PLATFORM(MAC)
</span><del>-TEST(WebKit, WKHTTPCookieStoreWithoutProcessPool)
</del><ins>+enum class ShouldEnableProcessPrewarming { No, Yes };
+void runWKHTTPCookieStoreWithoutProcessPool(ShouldEnableProcessPrewarming shouldEnableProcessPrewarming)
</ins><span class="cx"> {
</span><span class="cx">     RetainPtr<NSHTTPCookie> sessionCookie = [NSHTTPCookie cookieWithProperties:@{
</span><span class="cx">         NSHTTPCookiePath: @"/",
</span><span class="lines">@@ -516,10 +518,15 @@
</span><span class="cx">         }];
</span><span class="cx">     }];
</span><span class="cx">     TestWebKitAPI::Util::run(&finished);
</span><ins>+    finished = false;
</ins><span class="cx"> 
</span><del>-    finished = false;
</del><ins>+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().prewarmsProcessesAutomatically = shouldEnableProcessPrewarming == ShouldEnableProcessPrewarming::Yes;
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
</ins><span class="cx">     auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
</span><span class="cx">     configuration.get().websiteDataStore = ephemeralStoreWithCookies.get();
</span><ins>+    [configuration setProcessPool:processPool.get()];
</ins><span class="cx">     auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
</span><span class="cx">     auto delegate = adoptNS([[CookieUIDelegate alloc] init]);
</span><span class="cx">     webView.get().UIDelegate = delegate.get();
</span><span class="lines">@@ -569,5 +576,16 @@
</span><span class="cx">     [webView loadHTMLString:alertCookieHTML baseURL:[NSURL URLWithString:@"http://127.0.0.1"]];
</span><span class="cx">     TestWebKitAPI::Util::run(&finished);
</span><span class="cx"> }
</span><ins>+
+TEST(WebKit, WKHTTPCookieStoreWithoutProcessPoolWithoutPrewarming)
+{
+    runWKHTTPCookieStoreWithoutProcessPool(ShouldEnableProcessPrewarming::No);
+}
+
+TEST(WebKit, WKHTTPCookieStoreWithoutProcessPoolWithPrewarming)
+{
+    runWKHTTPCookieStoreWithoutProcessPool(ShouldEnableProcessPrewarming::Yes);
+}
+
</ins><span class="cx"> #endif // PLATFORM(MAC)
</span><span class="cx"> #endif
</span></span></pre>
</div>
</div>

</body>
</html>