Attachment 342960 Details for Bug 186784 – Patch

[patch] Patch

bug-186784-20180618121317.patch (text/plain), 8.75 KB, created by Chris Dumez on 2018-06-18 12:13:18 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-06-18 12:13:18 PDT

Size: 8.75 KB

patch

obsolete

>Subversion Revision: 232931
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index d2f64247e3e6dcdd2a3c77854328e32121a25629..9ad3801bd149fe850e9f711f7c79cf5a5f0ae442 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,26 @@
>+2018-06-18  Chris Dumez  <cdumez@apple.com>
>+
>+        Crash under WebProcessPool::networkProcessFailedToLaunch():
>+        https://bugs.webkit.org/show_bug.cgi?id=186784
>+        <rdar://problem/33535377>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * UIProcess/API/Cocoa/WKProcessPool.mm:
>+        (+[WKProcessPool _allProcessPoolsForTesting]):
>+        Add SPI to retrieve all WebProcessPool for testing purposes.
>+
>+        * UIProcess/API/Cocoa/WKProcessPoolPrivate.h:
>+        * UIProcess/Network/NetworkProcessProxy.cpp:
>+        (WebKit::NetworkProcessProxy::clearCallbackStates):
>+        Make iteration over completion handlers robust against completion handlers
>+        getting removed while we iterate.
>+
>+        (WebKit::NetworkProcessProxy::didClose):
>+        Ref the WebProcessPool (which keeps the NetworkProcessProxy alive too)
>+        as several calls within this method might cause the WebProcessPool /
>+        NetworkProcessProxy to get destroyed.
>+
> 2018-06-18  Karl Leplat  <karl.leplat_ext@softathome.com>
> 
>         [Threaded paintingEngine] Fix rendering glitches
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm b/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm
>index 576a585e6cd556adb647bc019f9850abfef32eb9..bac9c2795a78bd80883b0a69464b50556712d3b4 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKProcessPool.mm
>@@ -161,6 +161,15 @@ + (WKProcessPool *)_sharedProcessPool
>     return sharedProcessPool;
> }
> 
>++ (NSArray<WKProcessPool *> *)_allProcessPoolsForTesting
>+{
>+    auto& allPools = WebKit::WebProcessPool::allProcessPools();
>+    auto nsAllPools = adoptNS([[NSMutableArray alloc] initWithCapacity:allPools.size()]);
>+    for (auto* pool : allPools)
>+        [nsAllPools addObject:wrapper(*pool)];
>+    return nsAllPools.autorelease();
>+}
>+
> + (NSURL *)_websiteDataURLForContainerWithURL:(NSURL *)containerURL
> {
>     return [WKProcessPool _websiteDataURLForContainerWithURL:containerURL bundleIdentifierIfNotInContainer:nil];
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h b/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h
>index 46df02b3d5ea9aa221b47f6cb8753e8fc28f3a1d..db00c2ffe61ec1d1ebb26b8da74fb17f1e5d9610 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKProcessPoolPrivate.h
>@@ -42,6 +42,8 @@
> 
> + (WKProcessPool *)_sharedProcessPool;
> 
>++ (NSArray<WKProcessPool *> *)_allProcessPoolsForTesting WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
>+
> @property (nonatomic, readonly) _WKProcessPoolConfiguration *_configuration;
> 
> - (void)_setAllowsSpecificHTTPSCertificate:(NSArray *)certificateChain forHost:(NSString *)host;
>diff --git a/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp b/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp
>index d745025ea8335eda197845fdedb77f6f6fe96190..cbb182b65a6c0ace15300de964333013464624be 100644
>--- a/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp
>+++ b/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp
>@@ -223,17 +223,14 @@ void NetworkProcessProxy::networkProcessFailedToLaunch()
> 
> void NetworkProcessProxy::clearCallbackStates()
> {
>-    for (const auto& callback : m_pendingFetchWebsiteDataCallbacks.values())
>-        callback(WebsiteData());
>-    m_pendingFetchWebsiteDataCallbacks.clear();
>+    while (!m_pendingFetchWebsiteDataCallbacks.isEmpty())
>+        m_pendingFetchWebsiteDataCallbacks.take(m_pendingFetchWebsiteDataCallbacks.begin()->key)(WebsiteData { });
> 
>-    for (const auto& callback : m_pendingDeleteWebsiteDataCallbacks.values())
>-        callback();
>-    m_pendingDeleteWebsiteDataCallbacks.clear();
>+    while (!m_pendingDeleteWebsiteDataCallbacks.isEmpty())
>+        m_pendingDeleteWebsiteDataCallbacks.take(m_pendingDeleteWebsiteDataCallbacks.begin()->key)();
> 
>-    for (const auto& callback : m_pendingDeleteWebsiteDataForOriginsCallbacks.values())
>-        callback();
>-    m_pendingDeleteWebsiteDataForOriginsCallbacks.clear();
>+    while (!m_pendingDeleteWebsiteDataForOriginsCallbacks.isEmpty())
>+        m_pendingDeleteWebsiteDataForOriginsCallbacks.take(m_pendingDeleteWebsiteDataForOriginsCallbacks.begin()->key)();
> }
> 
> void NetworkProcessProxy::didReceiveMessage(IPC::Connection& connection, IPC::Decoder& decoder)
>@@ -257,6 +254,8 @@ void NetworkProcessProxy::didReceiveSyncMessage(IPC::Connection& connection, IPC
> 
> void NetworkProcessProxy::didClose(IPC::Connection&)
> {
>+    auto protectedProcessPool = makeRef(m_processPool);
>+
>     if (m_downloadProxyMap)
>         m_downloadProxyMap->processDidClose();
> #if ENABLE(LEGACY_CUSTOM_PROTOCOL_MANAGER)
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index c65cefb4ea40db1a17ede7ec78bb8a539d46280e..585cc698325d8425d9e54da756da1b9d8f9f34da 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-06-18  Chris Dumez  <cdumez@apple.com>
>+
>+        Crash under WebProcessPool::networkProcessFailedToLaunch():
>+        https://bugs.webkit.org/show_bug.cgi?id=186784
>+        <rdar://problem/33535377>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add API test coverage.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm:
>+        (TEST):
>+
> 2018-06-18  Zan Dobersek  <zdobersek@igalia.com>
> 
>         [webkitpy] WPTRunner should remove any metadata content before (re)generating it
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm
>index 797ea4e82d9fbed0d10a8d0161f02d124b7cf109..a8f0847a92a077d52591d1430d758de754f2da2e 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsiteDataStoreCustomPaths.mm
>@@ -285,6 +285,60 @@ TEST(WebKit, CustomDataStorePathsVersusCompletionHandlers)
>     TestWebKitAPI::Util::run(&readyToContinue);
> }
> 
>+TEST(WebKit, CustomDataStoreDestroyWhileFetchingNetworkProcessData)
>+{
>+    NSURL *cookieStorageFile = [NSURL fileURLWithPath:[@"~/Library/WebKit/TestWebKitAPI/CustomWebsiteData/CookieStorage/Cookie.File" stringByExpandingTildeInPath] isDirectory:NO];
>+    [[NSFileManager defaultManager] removeItemAtURL:cookieStorageFile error:nil];
>+
>+    auto websiteDataTypes = adoptNS([[NSSet alloc] initWithArray:@[WKWebsiteDataTypeCookies]]);
>+    static bool readyToContinue;
>+
>+    auto websiteDataStoreConfiguration = adoptNS([[_WKWebsiteDataStoreConfiguration alloc] init]);
>+    websiteDataStoreConfiguration.get()._cookieStorageFile = cookieStorageFile;
>+
>+    @autoreleasepool {
>+        auto dataStore = adoptNS([[WKWebsiteDataStore alloc] _initWithConfiguration:websiteDataStoreConfiguration.get()]);
>+
>+        // Fetch records
>+        [dataStore fetchDataRecordsOfTypes:websiteDataTypes.get() completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) {
>+            EXPECT_EQ((int)dataRecords.count, 0);
>+            readyToContinue = true;
>+        }];
>+        TestWebKitAPI::Util::run(&readyToContinue);
>+        readyToContinue = false;
>+
>+        // Fetch records again, this time releasing our reference to the data store while the request is in flight.
>+        [dataStore fetchDataRecordsOfTypes:websiteDataTypes.get() completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) {
>+            EXPECT_EQ((int)dataRecords.count, 0);
>+            readyToContinue = true;
>+        }];
>+        dataStore = nil;
>+    }
>+    TestWebKitAPI::Util::run(&readyToContinue);
>+    readyToContinue = false;
>+
>+    @autoreleasepool {
>+        auto dataStore = adoptNS([[WKWebsiteDataStore alloc] _initWithConfiguration:websiteDataStoreConfiguration.get()]);
>+        [dataStore fetchDataRecordsOfTypes:websiteDataTypes.get() completionHandler:^(NSArray<WKWebsiteDataRecord *> *dataRecords) {
>+            EXPECT_EQ((int)dataRecords.count, 0);
>+            readyToContinue = true;
>+        }];
>+
>+        // Terminate the network process while a query is pending.
>+        auto* allProcessPools = [WKProcessPool _allProcessPoolsForTesting];
>+        ASSERT_EQ(1U, [allProcessPools count]);
>+        auto* processPool = allProcessPools[0];
>+        while (![processPool _networkProcessIdentifier])
>+            TestWebKitAPI::Util::sleep(0.01);
>+        kill([processPool _networkProcessIdentifier], SIGKILL);
>+        allProcessPools = nil;
>+        dataStore = nil;
>+    }
>+
>+    TestWebKitAPI::Util::run(&readyToContinue);
>+    readyToContinue = false;
>+}
>+
> TEST(WebKit, WebsiteDataStoreEphemeral)
> {
>     RetainPtr<WebsiteDataStoreCustomPathsMessageHandler> handler = adoptNS([[WebsiteDataStoreCustomPathsMessageHandler alloc] init]);

Actions: View | Formatted Diff | Diff

Attachments on bug 186784: 342960 | 342971