Crash beneath ScriptedAnimationController::serviceScriptedAnimations after a requestA...
authormitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Dec 2017 21:41:02 +0000 (21:41 +0000)
committermitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 Dec 2017 21:41:02 +0000 (21:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181132
<rdar://problem/35143540>

Reviewed by Simon Fraser.

Source/WebCore:

Test: fast/animation/request-animation-frame-remove-iframe-in-callback.html

* dom/ScriptedAnimationController.cpp:
(WebCore::ScriptedAnimationController::serviceScriptedAnimations): Hold a reference to the
  document and pass that along to InspectorInstrumentation::willFireAnimationFrame rather
  than dereferencing the m_document member, which may have gotten cleared by an earlier
  callback.

LayoutTests:

* fast/animation/request-animation-frame-remove-iframe-in-callback-expected.txt: Added.
* fast/animation/request-animation-frame-remove-iframe-in-callback.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/animation/request-animation-frame-remove-iframe-in-callback-expected.txt [new file with mode: 0644]
LayoutTests/fast/animation/request-animation-frame-remove-iframe-in-callback.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/ScriptedAnimationController.cpp

index 32297e4..3be279a 100644 (file)
@@ -1,3 +1,14 @@
+2017-12-22  Dan Bernstein  <mitz@apple.com>
+
+        Crash beneath ScriptedAnimationController::serviceScriptedAnimations after a requestAnimationFrame callback removes the requesting iframe
+        https://bugs.webkit.org/show_bug.cgi?id=181132
+        <rdar://problem/35143540>
+
+        Reviewed by Simon Fraser.
+
+        * fast/animation/request-animation-frame-remove-iframe-in-callback-expected.txt: Added.
+        * fast/animation/request-animation-frame-remove-iframe-in-callback.html: Added.
+
 2017-12-22  Chris Dumez  <cdumez@apple.com>
 
         [Service Workers] Implement "Soft Update" algorithm
diff --git a/LayoutTests/fast/animation/request-animation-frame-remove-iframe-in-callback-expected.txt b/LayoutTests/fast/animation/request-animation-frame-remove-iframe-in-callback-expected.txt
new file mode 100644 (file)
index 0000000..0383162
--- /dev/null
@@ -0,0 +1 @@
+Test passes if it does not crash.
diff --git a/LayoutTests/fast/animation/request-animation-frame-remove-iframe-in-callback.html b/LayoutTests/fast/animation/request-animation-frame-remove-iframe-in-callback.html
new file mode 100644 (file)
index 0000000..c0dcd6f
--- /dev/null
@@ -0,0 +1,19 @@
+Test passes if it does not crash.
+<iframe id=target></iframe>
+<span id=result></span>
+<script>
+    if (window.testRunner) {
+        testRunner.dumpAsText();
+        testRunner.waitUntilDone();
+    }
+
+    const target = document.getElementById("target");
+    const contentWindow = target.contentWindow;
+    contentWindow.requestAnimationFrame(() => {
+        target.remove();
+        if (window.testRunner) {
+            setTimeout(() => { testRunner.notifyDone(); }, 0);
+        }
+    });
+    contentWindow.requestAnimationFrame(() => { });
+</script>
index 33196da..e92fb66 100644 (file)
@@ -1,3 +1,19 @@
+2017-12-22  Dan Bernstein  <mitz@apple.com>
+
+        Crash beneath ScriptedAnimationController::serviceScriptedAnimations after a requestAnimationFrame callback removes the requesting iframe
+        https://bugs.webkit.org/show_bug.cgi?id=181132
+        <rdar://problem/35143540>
+
+        Reviewed by Simon Fraser.
+
+        Test: fast/animation/request-animation-frame-remove-iframe-in-callback.html
+
+        * dom/ScriptedAnimationController.cpp:
+        (WebCore::ScriptedAnimationController::serviceScriptedAnimations): Hold a reference to the
+          document and pass that along to InspectorInstrumentation::willFireAnimationFrame rather
+          than dereferencing the m_document member, which may have gotten cleared by an earlier
+          callback.
+
 2017-12-22  Chris Dumez  <cdumez@apple.com>
 
         importScripts() inside a service worker should ensure that the response has a JavaScript MIME type
index 9ace901..94f4c1b 100644 (file)
@@ -209,11 +209,12 @@ void ScriptedAnimationController::serviceScriptedAnimations(double timestamp)
     // Invoking callbacks may detach elements from our document, which clears the document's
     // reference to us, so take a defensive reference.
     Ref<ScriptedAnimationController> protectedThis(*this);
+    Ref<Document> protectedDocument(*m_document);
 
     for (auto& callback : callbacks) {
         if (!callback->m_firedOrCancelled) {
             callback->m_firedOrCancelled = true;
-            InspectorInstrumentationCookie cookie = InspectorInstrumentation::willFireAnimationFrame(*m_document, callback->m_id);
+            InspectorInstrumentationCookie cookie = InspectorInstrumentation::willFireAnimationFrame(protectedDocument, callback->m_id);
             if (callback->m_useLegacyTimeBase)
                 callback->handleEvent(legacyHighResNowMs);
             else