Null check frame in Document::dispatchDisabledAdaptationsDidChangeForMainFrame and a few other places
https://bugs.webkit.org/show_bug.cgi?id=214715
<rdar://problem/65467702>
Reviewed by Geoffrey Garen.
Source/WebCore:
Test: security/mutation-observer-frame-detach.html
* dom/Document.cpp:
(WebCore::Document::didBecomeCurrentDocumentInFrame):
(WebCore::Document::initContentSecurityPolicy):
* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::commitData):
Add some null checks and early returns if the frame detaches.
* loader/SubframeLoader.cpp:
(WebCore::FrameLoader::SubframeLoader::loadSubframe):
Balance the call to incrementLoadEventDelayCount in the early return case or this test never finishes loading.
LayoutTests:
* security/mutation-observer-frame-detach-expected.txt: Added.
* security/mutation-observer-frame-detach.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@264880 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 6aa38ab..e3fd183 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,5 +1,16 @@
2020-07-24 Alex Christensen <achristensen@webkit.org>
+ Null check frame in Document::dispatchDisabledAdaptationsDidChangeForMainFrame and a few other places
+ https://bugs.webkit.org/show_bug.cgi?id=214715
+ <rdar://problem/65467702>
+
+ Reviewed by Geoffrey Garen.
+
+ * security/mutation-observer-frame-detach-expected.txt: Added.
+ * security/mutation-observer-frame-detach.html: Added.
+
+2020-07-24 Alex Christensen <achristensen@webkit.org>
+
Fix assertion when highlighting in detached frames and make loads complete
https://bugs.webkit.org/show_bug.cgi?id=214770
diff --git a/LayoutTests/security/mutation-observer-frame-detach-expected.txt b/LayoutTests/security/mutation-observer-frame-detach-expected.txt
new file mode 100644
index 0000000..aec0300
--- /dev/null
+++ b/LayoutTests/security/mutation-observer-frame-detach-expected.txt
@@ -0,0 +1,2 @@
+ALERT: test passed because nothing crashed
+
diff --git a/LayoutTests/security/mutation-observer-frame-detach.html b/LayoutTests/security/mutation-observer-frame-detach.html
new file mode 100644
index 0000000..a5f6351
--- /dev/null
+++ b/LayoutTests/security/mutation-observer-frame-detach.html
@@ -0,0 +1,29 @@
+<script>
+ function runTest() {
+ var observer = new MutationObserver(()=>{p1.replaceWith(p2)});
+ observer.observe(select,{childList:true});
+ select[2] = option;
+ document.head.appendChild(p2);
+ var object = document.createElement("object");
+ var frame = document.createElement("frame");
+ audio.appendChild(option);
+ p1.appendChild(object);
+ object.data = "abc";
+ document.all[9].appendChild(frame);
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ alert("test passed because nothing crashed");
+ }
+ }
+</script>
+<body onload=runTest()>
+ <p id="p1">
+ <p id="p2">
+ <audio id="audio">
+ <select id="select">
+ <option id="option"></option>
+ </select>
+ </audio>
+ </p>
+ </p>
+</body>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 307ae6f..49a267e 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,5 +1,25 @@
2020-07-24 Alex Christensen <achristensen@webkit.org>
+ Null check frame in Document::dispatchDisabledAdaptationsDidChangeForMainFrame and a few other places
+ https://bugs.webkit.org/show_bug.cgi?id=214715
+ <rdar://problem/65467702>
+
+ Reviewed by Geoffrey Garen.
+
+ Test: security/mutation-observer-frame-detach.html
+
+ * dom/Document.cpp:
+ (WebCore::Document::didBecomeCurrentDocumentInFrame):
+ (WebCore::Document::initContentSecurityPolicy):
+ * loader/DocumentLoader.cpp:
+ (WebCore::DocumentLoader::commitData):
+ Add some null checks and early returns if the frame detaches.
+ * loader/SubframeLoader.cpp:
+ (WebCore::FrameLoader::SubframeLoader::loadSubframe):
+ Balance the call to incrementLoadEventDelayCount in the early return case or this test never finishes loading.
+
+2020-07-24 Alex Christensen <achristensen@webkit.org>
+
Fix assertion when highlighting in detached frames and make loads complete
https://bugs.webkit.org/show_bug.cgi?id=214770
diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
index 48927c3..3d742e7 100644
--- a/Source/WebCore/dom/Document.cpp
+++ b/Source/WebCore/dom/Document.cpp
@@ -2386,16 +2386,24 @@
void Document::didBecomeCurrentDocumentInFrame()
{
- // FIXME: Are there cases where the document can be dislodged from the frame during the event handling below?
- // If so, then m_frame could become 0, and we need to do something about that.
-
m_frame->script().updateDocument();
+ // Many of these functions have event handlers which can detach the frame synchronously, so we must check repeatedly in this function.
+ if (!m_frame)
+ return;
+
if (!hasLivingRenderTree())
createRenderTree();
+ if (!m_frame)
+ return;
dispatchDisabledAdaptationsDidChangeForMainFrame();
+ if (!m_frame)
+ return;
+
updateViewportArguments();
+ if (!m_frame)
+ return;
// FIXME: Doing this only for the main frame is insufficient.
// Changing a subframe can also change the wheel event handler count.
@@ -2406,6 +2414,8 @@
// unless the documents they are replacing had wheel event handlers.
if (page() && m_frame->isMainFrame())
wheelEventHandlersChanged();
+ if (!m_frame)
+ return;
// Ensure that the scheduled task state of the document matches the DOM suspension state of the frame. It can
// be out of sync if the DOM suspension state changed while the document was not in the frame (possibly in the
@@ -6098,6 +6108,8 @@
void Document::initContentSecurityPolicy()
{
+ if (!m_frame)
+ return;
auto* parentFrame = m_frame->tree().parent();
if (parentFrame)
contentSecurityPolicy()->copyUpgradeInsecureRequestStateFrom(*parentFrame->document()->contentSecurityPolicy());
diff --git a/Source/WebCore/loader/DocumentLoader.cpp b/Source/WebCore/loader/DocumentLoader.cpp
index 2fc2149..408a13e 100644
--- a/Source/WebCore/loader/DocumentLoader.cpp
+++ b/Source/WebCore/loader/DocumentLoader.cpp
@@ -1087,7 +1087,10 @@
bool hasBegun = m_writer.begin(documentURL(), false);
m_writer.setDocumentWasLoadedAsPartOfNavigation();
- auto& document = *m_frame->document();
+ auto* documentOrNull = m_frame ? m_frame->document() : nullptr;
+ if (!documentOrNull)
+ return;
+ auto& document = *documentOrNull;
if (SecurityPolicy::allowSubstituteDataAccessToLocal() && m_originalSubstituteDataWasValid) {
// If this document was loaded with substituteData, then the document can
