Title: [268199] trunk
Revision
268199
Author
cdu...@apple.com
Date
2020-10-08 12:03:08 -0700 (Thu, 08 Oct 2020)

Log Message

Crash under WebKit::WebProcessCache::clear()
https://bugs.webkit.org/show_bug.cgi?id=217480

Reviewed by Geoffrey Garen.

Source/WebKit:

Protect |process| in the responsiveness check lambda in WebProcessCache::addProcessIfPossible().
If we fail to do so and WebProcessCache::clear() gets called while the responsiveness check is
pending, the WebProcessProxy destructor may get called while clear() clears m_pendingAddRequests,
which would resolve the responsiveness check with responsive=false, and cause the lambda to
try and remove the entry from m_pendingAddRequests (while clear() is clearing it).

* UIProcess/WebProcessCache.cpp:
(WebKit::WebProcessCache::addProcessIfPossible):

Tools:

Add API test coverage.

* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (268198 => 268199)


--- trunk/Source/WebKit/ChangeLog	2020-10-08 18:56:45 UTC (rev 268198)
+++ trunk/Source/WebKit/ChangeLog	2020-10-08 19:03:08 UTC (rev 268199)
@@ -1,3 +1,19 @@
+2020-10-08  Chris Dumez  <cdu...@apple.com>
+
+        Crash under WebKit::WebProcessCache::clear()
+        https://bugs.webkit.org/show_bug.cgi?id=217480
+
+        Reviewed by Geoffrey Garen.
+
+        Protect |process| in the responsiveness check lambda in WebProcessCache::addProcessIfPossible().
+        If we fail to do so and WebProcessCache::clear() gets called while the responsiveness check is
+        pending, the WebProcessProxy destructor may get called while clear() clears m_pendingAddRequests,
+        which would resolve the responsiveness check with responsive=false, and cause the lambda to
+        try and remove the entry from m_pendingAddRequests (while clear() is clearing it).
+
+        * UIProcess/WebProcessCache.cpp:
+        (WebKit::WebProcessCache::addProcessIfPossible):
+
 2020-10-08  David Quesada  <david_ques...@apple.com>
 
         WKWebViewConfiguration._shouldRelaxThirdPartyCookieBlocking should be available on iOS

Modified: trunk/Source/WebKit/UIProcess/WebProcessCache.cpp (268198 => 268199)


--- trunk/Source/WebKit/UIProcess/WebProcessCache.cpp	2020-10-08 18:56:45 UTC (rev 268198)
+++ trunk/Source/WebKit/UIProcess/WebProcessCache.cpp	2020-10-08 19:03:08 UTC (rev 268199)
@@ -93,7 +93,7 @@
     m_pendingAddRequests.add(requestIdentifier, makeUnique<CachedProcess>(process.copyRef()));
 
     WEBPROCESSCACHE_RELEASE_LOG("addProcessIfPossible: Checking if process is responsive before caching it", process->processIdentifier());
-    process->isResponsive([this, processPool = makeRef(process->processPool()), requestIdentifier](bool isResponsive) {
+    process->isResponsive([this, processPool = makeRef(process->processPool()), process, requestIdentifier](bool isResponsive) {
         auto cachedProcess = m_pendingAddRequests.take(requestIdentifier);
         if (!cachedProcess)
             return;

Modified: trunk/Tools/ChangeLog (268198 => 268199)


--- trunk/Tools/ChangeLog	2020-10-08 18:56:45 UTC (rev 268198)
+++ trunk/Tools/ChangeLog	2020-10-08 19:03:08 UTC (rev 268199)
@@ -1,3 +1,14 @@
+2020-10-08  Chris Dumez  <cdu...@apple.com>
+
+        Crash under WebKit::WebProcessCache::clear()
+        https://bugs.webkit.org/show_bug.cgi?id=217480
+
+        Reviewed by Geoffrey Garen.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
+
 2020-10-08  Sihui Liu  <sihui_...@apple.com>
 
         Adjust heuristic for checking whether view reaches visually non-empty state

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm (268198 => 268199)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm	2020-10-08 18:56:45 UTC (rev 268198)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm	2020-10-08 19:03:08 UTC (rev 268199)
@@ -6856,3 +6856,49 @@
         TestWebKitAPI::Util::run(&finishedRunningScript);
     }];
 }
+
+TEST(WebProcessCache, ClearWhenEnteringCache)
+{
+    auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
+    processPoolConfiguration.get().usesWebProcessCache = YES;
+    auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
+
+    auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [webViewConfiguration setProcessPool:processPool.get()];
+    auto handler = adoptNS([[PSONScheme alloc] init]);
+    [webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
+
+    @autoreleasepool {
+        auto webView1 = adoptNS([[WKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 800) configuration:webViewConfiguration.get()]);
+        auto webView2 = adoptNS([[WKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 800) configuration:webViewConfiguration.get()]);
+        auto webView3 = adoptNS([[WKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 800) configuration:webViewConfiguration.get()]);
+
+        auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
+        [webView1 setNavigationDelegate:delegate.get()];
+        [webView2 setNavigationDelegate:delegate.get()];
+        [webView3 setNavigationDelegate:delegate.get()];
+
+        NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
+        [webView1 loadRequest:request];
+
+        TestWebKitAPI::Util::run(&done);
+        done = false;
+
+        request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
+        [webView2 loadRequest:request];
+
+        TestWebKitAPI::Util::run(&done);
+        done = false;
+
+        request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.google.com/main.html"]];
+        [webView3 loadRequest:request];
+
+        TestWebKitAPI::Util::run(&done);
+        done = false;
+    }
+
+    TestWebKitAPI::Util::spinRunLoop();
+
+    // Clear the WebProcess cache while the processes are being checked for responsiveness.
+    [processPool _clearWebProcessCache];
+}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to