Changeset 264613 in webkit

Timestamp:

Jul 20, 2020, 1:12:44 PM (5 years ago)

Author:

commit-queue@webkit.org

Message:

REGRESSION(r263551) Main thread often hangs while saving cookies
​https://bugs.webkit.org/show_bug.cgi?id=214564
<rdar://problem/65779712>

Patch by Alex Christensen <​achristensen@webkit.org> on 2020-07-20
Reviewed by Geoffrey Garen.

I had assumed that CFNetwork wouldn't write cookies on the main thread, but I was wrong.
That change caused spins, so let's just do the work on the WebsiteDataStore's queue instead.

• UIProcess/API/APIHTTPCookieStore.h:
• UIProcess/API/Cocoa/APIHTTPCookieStoreCocoa.mm:

(API::HTTPCookieStore::flushDefaultUIProcessCookieStore):

• UIProcess/WebsiteData/WebsiteDataStore.cpp:

(WebKit::WebsiteDataStore::dispatchOnQueue):

• UIProcess/WebsiteData/WebsiteDataStore.h:

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• UIProcess/API/APIHTTPCookieStore.h (modified) (1 diff)
• UIProcess/API/Cocoa/APIHTTPCookieStoreCocoa.mm (modified) (2 diffs)
• UIProcess/WebsiteData/WebsiteDataStore.cpp (modified) (1 diff)
• UIProcess/WebsiteData/WebsiteDataStore.h (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-07-20  Alex Christensen  <achristensen@webkit.org>
+
+        REGRESSION(r263551) Main thread often hangs while saving cookies
+        https://bugs.webkit.org/show_bug.cgi?id=214564
+        <rdar://problem/65779712>
+
+        Reviewed by Geoffrey Garen.
+
+        I had assumed that CFNetwork wouldn't write cookies on the main thread, but I was wrong.
+        That change caused spins, so let's just do the work on the WebsiteDataStore's queue instead.
+
+        * UIProcess/API/APIHTTPCookieStore.h:
+        * UIProcess/API/Cocoa/APIHTTPCookieStoreCocoa.mm:
+        (API::HTTPCookieStore::flushDefaultUIProcessCookieStore):
+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+        (WebKit::WebsiteDataStore::dispatchOnQueue):
+        * UIProcess/WebsiteData/WebsiteDataStore.h:
+
 2020-07-20  David Kilzer  <ddkilzer@apple.com>
 

trunk/Source/WebKit/UIProcess/API/APIHTTPCookieStore.h

@@ -86 +86 @@
     void unregisterForNewProcessPoolNotifications();
 
-    static void flushDefaultUIProcessCookieStore(CompletionHandler<void()>&&);
+    void flushDefaultUIProcessCookieStore(CompletionHandler<void()>&&);
     static Vector<WebCore::Cookie> getAllDefaultUIProcessCookieStoreCookies();
     static void setCookieInDefaultUIProcessCookieStore(const WebCore::Cookie&);

trunk/Source/WebKit/UIProcess/API/Cocoa/APIHTTPCookieStoreCocoa.mm

@@ -27 +27 @@
 #import "APIHTTPCookieStore.h"
 
+#import "WebsiteDataStore.h"
 #import <WebCore/Cookie.h>
 #import <WebCore/CookieStorageObserver.h>
@@ -40 +41 @@
 #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()];
+    m_owningDataStore->dispatchOnQueue([completionHandler = WTFMove(completionHandler)] () mutable {
+        [[NSHTTPCookieStorage sharedHTTPCookieStorage] _saveCookies:makeBlockPtr([completionHandler = WTFMove(completionHandler)]() mutable {
+            RunLoop::main().dispatch(WTFMove(completionHandler));
+        }).get()];
+    });
 #else
     [[NSHTTPCookieStorage sharedHTTPCookieStorage] _saveCookies];

trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp

@@ -2222 +2222 @@
 }
 
+void WebsiteDataStore::dispatchOnQueue(Function<void()>&& function)
+{
+    m_queue->dispatch(WTFMove(function));
+}
+
 uint64_t WebsiteDataStore::perThirdPartyOriginStorageQuota() const
 {

trunk/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h

@@ -237 +237 @@
     void clearPendingCookies();
 
+    void dispatchOnQueue(Function<void()>&&);
+
 #if USE(CURL)
     void setNetworkProxySettings(WebCore::CurlProxySettings&&);