Attachment 340403 Details for Bug 185624 – Patch

[patch] Patch

bug-185624-20180515033420.patch (text/plain), 13.71 KB, created by Sihui Liu on 2018-05-15 03:34:21 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2018-05-15 03:34:21 PDT

Size: 13.71 KB

patch

obsolete

>Subversion Revision: 231789
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 65eb7171747e50fede6199f1ed75722c849fbd3e..86bcae55e74e7004a1a9c7419d908ec8bdb2203d 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,25 @@
>+2018-05-15  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. 
>+
>+        * 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-14  Brady Eidson  <beidson@apple.com>
> 
>         Add an API test to guard against regressions while re-entering setDefersLoading:.
>diff --git a/Source/WebKit/NetworkProcess/mac/RemoteNetworkingContext.mm b/Source/WebKit/NetworkProcess/mac/RemoteNetworkingContext.mm
>index 7aef9f1c0a30defd7e15237e9d070eab08cce741..0b88955c5bd247dc44750f6476d90dd3a6e4f138 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,13 @@ namespace WebKit {
> void RemoteNetworkingContext::ensureWebsiteDataStoreSession(WebsiteDataStoreParameters&& parameters)
> {
>     auto sessionID = parameters.networkSessionParameters.sessionID;
>-    if (NetworkStorageSession::storageSession(sessionID))
>+    if (auto* session = NetworkStorageSession::storageSession(sessionID)) {
>+        if (sessionID == PAL::SessionID::defaultSessionID()) {
>+            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..72cec86d581a4faaf9c70a76305f2b69ce0df054 100644
>--- a/Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp
>+++ b/Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp
>@@ -52,15 +52,14 @@ HTTPCookieStore::~HTTPCookieStore()
>     unregisterForNewProcessPoolNotifications();
> }
> 
>-void HTTPCookieStore::cookies(Function<void (const Vector<WebCore::Cookie>&)>&& completionHandler)
>+void HTTPCookieStore::cookies(Function<void(const Vector<WebCore::Cookie>&)>&& completionHandler)
> {
>     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();
>+        allCookies.appendVector(m_owningDataStore->pendingCookies());
> 
>         callOnMainThread([completionHandler = WTFMove(completionHandler), allCookies]() {
>             completionHandler(allCookies);
>@@ -78,7 +77,7 @@ void HTTPCookieStore::setCookie(const WebCore::Cookie& cookie, Function<void ()>
> {
>     auto* pool = m_owningDataStore->processPoolForCookieStorageOperations();
>     if (!pool) {
>-        if (m_owningDataStore->sessionID() == PAL::SessionID::defaultSessionID())
>+        if (m_owningDataStore->sessionID() == PAL::SessionID::defaultSessionID() && !cookie.session)
>             WebCore::NetworkStorageSession::defaultStorageSession().setCookie(cookie);
>         else
>             m_owningDataStore->addPendingCookie(cookie);
>@@ -99,10 +98,11 @@ void HTTPCookieStore::deleteCookie(const WebCore::Cookie& cookie, Function<void
> {
>     auto* pool = m_owningDataStore->processPoolForCookieStorageOperations();
>     if (!pool) {
>-        if (m_owningDataStore->sessionID() == PAL::SessionID::defaultSessionID())
>+        if (m_owningDataStore->sessionID() == PAL::SessionID::defaultSessionID() && !cookie.session)
>             WebCore::NetworkStorageSession::defaultStorageSession().deleteCookie(cookie);
>         else
>             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 b30d9172f9433206ff2d0fd520be88526288c8ce..89981662fc0aa7a129f0cbb19023b1d94e7fdb52 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-05-15  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-14  Brady Eidson  <beidson@apple.com>
> 
>         Add an API test to guard against regressions while re-entering setDefersLoading:.
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>index 9d5d7ecd6dc2a4719cd7d49fcc0a254ecd619798..e68b8d05ead43159aaa206189dc11facf6720e5b 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>@@ -376,62 +376,101 @@ @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:persistentCookie.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);
>+
>+    finished = false;
>+    [ephemeralStoreWithCookies.get().httpCookieStore setCookie:sessionCookie.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);
>     
>-    // FIXME: Get this to work on iOS. <rdar://problem/32260156>
>-#if !PLATFORM(IOS)
>+    // DefaultDataStore
>+    auto defaultStore = [WKWebsiteDataStore defaultDataStore];
>     finished = false;
>-    WKWebsiteDataStore *defaultStore = [WKWebsiteDataStore defaultDataStore];
>-    [defaultStore.httpCookieStore setCookie:cookie completionHandler:^ {
>+    [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