Skip to content

Commit ef0eb63

Browse files
committed
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: Canonical link: https://commits.webkit.org/230257@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@268199 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent a16b2d8 commit ef0eb63

4 files changed

Lines changed: 74 additions & 1 deletion

File tree

Source/WebKit/ChangeLog

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,19 @@
1+
2020-10-08 Chris Dumez <cdumez@apple.com>
2+
3+
Crash under WebKit::WebProcessCache::clear()
4+
https://bugs.webkit.org/show_bug.cgi?id=217480
5+
6+
Reviewed by Geoffrey Garen.
7+
8+
Protect |process| in the responsiveness check lambda in WebProcessCache::addProcessIfPossible().
9+
If we fail to do so and WebProcessCache::clear() gets called while the responsiveness check is
10+
pending, the WebProcessProxy destructor may get called while clear() clears m_pendingAddRequests,
11+
which would resolve the responsiveness check with responsive=false, and cause the lambda to
12+
try and remove the entry from m_pendingAddRequests (while clear() is clearing it).
13+
14+
* UIProcess/WebProcessCache.cpp:
15+
(WebKit::WebProcessCache::addProcessIfPossible):
16+
117
2020-10-08 David Quesada <david_quesada@apple.com>
218

319
WKWebViewConfiguration._shouldRelaxThirdPartyCookieBlocking should be available on iOS

Source/WebKit/UIProcess/WebProcessCache.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ bool WebProcessCache::addProcessIfPossible(Ref<WebProcessProxy>&& process)
9393
m_pendingAddRequests.add(requestIdentifier, makeUnique<CachedProcess>(process.copyRef()));
9494

9595
WEBPROCESSCACHE_RELEASE_LOG("addProcessIfPossible: Checking if process is responsive before caching it", process->processIdentifier());
96-
process->isResponsive([this, processPool = makeRef(process->processPool()), requestIdentifier](bool isResponsive) {
96+
process->isResponsive([this, processPool = makeRef(process->processPool()), process, requestIdentifier](bool isResponsive) {
9797
auto cachedProcess = m_pendingAddRequests.take(requestIdentifier);
9898
if (!cachedProcess)
9999
return;

Tools/ChangeLog

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
2020-10-08 Chris Dumez <cdumez@apple.com>
2+
3+
Crash under WebKit::WebProcessCache::clear()
4+
https://bugs.webkit.org/show_bug.cgi?id=217480
5+
6+
Reviewed by Geoffrey Garen.
7+
8+
Add API test coverage.
9+
10+
* TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm:
11+
112
2020-10-08 Sihui Liu <sihui_liu@apple.com>
213

314
Adjust heuristic for checking whether view reaches visually non-empty state

Tools/TestWebKitAPI/Tests/WebKitCocoa/ProcessSwapOnNavigation.mm

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6856,3 +6856,49 @@ static bool hasOverlay(CALayer *layer)
68566856
TestWebKitAPI::Util::run(&finishedRunningScript);
68576857
}];
68586858
}
6859+
6860+
TEST(WebProcessCache, ClearWhenEnteringCache)
6861+
{
6862+
auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
6863+
processPoolConfiguration.get().usesWebProcessCache = YES;
6864+
auto processPool = adoptNS([[WKProcessPool alloc] _initWithConfiguration:processPoolConfiguration.get()]);
6865+
6866+
auto webViewConfiguration = adoptNS([[WKWebViewConfiguration alloc] init]);
6867+
[webViewConfiguration setProcessPool:processPool.get()];
6868+
auto handler = adoptNS([[PSONScheme alloc] init]);
6869+
[webViewConfiguration setURLSchemeHandler:handler.get() forURLScheme:@"PSON"];
6870+
6871+
@autoreleasepool {
6872+
auto webView1 = adoptNS([[WKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 800) configuration:webViewConfiguration.get()]);
6873+
auto webView2 = adoptNS([[WKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 800) configuration:webViewConfiguration.get()]);
6874+
auto webView3 = adoptNS([[WKWebView alloc] initWithFrame:CGRectMake(0, 0, 800, 800) configuration:webViewConfiguration.get()]);
6875+
6876+
auto delegate = adoptNS([[PSONNavigationDelegate alloc] init]);
6877+
[webView1 setNavigationDelegate:delegate.get()];
6878+
[webView2 setNavigationDelegate:delegate.get()];
6879+
[webView3 setNavigationDelegate:delegate.get()];
6880+
6881+
NSURLRequest *request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.webkit.org/main.html"]];
6882+
[webView1 loadRequest:request];
6883+
6884+
TestWebKitAPI::Util::run(&done);
6885+
done = false;
6886+
6887+
request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.apple.com/main.html"]];
6888+
[webView2 loadRequest:request];
6889+
6890+
TestWebKitAPI::Util::run(&done);
6891+
done = false;
6892+
6893+
request = [NSURLRequest requestWithURL:[NSURL URLWithString:@"pson://www.google.com/main.html"]];
6894+
[webView3 loadRequest:request];
6895+
6896+
TestWebKitAPI::Util::run(&done);
6897+
done = false;
6898+
}
6899+
6900+
TestWebKitAPI::Util::spinRunLoop();
6901+
6902+
// Clear the WebProcess cache while the processes are being checked for responsiveness.
6903+
[processPool _clearWebProcessCache];
6904+
}

0 commit comments

Comments
 (0)