Title: [233488] trunk
- Revision
- 233488
- Author
- [email protected]
- Date
- 2018-07-03 17:05:39 -0700 (Tue, 03 Jul 2018)
Log Message
Make CallbackMap::invalidate() safe to re-enter
https://bugs.webkit.org/show_bug.cgi?id=187298
<rdar://problem/41057167>
Reviewed by Geoffrey Garen.
Source/WebKit:
Made it safe to re-enter CallbackMap::invalidate(), GenericCallback::performCallbackWithReturnValue(),
GenericCallback::invalidate() & invalidateCallbackMap() since those execute client blocks which may
re-enter WebKit.
* UIProcess/GenericCallback.h:
(WebKit::GenericCallback::performCallbackWithReturnValue):
(WebKit::invalidateCallbackMap):
Tools:
Add API test coverage.
* TestWebKitAPI/Tests/WebKitCocoa/WebContentProcessDidTerminate.mm:
(TEST):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (233487 => 233488)
--- trunk/Source/WebKit/ChangeLog 2018-07-03 23:54:46 UTC (rev 233487)
+++ trunk/Source/WebKit/ChangeLog 2018-07-04 00:05:39 UTC (rev 233488)
@@ -1,3 +1,19 @@
+2018-07-03 Chris Dumez <[email protected]>
+
+ Make CallbackMap::invalidate() safe to re-enter
+ https://bugs.webkit.org/show_bug.cgi?id=187298
+ <rdar://problem/41057167>
+
+ Reviewed by Geoffrey Garen.
+
+ Made it safe to re-enter CallbackMap::invalidate(), GenericCallback::performCallbackWithReturnValue(),
+ GenericCallback::invalidate() & invalidateCallbackMap() since those execute client blocks which may
+ re-enter WebKit.
+
+ * UIProcess/GenericCallback.h:
+ (WebKit::GenericCallback::performCallbackWithReturnValue):
+ (WebKit::invalidateCallbackMap):
+
2018-07-03 Brent Fulgham <[email protected]>
[iOS] Clean up sandbox warnings found during Public Beta
Modified: trunk/Source/WebKit/UIProcess/GenericCallback.h (233487 => 233488)
--- trunk/Source/WebKit/UIProcess/GenericCallback.h 2018-07-03 23:54:46 UTC (rev 233487)
+++ trunk/Source/WebKit/UIProcess/GenericCallback.h 2018-07-04 00:05:39 UTC (rev 233488)
@@ -105,9 +105,8 @@
if (!m_callback)
return;
- m_callback.value()(returnValue..., Error::None);
-
- m_callback = std::nullopt;
+ auto callback = std::exchange(m_callback, std::nullopt);
+ callback.value()(returnValue..., Error::None);
}
void performCallback()
@@ -122,9 +121,8 @@
if (!m_callback)
return;
- m_callback.value()(typename std::remove_reference<T>::type()..., error);
-
- m_callback = std::nullopt;
+ auto callback = std::exchange(m_callback, std::nullopt);
+ callback.value()(typename std::remove_reference<T>::type()..., error);
}
private:
@@ -163,10 +161,9 @@
template<typename T>
void invalidateCallbackMap(HashMap<uint64_t, T>& callbackMap, CallbackBase::Error error)
{
- for (auto& callback : copyToVector(callbackMap.values()))
+ auto map = WTFMove(callbackMap);
+ for (auto& callback : map.values())
callback->invalidate(error);
-
- callbackMap.clear();
}
class CallbackMap {
Modified: trunk/Tools/ChangeLog (233487 => 233488)
--- trunk/Tools/ChangeLog 2018-07-03 23:54:46 UTC (rev 233487)
+++ trunk/Tools/ChangeLog 2018-07-04 00:05:39 UTC (rev 233488)
@@ -1,5 +1,18 @@
2018-07-03 Chris Dumez <[email protected]>
+ Make CallbackMap::invalidate() safe to re-enter
+ https://bugs.webkit.org/show_bug.cgi?id=187298
+ <rdar://problem/41057167>
+
+ Reviewed by Geoffrey Garen.
+
+ Add API test coverage.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/WebContentProcessDidTerminate.mm:
+ (TEST):
+
+2018-07-03 Chris Dumez <[email protected]>
+
[Cocoa] Disable vnode guard related simulated crashes for WKTR / DRT and WebSQL
https://bugs.webkit.org/show_bug.cgi?id=187270
<rdar://problem/40674034>
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebContentProcessDidTerminate.mm (233487 => 233488)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebContentProcessDidTerminate.mm 2018-07-03 23:54:46 UTC (rev 233487)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebContentProcessDidTerminate.mm 2018-07-04 00:05:39 UTC (rev 233488)
@@ -42,6 +42,8 @@
static bool finishedLoad;
static bool shouldLoadAgainOnCrash;
static bool receivedScriptMessage;
+static bool calledAllCallbacks;
+static unsigned callbackCount;
static NSString *testHTML = @"<script>window.webkit.messageHandlers.testHandler.postMessage('LOADED');</script>";
@@ -206,4 +208,72 @@
EXPECT_FALSE(startedLoad);
}
+TEST(WKNavigation, ProcessCrashDuringCallback)
+{
+ auto webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+
+ auto delegate = adoptNS([[BasicNavigationDelegateWithoutCrashHandler alloc] init]);
+ [webView setNavigationDelegate:delegate.get()];
+
+ startedLoad = false;
+ finishedLoad = false;
+
+ [webView loadRequest:[NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"rich-and-plain-text" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]]];
+ TestWebKitAPI::Util::run(&finishedLoad);
+
+ startedLoad = false;
+ finishedLoad = false;
+
+ callbackCount = 0;
+ calledAllCallbacks = false;
+
+ __block WKWebView *view = webView.get();
+ [webView _getContentsAsStringWithCompletionHandler:^(NSString *contents, NSError *error) {
+ EXPECT_TRUE(!!error);
+ ++callbackCount;
+ if (callbackCount == 6)
+ calledAllCallbacks = true;
+ }];
+ [webView _getContentsAsStringWithCompletionHandler:^(NSString *contents, NSError *error) {
+ EXPECT_TRUE(!!error);
+ ++callbackCount;
+ if (callbackCount == 6)
+ calledAllCallbacks = true;
+ }];
+ [webView _getContentsAsStringWithCompletionHandler:^(NSString *contents, NSError *error) {
+ EXPECT_TRUE(!!error);
+ [view _close]; // Calling _close will also invalidate all callbacks.
+ ++callbackCount;
+ if (callbackCount == 6)
+ calledAllCallbacks = true;
+ }];
+ [webView _getContentsAsStringWithCompletionHandler:^(NSString *contents, NSError *error) {
+ EXPECT_TRUE(!!error);
+ ++callbackCount;
+ if (callbackCount == 6)
+ calledAllCallbacks = true;
+ }];
+ [webView _getContentsAsStringWithCompletionHandler:^(NSString *contents, NSError *error) {
+ EXPECT_TRUE(!!error);
+ ++callbackCount;
+ if (callbackCount == 6)
+ calledAllCallbacks = true;
+ }];
+ [webView _getContentsAsStringWithCompletionHandler:^(NSString *contents, NSError *error) {
+ EXPECT_TRUE(!!error);
+ ++callbackCount;
+ if (callbackCount == 6)
+ calledAllCallbacks = true;
+ }];
+
+ // Simulate a crash, which should invalidate all pending callbacks.
+ [webView _killWebContentProcess];
+
+ webView = nullptr;
+
+ TestWebKitAPI::Util::run(&calledAllCallbacks);
+ TestWebKitAPI::Util::sleep(0.5);
+ EXPECT_EQ(6U, callbackCount);
+}
+
#endif // WK_API_ENABLED
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
