- Revision
- 287850
- Author
- [email protected]
- Date
- 2022-01-10 12:09:53 -0800 (Mon, 10 Jan 2022)
Log Message
Cherry-pick r287814. rdar://problem/86845512
Null pointer crash when calling into `-[WebView close]` in `-webView:didCommitLoadForFrame:`
https://bugs.webkit.org/show_bug.cgi?id=234994
rdar://86845512
Reviewed by Chris Dumez and Geoff Garen.
Source/WebCore:
Add a null check in `FrameLoader::dispatchDidCommitLoad()` before calling into any methods on the frame's Page.
Since we called into the client layer to `dispatchDidCommitLoad()` immediately prior to this, a legacy WebKit
client may have already caused the page to be destroyed (e.g. by closing the web view). Since Frame's pointer
to Page is a WeakPtr, this triggers null derefs when attempting to call either `didCommitLoad()` or
`remoteInspectorInformationDidChange()`.
Test: WebKitLegacy.CloseWhileCommittingLoad
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::dispatchDidCommitLoad):
Tools:
Add a new API test to exercise the fix.
* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKitCocoa/CloseWhileCommittingLoad.mm: Added.
(-[CloseWhileCommittingLoadDelegate webView:didCommitLoadForFrame:]):
(TestWebKitAPI::TEST):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@287814 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Added Paths
Diff
Modified: branches/safari-612.4.9.1-branch/Source/WebCore/ChangeLog (287849 => 287850)
--- branches/safari-612.4.9.1-branch/Source/WebCore/ChangeLog 2022-01-10 19:19:54 UTC (rev 287849)
+++ branches/safari-612.4.9.1-branch/Source/WebCore/ChangeLog 2022-01-10 20:09:53 UTC (rev 287850)
@@ -1,3 +1,56 @@
+2022-01-10 Russell Epstein <[email protected]>
+
+ Cherry-pick r287814. rdar://problem/86845512
+
+ Null pointer crash when calling into `-[WebView close]` in `-webView:didCommitLoadForFrame:`
+ https://bugs.webkit.org/show_bug.cgi?id=234994
+ rdar://86845512
+
+ Reviewed by Chris Dumez and Geoff Garen.
+
+ Source/WebCore:
+
+ Add a null check in `FrameLoader::dispatchDidCommitLoad()` before calling into any methods on the frame's Page.
+ Since we called into the client layer to `dispatchDidCommitLoad()` immediately prior to this, a legacy WebKit
+ client may have already caused the page to be destroyed (e.g. by closing the web view). Since Frame's pointer
+ to Page is a WeakPtr, this triggers null derefs when attempting to call either `didCommitLoad()` or
+ `remoteInspectorInformationDidChange()`.
+
+ Test: WebKitLegacy.CloseWhileCommittingLoad
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::dispatchDidCommitLoad):
+
+ Tools:
+
+ Add a new API test to exercise the fix.
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WebKitCocoa/CloseWhileCommittingLoad.mm: Added.
+ (-[CloseWhileCommittingLoadDelegate webView:didCommitLoadForFrame:]):
+ (TestWebKitAPI::TEST):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@287814 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2022-01-08 Wenson Hsieh <[email protected]>
+
+ Null pointer crash when calling into `-[WebView close]` in `-webView:didCommitLoadForFrame:`
+ https://bugs.webkit.org/show_bug.cgi?id=234994
+ rdar://86845512
+
+ Reviewed by Chris Dumez and Geoff Garen.
+
+ Add a null check in `FrameLoader::dispatchDidCommitLoad()` before calling into any methods on the frame's Page.
+ Since we called into the client layer to `dispatchDidCommitLoad()` immediately prior to this, a legacy WebKit
+ client may have already caused the page to be destroyed (e.g. by closing the web view). Since Frame's pointer
+ to Page is a WeakPtr, this triggers null derefs when attempting to call either `didCommitLoad()` or
+ `remoteInspectorInformationDidChange()`.
+
+ Test: WebKitLegacy.CloseWhileCommittingLoad
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::dispatchDidCommitLoad):
+
2022-01-07 Russell Epstein <[email protected]>
Cherry-pick r286094. rdar://problem/87125111
Modified: branches/safari-612.4.9.1-branch/Source/WebCore/loader/FrameLoader.cpp (287849 => 287850)
--- branches/safari-612.4.9.1-branch/Source/WebCore/loader/FrameLoader.cpp 2022-01-10 19:19:54 UTC (rev 287849)
+++ branches/safari-612.4.9.1-branch/Source/WebCore/loader/FrameLoader.cpp 2022-01-10 20:09:53 UTC (rev 287850)
@@ -4032,14 +4032,14 @@
m_client->dispatchDidCommitLoad(initialHasInsecureContent, initialUsedLegacyTLS);
- if (m_frame.isMainFrame())
- m_frame.page()->didCommitLoad();
+ if (auto* page = m_frame.page(); page && m_frame.isMainFrame())
+ page->didCommitLoad();
InspectorInstrumentation::didCommitLoad(m_frame, m_documentLoader.get());
#if ENABLE(REMOTE_INSPECTOR)
- if (m_frame.isMainFrame())
- m_frame.page()->remoteInspectorInformationDidChange();
+ if (auto* page = m_frame.page(); page && m_frame.isMainFrame())
+ page->remoteInspectorInformationDidChange();
#endif
}
Modified: branches/safari-612.4.9.1-branch/Tools/ChangeLog (287849 => 287850)
--- branches/safari-612.4.9.1-branch/Tools/ChangeLog 2022-01-10 19:19:54 UTC (rev 287849)
+++ branches/safari-612.4.9.1-branch/Tools/ChangeLog 2022-01-10 20:09:53 UTC (rev 287850)
@@ -1,3 +1,52 @@
+2022-01-10 Russell Epstein <[email protected]>
+
+ Cherry-pick r287814. rdar://problem/86845512
+
+ Null pointer crash when calling into `-[WebView close]` in `-webView:didCommitLoadForFrame:`
+ https://bugs.webkit.org/show_bug.cgi?id=234994
+ rdar://86845512
+
+ Reviewed by Chris Dumez and Geoff Garen.
+
+ Source/WebCore:
+
+ Add a null check in `FrameLoader::dispatchDidCommitLoad()` before calling into any methods on the frame's Page.
+ Since we called into the client layer to `dispatchDidCommitLoad()` immediately prior to this, a legacy WebKit
+ client may have already caused the page to be destroyed (e.g. by closing the web view). Since Frame's pointer
+ to Page is a WeakPtr, this triggers null derefs when attempting to call either `didCommitLoad()` or
+ `remoteInspectorInformationDidChange()`.
+
+ Test: WebKitLegacy.CloseWhileCommittingLoad
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::dispatchDidCommitLoad):
+
+ Tools:
+
+ Add a new API test to exercise the fix.
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WebKitCocoa/CloseWhileCommittingLoad.mm: Added.
+ (-[CloseWhileCommittingLoadDelegate webView:didCommitLoadForFrame:]):
+ (TestWebKitAPI::TEST):
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@287814 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2022-01-08 Wenson Hsieh <[email protected]>
+
+ Null pointer crash when calling into `-[WebView close]` in `-webView:didCommitLoadForFrame:`
+ https://bugs.webkit.org/show_bug.cgi?id=234994
+ rdar://86845512
+
+ Reviewed by Chris Dumez and Geoff Garen.
+
+ Add a new API test to exercise the fix.
+
+ * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+ * TestWebKitAPI/Tests/WebKitCocoa/CloseWhileCommittingLoad.mm: Added.
+ (-[CloseWhileCommittingLoadDelegate webView:didCommitLoadForFrame:]):
+ (TestWebKitAPI::TEST):
+
2022-01-05 Russell Epstein <[email protected]>
Cherry-pick r287039. rdar://problem/85015428
Modified: branches/safari-612.4.9.1-branch/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj (287849 => 287850)
--- branches/safari-612.4.9.1-branch/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2022-01-10 19:19:54 UTC (rev 287849)
+++ branches/safari-612.4.9.1-branch/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj 2022-01-10 20:09:53 UTC (rev 287850)
@@ -1199,6 +1199,7 @@
F42D634422A1729F00D2FB3A /* AutocorrectionTestsIOS.mm in Sources */ = {isa = PBXBuildFile; fileRef = F42D634322A1729F00D2FB3A /* AutocorrectionTestsIOS.mm */; };
F42DA5161D8CEFE400336F40 /* large-input-field-focus-onload.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F42DA5151D8CEFDB00336F40 /* large-input-field-focus-onload.html */; };
F434CA1A22E65BCA005DDB26 /* ScrollToRevealSelection.mm in Sources */ = {isa = PBXBuildFile; fileRef = F434CA1922E65BCA005DDB26 /* ScrollToRevealSelection.mm */; };
+ F43CAB1D278A326500C8D0A2 /* CloseWhileCommittingLoad.mm in Sources */ = {isa = PBXBuildFile; fileRef = F43CAB1C278A326500C8D0A2 /* CloseWhileCommittingLoad.mm */; };
F43E3BBF20DADA1E00A4E7ED /* WKScrollViewTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = F43E3BBE20DADA1E00A4E7ED /* WKScrollViewTests.mm */; };
F43E3BC120DADBC500A4E7ED /* fixed-nav-bar.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F43E3BC020DADB8000A4E7ED /* fixed-nav-bar.html */; };
F442851D2140DF2900CCDA22 /* NSFontPanelTesting.mm in Sources */ = {isa = PBXBuildFile; fileRef = F442851C2140DF2900CCDA22 /* NSFontPanelTesting.mm */; };
@@ -3046,6 +3047,7 @@
F42D634322A1729F00D2FB3A /* AutocorrectionTestsIOS.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = AutocorrectionTestsIOS.mm; sourceTree = "<group>"; };
F42DA5151D8CEFDB00336F40 /* large-input-field-focus-onload.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; name = "large-input-field-focus-onload.html"; path = "Tests/WebKitCocoa/large-input-field-focus-onload.html"; sourceTree = SOURCE_ROOT; };
F434CA1922E65BCA005DDB26 /* ScrollToRevealSelection.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = ScrollToRevealSelection.mm; sourceTree = "<group>"; };
+ F43CAB1C278A326500C8D0A2 /* CloseWhileCommittingLoad.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CloseWhileCommittingLoad.mm; sourceTree = "<group>"; };
F43E3BBE20DADA1E00A4E7ED /* WKScrollViewTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = WKScrollViewTests.mm; sourceTree = "<group>"; };
F43E3BC020DADB8000A4E7ED /* fixed-nav-bar.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "fixed-nav-bar.html"; sourceTree = "<group>"; };
F442851B2140DF2900CCDA22 /* NSFontPanelTesting.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = NSFontPanelTesting.h; sourceTree = "<group>"; };
@@ -3140,6 +3142,7 @@
F4D5E4E71F0C5D27008C1A49 /* dragstart-clear-selection.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "dragstart-clear-selection.html"; sourceTree = "<group>"; };
F4D65DA71F5E46C0009D8C27 /* selected-text-image-link-and-editable.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "selected-text-image-link-and-editable.html"; sourceTree = "<group>"; };
F4D9818E2196B911008230FC /* editable-nested-lists.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "editable-nested-lists.html"; sourceTree = "<group>"; };
+ F4DBE5DC278906A300642BC0 /* CloseWhileCommittingLoad.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = CloseWhileCommittingLoad.mm; sourceTree = "<group>"; };
F4DEF6EC1E9B4D950048EF61 /* image-in-link-and-input.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "image-in-link-and-input.html"; sourceTree = "<group>"; };
F4E0A28E211E5D5B00AF7C7F /* DragAndDropTestsMac.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = DragAndDropTestsMac.mm; sourceTree = "<group>"; };
F4E0A295211FC5A300AF7C7F /* selected-text-and-textarea.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "selected-text-and-textarea.html"; sourceTree = "<group>"; };
@@ -3442,6 +3445,7 @@
F422A3E8235ACA9B00CF00CA /* ClipboardTests.mm */,
CDF92236216D186400647AA7 /* CloseWebViewAfterEnterFullscreen.mm */,
CDF0B789216D484300421ECC /* CloseWebViewDuringEnterFullscreen.mm */,
+ F4DBE5DC278906A300642BC0 /* CloseWhileCommittingLoad.mm */,
1AAD19F51C7CE20300831E47 /* Coding.mm */,
7C3DB8E21D12129B00AE8CC3 /* CommandBackForward.mm */,
5C4A84941F7EEFD400ACFC54 /* Configuration.mm */,
@@ -4753,6 +4757,7 @@
26DF5A5D15A29BAA003689C2 /* CancelLoadFromResourceLoadDelegate.mm */,
93CFA8681CEBCFED000565A8 /* CandidateTests.mm */,
290A9BB51735DE8A00D71BBC /* CloseNewWindowInNavigationPolicyDelegate.mm */,
+ F43CAB1C278A326500C8D0A2 /* CloseWhileCommittingLoad.mm */,
A1146A8A1D2D704F000FE710 /* ContentFiltering.mm */,
5142B2701517C88B00C32B19 /* ContextMenuCanCopyURL.mm */,
37FB72951DB2E82F00E41BE4 /* ContextMenuDefaultItemsHaveTags.mm */,
@@ -5430,6 +5435,7 @@
7CCE7EE51A411AE600447C4C /* CloseThenTerminate.cpp in Sources */,
CDF92237216D186400647AA7 /* CloseWebViewAfterEnterFullscreen.mm in Sources */,
CDF0B78A216D48DC00421ECC /* CloseWebViewDuringEnterFullscreen.mm in Sources */,
+ F43CAB1D278A326500C8D0A2 /* CloseWhileCommittingLoad.mm in Sources */,
6B306106218A372900F5A802 /* ClosingWebView.mm in Sources */,
7C83E0BA1D0A64FB00FEBCF3 /* Coding.mm in Sources */,
7C3965061CDD74F90094DBB8 /* ColorTests.cpp in Sources */,
Added: branches/safari-612.4.9.1-branch/Tools/TestWebKitAPI/Tests/mac/CloseWhileCommittingLoad.mm (0 => 287850)
--- branches/safari-612.4.9.1-branch/Tools/TestWebKitAPI/Tests/mac/CloseWhileCommittingLoad.mm (rev 0)
+++ branches/safari-612.4.9.1-branch/Tools/TestWebKitAPI/Tests/mac/CloseWhileCommittingLoad.mm 2022-01-10 20:09:53 UTC (rev 287850)
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2022 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#import "config.h"
+#import "PlatformUtilities.h"
+#import <WebKit/WebFrameLoadDelegate.h>
+#import <WebKit/WebViewPrivate.h>
+#import <wtf/RetainPtr.h>
+
+static bool didCloseWhileCommittingLoad;
+
+@interface CloseWhileCommittingLoadDelegate : NSObject <WebFrameLoadDelegate>
+@end
+
+@implementation CloseWhileCommittingLoadDelegate
+
+- (void)webView:(WebView *)sender didCommitLoadForFrame:(WebFrame *)frame
+{
+ [sender close];
+
+ didCloseWhileCommittingLoad = true;
+}
+
+@end
+
+namespace TestWebKitAPI {
+
+TEST(WebKitLegacy, CloseWhileCommittingLoad)
+{
+ auto webView = adoptNS([WebView new]);
+ auto delegate = adoptNS([CloseWhileCommittingLoadDelegate new]);
+ [webView setFrameLoadDelegate:delegate.get()];
+
+ didCloseWhileCommittingLoad = false;
+ [[webView mainFrame] loadRequest:[NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"simple" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]]];
+
+ Util::run(&didCloseWhileCommittingLoad);
+}
+
+} // namespace TestWebKitAPI
