Attachment 340362 Details for Bug 185624 – Patch

[patch] Patch

bug-185624-20180514144230.patch (text/plain), 13.71 KB, created by Sihui Liu on 2018-05-14 14:42:31 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2018-05-14 14:42:31 PDT

Size: 13.71 KB

patch

obsolete

>Subversion Revision: 231737
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index f2ec028e926944fc7bc0c22f1001c98270c19bd3..5e619d0cf0f006537eaa92a10ab428737a53d337 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,27 @@
>+2018-05-14  Sihui Liu  <sihui_liu@apple.com>
>+
>+        Session cookies aren't reliably set when using default WKWebSiteDataStore
>+        https://bugs.webkit.org/show_bug.cgi?id=185624
>+        <rdar://problem/39111626>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Session cookies of default session were set in UI Process when there was no process pool, 
>+        but they were not synced or synced slowly to Network Process. To make these cookies visible
>+        as soon as they were set through API, we could manually set those cookies in Network Process
>+        during its initilization. 
>+
>+        Test: WebKit.WKHTTPCookieStoreWithoutProcessPool
>+
>+        * NetworkProcess/mac/RemoteNetworkingContext.mm:
>+        (WebKit::RemoteNetworkingContext::ensureWebsiteDataStoreSession):
>+        * UIProcess/API/APIHTTPCookieStore.cpp:
>+        (API::HTTPCookieStore::cookies):
>+        (API::HTTPCookieStore::setCookie):
>+        (API::HTTPCookieStore::deleteCookie):
>+        * UIProcess/WebProcessPool.cpp:
>+        (WebKit::WebProcessPool::ensureNetworkProcess):
>+
> 2018-05-11  Daniel Bates  <dabates@apple.com>
> 
>         X-Frame-Options: SAMEORIGIN needs to check all ancestor frames
>diff --git a/Source/WebKit/NetworkProcess/mac/RemoteNetworkingContext.mm b/Source/WebKit/NetworkProcess/mac/RemoteNetworkingContext.mm
>index 7aef9f1c0a30defd7e15237e9d070eab08cce741..12ba375b1c24f30d7e2ede3978cc053e2c06ba4c 100644
>--- a/Source/WebKit/NetworkProcess/mac/RemoteNetworkingContext.mm
>+++ b/Source/WebKit/NetworkProcess/mac/RemoteNetworkingContext.mm
>@@ -36,6 +36,7 @@
> #import "WebsiteDataStoreParameters.h"
> #import <WebCore/NetworkStorageSession.h>
> #import <WebCore/ResourceError.h>
>+#import <pal/SessionID.h>
> #import <wtf/MainThread.h>
> 
> using namespace WebCore;
>@@ -45,8 +46,14 @@ namespace WebKit {
> void RemoteNetworkingContext::ensureWebsiteDataStoreSession(WebsiteDataStoreParameters&& parameters)
> {
>     auto sessionID = parameters.networkSessionParameters.sessionID;
>-    if (NetworkStorageSession::storageSession(sessionID))
>+    if (NetworkStorageSession::storageSession(sessionID)) {
>+        if (sessionID == PAL::SessionID::defaultSessionID()) {
>+            auto* session = NetworkStorageSession::storageSession(sessionID);
>+            for (const auto& cookie : parameters.pendingCookies)
>+                session->setCookie(cookie);
>+        }
>         return;
>+    }
> 
>     String base;
>     if (SessionTracker::getIdentifierBase().isNull())
>diff --git a/Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp b/Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp
>index f5bf74f5f503c4ec250b3947ed1bb041f4e53130..5767455faa3ac57fe6cbf7261f2a27d13b54b356 100644
>--- a/Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp
>+++ b/Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp
>@@ -56,13 +56,7 @@ void HTTPCookieStore::cookies(Function<void (const Vector<WebCore::Cookie>&)>&&
> {
>     auto* pool = m_owningDataStore->processPoolForCookieStorageOperations();
>     if (!pool) {
>-        Vector<WebCore::Cookie> allCookies;
>-        if (m_owningDataStore->sessionID() == PAL::SessionID::defaultSessionID())
>-            allCookies = WebCore::NetworkStorageSession::defaultStorageSession().getAllCookies();
>-        else
>-            allCookies = m_owningDataStore->pendingCookies();
>-
>-        callOnMainThread([completionHandler = WTFMove(completionHandler), allCookies]() {
>+        callOnMainThread([completionHandler = WTFMove(completionHandler), allCookies = m_owningDataStore->pendingCookies()]() {
>             completionHandler(allCookies);
>         });
>         return;
>@@ -78,11 +72,7 @@ void HTTPCookieStore::setCookie(const WebCore::Cookie& cookie, Function<void ()>
> {
>     auto* pool = m_owningDataStore->processPoolForCookieStorageOperations();
>     if (!pool) {
>-        if (m_owningDataStore->sessionID() == PAL::SessionID::defaultSessionID())
>-            WebCore::NetworkStorageSession::defaultStorageSession().setCookie(cookie);
>-        else
>-            m_owningDataStore->addPendingCookie(cookie);
>-
>+        m_owningDataStore->addPendingCookie(cookie);
>         callOnMainThread([completionHandler = WTFMove(completionHandler)]() {
>             completionHandler();
>         });
>@@ -99,10 +89,7 @@ void HTTPCookieStore::deleteCookie(const WebCore::Cookie& cookie, Function<void
> {
>     auto* pool = m_owningDataStore->processPoolForCookieStorageOperations();
>     if (!pool) {
>-        if (m_owningDataStore->sessionID() == PAL::SessionID::defaultSessionID())
>-            WebCore::NetworkStorageSession::defaultStorageSession().deleteCookie(cookie);
>-        else
>-            m_owningDataStore->removePendingCookie(cookie);
>+        m_owningDataStore->removePendingCookie(cookie);
>         callOnMainThread([completionHandler = WTFMove(completionHandler)]() {
>             completionHandler();
>         });
>diff --git a/Source/WebKit/UIProcess/WebProcessPool.cpp b/Source/WebKit/UIProcess/WebProcessPool.cpp
>index a73cecd6514c96ee13b6c1c5e1dc537ec81f3cd0..d78dc6c83957877bcfc2fd806406df29663f716e 100644
>--- a/Source/WebKit/UIProcess/WebProcessPool.cpp
>+++ b/Source/WebKit/UIProcess/WebProcessPool.cpp
>@@ -515,6 +515,9 @@ NetworkProcessProxy& WebProcessPool::ensureNetworkProcess(WebsiteDataStore* with
> #if PLATFORM(COCOA)
>     m_networkProcess->send(Messages::NetworkProcess::SetQOS(networkProcessLatencyQOS(), networkProcessThroughputQOS()), 0);
> #endif
>+    
>+    if (m_websiteDataStore)
>+        m_networkProcess->send(Messages::NetworkProcess::AddWebsiteDataStore(m_websiteDataStore->websiteDataStore().parameters()), 0);
> 
>     if (m_didNetworkProcessCrash) {
>         m_didNetworkProcessCrash = false;
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 8cb18d9cff772b2b6c4a975a62af39fecd83940e..1bc2f04442eb05923c23f5ed22065c26443c4aa1 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-05-14  Sihui Liu  <sihui_liu@apple.com>
>+
>+        Session cookies aren't reliably set when using default WKWebSiteDataStore
>+        https://bugs.webkit.org/show_bug.cgi?id=185624
>+        <rdar://problem/39111626>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Modified and enabled WebKit.WKHTTPCookieStoreWithoutProcessPool.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:
>+        (-[CookieUIDelegate webView:runJavaScriptAlertPanelWithMessage:initiatedByFrame:completionHandler:]):
>+        (TEST):
>+
> 2018-05-11  Leo Balter  <leonardo.balter@gmail.com>
> 
>         Test262 Runner should search for the Debug JSC by default
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>index 9d5d7ecd6dc2a4719cd7d49fcc0a254ecd619798..3acc697f9b754e3974c2b942a727b990187b2a88 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>@@ -376,62 +376,104 @@ @end
> @implementation CookieUIDelegate
> - (void)webView:(WKWebView *)webView runJavaScriptAlertPanelWithMessage:(NSString *)message initiatedByFrame:(WKFrameInfo *)frame completionHandler:(void (^)(void))completionHandler
> {
>-    EXPECT_STREQ("cookie:cookiename=cookievalue", message.UTF8String);
>+    EXPECT_STREQ("cookie:PersistentCookieName=CookieValue; SessionCookieName=CookieValue", message.UTF8String);
>     finished = true;
>     completionHandler();
> }
> @end
> 
>-// FIXME: This should be removed once <rdar://problem/35344202> is resolved and bots are updated.
>-#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MAX_ALLOWED <= 101301) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MAX_ALLOWED <= 110102)
> TEST(WebKit, WKHTTPCookieStoreWithoutProcessPool)
> {
>-    NSHTTPCookie *cookie = [NSHTTPCookie cookieWithProperties:[NSDictionary dictionaryWithObjectsAndKeys:@"127.0.0.1", NSHTTPCookieDomain, @"/", NSHTTPCookiePath, @"cookiename", NSHTTPCookieName, @"cookievalue", NSHTTPCookieValue, [NSDate distantFuture], NSHTTPCookieExpires, nil]];
>+    RetainPtr<NSHTTPCookie> sessionCookie = [NSHTTPCookie cookieWithProperties:@{
>+        NSHTTPCookiePath: @"/",
>+        NSHTTPCookieName: @"SessionCookieName",
>+        NSHTTPCookieValue: @"CookieValue",
>+        NSHTTPCookieDomain: @"127.0.0.1",
>+    }];
>+    RetainPtr<NSHTTPCookie> persistentCookie = [NSHTTPCookie cookieWithProperties:@{
>+        NSHTTPCookiePath: @"/",
>+        NSHTTPCookieName: @"PersistentCookieName",
>+        NSHTTPCookieValue: @"CookieValue",
>+        NSHTTPCookieDomain: @"127.0.0.1",
>+        NSHTTPCookieExpires: [NSDate distantFuture],
>+    }];
>     NSString *alertCookieHTML = @"<script>alert('cookie:'+document.cookie);</script>";
>     
>+
>+    // NonPersistentDataStore
>+    RetainPtr<WKWebsiteDataStore> ephemeralStoreWithCookies = [WKWebsiteDataStore nonPersistentDataStore];
>+
>     finished = false;
>-    WKWebsiteDataStore *ephemeralStoreWithCookies = [WKWebsiteDataStore nonPersistentDataStore];
>-    [ephemeralStoreWithCookies.httpCookieStore setCookie:cookie completionHandler:^ {
>+    [ephemeralStoreWithCookies.get().httpCookieStore setCookie:sessionCookie.get() completionHandler:^{
>         WKWebsiteDataStore *ephemeralStoreWithIndependentCookieStorage = [WKWebsiteDataStore nonPersistentDataStore];
>         [ephemeralStoreWithIndependentCookieStorage.httpCookieStore getAllCookies:^(NSArray<NSHTTPCookie *> *cookies) {
>-            ASSERT_EQ(cookies.count, 0u);
>-            
>-            WKWebViewConfiguration *configuration = [[WKWebViewConfiguration alloc] init];
>-            configuration.websiteDataStore = ephemeralStoreWithCookies;
>-            WKWebView *view = [[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration];
>-            view.UIDelegate = [[CookieUIDelegate alloc] init];
>-
>-            [view loadHTMLString:alertCookieHTML baseURL:[NSURL URLWithString:@"http://127.0.0.1/"]];
>+            ASSERT_EQ(0u, cookies.count);
>+            finished = true;
>         }];
>     }];
>     TestWebKitAPI::Util::run(&finished);
>-    
>-    // FIXME: Get this to work on iOS. <rdar://problem/32260156>
>-#if !PLATFORM(IOS)
>+
>     finished = false;
>-    WKWebsiteDataStore *defaultStore = [WKWebsiteDataStore defaultDataStore];
>-    [defaultStore.httpCookieStore setCookie:cookie completionHandler:^ {
>+    [ephemeralStoreWithCookies.get().httpCookieStore setCookie:persistentCookie.get() completionHandler:^{
>+        [ephemeralStoreWithCookies.get().httpCookieStore getAllCookies:^(NSArray<NSHTTPCookie *> *cookies) {
>+            ASSERT_EQ(2u, cookies.count);
>+            finished = true;
>+        }];
>+    }];
>+    TestWebKitAPI::Util::run(&finished);
>+
>+    finished = false;
>+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
>+    configuration.get().websiteDataStore = ephemeralStoreWithCookies.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();
>+    [webView loadHTMLString:alertCookieHTML baseURL:[NSURL URLWithString:@"http://127.0.0.1"]];
>+    TestWebKitAPI::Util::run(&finished);
>+
>+    finished = false;
>+    [ephemeralStoreWithCookies.get().httpCookieStore deleteCookie:sessionCookie.get() completionHandler:^{
>+        [ephemeralStoreWithCookies.get().httpCookieStore getAllCookies:^(NSArray<NSHTTPCookie *> *cookies) {
>+            ASSERT_EQ(1u, cookies.count);
>+            finished = true;
>+        }];
>+    }];
>+    TestWebKitAPI::Util::run(&finished);
>+
>+
>+    // DefaultDataStore
>+    auto defaultStore = [WKWebsiteDataStore defaultDataStore];
>+
>+    finished = false;
>+    [defaultStore removeDataOfTypes:[WKWebsiteDataStore allWebsiteDataTypes] modifiedSince:[NSDate distantPast] completionHandler:[] {
>+        finished = true;
>+    }];
>+    TestWebKitAPI::Util::run(&finished);
>+
>+    finished = false;
>+    [defaultStore.httpCookieStore setCookie:sessionCookie.get() completionHandler:^{
>         [defaultStore.httpCookieStore getAllCookies:^(NSArray<NSHTTPCookie *> *cookies) {
>-            ASSERT_EQ(cookies.count, 1u);
>-            
>-            WKWebViewConfiguration *configuration = [[WKWebViewConfiguration alloc] init];
>-            configuration.websiteDataStore = defaultStore;
>-            WKWebView *view = [[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration];
>-            view.UIDelegate = [[CookieUIDelegate alloc] init];
>-            
>-            [view loadHTMLString:alertCookieHTML baseURL:[NSURL URLWithString:@"http://127.0.0.1/"]];
>+            ASSERT_EQ(1u, cookies.count);
>+            finished = true;
>         }];
>     }];
>     TestWebKitAPI::Util::run(&finished);
>-    
>-    [defaultStore.httpCookieStore deleteCookie:cookie completionHandler:^ {
>+
>+    finished = false;
>+    [defaultStore.httpCookieStore setCookie:persistentCookie.get() completionHandler:^{
>         [defaultStore.httpCookieStore getAllCookies:^(NSArray<NSHTTPCookie *> *cookies) {
>-            ASSERT_EQ(cookies.count, 0u);
>+            ASSERT_EQ(2u, cookies.count);
>             finished = true;
>         }];
>     }];
>     TestWebKitAPI::Util::run(&finished);
>-#endif
>+
>+    finished = false;
>+    configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
>+    configuration.get().websiteDataStore = defaultStore;
>+    webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
>+    webView.get().UIDelegate = delegate.get();
>+    [webView loadHTMLString:alertCookieHTML baseURL:[NSURL URLWithString:@"http://127.0.0.1"]];
>+    TestWebKitAPI::Util::run(&finished);
> }
>-#endif // (PLATFORM(MAC) && __MAC_OS_X_VERSION_MAX_ALLOWED <= 101301) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MAX_ALLOWED <= 110102)
> #endif

Actions: View | Formatted Diff | Diff

Attachments on bug 185624: 340362 | 340373 | 340403 | 340441 | 340484 | 340746 | 340750