Attachment 346311 Details for Bug 188241 – Patch

[patch] Patch

bug-188241-20180801160550.patch (text/plain), 8.67 KB, created by Chris Dumez on 2018-08-01 16:05:51 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-08-01 16:05:51 PDT

Size: 8.67 KB

patch

obsolete

>Subversion Revision: 234483
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 2507ace3cc64473e6c3f41f2a61d1f80934dc168..4b46a28c3d6bac50a014fcb223f457725626b17d 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,31 @@
>+2018-08-01  Chris Dumez  <cdumez@apple.com>
>+
>+        Make sure cookies get flushed to disk before exiting or suspending the network process
>+        https://bugs.webkit.org/show_bug.cgi?id=188241
>+        <rdar://problem/42831831>
>+
>+        Reviewed by Alex Christensen and Geoffrey Garen.
>+
>+        Make sure cookies get flushed to disk before exiting or suspending the network process,
>+        to make sure they do not get lost.
>+
>+        * NetworkProcess/NetworkProcess.cpp:
>+        (WebKit::NetworkProcess::didClose):
>+        (WebKit::NetworkProcess::actualPrepareToSuspend):
>+        (WebKit::NetworkProcess::platformSyncAllCookies):
>+        * NetworkProcess/NetworkProcess.h:
>+        * NetworkProcess/cocoa/NetworkProcessCocoa.mm:
>+        (WebKit::NetworkProcess::syncAllCookies):
>+        (WebKit::NetworkProcess::platformSyncAllCookies):
>+        * Shared/ChildProcess.cpp:
>+        (WebKit::ChildProcess::didClose):
>+        (WebKit::callExitNow):
>+        (WebKit::callExitSoon):
>+        (WebKit::ChildProcess::initialize):
>+        (WebKit::didCloseOnConnectionWorkQueue): Deleted.
>+        * Shared/ChildProcess.h:
>+        (WebKit::ChildProcess::shouldCallExitWhenConnectionIsClosed const):
>+
> 2018-08-01  Alex Christensen  <achristensen@webkit.org>
> 
>         Move all calls to ResourceLoader::start to WebKitLegacy
>diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.cpp b/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>index 7c0cdd41fc1d2c59b73fc46be337e57a6a150e8b..14b4ef5675901a03857a030267d64dd69b65c429 100644
>--- a/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>@@ -190,6 +190,16 @@ void NetworkProcess::didReceiveSyncMessage(IPC::Connection& connection, IPC::Dec
>     didReceiveSyncNetworkProcessMessage(connection, decoder, replyEncoder);
> }
> 
>+void NetworkProcess::didClose(IPC::Connection&)
>+{
>+    ASSERT(RunLoop::isMain());
>+
>+    // Make sure we flush all cookies to disk before exiting.
>+    platformSyncAllCookies([this] {
>+        stopRunLoop();
>+    });
>+}
>+
> void NetworkProcess::didCreateDownload()
> {
>     disableTermination();
>@@ -846,6 +856,8 @@ void NetworkProcess::actualPrepareToSuspend(ShouldAcknowledgeWhenReadyToSuspend
>         }));
>     }
> 
>+    platformSyncAllCookies([delayedTaskCounter] { });
>+
>     for (auto& connection : m_webProcessConnections)
>         connection->cleanupForSuspension([delayedTaskCounter] { });
> }
>@@ -981,6 +993,11 @@ void NetworkProcess::syncAllCookies()
> {
> }
> 
>+void NetworkProcess::platformSyncAllCookies(CompletionHandler<void()>&& completionHandler)
>+{
>+    completionHandler();
>+}
>+
> #endif
> 
> } // namespace WebKit
>diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.h b/Source/WebKit/NetworkProcess/NetworkProcess.h
>index c9e9da2b0eeb56a1dba7df87061af7de05f3020d..de81924d8b94c6c1c788ff6e7a2d2b67e5205a81 100644
>--- a/Source/WebKit/NetworkProcess/NetworkProcess.h
>+++ b/Source/WebKit/NetworkProcess/NetworkProcess.h
>@@ -189,10 +189,12 @@ private:
>     void initializeSandbox(const ChildProcessInitializationParameters&, SandboxInitializationParameters&) override;
>     void initializeConnection(IPC::Connection*) override;
>     bool shouldTerminate() override;
>+    bool shouldCallExitWhenConnectionIsClosed() const final { return false; } // We override didClose() and want it to be called.
> 
>     // IPC::Connection::Client
>     void didReceiveMessage(IPC::Connection&, IPC::Decoder&) override;
>     void didReceiveSyncMessage(IPC::Connection&, IPC::Decoder&, std::unique_ptr<IPC::Encoder>&) override;
>+    void didClose(IPC::Connection&) override;
> 
>     // DownloadManager::Client
>     void didCreateDownload() override;
>@@ -252,6 +254,8 @@ private:
>     static void setSharedHTTPCookieStorage(const Vector<uint8_t>& identifier);
> #endif
> 
>+    void platformSyncAllCookies(CompletionHandler<void()>&&);
>+
>     void registerURLSchemeAsSecure(const String&) const;
>     void registerURLSchemeAsBypassingContentSecurityPolicy(const String&) const;
>     void registerURLSchemeAsLocal(const String&) const;
>diff --git a/Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm b/Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm
>index 5bc74986539f5479e61c85507a69fa4a69e9e998..76bae116687302bd6a169ebc1eab46bbf6652018 100644
>--- a/Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm
>+++ b/Source/WebKit/NetworkProcess/cocoa/NetworkProcessCocoa.mm
>@@ -207,20 +207,24 @@ void NetworkProcess::setStorageAccessAPIEnabled(bool enabled)
> 
> void NetworkProcess::syncAllCookies()
> {
>+    platformSyncAllCookies([this] {
>+        didSyncAllCookies();
>+    });
>+}
>+
>+void NetworkProcess::platformSyncAllCookies(CompletionHandler<void()>&& completionHander) {
>     ASSERT(hasProcessPrivilege(ProcessPrivilege::CanAccessRawCookies));
> #pragma clang diagnostic push
> #pragma clang diagnostic ignored "-Wdeprecated-declarations"
>     
> #if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 120000)
>-    RefPtr<CallbackAggregator> callbackAggregator = CallbackAggregator::create([this] {
>-        didSyncAllCookies();
>-    });
>+    RefPtr<CallbackAggregator> callbackAggregator = CallbackAggregator::create(WTFMove(completionHander));
>     WebCore::NetworkStorageSession::forEach([&] (auto& networkStorageSession) {
>         [networkStorageSession.nsCookieStorage() _saveCookies:[callbackAggregator] { }];
>     });
> #else
>     _CFHTTPCookieStorageFlushCookieStores();
>-    didSyncAllCookies();
>+    completionHander();
> #endif
> 
> #pragma clang diagnostic pop
>diff --git a/Source/WebKit/Shared/ChildProcess.cpp b/Source/WebKit/Shared/ChildProcess.cpp
>index 4534243835d0f0ced3bb839562bf59fd565e90bf..e66396248b801923e617d1d3c9e4a27e7a2fadd6 100644
>--- a/Source/WebKit/Shared/ChildProcess.cpp
>+++ b/Source/WebKit/Shared/ChildProcess.cpp
>@@ -56,15 +56,28 @@ ChildProcess::~ChildProcess()
> 
> void ChildProcess::didClose(IPC::Connection&)
> {
>-    // We call _exit() in didCloseOnConnectionWorkQueue.
>-    ASSERT_NOT_REACHED();
> }
> 
>-NO_RETURN static void didCloseOnConnectionWorkQueue(IPC::Connection*)
>+NO_RETURN static void callExitNow(IPC::Connection*)
> {
>     _exit(EXIT_SUCCESS);
> }
> 
>+static void callExitSoon(IPC::Connection*)
>+{
>+    // If the connection has been closed and we haven't responded in the main thread for 10 seconds
>+    // the process will exit forcibly.
>+    auto watchdogDelay = 10_s;
>+
>+    WorkQueue::create("com.apple.WebKit.ChildProcess.WatchDogQueue")->dispatchAfter(watchdogDelay, [] {
>+        // We use _exit here since the watchdog callback is called from another thread and we don't want
>+        // global destructors or atexit handlers to be called from this thread while the main thread is busy
>+        // doing its thing.
>+        RELEASE_LOG_ERROR(IPC, "Exiting process early due to unacknowledged closed-connection");
>+        _exit(EXIT_FAILURE);
>+    });
>+}
>+
> void ChildProcess::initialize(const ChildProcessInitializationParameters& parameters)
> {
>     RELEASE_ASSERT_WITH_MESSAGE(parameters.processIdentifier, "Unable to initialize child process without a WebCore process identifier");
>@@ -86,7 +99,11 @@ void ChildProcess::initialize(const ChildProcessInitializationParameters& parame
>     PAL::SessionID::enableGenerationProtection();
> 
>     m_connection = IPC::Connection::createClientConnection(parameters.connectionIdentifier, *this);
>-    m_connection->setDidCloseOnConnectionWorkQueueCallback(didCloseOnConnectionWorkQueue);
>+    if (shouldCallExitWhenConnectionIsClosed())
>+        m_connection->setDidCloseOnConnectionWorkQueueCallback(callExitNow);
>+    else
>+        m_connection->setDidCloseOnConnectionWorkQueueCallback(callExitSoon);
>+
>     initializeConnection(m_connection.get());
>     m_connection->open();
> }
>diff --git a/Source/WebKit/Shared/ChildProcess.h b/Source/WebKit/Shared/ChildProcess.h
>index a28772b2ed84b0cfab5ec989afe42fc539e019bf..95a310470efb5c3bd8e4c325249883b16378f8cd 100644
>--- a/Source/WebKit/Shared/ChildProcess.h
>+++ b/Source/WebKit/Shared/ChildProcess.h
>@@ -94,6 +94,7 @@ protected:
>     virtual bool shouldTerminate() = 0;
>     virtual void terminate();
> 
>+    virtual bool shouldCallExitWhenConnectionIsClosed() const { return true; }
>     virtual void stopRunLoop();
> 
> #if USE(APPKIT)
>@@ -120,7 +121,7 @@ private:
> 
>     // IPC::Connection::Client.
>     void didReceiveInvalidMessage(IPC::Connection&, IPC::StringReference messageReceiverName, IPC::StringReference messageName) final;
>-    void didClose(IPC::Connection&) final;
>+    void didClose(IPC::Connection&) override;
> 
>     void shutDown();
>

Actions: View | Formatted Diff | Diff

Attachments on bug 188241: 346306 | 346311