nullptr crash in EventPath::buildPath via FullscreenManager::dispatch…

…FullscreenChangeOrErrorEvent https://bugs.webkit.org/show_bug.cgi?id=242345 Reviewed by Chris Dumez. The bug was caused by EventPath::buildPath stumbling upon an element inside a shadow tree which had already been disassociated with its shadow host as a way of destruction. This doesn't ordinarily happen since the shadow host of a shadow root gets kept alive by its JS wrapper. The bug is precisely that FullscreenManager doesn't keep alive JS wrappers of nodes to dispatch change and error events. This patch addresses this problem by deploying GCReachableRef in FullscreenManager. * LayoutTests/fullscreen/full-screen-inside-shadow-event-path-crash-expected.txt: Added. * LayoutTests/fullscreen/full-screen-inside-shadow-event-path-crash.html: Added. * Source/WebCore/dom/FullscreenManager.cpp: (WebCore::FullscreenManager::dispatchFullscreenChangeEvents): (WebCore::FullscreenManager::dispatchFullscreenChangeOrErrorEvent): (WebCore::FullscreenManager::addDocumentToFullscreenChangeEventQueue): * Source/WebCore/dom/FullscreenManager.h: Canonical link: https://commits.webkit.org/253227@main
WebKit · Aug 8, 2022 · 29c4919 · 29c4919
1 parent cc7b062
commit 29c4919
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 17 deletions.
diff --git a/LayoutTests/fullscreen/full-screen-inside-shadow-event-path-crash-expected.txt b/LayoutTests/fullscreen/full-screen-inside-shadow-event-path-crash-expected.txt
@@ -0,0 +1 @@
+PASS
diff --git a/LayoutTests/fullscreen/full-screen-inside-shadow-event-path-crash.html b/LayoutTests/fullscreen/full-screen-inside-shadow-event-path-crash.html
@@ -0,0 +1,22 @@
+<script>
+if (window.testRunner) {
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+}
+
+onload = async () => {
+    let shadowRoot = document.createElement('div').attachShadow({mode: 'open'});
+    window.r = new WeakRef(shadowRoot.children);
+    let div2 = document.createElement('div');
+    div2.webkitRequestFullscreen();
+    shadowRoot.replaceChildren(div2);
+    await undefined;
+    GCController.collect();
+    location.href = `data:text/html,<!DOCTYPE html>
+<p>PASS</p>
+<script>
+if (window.testRunner)
+    testRunner.notifyDone();
+</` + 'script>'
+};
+</script>
diff --git a/Source/WebCore/dom/FullscreenManager.cpp b/Source/WebCore/dom/FullscreenManager.cpp
@@ -573,33 +573,30 @@ void FullscreenManager::dispatchFullscreenChangeEvents()
     // document will be detached and GC'd. We protect it here to make sure we
     // can finish the function successfully.
     Ref<Document> protectedDocument(document());
-    Deque<RefPtr<Node>> changeQueue;
+    Deque<GCReachableRef<Node>> changeQueue;
     m_fullscreenChangeEventTargetQueue.swap(changeQueue);
-    Deque<RefPtr<Node>> errorQueue;
+    Deque<GCReachableRef<Node>> errorQueue;
     m_fullscreenErrorEventTargetQueue.swap(errorQueue);
     dispatchFullscreenChangeOrErrorEvent(changeQueue, eventNames().webkitfullscreenchangeEvent, /* shouldNotifyMediaElement */ true);
     dispatchFullscreenChangeOrErrorEvent(errorQueue, eventNames().webkitfullscreenerrorEvent, /* shouldNotifyMediaElement */ false);
 }
 
-void FullscreenManager::dispatchFullscreenChangeOrErrorEvent(Deque<RefPtr<Node>>& queue, const AtomString& eventName, bool shouldNotifyMediaElement)
+void FullscreenManager::dispatchFullscreenChangeOrErrorEvent(Deque<GCReachableRef<Node>>& queue, const AtomString& eventName, bool shouldNotifyMediaElement)
 {
     // Step 3 of https://fullscreen.spec.whatwg.org/#run-the-fullscreen-steps
     while (!queue.isEmpty()) {
-        RefPtr<Node> node = queue.takeFirst();
-        if (!node)
-            node = documentElement();
-        // The dispatchEvent below may have blown away our documentElement.
-        if (!node)
-            continue;
+        auto node = queue.takeFirst();
 
         // If the element was removed from our tree, also message the documentElement. Since we may
         // have a document hierarchy, check that node isn't in another document.
-        if (!node->isConnected())
-            queue.append(documentElement());
+        if (!node->isConnected()) {
+            if (auto* element = documentElement())
+                queue.append(*element);
+        }
 
 #if ENABLE(VIDEO)
-        if (shouldNotifyMediaElement && is<HTMLMediaElement>(*node))
-            downcast<HTMLMediaElement>(*node).enteredOrExitedFullscreen();
+        if (shouldNotifyMediaElement && is<HTMLMediaElement>(node.get()))
+            downcast<HTMLMediaElement>(node.get()).enteredOrExitedFullscreen();
 #else
         UNUSED_PARAM(shouldNotifyMediaElement);
 #endif
@@ -703,7 +700,7 @@ void FullscreenManager::addDocumentToFullscreenChangeEventQueue(Document& docume
         target = document.fullscreenManager().currentFullscreenElement();
     if (!target)
         target = &document;
-    m_fullscreenChangeEventTargetQueue.append(target);
+    m_fullscreenChangeEventTargetQueue.append(GCReachableRef(*target));
 }
 
 #if !RELEASE_LOG_DISABLED

diff --git a/Source/WebCore/dom/FullscreenManager.h b/Source/WebCore/dom/FullscreenManager.h
@@ -29,6 +29,7 @@
 
 #include "Document.h"
 #include "FrameDestructionObserverInlines.h"
+#include "GCReachableRef.h"
 #include "LayoutRect.h"
 #include <wtf/Deque.h>
 #include <wtf/WeakPtr.h>
@@ -97,7 +98,7 @@ class FullscreenManager final : public CanMakeWeakPtr<FullscreenManager> {
 protected:
     friend class Document;
 
-    void dispatchFullscreenChangeOrErrorEvent(Deque<RefPtr<Node>>&, const AtomString& eventName, bool shouldNotifyMediaElement);
+    void dispatchFullscreenChangeOrErrorEvent(Deque<GCReachableRef<Node>>&, const AtomString& eventName, bool shouldNotifyMediaElement);
     void clearFullscreenElementStack();
     void popFullscreenElementStack();
     void pushFullscreenElementStack(Element&);
@@ -120,8 +121,8 @@ class FullscreenManager final : public CanMakeWeakPtr<FullscreenManager> {
     RefPtr<Element> m_fullscreenElement;
     Vector<RefPtr<Element>> m_fullscreenElementStack;
     WeakPtr<RenderFullScreen> m_fullscreenRenderer { nullptr };
-    Deque<RefPtr<Node>> m_fullscreenChangeEventTargetQueue;
-    Deque<RefPtr<Node>> m_fullscreenErrorEventTargetQueue;
+    Deque<GCReachableRef<Node>> m_fullscreenChangeEventTargetQueue;
+    Deque<GCReachableRef<Node>> m_fullscreenErrorEventTargetQueue;
     LayoutRect m_savedPlaceholderFrameRect;
     std::unique_ptr<RenderStyle> m_savedPlaceholderRenderStyle;