[webkit-changes] [263551] trunk/Source/WebKit

Thu, 25 Jun 2020 21:37:54 -0700

Title: [263551] trunk/Source/WebKit
Revision
263551
Author
[email protected]
Date
2020-06-25 21:36:19 -0700 (Thu, 25 Jun 2020)

Log Message

WKHTTPCookieStore.setCookie should flush the cookies to disk before calling completion handler when we are using the UIProcess's default cookie storage
https://bugs.webkit.org/show_bug.cgi?id=213636

Patch by Alex Christensen <[email protected]> on 2020-06-25
Reviewed by Brady Eidson.

This fixes a race condition that many, many third party developers run into, causing them to not be logged in when they thought they did.

* NetworkProcess/cocoa/NetworkProcessCocoa.mm:
(WebKit::saveCookies):
* UIProcess/API/APIHTTPCookieStore.cpp:
(API::HTTPCookieStore::setCookies):
(API::HTTPCookieStore::registerForNewProcessPoolNotifications):
(API::HTTPCookieStore::flushDefaultUIProcessCookieStore):
* UIProcess/API/APIHTTPCookieStore.h:
* UIProcess/API/Cocoa/APIHTTPCookieStoreCocoa.mm:
(API::HTTPCookieStore::flushDefaultUIProcessCookieStore):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (263550 => 263551)


--- trunk/Source/WebKit/ChangeLog	2020-06-26 04:17:47 UTC (rev 263550)
+++ trunk/Source/WebKit/ChangeLog	2020-06-26 04:36:19 UTC (rev 263551)
@@ -1,3 +1,22 @@
+2020-06-25  Alex Christensen  <[email protected]>
+
+        WKHTTPCookieStore.setCookie should flush the cookies to disk before calling completion handler when we are using the UIProcess's default cookie storage
+        https://bugs.webkit.org/show_bug.cgi?id=213636
+
+        Reviewed by Brady Eidson.
+
+        This fixes a race condition that many, many third party developers run into, causing them to not be logged in when they thought they did.
+
+        * NetworkProcess/cocoa/NetworkProcessCocoa.mm:
+        (WebKit::saveCookies):
+        * UIProcess/API/APIHTTPCookieStore.cpp:
+        (API::HTTPCookieStore::setCookies):
+        (API::HTTPCookieStore::registerForNewProcessPoolNotifications):
+        (API::HTTPCookieStore::flushDefaultUIProcessCookieStore):
+        * UIProcess/API/APIHTTPCookieStore.h:
+        * UIProcess/API/Cocoa/APIHTTPCookieStoreCocoa.mm:
+        (API::HTTPCookieStore::flushDefaultUIProcessCookieStore):
+
 2020-06-25  Megan Gardner  <[email protected]>
 
         Upstream date/time picker style

Modified: trunk/Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm (263550 => 263551)


--- trunk/Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm	2020-06-26 04:17:47 UTC (rev 263550)
+++ trunk/Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm	2020-06-26 04:36:19 UTC (rev 263551)
@@ -216,9 +216,7 @@
     ASSERT(RunLoop::isMain());
     [cookieStorage _saveCookies:makeBlockPtr([completionHandler = WTFMove(completionHandler)]() mutable {
         // CFNetwork may call the completion block on a background queue, so we need to redispatch to the main thread.
-        RunLoop::main().dispatch([completionHandler = WTFMove(completionHandler)]() mutable {
-            completionHandler();
-        });
+        RunLoop::main().dispatch(WTFMove(completionHandler));
     }).get()];
 }
 #endif

Modified: trunk/Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp (263550 => 263551)


