- Revision
- 254711
- Author
- [email protected]
- Date
- 2020-01-16 14:57:43 -0800 (Thu, 16 Jan 2020)
Log Message
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):
Modified Paths
Added Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (254710 => 254711)
--- trunk/Source/WebKit/ChangeLog 2020-01-16 22:54:02 UTC (rev 254710)
+++ trunk/Source/WebKit/ChangeLog 2020-01-16 22:57:43 UTC (rev 254711)
@@ -1,3 +1,30 @@
+2020-01-16 Chris Dumez <[email protected]>
+
+ 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 <[email protected]>
Add finite timeout when synchronously terminating a service worker
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (254710 => 254711)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm 2020-01-16 22:54:02 UTC (rev 254710)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm 2020-01-16 22:57:43 UTC (rev 254711)
@@ -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);
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h (254710 => 254711)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h 2020-01-16 22:54:02 UTC (rev 254710)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewPrivate.h 2020-01-16 22:57:43 UTC (rev 254711)
@@ -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));
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (254710 => 254711)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2020-01-16 22:54:02 UTC (rev 254710)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2020-01-16 22:57:43 UTC (rev 254711)
@@ -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));
}
Modified: trunk/Tools/ChangeLog (254710 => 254711)
--- trunk/Tools/ChangeLog 2020-01-16 22:54:02 UTC (rev 254710)
+++ trunk/Tools/ChangeLog 2020-01-16 22:57:43 UTC (rev 254711)
@@ -1,3 +1,20 @@
+2020-01-16 Chris Dumez <[email protected]>
+
+ 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 <[email protected]>
[WebAuthn] User Verification (UV) option present on a CTAP2 authenticatorMakeCredential while the authenticator has not advertised support for it
Modified: trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (254710 => 254711)
--- trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2020-01-16 22:54:02 UTC (rev 254710)
+++ trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2020-01-16 22:57:43 UTC (rev 254711)
@@ -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 */,
Added: trunk/Tools/TestWebKitAPI/Tests/WebKit/beforeunload.html (0 => 254711)
--- trunk/Tools/TestWebKitAPI/Tests/WebKit/beforeunload.html (rev 0)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit/beforeunload.html 2020-01-16 22:57:43 UTC (rev 254711)
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+_onbeforeunload_ = (ev) => {
+ ev.preventDefault();
+ return "foo";
+}
+</script>
+</head>
+</html>
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalAlerts.mm (254710 => 254711)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalAlerts.mm 2020-01-16 22:54:02 UTC (rev 254710)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ModalAlerts.mm 2020-01-16 22:57:43 UTC (rev 254711)
@@ -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