Changeset 267042 in webkit


Ignore:
Timestamp:
Sep 14, 2020 2:50:38 PM (4 years ago)
Author:
achristensen@apple.com
Message:

Move cookie flushing SPI from WKProcessPool to WKHTTPCookieStore
​https://bugs.webkit.org/show_bug.cgi?id=216493

Reviewed by Chris Dumez.

Source/WebKit:

In order for ​https://bugs.webkit.org/show_bug.cgi?id=216041 to be possible, our interface for flushing cookies to disk can't be
on the WKProcessPool. It should be with the other cookie operations, in WKHTTPCookieStore.
AuxiliaryProcessProxy::sendWithAsyncReply takes care of the background assertion, so we remove that code from NetworkProcessProxy::syncAllCookies.
rdar://problem/68872711 tracks adoption in Safari, which I will land within an hour of landing this to avoid cookie regressions.

  • NetworkProcess/NetworkProcess.cpp:

(WebKit::NetworkProcess::syncAllCookies):
(WebKit::NetworkProcess::didSyncAllCookies): Deleted.

  • NetworkProcess/NetworkProcess.h:
  • NetworkProcess/NetworkProcess.messages.in:
  • NetworkProcess/cocoa/NetworkProcessCocoa.mm:

(WebKit::NetworkProcess::syncAllCookies):

  • UIProcess/API/APIHTTPCookieStore.cpp:

(API::HTTPCookieStore::syncCookies):

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

(-[WKHTTPCookieStore _flushCookiesToDiskWithCompletionHandler:]):

  • UIProcess/API/Cocoa/WKHTTPCookieStorePrivate.h:
  • UIProcess/API/Cocoa/WKProcessPool.mm:

(-[WKProcessPool _syncNetworkProcessCookies]):

  • UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
  • UIProcess/Network/NetworkProcessProxy.cpp:

(WebKit::NetworkProcessProxy::didClose):
(WebKit::NetworkProcessProxy::syncAllCookies):
(WebKit::NetworkProcessProxy::didSyncAllCookies): Deleted.

  • UIProcess/Network/NetworkProcessProxy.h:
  • UIProcess/Network/NetworkProcessProxy.messages.in:
  • UIProcess/WebProcessPool.cpp:

(WebKit::WebProcessPool::syncCookies):
(WebKit::WebProcessPool::syncNetworkProcessCookies): Deleted.

  • UIProcess/WebProcessPool.h:
  • UIProcess/WebsiteData/WebsiteDataStore.cpp:

(WebKit::WebsiteDataStore::syncCookies):

  • UIProcess/WebsiteData/WebsiteDataStore.h:

Tools:

  • TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:

(runWebsiteDataStoreCustomPaths):
(TEST):

