Regression(r253224) No longer able to prevent a tab from closing via the beforeunload prompt
https://bugs.webkit.org/show_bug.cgi?id=206366
<rdar://problem/58537467>
Reviewed by Geoffrey Garen.
Source/WebKit:
Change is covered by new API test.
* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _tryClose]):
(-[WKWebView _isClosed]):
* UIProcess/API/Cocoa/WKWebViewPrivate.h:
Add new _tryClose / _isClosed SPI on WKWebView in order to write an API test to cover
the change.
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::runBeforeUnloadConfirmPanel):
In WebPageProxy::tryClose(), we start a timer before sending the TryClose async IPC to the
WebProcess. We would then stop the timer when receiving the response to the TryClose IPC.
If the timer fires, we would forcefully close the page. The issue was that before answering
the TryClose IPC, the WebContent process would send a sync RunBeforeUnloadConfirmPanel IPC
to the UIProcess to show the confirmation prompt, and this would fail to stop the timer.
WebPageProxy::runBeforeUnloadConfirmPanel() would spin a nested run loop to show the prompt
and we would time out while showing the prompt.
Tools:
tryClose_timeout_fix
* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKit/beforeunload.html: Added.
* TestWebKitAPI/Tests/WebKitCocoa/ModalAlerts.mm:
(-[SlowBeforeUnloadPromptUIDelegate _webView:runBeforeUnloadConfirmPanelWithMessage:initiatedByFrame:completionHandler:]):
(-[SlowBeforeUnloadPromptUIDelegate webViewDidClose:]):
(TEST):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@254711 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index 918a3e5..c6720c8 100644
--- a/Source/WebKit/ChangeLog
+++ b/Source/WebKit/ChangeLog
@@ -1,3 +1,30 @@
+2020-01-16 Chris Dumez <cdumez@apple.com>
+
+ Regression(r253224) No longer able to prevent a tab from closing via the beforeunload prompt
+ https://bugs.webkit.org/show_bug.cgi?id=206366
+ <rdar://problem/58537467>
+
+ Reviewed by Geoffrey Garen.
+
+ Change is covered by new API test.
+
+ * UIProcess/API/Cocoa/WKWebView.mm:
+ (-[WKWebView _tryClose]):
+ (-[WKWebView _isClosed]):
+ * UIProcess/API/Cocoa/WKWebViewPrivate.h:
+ Add new _tryClose / _isClosed SPI on WKWebView in order to write an API test to cover
+ the change.
+
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::runBeforeUnloadConfirmPanel):
+ In WebPageProxy::tryClose(), we start a timer before sending the TryClose async IPC to the
+ WebProcess. We would then stop the timer when receiving the response to the TryClose IPC.
+ If the timer fires, we would forcefully close the page. The issue was that before answering
+ the TryClose IPC, the WebContent process would send a sync RunBeforeUnloadConfirmPanel IPC
+ to the UIProcess to show the confirmation prompt, and this would fail to stop the timer.
+ WebPageProxy::runBeforeUnloadConfirmPanel() would spin a nested run loop to show the prompt
+ and we would time out while showing the prompt.
+
2020-01-16 Alex Christensen <achristensen@webkit.org>
Add finite timeout when synchronously terminating a service worker
diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
index 74564db..4a54001 100644
--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
@@ -2041,6 +2041,16 @@
_page->close();
}
+- (void)_tryClose
+{
+ _page->tryClose();
+}
+
+- (BOOL)_isClosed
+{
+ return _page->isClosed();
+}
+
- (_WKAttachment *)_insertAttachmentWithFilename:(NSString *)filename contentType:(NSString *)contentType data:(NSData *)data options:(_WKAttachmentDisplayOptions *)options completion:(void(^)(BOOL success))completionHandler
{
UNUSED_PARAM(options);
diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h
index efc1b60..4af6dcf 100644
--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h
+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h
@@ -188,6 +188,8 @@
@property (nonatomic, readonly, getter=_isShowingNavigationGestureSnapshot) BOOL _showingNavigationGestureSnapshot;
- (void)_close;
+- (void)_tryClose WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
+- (BOOL)_isClosed WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
- (void)_updateWebsitePolicies:(_WKWebsitePolicies *)websitePolicies WK_API_DEPRECATED_WITH_REPLACEMENT("-_updateWebpagePreferences:", macos(10.13, WK_MAC_TBA), ios(11.3, WK_IOS_TBA));
- (void)_updateWebpagePreferences:(WKWebpagePreferences *)webpagePreferences WK_API_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA));
diff --git a/Source/WebKit/UIProcess/WebPageProxy.cpp b/Source/WebKit/UIProcess/WebPageProxy.cpp
index 7bb955f..a5114e8 100644
--- a/Source/WebKit/UIProcess/WebPageProxy.cpp
+++ b/Source/WebKit/UIProcess/WebPageProxy.cpp
@@ -5628,8 +5628,9 @@
}
}
- // Since runBeforeUnloadConfirmPanel() can spin a nested run loop we need to turn off the responsiveness timer.
+ // Since runBeforeUnloadConfirmPanel() can spin a nested run loop we need to turn off the responsiveness timer and the tryClose timer.
m_process->responsivenessTimer().stop();
+ m_tryCloseTimeoutTimer.stop();
m_uiClient->runBeforeUnloadConfirmPanel(*this, message, frame, WTFMove(securityOrigin), WTFMove(reply));
}
diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 75a4b68..8b9c93b 100644
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,20 @@
+2020-01-16 Chris Dumez <cdumez@apple.com>
+
+ Regression(r253224) No longer able to prevent a tab from closing via the beforeunload prompt
+ https://bugs.webkit.org/show_bug.cgi?id=206366
+ <rdar://problem/58537467>
+
+ Reviewed by Geoffrey Garen.
+
+ tryClose_timeout_fix
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WebKit/beforeunload.html: Added.
+ * TestWebKitAPI/Tests/WebKitCocoa/ModalAlerts.mm:
+ (-[SlowBeforeUnloadPromptUIDelegate _webView:runBeforeUnloadConfirmPanelWithMessage:initiatedByFrame:completionHandler:]):
+ (-[SlowBeforeUnloadPromptUIDelegate webViewDidClose:]):
+ (TEST):
+
2020-01-16 Jiewen Tan <jiewen_tan@apple.com>
[WebAuthn] User Verification (UV) option present on a CTAP2 authenticatorMakeCredential while the authenticator has not advertised support for it
diff --git a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
index cc82f40..12d4759 100644
--- a/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
+++ b/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
@@ -241,6 +241,7 @@
46918EFC2237283C00468DFE /* DeviceOrientation.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46918EFB2237283500468DFE /* DeviceOrientation.mm */; };
46A911592108E6780078D40D /* CustomUserAgent.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46A911582108E66B0078D40D /* CustomUserAgent.mm */; };
46AE5A3720F9066D00E0873E /* SimpleServiceWorkerRegistrations-4.sqlite3 in Copy Resources */ = {isa = PBXBuildFile; fileRef = 4656A75720F9054F0002E21F /* SimpleServiceWorkerRegistrations-4.sqlite3 */; };
+ 46C3AEB323D0E529001B0680 /* beforeunload.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 46C3AEB223D0E50F001B0680 /* beforeunload.html */; };
46C519DA1D355AB200DAA51A /* LocalStorageNullEntries.mm in Sources */ = {isa = PBXBuildFile; fileRef = 46C519D81D355A7300DAA51A /* LocalStorageNullEntries.mm */; };
46C519E61D3563FD00DAA51A /* LocalStorageNullEntries.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 46C519E21D35629600DAA51A /* LocalStorageNullEntries.html */; };
46C519E71D3563FD00DAA51A /* LocalStorageNullEntries.localstorage in Copy Resources */ = {isa = PBXBuildFile; fileRef = 46C519E31D35629600DAA51A /* LocalStorageNullEntries.localstorage */; };
@@ -1157,6 +1158,7 @@
dstPath = TestWebKitAPI.resources;
dstSubfolderSpec = 7;
files = (
+ 46C3AEB323D0E529001B0680 /* beforeunload.html in Copy Resources */,
55A817FF2181021A0004A39A /* 100x100-red.tga in Copy Resources */,
1A9E52C913E65EF4006917F5 /* 18-characters.html in Copy Resources */,
55A81800218102210004A39A /* 400x400-green.png in Copy Resources */,
@@ -1795,6 +1797,7 @@
468F2F932368DAA700F4B864 /* window-open-then-document-open.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "window-open-then-document-open.html"; sourceTree = "<group>"; };
46918EFB2237283500468DFE /* DeviceOrientation.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = DeviceOrientation.mm; sourceTree = "<group>"; };
46A911582108E66B0078D40D /* CustomUserAgent.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CustomUserAgent.mm; sourceTree = "<group>"; };
+ 46C3AEB223D0E50F001B0680 /* beforeunload.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = beforeunload.html; sourceTree = "<group>"; };
46C519D81D355A7300DAA51A /* LocalStorageNullEntries.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = LocalStorageNullEntries.mm; sourceTree = "<group>"; };
46C519E21D35629600DAA51A /* LocalStorageNullEntries.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = LocalStorageNullEntries.html; sourceTree = "<group>"; };
46C519E31D35629600DAA51A /* LocalStorageNullEntries.localstorage */ = {isa = PBXFileReference; lastKnownFileType = file; path = LocalStorageNullEntries.localstorage; sourceTree = "<group>"; };
@@ -3867,6 +3870,7 @@
C95984F31E36BC55002C0D45 /* autoplay-no-audio-check.html */,
C99B675A1E3971FC00FC6C80 /* autoplay-with-controls.html */,
C99BDF881E8097E300C7170E /* autoplay-zero-volume-check.html */,
+ 46C3AEB223D0E50F001B0680 /* beforeunload.html */,
7C486BA01AA1254B003F6F9B /* bundle-file.html */,
9BD4239B1E04BFD000200395 /* chinese-character-with-image.html */,
1A50AA1F1A2A4EA500F4C345 /* close-from-within-create-page.html */,
diff --git a/Tools/TestWebKitAPI/Tests/WebKit/beforeunload.html b/Tools/TestWebKitAPI/Tests/WebKit/beforeunload.html
new file mode 100644
index 0000000..3336f39
--- /dev/null
+++ b/Tools/TestWebKitAPI/Tests/WebKit/beforeunload.html
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+onbeforeunload = (ev) => {
+ ev.preventDefault();
+ return "foo";
+}
+</script>
+</head>
+</html>
diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalAlerts.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalAlerts.mm
index 2608604..dad92ff 100644
--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalAlerts.mm
+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalAlerts.mm
@@ -31,6 +31,7 @@
#import <WebKit/WKUIDelegatePrivate.h>
#import <WebKit/WKWebView.h>
#import <WebKit/WKWebViewConfiguration.h>
+#import <WebKit/WKWebViewPrivate.h>
#import <wtf/RetainPtr.h>
@class ModalAlertsUIDelegate;
@@ -141,3 +142,52 @@
TestWebKitAPI::Util::run(&isDone);
}
+
+#if PLATFORM(MAC)
+
+static bool didRejectTryClose = false;
+
+@interface SlowBeforeUnloadPromptUIDelegate : NSObject <WKUIDelegate>
+@end
+
+@implementation SlowBeforeUnloadPromptUIDelegate
+
+- (void)_webView:(WKWebView *)webView runBeforeUnloadConfirmPanelWithMessage:(NSString *)message initiatedByFrame:(WKFrameInfo *)frame completionHandler:(void (^)(BOOL result))completionHandler
+{
+ dispatch_after(dispatch_time(DISPATCH_TIME_NOW, 0.5 * NSEC_PER_SEC), dispatch_get_main_queue(), ^{
+ completionHandler(NO);
+ didRejectTryClose = true;
+ });
+}
+
+- (void)webViewDidClose:(WKWebView *)webView
+{
+ [webView _close];
+}
+
+@end
+
+TEST(WebKit, SlowBeforeUnloadPrompt)
+{
+ auto slowBeforeUnloadPromptUIDelegate = adoptNS([[SlowBeforeUnloadPromptUIDelegate alloc] init]);
+ auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+ [webView setUIDelegate:slowBeforeUnloadPromptUIDelegate.get()];
+ [webView synchronouslyLoadTestPageNamed:@"beforeunload"];
+
+ TestWebKitAPI::Util::spinRunLoop(10);
+
+ // Need a user gesture on the page before being allowed to show the beforeunload prompt.
+ [webView sendClicksAtPoint:NSMakePoint(5, 5) numberOfClicks:1];
+
+ TestWebKitAPI::Util::spinRunLoop(10);
+
+ [webView _tryClose];
+
+ TestWebKitAPI::Util::run(&didRejectTryClose);
+ EXPECT_FALSE([webView _isClosed]);
+
+ TestWebKitAPI::Util::sleep(0.1);
+ EXPECT_FALSE([webView _isClosed]);
+}
+
+#endif