--- trunk/Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp	2020-06-26 04:17:47 UTC (rev 263550)
+++ trunk/Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp	2020-06-26 04:36:19 UTC (rev 263551)
@@ -117,17 +117,22 @@
 void HTTPCookieStore::setCookies(const Vector<WebCore::Cookie>& cookies, CompletionHandler<void()>&& completionHandler)
 {
     filterAppBoundCookies(cookies, [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)] (auto&& appBoundCookies) mutable {
+        bool needsFlush = false;
         auto* pool = m_owningDataStore->processPoolForCookieStorageOperations();
         if (!pool) {
             for (auto& cookie : appBoundCookies) {
                 // 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() && !cookie.session) {
+                    needsFlush = true;
                     setCookieInDefaultUIProcessCookieStore(cookie);
-                else
+                } else
                     m_owningDataStore->addPendingCookie(cookie);
             }
 
-            RunLoop::main().dispatch(WTFMove(completionHandler));
+            if (needsFlush)
+                flushDefaultUIProcessCookieStore(WTFMove(completionHandler));
+            else
+                RunLoop::main().dispatch(WTFMove(completionHandler));
             return;
         }
 
@@ -294,7 +299,7 @@
 
         // Now that an associated process pool exists, we need to flush the UI process cookie store
         // to make sure any changes are reflected within the new process pool.
-        flushDefaultUIProcessCookieStore();
+        flushDefaultUIProcessCookieStore([] { });
         newProcessPool.ensureNetworkProcess();
 
         if (m_cookieManagerProxyObserver) {
@@ -314,7 +319,7 @@
 }
 
 #if !PLATFORM(COCOA)
-void HTTPCookieStore::flushDefaultUIProcessCookieStore() { }
+void HTTPCookieStore::flushDefaultUIProcessCookieStore(CompletionHandler<void()>&& completionHandler) { completionHandler(); }
 Vector<WebCore::Cookie> HTTPCookieStore::getAllDefaultUIProcessCookieStoreCookies() { return { }; }
 void HTTPCookieStore::setCookieInDefaultUIProcessCookieStore(const WebCore::Cookie&) { }
 void HTTPCookieStore::deleteCookieFromDefaultUIProcessCookieStore(const WebCore::Cookie&) { }

Modified: trunk/Source/WebKit/UIProcess/API/APIHTTPCookieStore.h (263550 => 263551)


--- trunk/Source/WebKit/UIProcess/API/APIHTTPCookieStore.h	2020-06-26 04:17:47 UTC (rev 263550)
+++ trunk/Source/WebKit/UIProcess/API/APIHTTPCookieStore.h	2020-06-26 04:36:19 UTC (rev 263551)
@@ -85,7 +85,7 @@
     void registerForNewProcessPoolNotifications();
     void unregisterForNewProcessPoolNotifications();
 
-    static void flushDefaultUIProcessCookieStore();
+    static void flushDefaultUIProcessCookieStore(CompletionHandler<void()>&&);
     static Vector<WebCore::Cookie> getAllDefaultUIProcessCookieStoreCookies();
     static void setCookieInDefaultUIProcessCookieStore(const WebCore::Cookie&);
     static void deleteCookieFromDefaultUIProcessCookieStore(const WebCore::Cookie&);

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/APIHTTPCookieStoreCocoa.mm (263550 => 263551)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/APIHTTPCookieStoreCocoa.mm	2020-06-26 04:17:47 UTC (rev 263550)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/APIHTTPCookieStoreCocoa.mm	2020-06-26 04:36:19 UTC (rev 263551)
@@ -30,12 +30,23 @@
 #import <WebCore/CookieStorageObserver.h>
 #import <WebCore/HTTPCookieAcceptPolicy.h>
 #import <pal/spi/cf/CFNetworkSPI.h>
+#import <wtf/BlockPtr.h>
+#import <wtf/RunLoop.h>
 
 namespace API {
 
-void HTTPCookieStore::flushDefaultUIProcessCookieStore()
+void HTTPCookieStore::flushDefaultUIProcessCookieStore(CompletionHandler<void()>&& completionHandler)
 {
+#if HAVE(FOUNDATION_WITH_SAVE_COOKIES_WITH_COMPLETION_HANDLER)
+    ASSERT(RunLoop::isMain());
+    [[NSHTTPCookieStorage sharedHTTPCookieStorage] _saveCookies:makeBlockPtr([completionHandler = WTFMove(completionHandler)]() mutable {
+        // CFNetwork may call the completion block on a background queue, so we need to redispatch to the main thread.
+        RunLoop::main().dispatch(WTFMove(completionHandler));
+    }).get()];
+#else
     [[NSHTTPCookieStorage sharedHTTPCookieStorage] _saveCookies];
+    RunLoop::main().dispatch(WTFMove(completionHandler));
+#endif
 }
 
 Vector<WebCore::Cookie> HTTPCookieStore::getAllDefaultUIProcessCookieStoreCookies()

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to