WebKit Bugzilla
Attachment 340441 Details for
Bug 185624
: Session cookies aren't reliably set when using default WKWebSiteDataStore
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[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:
Patch
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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 185624
:
340362
|
340373
|
340403
|
340441
|
340484
|
340746
|
340750