Location:
trunk
Files:
20 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r267034 r267042  
     12020-09-14  Alex Christensen  <achristensen@webkit.org>
     2
     3        Move cookie flushing SPI from WKProcessPool to WKHTTPCookieStore
     4        https://bugs.webkit.org/show_bug.cgi?id=216493
     5
     6        Reviewed by Chris Dumez.
     7
     8        In order for https://bugs.webkit.org/show_bug.cgi?id=216041 to be possible, our interface for flushing cookies to disk can't be
     9        on the WKProcessPool.  It should be with the other cookie operations, in WKHTTPCookieStore.
     10        AuxiliaryProcessProxy::sendWithAsyncReply takes care of the background assertion, so we remove that code from NetworkProcessProxy::syncAllCookies.
     11        rdar://problem/68872711 tracks adoption in Safari, which I will land within an hour of landing this to avoid cookie regressions.
     12
     13        * NetworkProcess/NetworkProcess.cpp:
     14        (WebKit::NetworkProcess::syncAllCookies):
     15        (WebKit::NetworkProcess::didSyncAllCookies): Deleted.
     16        * NetworkProcess/NetworkProcess.h:
     17        * NetworkProcess/NetworkProcess.messages.in:
     18        * NetworkProcess/cocoa/NetworkProcessCocoa.mm:
     19        (WebKit::NetworkProcess::syncAllCookies):
     20        * UIProcess/API/APIHTTPCookieStore.cpp:
     21        (API::HTTPCookieStore::syncCookies):
     22        * UIProcess/API/APIHTTPCookieStore.h:
     23        * UIProcess/API/Cocoa/WKHTTPCookieStore.mm:
     24        (-[WKHTTPCookieStore _flushCookiesToDiskWithCompletionHandler:]):
     25        * UIProcess/API/Cocoa/WKHTTPCookieStorePrivate.h:
     26        * UIProcess/API/Cocoa/WKProcessPool.mm:
     27        (-[WKProcessPool _syncNetworkProcessCookies]):
     28        * UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
     29        * UIProcess/Network/NetworkProcessProxy.cpp:
     30        (WebKit::NetworkProcessProxy::didClose):
     31        (WebKit::NetworkProcessProxy::syncAllCookies):
     32        (WebKit::NetworkProcessProxy::didSyncAllCookies): Deleted.
     33        * UIProcess/Network/NetworkProcessProxy.h:
     34        * UIProcess/Network/NetworkProcessProxy.messages.in:
     35        * UIProcess/WebProcessPool.cpp:
     36        (WebKit::WebProcessPool::syncCookies):
     37        (WebKit::WebProcessPool::syncNetworkProcessCookies): Deleted.
     38        * UIProcess/WebProcessPool.h:
     39        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
     40        (WebKit::WebsiteDataStore::syncCookies):
     41        * UIProcess/WebsiteData/WebsiteDataStore.h:
     42
    1432020-09-14  Fujii Hironori  <Hironori.Fujii@sony.com>
    244

  • trunk/Source/WebKit/NetworkProcess/NetworkProcess.cpp

    r266929 r267042  
    276276    });
    277277#endif
    278     platformSyncAllCookies([callbackAggregator] { });
     278    forEachNetworkSession([&] (auto& networkSession) {
     279        platformFlushCookies(networkSession.sessionID(), [callbackAggregator] { });
     280    });
    279281}
    280282
    … …  
    22812283#endif
    22822284
    2283     platformSyncAllCookies([callbackAggregator] { });
     2285    forEachNetworkSession([&] (auto& networkSession) {
     2286        platformFlushCookies(networkSession.sessionID(), [callbackAggregator] { });
     2287    });
    22842288
    22852289    for (auto& connection : m_webProcessConnections.values())
    … …  
    23812385{
    23822386    LegacySchemeRegistry::registerURLSchemeAsNoAccess(scheme);
    2383 }
    2384 
    2385 void NetworkProcess::didSyncAllCookies()
    2386 {
    2387     parentProcessConnection()->send(Messages::NetworkProcessProxy::DidSyncAllCookies(), 0);
    23882387}
    23892388
    … …  
    26132612}
    26142613
    2615 void NetworkProcess::syncAllCookies()
    2616 {
    2617 }
    2618 
    2619 void NetworkProcess::platformSyncAllCookies(CompletionHandler<void()>&& completionHandler)
     2614void NetworkProcess::flushCookies(const PAL::SessionID&, CompletionHandler<void()>&& completionHandler)
     2615{
     2616    completionHandler();
     2617}
     2618
     2619void NetworkProcess::platformFlushCookies(const PAL::SessionID&, CompletionHandler<void()>&& completionHandler)
    26202620{
    26212621    completionHandler();

  • trunk/Source/WebKit/NetworkProcess/NetworkProcess.h

    r266829 r267042  
    444444    void setAllowsAnySSLCertificateForWebSocket(bool, CompletionHandler<void()>&&);
    445445   
    446     void syncAllCookies();
    447     void didSyncAllCookies();
     446    void flushCookies(const PAL::SessionID&, CompletionHandler<void()>&&);
    448447
    449448#if USE(SOUP)
    … …  
    462461#endif
    463462
    464     void platformSyncAllCookies(CompletionHandler<void()>&&);
     463    void platformFlushCookies(const PAL::SessionID&, CompletionHandler<void()>&&);
    465464   
    466465    void registerURLSchemeAsSecure(const String&) const;

  • trunk/Source/WebKit/NetworkProcess/NetworkProcess.messages.in

    r266829 r267042  
    6868    SetAllowsAnySSLCertificateForWebSocket(bool enabled) -> () Synchronous
    6969
    70     SyncAllCookies()
     70    FlushCookies(PAL::SessionID sessionID) -> () Async
    7171
    7272    AllowSpecificHTTPSCertificateForHost(WebCore::CertificateInfo certificate, String host)

  • trunk/Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm

    r266133 r267042  
    224224}
    225225
    226 void NetworkProcess::syncAllCookies()
    227 {
    228     platformSyncAllCookies([this] {
    229         didSyncAllCookies();
    230     });
     226void NetworkProcess::flushCookies(const PAL::SessionID& sessionID, CompletionHandler<void()>&& completionHandler)
     227{
     228    platformFlushCookies(sessionID, WTFMove(completionHandler));
    231229}
    232230
    … …  
    235233{
    236234    ASSERT(RunLoop::isMain());
     235    ASSERT(cookieStorage);
    237236    [cookieStorage _saveCookies:makeBlockPtr([completionHandler = WTFMove(completionHandler)]() mutable {
    238237        // CFNetwork may call the completion block on a background queue, so we need to redispatch to the main thread.
    … …  
    242241#endif
    243242
    244 void NetworkProcess::platformSyncAllCookies(CompletionHandler<void()>&& completionHander) {
     243void NetworkProcess::platformFlushCookies(const PAL::SessionID& sessionID, CompletionHandler<void()>&& completionHandler)
     244{
    245245    ASSERT(hasProcessPrivilege(ProcessPrivilege::CanAccessRawCookies));
     246#if HAVE(FOUNDATION_WITH_SAVE_COOKIES_WITH_COMPLETION_HANDLER)
     247    if (auto* networkStorageSession = storageSession(sessionID))
     248        saveCookies(networkStorageSession->nsCookieStorage(), WTFMove(completionHandler));
     249    else
     250        completionHandler();
     251#else
    246252    ALLOW_DEPRECATED_DECLARATIONS_BEGIN
    247 
    248 #if HAVE(FOUNDATION_WITH_SAVE_COOKIES_WITH_COMPLETION_HANDLER)
    249     RefPtr<CallbackAggregator> callbackAggregator = CallbackAggregator::create(WTFMove(completionHander));
    250     forEachNetworkStorageSession([&] (auto& networkStorageSession) {
    251         saveCookies(networkStorageSession.nsCookieStorage(), [callbackAggregator] { });
    252     });
    253 #else
    254253    _CFHTTPCookieStorageFlushCookieStores();
    255     completionHander();
    256 #endif
    257 
    258254    ALLOW_DEPRECATED_DECLARATIONS_END
     255    completionHandler();
     256#endif
    259257}
    260258

  • trunk/Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp

    r266829 r267042  
    190190}
    191191
     192void HTTPCookieStore::flushCookies(CompletionHandler<void()>&& completionHandler)
     193{
     194    m_owningDataStore->flushCookies(WTFMove(completionHandler));
     195}
     196
    192197class APIWebCookieManagerProxyObserver : public WebCookieManagerProxy::Observer {
    193198    WTF_MAKE_FAST_ALLOCATED;

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

    r265727 r267042  
    6565    void deleteAllCookies(CompletionHandler<void()>&&);
    6666    void setHTTPCookieAcceptPolicy(WebCore::HTTPCookieAcceptPolicy, CompletionHandler<void()>&&);
     67    void flushCookies(CompletionHandler<void()>&&);
    6768
    6869    class Observer {

  • trunk/Source/WebKit/UIProcess/API/Cocoa/WKHTTPCookieStore.mm

    r266951 r267042  
    147147}
    148148
     149- (void)_flushCookiesToDiskWithCompletionHandler:(void(^)(void))completionHandler
     150{
     151    _cookieStore->flushCookies([completionHandler = makeBlockPtr(completionHandler)]() {
     152        completionHandler();
     153    });
     154}
     155
    149156@end

  • trunk/Source/WebKit/UIProcess/API/Cocoa/WKHTTPCookieStorePrivate.h

    r266951 r267042  
    2828@interface WKHTTPCookieStore (WKPrivate)
    2929- (void)_getCookiesForURL:(NSURL *)url completionHandler:(void (^)(NSArray<NSHTTPCookie *> *))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
    30 - (void)_setCookieAcceptPolicy:(NSHTTPCookieAcceptPolicy)policy completionHandler:(void (^)())completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
     30- (void)_setCookieAcceptPolicy:(NSHTTPCookieAcceptPolicy)policy completionHandler:(void (^)(void))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
     31- (void)_flushCookiesToDiskWithCompletionHandler:(void(^)(void))completionHandler WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
    3132@end

  • trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm

    r266951 r267042  
    452452- (void)_syncNetworkProcessCookies
    453453{
    454     _processPool->syncNetworkProcessCookies();
    455454}
    456455

  • trunk/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h

    r266951 r267042  
    105105- (size_t)_pluginProcessCount WK_API_AVAILABLE(macos(10.13.4), ios(11.3));
    106106- (size_t)_serviceWorkerProcessCount WK_API_AVAILABLE(macos(10.14), ios(12.0));
    107 - (void)_syncNetworkProcessCookies WK_API_AVAILABLE(macos(10.13), ios(11.0));
     107- (void)_syncNetworkProcessCookies WK_API_DEPRECATED_WITH_REPLACEMENT("WKHTTPCookieStore._flushCookiesToDiskWithCompletionHandler:", macos(10.13, WK_MAC_TBA), ios(11.0, WK_IOS_TBA));
    108108- (void)_makeNextWebProcessLaunchFailForTesting WK_API_AVAILABLE(macos(10.14), ios(12.0));
    109109- (void)_makeNextNetworkProcessLaunchFailForTesting WK_API_AVAILABLE(macos(10.14), ios(12.0));

  • trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp

    r266829 r267042  
    284284
    285285    m_activityForHoldingLockedFiles = nullptr;
    286    
    287     m_syncAllCookiesActivity = nullptr;
    288     m_syncAllCookiesCounter = 0;
    289286
    290287    m_uploadActivity = WTF::nullopt;
    … …  
    12511248}
    12521249
    1253 void NetworkProcessProxy::syncAllCookies()
    1254 {
    1255     send(Messages::NetworkProcess::SyncAllCookies(), 0);
    1256    
    1257     ++m_syncAllCookiesCounter;
    1258     if (m_syncAllCookiesActivity)
    1259         return;
    1260    
    1261     RELEASE_LOG(ProcessSuspension, "%p - NetworkProcessProxy is taking a background assertion because the Network process is syncing cookies", this);
    1262     m_syncAllCookiesActivity = throttler().backgroundActivity("Syncing cookies"_s).moveToUniquePtr();
    1263 }
    1264    
    1265 void NetworkProcessProxy::didSyncAllCookies()
    1266 {
    1267     ASSERT(m_syncAllCookiesCounter);
    1268 
    1269     --m_syncAllCookiesCounter;
    1270     if (!m_syncAllCookiesCounter) {
    1271         RELEASE_LOG(ProcessSuspension, "%p - NetworkProcessProxy is releasing a background assertion because the Network process is done syncing cookies", this);
    1272         m_syncAllCookiesActivity = nullptr;
    1273     }
     1250void NetworkProcessProxy::flushCookies(const PAL::SessionID& sessionID, CompletionHandler<void()>&& completionHandler)
     1251{
     1252    sendWithAsyncReply(Messages::NetworkProcess::FlushCookies(sessionID), WTFMove(completionHandler));
    12741253}
    12751254

  • trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h

    r266829 r267042  
    199199    void setIsHoldingLockedFiles(bool);
    200200
    201     void syncAllCookies();
    202     void didSyncAllCookies();
     201    void flushCookies(const PAL::SessionID&, CompletionHandler<void()>&&);
    203202
    204203    void testProcessIncomingSyncMessagesWhenWaitingForSyncReply(WebPageProxyIdentifier, Messages::NetworkProcessProxy::TestProcessIncomingSyncMessagesWhenWaitingForSyncReplyDelayedReply&&);
    … …  
    326325    ProcessThrottler m_throttler;
    327326    std::unique_ptr<ProcessThrottler::BackgroundActivity> m_activityForHoldingLockedFiles;
    328     std::unique_ptr<ProcessThrottler::BackgroundActivity> m_syncAllCookiesActivity;
    329327    ProcessThrottler::ActivityVariant m_activityFromWebProcesses;
    330    
    331     unsigned m_syncAllCookiesCounter { 0 };
    332328
    333329#if ENABLE(CONTENT_EXTENSIONS)

  • trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in

    r266829 r267042  
    2929    DidDeleteWebsiteData(WebKit::CallbackID callbackID)
    3030    DidDeleteWebsiteDataForOrigins(WebKit::CallbackID callbackID)
    31 
    32     DidSyncAllCookies()
    3331
    3432    TestProcessIncomingSyncMessagesWhenWaitingForSyncReply(WebKit::WebPageProxyIdentifier pageID) -> (bool handled) Synchronous

  • trunk/Source/WebKit/UIProcess/WebProcessPool.cpp

    r266951 r267042  
    18341834}
    18351835
    1836 void WebProcessPool::syncNetworkProcessCookies()
    1837 {
    1838     ensureNetworkProcess().syncAllCookies();
     1836void WebProcessPool::flushCookies(const PAL::SessionID& sessionID, CompletionHandler<void()>&& completionHandler)
     1837{
     1838    ensureNetworkProcess().flushCookies(sessionID, WTFMove(completionHandler));
    18391839}
    18401840

  • trunk/Source/WebKit/UIProcess/WebProcessPool.h

    r266951 r267042  
    329329    void terminateServiceWorkers();
    330330
    331     void syncNetworkProcessCookies();
     331    void flushCookies(const PAL::SessionID&, CompletionHandler<void()>&&);
    332332    void syncLocalStorage(CompletionHandler<void()>&& callback);
    333333    void clearLegacyPrivateBrowsingLocalStorage(CompletionHandler<void()>&& callback);

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

    r266829 r267042  
    22552255}
    22562256
     2257void WebsiteDataStore::flushCookies(CompletionHandler<void()>&& completionHandler)
     2258{
     2259    auto callbackAggregator = CallbackAggregator::create(WTFMove(completionHandler));
     2260    for (auto processPool : WebProcessPool::allProcessPools())
     2261        processPool->flushCookies(sessionID(), [callbackAggregator] { });
     2262}
     2263
    22572264void WebsiteDataStore::dispatchOnQueue(Function<void()>&& function)
    22582265{

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

    r266829 r267042  
    239239    void removePendingCookie(const WebCore::Cookie&);
    240240    void clearPendingCookies();
     241    void flushCookies(CompletionHandler<void()>&&);
    241242
    242243    void dispatchOnQueue(Function<void()>&&);

  • trunk/Tools/ChangeLog

    r267035 r267042  
     12020-09-14  Alex Christensen  <achristensen@webkit.org>
     2
     3        Move cookie flushing SPI from WKProcessPool to WKHTTPCookieStore
     4        https://bugs.webkit.org/show_bug.cgi?id=216493
     5
     6        Reviewed by Chris Dumez.
     7
     8        * TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:
     9        (runWebsiteDataStoreCustomPaths):
     10        (TEST):
     11
    1122020-09-14  Jonathan Bedard  <jbedard@apple.com>
    213

  • trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm

    r266150 r267042  
    3333#import "TestWKWebView.h"
    3434#import <JavaScriptCore/JSCConfig.h>
     35#import <WebKit/WKHTTPCookieStorePrivate.h>
    3536#import <WebKit/WKPreferencesRef.h>
    3637#import <WebKit/WKProcessPoolPrivate.h>
    … …  
    149150    EXPECT_STREQ([getNextMessage().body UTF8String], "Success opening indexed database");
    150151
    151     [[[webView configuration] processPool] _syncNetworkProcessCookies];
     152    __block bool flushed = false;
     153    [configuration.get().websiteDataStore.httpCookieStore _flushCookiesToDiskWithCompletionHandler:^{
     154        flushed = true;
     155    }];
     156    TestWebKitAPI::Util::run(&flushed);
    152157
    153158    // Forcibly shut down everything of WebKit that we can.
    … …  
    419424    [webView loadRequest:request];
    420425
    421     [[[webView configuration] processPool] _syncNetworkProcessCookies];
     426    __block bool flushed = false;
     427    [configuration.get().websiteDataStore.httpCookieStore _flushCookiesToDiskWithCompletionHandler:^{
     428        flushed = true;
     429    }];
     430    TestWebKitAPI::Util::run(&flushed);
    422431
    423432    // Forcibly shut down everything of WebKit that we can.
    … …  
    494503    [webView loadRequest:request];
    495504
    496     [[[webView configuration] processPool] _syncNetworkProcessCookies];
     505    __block bool flushed = false;
     506    [configuration.get().websiteDataStore.httpCookieStore _flushCookiesToDiskWithCompletionHandler:^{
     507        flushed = true;
     508    }];
     509    TestWebKitAPI::Util::run(&flushed);
    497510
    498511    // Forcibly shut down everything of WebKit that we can.
Note: See TracChangeset for help on using the changeset viewer.