Attachment 342739 Details for Bug 186625 – Patch

[patch] Patch

bug-186625-20180614103554.patch (text/plain), 16.55 KB, created by Sihui Liu on 2018-06-14 10:35:55 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2018-06-14 10:35:55 PDT

Size: 16.55 KB

patch

obsolete

>Subversion Revision: 232823
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index cad66597a6231a293b7b9ac880cecd5b72fa6b99..fd4520fda5c4c3ec045c2ddd178616e031ea0413 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,26 @@
>+2018-06-14  Sihui Liu  <sihui_liu@apple.com>
>+
>+        REVERT: Session cookies aren't reliably set when using default WKWebSiteDataStore (185624)
>+        https://bugs.webkit.org/show_bug.cgi?id=186625
>+
>+        Temporary fix for regression <rdar://problem/41068016>.
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * 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::removePendingCookie):
>+        (WebKit::WebsiteDataStore::clearPendingCookies): Deleted.
>+        * UIProcess/WebsiteData/WebsiteDataStore.h:
>+
> 2018-06-13  Dean Jackson  <dino@apple.com>
> 
>         Disable AR support in WKWebView clients
>diff --git a/Source/WebKit/NetworkProcess/mac/RemoteNetworkingContext.mm b/Source/WebKit/NetworkProcess/mac/RemoteNetworkingContext.mm
>index e989bd8e99d4111cfd737a06503775f3ddc005f6..7aef9f1c0a30defd7e15237e9d070eab08cce741 100644
>--- a/Source/WebKit/NetworkProcess/mac/RemoteNetworkingContext.mm
>+++ b/Source/WebKit/NetworkProcess/mac/RemoteNetworkingContext.mm
>@@ -36,7 +36,6 @@
> #import "WebsiteDataStoreParameters.h"
> #import <WebCore/NetworkStorageSession.h>
> #import <WebCore/ResourceError.h>
>-#import <pal/SessionID.h>
> #import <wtf/MainThread.h>
> 
> using namespace WebCore;
>@@ -46,12 +45,8 @@ namespace WebKit {
> void RemoteNetworkingContext::ensureWebsiteDataStoreSession(WebsiteDataStoreParameters&& parameters)
> {
>     auto sessionID = parameters.networkSessionParameters.sessionID;
>-    if (auto* session = NetworkStorageSession::storageSession(sessionID)) {
>-        ASSERT(parameters.pendingCookies.isEmpty() || sessionID == PAL::SessionID::defaultSessionID());
>-        for (const auto& cookie : parameters.pendingCookies)
>-            session->setCookie(cookie);
>+    if (NetworkStorageSession::storageSession(sessionID))
>         return;
>-    }
> 
>     String base;
>     if (SessionTracker::getIdentifierBase().isNull())
>diff --git a/Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp b/Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp
>index ad9215793925b8afc3193348beb1c625d1f0f6ca..6987cf9e03fb230f104e83bbb10a42efa53ecf09 100644
>--- a/Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp
>+++ b/Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp
>@@ -61,7 +61,8 @@ void HTTPCookieStore::cookies(Function<void(const Vector<WebCore::Cookie>&)>&& c
>         Vector<WebCore::Cookie> allCookies;
>         if (m_owningDataStore->sessionID() == PAL::SessionID::defaultSessionID())
>             allCookies = WebCore::NetworkStorageSession::defaultStorageSession().getAllCookies();
>-        allCookies.appendVector(m_owningDataStore->pendingCookies());
>+        else
>+            allCookies = m_owningDataStore->pendingCookies();
> 
>         callOnMainThread([completionHandler = WTFMove(completionHandler), allCookies]() {
>             completionHandler(allCookies);
>@@ -79,8 +80,7 @@ void HTTPCookieStore::setCookie(const WebCore::Cookie& cookie, Function<void ()>
> {
>     auto* pool = m_owningDataStore->processPoolForCookieStorageOperations();
>     if (!pool) {
>-        // 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)
>+        if (m_owningDataStore->sessionID() == PAL::SessionID::defaultSessionID())
>             WebCore::NetworkStorageSession::defaultStorageSession().setCookie(cookie);
>         else
>             m_owningDataStore->addPendingCookie(cookie);
>@@ -101,7 +101,7 @@ void HTTPCookieStore::deleteCookie(const WebCore::Cookie& cookie, Function<void
> {
>     auto* pool = m_owningDataStore->processPoolForCookieStorageOperations();
>     if (!pool) {
>-        if (m_owningDataStore->sessionID() == PAL::SessionID::defaultSessionID() && !cookie.session)
>+        if (m_owningDataStore->sessionID() == PAL::SessionID::defaultSessionID())
>             WebCore::NetworkStorageSession::defaultStorageSession().deleteCookie(cookie);
>         else
>             m_owningDataStore->removePendingCookie(cookie);
>diff --git a/Source/WebKit/UIProcess/WebProcessPool.cpp b/Source/WebKit/UIProcess/WebProcessPool.cpp
>index 4c0f00e0fc7d1f73e845de7be5765cec835f3d78..23fcae80549bc9fd8486c21ffc259168944237b2 100644
>--- a/Source/WebKit/UIProcess/WebProcessPool.cpp
>+++ b/Source/WebKit/UIProcess/WebProcessPool.cpp
>@@ -447,10 +447,8 @@ 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;
>     }
> 
>@@ -535,16 +533,9 @@ NetworkProcessProxy& WebProcessPool::ensureNetworkProcess(WebsiteDataStore* with
>         if (m_websiteDataStore)
>             m_websiteDataStore->websiteDataStore().networkProcessDidCrash();
>     }
>-
>-    if (m_websiteDataStore) {
>-        m_networkProcess->send(Messages::NetworkProcess::AddWebsiteDataStore(m_websiteDataStore->websiteDataStore().parameters()), 0);
>-        m_websiteDataStore->websiteDataStore().clearPendingCookies();
>-    }
>     
>-    if (withWebsiteDataStore) {
>+    if (withWebsiteDataStore)
>         m_networkProcess->send(Messages::NetworkProcess::AddWebsiteDataStore(withWebsiteDataStore->parameters()), 0);
>-        withWebsiteDataStore->clearPendingCookies();
>-    }
> 
>     return *m_networkProcess;
> }
>@@ -1199,11 +1190,9 @@ 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 68192de1129dc43f736b91f3d370f06b9cb8ff9e..4f7c24f252da80b4f1a705d86d0a2c5c204e1a29 100644
>--- a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>+++ b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
>@@ -1534,11 +1534,6 @@ 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 978c3d945c5c083cbcfcf67c842ab2639d0071aa..5f26d740b8155e4f0415379d75cd600340b6ca7a 100644
>--- a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h
>+++ b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h
>@@ -162,7 +162,6 @@ 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 2708476459870b3f61b250ccf4d0b2cf87e1bc2e..c0017f0bcdadf81f9208f92164cef7fea33da502 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-06-14  Sihui Liu  <sihui_liu@apple.com>
>+
>+        REVERT: Session cookies aren't reliably set when using default WKWebSiteDataStore (185624)
>+        https://bugs.webkit.org/show_bug.cgi?id=186625
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:
>+        (-[CookieUIDelegate webView:runJavaScriptAlertPanelWithMessage:initiatedByFrame:completionHandler:]):
>+        (TEST):
>+
> 2018-06-11  Carlos Alberto Lopez Perez  <clopez@igalia.com>
> 
>         [GTK] Add a script to upload the data generated with run-benchmark to a browserperfdash server and use it on the gtk perf bot
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>index 82fdedf2e27ea559fc558add9efc1c14949fc64b..bc30ad4eebccbc0788bf20731f9785cac99c37b4 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm
>@@ -468,101 +468,62 @@ @end
> @implementation CookieUIDelegate
> - (void)webView:(WKWebView *)webView runJavaScriptAlertPanelWithMessage:(NSString *)message initiatedByFrame:(WKFrameInfo *)frame completionHandler:(void (^)(void))completionHandler
> {
>-    EXPECT_STREQ("PersistentCookieName=CookieValue; SessionCookieName=CookieValue", message.UTF8String);
>+    EXPECT_STREQ("cookie:cookiename=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)
> {
>-    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>var cookies = document.cookie.split(';'); for (let i = 0; i < cookies.length; i ++) { cookies[i] = cookies[i].trim(); } cookies.sort(); alert(cookies.join('; '));</script>";
>-
>-    // NonPersistentDataStore
>-    RetainPtr<WKWebsiteDataStore> ephemeralStoreWithCookies = [WKWebsiteDataStore nonPersistentDataStore];
>+    NSHTTPCookie *cookie = [NSHTTPCookie cookieWithProperties:[NSDictionary dictionaryWithObjectsAndKeys:@"127.0.0.1", NSHTTPCookieDomain, @"/", NSHTTPCookiePath, @"cookiename", NSHTTPCookieName, @"cookievalue", NSHTTPCookieValue, [NSDate distantFuture], NSHTTPCookieExpires, nil]];
>+    NSString *alertCookieHTML = @"<script>alert('cookie:'+document.cookie);</script>";
> 
>     finished = false;
>-    [ephemeralStoreWithCookies.get().httpCookieStore setCookie:persistentCookie.get() completionHandler:^{
>+    WKWebsiteDataStore *ephemeralStoreWithCookies = [WKWebsiteDataStore nonPersistentDataStore];
>+    [ephemeralStoreWithCookies.httpCookieStore setCookie:cookie completionHandler:^{
>         WKWebsiteDataStore *ephemeralStoreWithIndependentCookieStorage = [WKWebsiteDataStore nonPersistentDataStore];
>         [ephemeralStoreWithIndependentCookieStorage.httpCookieStore getAllCookies:^(NSArray<NSHTTPCookie *> *cookies) {
>-            ASSERT_EQ(0u, cookies.count);
>-            finished = true;
>-        }];
>-    }];
>-    TestWebKitAPI::Util::run(&finished);
>+            ASSERT_EQ(cookies.count, 0u);
> 
>-    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);
>+            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];
> 
>-    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;
>+            [view loadHTMLString:alertCookieHTML baseURL:[NSURL URLWithString:@"http://127.0.0.1/"]];
>         }];
>     }];
>     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);
> 
>+    // FIXME: Get this to work on iOS. <rdar://problem/32260156>
>+#if !PLATFORM(IOS)
>     finished = false;
>-    [defaultStore.httpCookieStore setCookie:persistentCookie.get() completionHandler:^{
>+    WKWebsiteDataStore *defaultStore = [WKWebsiteDataStore defaultDataStore];
>+    [defaultStore.httpCookieStore setCookie:cookie completionHandler:^{
>         [defaultStore.httpCookieStore getAllCookies:^(NSArray<NSHTTPCookie *> *cookies) {
>-            ASSERT_EQ(1u, cookies.count);
>-            finished = true;
>+            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/"]];
>         }];
>     }];
>     TestWebKitAPI::Util::run(&finished);
> 
>-    finished = false;
>-    [defaultStore.httpCookieStore setCookie:sessionCookie.get() completionHandler:^{
>+    [defaultStore.httpCookieStore deleteCookie:cookie completionHandler:^{
>         [defaultStore.httpCookieStore getAllCookies:^(NSArray<NSHTTPCookie *> *cookies) {
>-            ASSERT_EQ(2u, cookies.count);
>+            ASSERT_EQ(cookies.count, 0u);
>             finished = true;
>         }];
>     }];
>     TestWebKitAPI::Util::run(&finished);
>-
>-    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
> }
>+#endif // (PLATFORM(MAC) && __MAC_OS_X_VERSION_MAX_ALLOWED <= 101301) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MAX_ALLOWED <= 110102)
> #endif

Flags:

ggaren: review-

Actions: View | Formatted Diff | Diff

Attachments on bug 186625: 342739