Title: [282914] branches/safari-612-branch
Revision
282914
Author
repst...@apple.com
Date
2021-09-22 21:29:25 -0700 (Wed, 22 Sep 2021)

Log Message

Cherry-pick r282365. rdar://problem/83429982

    Crash under WebPage::runJavaScript()
    https://bugs.webkit.org/show_bug.cgi?id=230223
    <rdar://80172436>

    Reviewed by Brady Eidson.

    Source/WebKit:

    The resolveFunction() was capturing `frame = makeRef(frame)` and then calling `frame->coreFrame()->script()`.
    This would lead to a null dereference crash in the case where the core frame gets destroyed before the JS
    promise gets resolved. Protecting the WebFrame does not keep the core Frame alive as WebFrame::m_coreFrame
    is merely a WeakPtr. To address the issue, the lambda now also protects the core frame and uses it to get
    the script controller.

    * WebProcess/WebPage/WebPage.cpp:
    (WebKit::WebPage::runJavaScript):

    Tools:

    Add API test coverage.

    * TestWebKitAPI/Tests/WebKitCocoa/AsyncFunction.mm:
    (-[AsyncJSUIDelegate initWithAlertHandler:]):
    (-[AsyncJSUIDelegate webView:runJavaScriptAlertPanelWithMessage:initiatedByFrame:completionHandler:]):
    (TestWebKitAPI::TEST):

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@282365 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Diff

Modified: branches/safari-612-branch/Source/WebKit/ChangeLog (282913 => 282914)


--- branches/safari-612-branch/Source/WebKit/ChangeLog	2021-09-23 04:29:22 UTC (rev 282913)
+++ branches/safari-612-branch/Source/WebKit/ChangeLog	2021-09-23 04:29:25 UTC (rev 282914)
@@ -1,5 +1,55 @@
 2021-09-22  Alan Coon  <alanc...@apple.com>
 
+        Cherry-pick r282365. rdar://problem/83429982
+
+    Crash under WebPage::runJavaScript()
+    https://bugs.webkit.org/show_bug.cgi?id=230223
+    <rdar://80172436>
+    
+    Reviewed by Brady Eidson.
+    
+    Source/WebKit:
+    
+    The resolveFunction() was capturing `frame = makeRef(frame)` and then calling `frame->coreFrame()->script()`.
+    This would lead to a null dereference crash in the case where the core frame gets destroyed before the JS
+    promise gets resolved. Protecting the WebFrame does not keep the core Frame alive as WebFrame::m_coreFrame
+    is merely a WeakPtr. To address the issue, the lambda now also protects the core frame and uses it to get
+    the script controller.
+    
+    * WebProcess/WebPage/WebPage.cpp:
+    (WebKit::WebPage::runJavaScript):
+    
+    Tools:
+    
+    Add API test coverage.
+    
+    * TestWebKitAPI/Tests/WebKitCocoa/AsyncFunction.mm:
+    (-[AsyncJSUIDelegate initWithAlertHandler:]):
+    (-[AsyncJSUIDelegate webView:runJavaScriptAlertPanelWithMessage:initiatedByFrame:completionHandler:]):
+    (TestWebKitAPI::TEST):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@282365 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2021-09-13  Chris Dumez  <cdu...@apple.com>
+
+            Crash under WebPage::runJavaScript()
+            https://bugs.webkit.org/show_bug.cgi?id=230223
+            <rdar://80172436>
+
+            Reviewed by Brady Eidson.
+
+            The resolveFunction() was capturing `frame = makeRef(frame)` and then calling `frame->coreFrame()->script()`.
+            This would lead to a null dereference crash in the case where the core frame gets destroyed before the JS
+            promise gets resolved. Protecting the WebFrame does not keep the core Frame alive as WebFrame::m_coreFrame
+            is merely a WeakPtr. To address the issue, the lambda now also protects the core frame and uses it to get
+            the script controller.
+
+            * WebProcess/WebPage/WebPage.cpp:
+            (WebKit::WebPage::runJavaScript):
+
+2021-09-22  Alan Coon  <alanc...@apple.com>
+
         Cherry-pick r282358. rdar://problem/83429732
 
     AX: Make PDFs loaded via <embed> accessible

Modified: branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp (282913 => 282914)


--- branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2021-09-23 04:29:22 UTC (rev 282913)
+++ branches/safari-612-branch/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2021-09-23 04:29:25 UTC (rev 282914)
@@ -3726,11 +3726,11 @@
     }
 #endif
 
