Title: [254711] trunk
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
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to