Attachment 340441 Details for Bug 185624 – Patch

[patch] Patch

bug-185624-20180515162417.patch (text/plain), 17.09 KB, created by Sihui Liu on 2018-05-15 16:24:16 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2018-05-15 16:24:16 PDT

Size: 17.09 KB

patch

obsolete

>Subversion Revision: 231789
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 65eb7171747e50fede6199f1ed75722c849fbd3e..76dcc1a98df1d938ba587f6994b0a2bdd4ff9527 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,29 @@
>+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):
>+        (WebKit::WebProcessPool::pageBeginUsingWebsiteDataStore):
>+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
>+        (WebKit::WebsiteDataStore::clearPendingCookies):
>+        * UIProcess/WebsiteData/WebsiteDataStore.h:
>+
> 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..e989bd8e99d4111cfd737a06503775f3ddc005f6 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,12 @@ namespace WebKit {
> void RemoteNetworkingContext::ensureWebsiteDataStoreSession(WebsiteDataStoreParameters&& parameters)
> {
>     auto sessionID = parameters.networkSessionParameters.sessionID;
>-    if (NetworkStorageSession::storageSession(sessionID))
>+    if (auto* session = NetworkStorageSession::storageSession(sessionID)) {
>+        ASSERT(parameters.pendingCookies.isEmpty() || 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..ed5d7385964db80726343b7673ba0ce5a91afee6 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,8 @@ void HTTPCookieStore::setCookie(const WebCore::Cookie& cookie, Function<void ()>
> {
>     auto* pool = m_owningDataStore->processPoolForCookieStorageOperations();
>     if (!pool) {
>-        if (m_owningDataStore->sessionID() == PAL::SessionID::defaultSessionID())
>+        // FIXME: pendingCookies used for defaultSession because session cookies cannot be propagated to Network Process with uiProcessCookieStorageIdentifier.
>+        if (m_owningDataStore->sessionID() == PAL::SessionID::defaultSessionID() && !cookie.session)
>             WebCore::NetworkStorageSession::defaultStorageSession().setCookie(cookie);
>         else
>             m_owningDataStore->addPendingCookie(cookie);
>@@ -99,10 +99,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..91523ffb5c629a3099b4cb1d6de6be722ead415a 100644
>--- a/Source/WebKit/UIProcess/WebProcessPool.cpp
>+++ b/Source/WebKit/UIProcess/WebProcessPool.cpp
>@@ -429,8 +429,10 @@ void WebProcessPool::screenPropertiesStateChanged()
> NetworkProcessProxy& WebProcessPool::ensureNetworkProcess(WebsiteDataStore* withWebsiteDataStore)
> {
>     if (m_networkProcess) {
>-        if (withWebsiteDataStore)
>+        if (withWebsiteDataStore) {
>             m_networkProcess->send(Messages::NetworkProcess::AddWebsiteDataStore(withWebsiteDataStore->parameters()), 0);
>+            withWebsiteDataStore->clearPendingCookies();
>+        }
>         return *m_networkProcess;
>     }
> 
>@@ -523,8 +525,15 @@ NetworkProcessProxy& WebProcessPool::ensureNetworkProcess(WebsiteDataStore* with
>             m_websiteDataStore->websiteDataStore().networkProcessDidCrash();
>     }
> 
>-    if (withWebsiteDataStore)
>+    if (m_websiteDataStore) {
>+        m_networkProcess->send(Messages::NetworkProcess::AddWebsiteDataStore(m_websiteDataStore->websiteDataStore().parameters()), 0);
>+        m_websiteDataStore->websiteDataStore().clearPendingCookies();
>+    }
>+    
>+    if (withWebsiteDataStore) {
>         m_networkProcess->send(Messages::NetworkProcess::AddWebsiteDataStore(withWebsiteDataStore->parameters()), 0);
>+        withWebsiteDataStore->clearPendingCookies();
>+    }
> 
>     return *m_networkProcess;
> }
>@@ -1168,9 +1177,11 @@ void WebProcessPool::pageBeginUsingWebsiteDataStore(WebPageProxy& page)
>         ASSERT(page.websiteDataStore().parameters().networkSessionParameters.sessionID == sessionID);
>         sendToNetworkingProcess(Messages::NetworkProcess::AddWebsiteDataStore(page.websiteDataStore().parameters()));
>         page.process().send(Messages::WebProcess::AddWebsiteDataStore(WebsiteDataStoreParameters::privateSessionParameters(sessionID)), 0);
>+        page.websiteDataStore().clearPendingCookies();
>     } else if (sessionID != PAL::SessionID::defaultSessionID()) {
>         sendToNetworkingProcess(Messages::NetworkProcess::AddWebsiteDataStore(page.websiteDataStore().parameters()));
>         page.process().send(Messages::WebProcess::AddWebsiteDataStore(page.websiteDataStore().parameters()), 0);
>+        page.websiteDataStore().clearPendingCookies();
> 
> #if ENABLE(INDEXED_DATABASE)
>         if (!page.websiteDataStore().resolvedIndexedDatabaseDirectory().isEmpty())
>diff --git a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>index e07472f2fe389968d94a4c27498195a1d8b4d02b..0db39b498edb0ed1e9b4881c2d5d8977029a04ea 100644
>--- a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>+++ b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>@@ -1523,6 +1523,11 @@ void WebsiteDataStore::removePendingCookie(const WebCore::Cookie& cookie)
> {
>     m_pendingCookies.remove(cookie);
> }
>+    
>+void WebsiteDataStore::clearPendingCookies()
>+{
>+    m_pendingCookies.clear();
>+}
> 
> #if !PLATFORM(COCOA)
> WebsiteDataStoreParameters WebsiteDataStore::parameters()
>diff --git a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h
>index 231b7861ef45705d19fcc4dd5642f576f7b6e125..aecdacaae360e054803ba02fb7ef9b52cbea9780 100644
>--- a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h
>+++ b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h
>@@ -160,6 +160,7 @@ public:
>     Vector<WebCore::Cookie> pendingCookies() const;
>     void addPendingCookie(const WebCore::Cookie&);
>     void removePendingCookie(const WebCore::Cookie&);
>+    void clearPendingCookies();
> 
>     void enableResourceLoadStatisticsAndSetTestingCallback(Function<void (const String&)>&& callback);
> 
>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..ae13bbb80cb82bf60c50ef05858d9b6883deb336 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:persistentCookie.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:sessionCookie.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