Changeset 215598 in webkit

Timestamp:

Apr 20, 2017 6:02:47 PM (7 years ago)

Author:

beidson@apple.com

Message:

WebContent process becomes unresponsive after returning nil from async version of -webView:createWebViewWithConfiguration:...
<rdar://problem/31739023> and ​https://bugs.webkit.org/show_bug.cgi?id=171090

Reviewed by Andy Estes.

Source/WebKit2:

• UIProcess/Cocoa/UIDelegate.mm:

(WebKit::UIDelegate::UIClient::createNewPageCommon): Handle the perfectly valid "nil webView" case correctly.

Tools:

• TestWebKitAPI/Tests/WebKit2/open-and-close-window.html:
• TestWebKitAPI/Tests/WebKit2Cocoa/OpenAndCloseWindow.mm:

(resetToConsistentState):
(-[OpenAndCloseWindowUIDelegate webViewDidClose:]):
(-[OpenAndCloseWindowUIDelegate webView:createWebViewWithConfiguration:forNavigationAction:windowFeatures:]):
(TEST):
(-[OpenAndCloseWindowUIDelegateAsync _webView:createWebViewWithConfiguration:forNavigationAction:windowFeatures:completionHandler:]):

Location:

trunk

Files:

• Source/WebKit2/ChangeLog (modified) (1 diff)
• Source/WebKit2/UIProcess/Cocoa/UIDelegate.mm (modified) (2 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKit2/open-and-close-window.html (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKit2Cocoa/OpenAndCloseWindow.mm (modified) (8 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2017-04-20  Brady Eidson  <beidson@apple.com>
+
+        WebContent process becomes unresponsive after returning nil from async version of -webView:createWebViewWithConfiguration:...
+        <rdar://problem/31739023> and https://bugs.webkit.org/show_bug.cgi?id=171090
+
+        Reviewed by Andy Estes.
+
+        * UIProcess/Cocoa/UIDelegate.mm:
+        (WebKit::UIDelegate::UIClient::createNewPageCommon): Handle the perfectly valid "nil webView" case correctly.
+
 2017-04-20  Dan Bernstein  <mitz@apple.com>
 

trunk/Source/WebKit2/UIProcess/Cocoa/UIDelegate.mm

@@ -195 +195 @@
             checker->didCallCompletionHandler();
 
-            if (!webView)
+            if (!webView) {
+                completionHandler(nullptr);
                 return;
+            }
 
             if ([webView->_configuration _relatedWebView] != relatedWebView.get())
@@ -202 +204 @@
 
             completionHandler(webView->_page.get());
-
         }).get()];
 

trunk/Tools/ChangeLog

@@ +1 @@
+2017-04-20  Brady Eidson  <beidson@apple.com>
+
+        WebContent process becomes unresponsive after returning nil from async version of -webView:createWebViewWithConfiguration:...
+        <rdar://problem/31739023> and https://bugs.webkit.org/show_bug.cgi?id=171090
+
+        Reviewed by Andy Estes.
+
+        * TestWebKitAPI/Tests/WebKit2/open-and-close-window.html:
+        * TestWebKitAPI/Tests/WebKit2Cocoa/OpenAndCloseWindow.mm:
+        (resetToConsistentState):
+        (-[OpenAndCloseWindowUIDelegate webViewDidClose:]):
+        (-[OpenAndCloseWindowUIDelegate webView:createWebViewWithConfiguration:forNavigationAction:windowFeatures:]):
+        (TEST):
+        (-[OpenAndCloseWindowUIDelegateAsync _webView:createWebViewWithConfiguration:forNavigationAction:windowFeatures:completionHandler:]):
+
 2017-04-20  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKit2/open-and-close-window.html

@@ -5 +5 @@
 if (document.location.search === "?close-window")
     window.close();
-else
-    window.open("open-and-close-window.html?close-window");
+else {
+    if (!window.open("open-and-close-window.html?close-window"))
+        window.close();
+}
 </script>
 </head>

trunk/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/OpenAndCloseWindow.mm

@@ -43 +43 @@
 static RetainPtr<OpenAndCloseWindowUIDelegateAsync> sharedUIDelegateAsync;
 
+static void resetToConsistentState()
+{
+    isDone = false;
+    openedWebView = nil;
+    sharedUIDelegate = nil;
+    sharedUIDelegateAsync = nil;
+}
+
 @interface OpenAndCloseWindowUIDelegate : NSObject <WKUIDelegate>
+@property (nonatomic, assign) WKWebView *expectedClosingView;
 @end
 
@@ -50 +59 @@
 - (void)webViewDidClose:(WKWebView *)webView
 {
-    EXPECT_EQ(openedWebView, webView);
+    EXPECT_EQ(_expectedClosingView, webView);
     isDone = true;
 }
@@ -58 +67 @@
     openedWebView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration]);
     [openedWebView setUIDelegate:sharedUIDelegate.get()];
+    _expectedClosingView = openedWebView.get();
     return openedWebView.get();
 }
@@ -65 +75 @@
 TEST(WebKit2, OpenAndCloseWindow)
 {
-    openedWebView = nullptr;
-    isDone = false;
+    resetToConsistentState();
 
     RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
@@ -84 +93 @@
 @property (nonatomic) BOOL shouldCallback;
 @property (nonatomic, assign) id savedCompletionHandler;
-
+@property (nonatomic) BOOL shouldCallbackWithNil;
 @end
 
@@ -104 +113 @@
 {
     if (_shouldCallback) {
-        dispatch_async(dispatch_get_main_queue(), ^ {
-            openedWebView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration]);
-            [openedWebView setUIDelegate:sharedUIDelegateAsync.get()];
-            completionHandler(openedWebView.get());
-        });
+        if (!_shouldCallbackWithNil) {
+            dispatch_async(dispatch_get_main_queue(), ^ {
+                openedWebView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration]);
+                [openedWebView setUIDelegate:sharedUIDelegateAsync.get()];
+                self.expectedClosingView = openedWebView.get();
+                completionHandler(openedWebView.get());
+            });
+        } else {
+            dispatch_async(dispatch_get_main_queue(), ^ {
+                self.expectedClosingView = webView;
+                completionHandler(nil);
+            });
+        }
         return;
     }
@@ -120 +137 @@
 TEST(WebKit2, OpenAndCloseWindowAsync)
 {
-    openedWebView = nullptr;
-    isDone = false;
+    resetToConsistentState();
 
     RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
@@ -137 +153 @@
 }
 
+TEST(WebKit2, OpenAsyncWithNil)
+{
+    resetToConsistentState();
+
+    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+
+    sharedUIDelegateAsync = adoptNS([[OpenAndCloseWindowUIDelegateAsync alloc] init]);
+    sharedUIDelegateAsync.get().shouldCallback = YES;
+    sharedUIDelegateAsync.get().shouldCallbackWithNil = YES;
+    [webView setUIDelegate:sharedUIDelegateAsync.get()];
+
+    [webView configuration].preferences.javaScriptCanOpenWindowsAutomatically = YES;
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"open-and-close-window" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
+    [webView loadRequest:request];
+
+    TestWebKitAPI::Util::run(&isDone);
+}
+
 // https://bugs.webkit.org/show_bug.cgi?id=171083 - Try to figure out why this fails for some configs but not others, and resolve.
 //TEST(WebKit2, OpenAndCloseWindowAsyncCallbackException)
 //{
-//    openedWebView = nullptr;
-//    isDone = false;
+//    resetToConsistentState();
 //
 //    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);