Cherry-pick 272448.253@safari-7618-branch (b417dff). https://bugs.web…

…kit.org/show_bug.cgi?id=267739 Crash in ImageEventListener::handleEvent https://bugs.webkit.org/show_bug.cgi?id=267739 rdar://118761846 Reviewed by Chris Dumez. Use WeakPtr instead of a raw reference. * LayoutTests/fast/images/image-document-event-handler-crash-expected.txt: Added. * LayoutTests/fast/images/image-document-event-handler-crash.html: Added. * Source/WebCore/html/ImageDocument.cpp: (WebCore::ImageEventListener::handleEvent): Canonical link: https://commits.webkit.org/272448.253@safari-7618-branch Canonical link: https://commits.webkit.org/274313.69@webkitglib/2.44
WebKit · Mar 11, 2024 · bb20f3e · bb20f3e
1 parent 8240207
commit bb20f3e
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 5 deletions.
diff --git a/LayoutTests/fast/images/image-document-event-handler-crash-expected.txt b/LayoutTests/fast/images/image-document-event-handler-crash-expected.txt
@@ -0,0 +1,3 @@
+This test passes if WebKit does not crash in ASAN builds.
+
+PASS
diff --git a/LayoutTests/fast/images/image-document-event-handler-crash.html b/LayoutTests/fast/images/image-document-event-handler-crash.html
@@ -0,0 +1,38 @@
+<html>
+<body onload="main()">
+<p>This test passes if WebKit does not crash in ASAN builds.</p>
+<div id="result"></div>
+<script>
+
+if (window.testRunner) {
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+}
+
+function main() {
+    const img_window = window.open(URL.createObjectURL(new Blob([new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, 0x49, 0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x01, 0x03, 0x00, 0x00, 0x00, 0x25, 0xdb, 0x56, 0xca, 0x00, 0x00, 0x00, 0x03, 0x50, 0x4c, 0x54, 0x45, 0x00, 0x00, 0x00, 0xa7, 0x7a, 0x3d, 0xda, 0x00, 0x00, 0x00, 0x01, 0x74, 0x52, 0x4e, 0x53, 0x00, 0x40, 0xe6, 0xd8, 0x66, 0x00, 0x00, 0x00, 0x0a, 0x49, 0x44, 0x41, 0x54, 0x08, 0xd7, 0x63, 0x60, 0x00, 0x00, 0x00, 0x02, 0x00, 0x01, 0xe2, 0x21, 0xbc, 0x33, 0x00, 0x00, 0x00, 0x00, 0x49, 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82])], {type: 'image/png'})));
+
+    img_window.onload = () => {
+        const div = document.createElement('div');
+        div.appendChild(img_window.document.body);
+
+        const img = div.querySelector('img');
+        img.remove();
+
+        div.firstChild.remove();
+        img_window.location.reload();
+
+        setTimeout(() => {
+            if (window.GCController)
+                GCController.collect();
+            img.dispatchEvent(new MouseEvent('click'));
+            result.textContent = 'PASS';
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }, 10);
+    }
+}
+
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/html/ImageDocument.cpp b/Source/WebCore/html/ImageDocument.cpp
@@ -73,7 +73,7 @@ class ImageEventListener final : public EventListener {
     bool operator==(const EventListener&) const override;
     void handleEvent(ScriptExecutionContext&, Event&) override;
 
-    ImageDocument& m_document;
+    WeakPtr<ImageDocument, WeakPtrImplWithEventTargetData> m_document;
 };
 #endif
 
@@ -104,14 +104,14 @@ class ImageDocumentElement final : public HTMLImageElement {
 private:
     ImageDocumentElement(ImageDocument& document)
         : HTMLImageElement(imgTag, document)
-        , m_imageDocument(&document)
+        , m_imageDocument(document)
     {
     }
 
     virtual ~ImageDocumentElement();
     void didMoveToNewDocument(Document& oldDocument, Document& newDocument) override;
 
-    ImageDocument* m_imageDocument;
+    WeakPtr<ImageDocument, WeakPtrImplWithEventTargetData> m_imageDocument;
 };
 
 inline Ref<ImageDocumentElement> ImageDocumentElement::create(ImageDocument& document)
@@ -422,8 +422,9 @@ void ImageDocument::imageClicked(int x, int y)
 
 void ImageEventListener::handleEvent(ScriptExecutionContext&, Event& event)
 {
-    if (auto* mouseEvent = dynamicDowncast<MouseEvent>(event); mouseEvent && event.type() == eventNames().clickEvent)
-        m_document.imageClicked(mouseEvent->offsetX(), mouseEvent->offsetY());
+    RefPtr document = m_document.get();
+    if (auto* mouseEvent = dynamicDowncast<MouseEvent>(event); mouseEvent && event.type() == eventNames().clickEvent && document)
+        document->imageClicked(mouseEvent->offsetX(), mouseEvent->offsetY());
 }
 
 bool ImageEventListener::operator==(const EventListener& other) const