Crash in WebCore::Document::fullScreenChangeDelayTimerFired
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Sep 2012 23:20:08 +0000 (23:20 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Sep 2012 23:20:08 +0000 (23:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=97367

Patch by Jeremy Apthorp <jeremya@chromium.org> on 2012-09-21
Reviewed by Abhishek Arya.

The document could be destroyed during the processing of the
fullscreenchange event, if the document was destroyed as a result of
one of the dispatchEvent calls.

This bug isn't reliably reproducible, so no new tests.

* dom/Document.cpp:
(WebCore::Document::fullScreenChangeDelayTimerFired):

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

Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp

index 236826b..d4dc1df 100644 (file)
@@ -1,3 +1,19 @@
+2012-09-21  Jeremy Apthorp  <jeremya@chromium.org>
+
+        Crash in WebCore::Document::fullScreenChangeDelayTimerFired
+        https://bugs.webkit.org/show_bug.cgi?id=97367
+
+        Reviewed by Abhishek Arya.
+
+        The document could be destroyed during the processing of the
+        fullscreenchange event, if the document was destroyed as a result of
+        one of the dispatchEvent calls.
+
+        This bug isn't reliably reproducible, so no new tests.
+
+        * dom/Document.cpp:
+        (WebCore::Document::fullScreenChangeDelayTimerFired):
+
 2012-09-21  Pratik Solanki  <psolanki@apple.com>
 
         No need to pass order file for WebCoreTestSupport build
index ab48ca3..58e5c68 100644 (file)
@@ -5768,6 +5768,10 @@ void Document::setFullScreenRendererBackgroundColor(Color backgroundColor)
     
 void Document::fullScreenChangeDelayTimerFired(Timer<Document>*)
 {
+    // Since we dispatch events in this function, it's possible that the
+    // document will be detached and GC'd. We protect it here to make sure we
+    // can finish the function successfully.
+    RefPtr<Document> protectDocument(this);
     Deque<RefPtr<Node> > changeQueue;
     m_fullScreenChangeEventTargetQueue.swap(changeQueue);
 
@@ -5775,6 +5779,9 @@ void Document::fullScreenChangeDelayTimerFired(Timer<Document>*)
         RefPtr<Node> node = changeQueue.takeFirst();
         if (!node)
             node = documentElement();
+        // The dispatchEvent below may have blown away our documentElement.
+        if (!node)
+            continue;
 
         // 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.
@@ -5791,6 +5798,9 @@ void Document::fullScreenChangeDelayTimerFired(Timer<Document>*)
         RefPtr<Node> node = errorQueue.takeFirst();
         if (!node)
             node = documentElement();
+        // The dispatchEvent below may have blown away our documentElement.
+        if (!node)
+            continue;
         
         // 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.