-    auto resolveFunction = [world = makeRef(*world), frame = makeRef(*frame), completionHandler = WTFMove(completionHandler)] (ValueOrException result) mutable {
+    auto resolveFunction = [world = makeRef(*world), frame = Ref { *frame }, coreFrame = Ref { *frame->coreFrame() }, completionHandler = WTFMove(completionHandler)] (ValueOrException result) mutable {
         RefPtr<SerializedScriptValue> serializedResultValue;
         if (result) {
             serializedResultValue = SerializedScriptValue::create(frame->jsContextForWorld(world.ptr()),
-                toRef(frame->coreFrame()->script().globalObject(world->coreWorld()), result.value()), nullptr);
+                toRef(coreFrame->script().globalObject(world->coreWorld()), result.value()), nullptr);
         }
 
         IPC::DataReference dataReference;

Modified: branches/safari-612-branch/Tools/ChangeLog (282913 => 282914)


--- branches/safari-612-branch/Tools/ChangeLog	2021-09-23 04:29:22 UTC (rev 282913)
+++ branches/safari-612-branch/Tools/ChangeLog	2021-09-23 04:29:25 UTC (rev 282914)
@@ -1,5 +1,53 @@
 2021-09-22  Alan Coon  <alanc...@apple.com>
 
+        Cherry-pick r282365. rdar://problem/83429982
+
+    Crash under WebPage::runJavaScript()
+    https://bugs.webkit.org/show_bug.cgi?id=230223
+    <rdar://80172436>
+    
+    Reviewed by Brady Eidson.
+    
+    Source/WebKit:
+    
+    The resolveFunction() was capturing `frame = makeRef(frame)` and then calling `frame->coreFrame()->script()`.
+    This would lead to a null dereference crash in the case where the core frame gets destroyed before the JS
+    promise gets resolved. Protecting the WebFrame does not keep the core Frame alive as WebFrame::m_coreFrame
+    is merely a WeakPtr. To address the issue, the lambda now also protects the core frame and uses it to get
+    the script controller.
+    
+    * WebProcess/WebPage/WebPage.cpp:
+    (WebKit::WebPage::runJavaScript):
+    
+    Tools:
+    
+    Add API test coverage.
+    
+    * TestWebKitAPI/Tests/WebKitCocoa/AsyncFunction.mm:
+    (-[AsyncJSUIDelegate initWithAlertHandler:]):
+    (-[AsyncJSUIDelegate webView:runJavaScriptAlertPanelWithMessage:initiatedByFrame:completionHandler:]):
+    (TestWebKitAPI::TEST):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@282365 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2021-09-13  Chris Dumez  <cdu...@apple.com>
+
+            Crash under WebPage::runJavaScript()
+            https://bugs.webkit.org/show_bug.cgi?id=230223
+            <rdar://80172436>
+
+            Reviewed by Brady Eidson.
+
+            Add API test coverage.
+
+            * TestWebKitAPI/Tests/WebKitCocoa/AsyncFunction.mm:
+            (-[AsyncJSUIDelegate initWithAlertHandler:]):
+            (-[AsyncJSUIDelegate webView:runJavaScriptAlertPanelWithMessage:initiatedByFrame:completionHandler:]):
+            (TestWebKitAPI::TEST):
+
+2021-09-22  Alan Coon  <alanc...@apple.com>
+
         Cherry-pick r282358. rdar://problem/83429732
 
     AX: Make PDFs loaded via <embed> accessible

Modified: branches/safari-612-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/AsyncFunction.mm (282913 => 282914)


--- branches/safari-612-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/AsyncFunction.mm	2021-09-23 04:29:22 UTC (rev 282913)
+++ branches/safari-612-branch/Tools/TestWebKitAPI/Tests/WebKitCocoa/AsyncFunction.mm	2021-09-23 04:29:25 UTC (rev 282914)
@@ -27,8 +27,10 @@
 
 #import "PlatformUtilities.h"
 #import "Test.h"
+#import "TestUIDelegate.h"
 #import "TestWKWebView.h"
 #import <WebKit/WKContentWorld.h>
+#import <WebKit/WKFrameInfo.h>
 #import <WebKit/WKProcessPoolPrivate.h>
 #import <WebKit/WKWebViewPrivate.h>
 
@@ -280,5 +282,43 @@
     TestWebKitAPI::Util::run(&done);
 }
 
+TEST(AsyncFunction, PromiseDetachedFrame)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+    auto uiDelegate = adoptNS([[TestUIDelegate alloc] init]);
+    [webView setUIDelegate:uiDelegate.get()];
+
+    __block RetainPtr<WKFrameInfo> subframe;
+    __block bool doneGettingSubframe = false;
+    uiDelegate.get().runJavaScriptAlertPanelWithMessage = ^(WKWebView *, NSString *, WKFrameInfo * frame, void (^completionHandler)(void)) {
+        subframe = frame;
+        doneGettingSubframe = true;
+        completionHandler();
+    };
+
+    [webView synchronouslyLoadHTMLString:@"<iframe src=''></iframe>"];
+
+    TestWebKitAPI::Util::run(&doneGettingSubframe);
+
+    ASSERT_TRUE(!!subframe);
+    EXPECT_FALSE([subframe isMainFrame]);
+
+    auto pid = [webView _webProcessIdentifier];
+    EXPECT_NE(pid, 0);
+
+    NSString *functionBody = @"return new Promise(function(resolve, reject) { setTimeout(function(){ top.setTimeout(function() { resolve(42); }, 100); top.document.querySelector('iframe').remove(); }, 0); })";
+
+    bool done = false;
+    [webView callAsyncJavaScript:functionBody arguments:nil inFrame:subframe.get() inContentWorld:WKContentWorld.pageWorld completionHandler:[&] (id result, NSError *error) {
+        EXPECT_NULL(error);
+        EXPECT_TRUE([result isKindOfClass:[NSNumber class]]);
+        EXPECT_TRUE([result isEqualToNumber:@42]);
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+
+    EXPECT_EQ([webView _webProcessIdentifier], pid);
 }
 
+}
+
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to