Source/WebCore: Improve handling of frame removal during requestAnimationFrame callback invocation
https://bugs.webkit.org/show_bug.cgi?id=74036
Reviewed by Adam Barth.
See bug for details.
Test: fast/animation/request-animation-frame-detach-element.html
* dom/Document.cpp:
(WebCore::Document::removedLastRef):
(WebCore::Document::detach):
* dom/Document.h:
* dom/ScriptedAnimationController.cpp:
(WebCore::ScriptedAnimationController::~ScriptedAnimationController):
(WebCore::ScriptedAnimationController::serviceScriptedAnimations):
(WebCore::ScriptedAnimationController::scheduleAnimation):
* dom/ScriptedAnimationController.h:
(WebCore::ScriptedAnimationController::create):
(WebCore::ScriptedAnimationController::clearDocumentPointer):
* page/FrameView.cpp:
(WebCore::FrameView::serviceScriptedAnimations):
LayoutTests: Add some tests for removing frames from the document while servicing requestAnimationFrame callbacks
https://bugs.webkit.org/show_bug.cgi?id=74036
Reviewed by Adam Barth.
* fast/animation/request-animation-frame-detach-element-expected.txt: Added.
* fast/animation/request-animation-frame-detach-element.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@102405 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 1bae626..e8f7766 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2011-12-08 James Robinson <jamesr@chromium.org>
+
+ Add some tests for removing frames from the document while servicing requestAnimationFrame callbacks
+ https://bugs.webkit.org/show_bug.cgi?id=74036
+
+ Reviewed by Adam Barth.
+
+ * fast/animation/request-animation-frame-detach-element-expected.txt: Added.
+ * fast/animation/request-animation-frame-detach-element.html: Added.
+
2011-12-08 Jacob Goldstein <jacobg@adobe.com>
Convert some fast/regions pixel tests to reftests
diff --git a/LayoutTests/fast/animation/request-animation-frame-detach-element-expected.txt b/LayoutTests/fast/animation/request-animation-frame-detach-element-expected.txt
new file mode 100644
index 0000000..797a749
--- /dev/null
+++ b/LayoutTests/fast/animation/request-animation-frame-detach-element-expected.txt
@@ -0,0 +1 @@
+Test passes is there is no crash.
diff --git a/LayoutTests/fast/animation/request-animation-frame-detach-element.html b/LayoutTests/fast/animation/request-animation-frame-detach-element.html
new file mode 100644
index 0000000..2acd468
--- /dev/null
+++ b/LayoutTests/fast/animation/request-animation-frame-detach-element.html
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<script>
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+}
+window.onload = function() {
+ var el = document.getElementsByTagName("iframe")[0];
+ window.frames[0].webkitRequestAnimationFrame(function() {
+ el.parentNode.removeChild(el);
+ });
+ window.frames[1].webkitRequestAnimationFrame(function() {
+ });
+
+ if (window.layoutTestController) {
+ window.setTimeout(function() {
+ layoutTestController.display();
+ layoutTestController.notifyDone();
+ }, 50);
+ }
+}
+</script>
+Test passes is there is no crash.
+<iframe></iframe>
+<iframe></iframe>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 0cea301..82a293f 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2011-12-08 James Robinson <jamesr@chromium.org>
+
+ Improve handling of frame removal during requestAnimationFrame callback invocation
+ https://bugs.webkit.org/show_bug.cgi?id=74036
+
+ Reviewed by Adam Barth.
+
+ See bug for details.
+
+ Test: fast/animation/request-animation-frame-detach-element.html
+
+ * dom/Document.cpp:
+ (WebCore::Document::removedLastRef):
+ (WebCore::Document::detach):
+ * dom/Document.h:
+ * dom/ScriptedAnimationController.cpp:
+ (WebCore::ScriptedAnimationController::~ScriptedAnimationController):
+ (WebCore::ScriptedAnimationController::serviceScriptedAnimations):
+ (WebCore::ScriptedAnimationController::scheduleAnimation):
+ * dom/ScriptedAnimationController.h:
+ (WebCore::ScriptedAnimationController::create):
+ (WebCore::ScriptedAnimationController::clearDocumentPointer):
+ * page/FrameView.cpp:
+ (WebCore::FrameView::serviceScriptedAnimations):
+
2011-12-08 Yongjun Zhang <yongjun_zhang@apple.com>
Use bitfield for bool data members in BitmapImage.
diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
index a59b7a6..7085e1a 100644
--- a/Source/WebCore/dom/Document.cpp
+++ b/Source/WebCore/dom/Document.cpp
@@ -613,7 +613,9 @@
#if ENABLE(REQUEST_ANIMATION_FRAME)
// FIXME: consider using ActiveDOMObject.
- m_scriptedAnimationController = nullptr;
+ if (m_scriptedAnimationController)
+ m_scriptedAnimationController->clearDocumentPointer();
+ m_scriptedAnimationController.clear();
#endif
#ifndef NDEBUG
@@ -1834,7 +1836,9 @@
#if ENABLE(REQUEST_ANIMATION_FRAME)
// FIXME: consider using ActiveDOMObject.
- m_scriptedAnimationController = nullptr;
+ if (m_scriptedAnimationController)
+ m_scriptedAnimationController->clearDocumentPointer();
+ m_scriptedAnimationController.clear();
#endif
RenderObject* render = renderer();
diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
index 43c7ebd..f4d0bba 100644
--- a/Source/WebCore/dom/Document.h
+++ b/Source/WebCore/dom/Document.h
@@ -1441,7 +1441,7 @@
unsigned m_wheelEventHandlerCount;
#if ENABLE(REQUEST_ANIMATION_FRAME)
- OwnPtr<ScriptedAnimationController> m_scriptedAnimationController;
+ RefPtr<ScriptedAnimationController> m_scriptedAnimationController;
#endif
Timer<Document> m_pendingTasksTimer;
diff --git a/Source/WebCore/dom/ScriptedAnimationController.cpp b/Source/WebCore/dom/ScriptedAnimationController.cpp
index 3e50a88..8875f55 100644
--- a/Source/WebCore/dom/ScriptedAnimationController.cpp
+++ b/Source/WebCore/dom/ScriptedAnimationController.cpp
@@ -61,6 +61,10 @@
windowScreenDidChange(displayID);
}
+ScriptedAnimationController::~ScriptedAnimationController()
+{
+}
+
void ScriptedAnimationController::suspend()
{
++m_suspendCount;
@@ -119,8 +123,17 @@
// missing any callbacks, we keep iterating through the list of candiate callbacks and firing
// them until nothing new becomes visible.
bool firedCallback;
+
+ // Invoking callbacks may detach elements from our document, which clear's the document's
+ // reference to us, so take a defensive reference.
+ RefPtr<ScriptedAnimationController> protector(this);
do {
firedCallback = false;
+ // A previous iteration may have detached this Document from the DOM tree.
+ // If so, then we do not need to process any more callbacks.
+ if (!m_document)
+ continue;
+
// A previous iteration may have invalidated style (or layout). Update styles for each iteration
// for now since all we check is the existence of a renderer.
m_document->updateStyleIfNeeded();
@@ -161,6 +174,9 @@
void ScriptedAnimationController::scheduleAnimation()
{
+ if (!m_document)
+ return;
+
#if USE(REQUEST_ANIMATION_FRAME_TIMER)
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
if (!m_useTimer) {
diff --git a/Source/WebCore/dom/ScriptedAnimationController.h b/Source/WebCore/dom/ScriptedAnimationController.h
index dfdd503..d2c9c0c 100644
--- a/Source/WebCore/dom/ScriptedAnimationController.h
+++ b/Source/WebCore/dom/ScriptedAnimationController.h
@@ -35,8 +35,7 @@
#include "Timer.h"
#endif
#include "PlatformScreen.h"
-#include <wtf/Noncopyable.h>
-#include <wtf/PassOwnPtr.h>
+#include <wtf/RefCounted.h>
#include <wtf/RefPtr.h>
#include <wtf/Vector.h>
@@ -46,17 +45,18 @@
class Element;
class RequestAnimationFrameCallback;
-class ScriptedAnimationController
+class ScriptedAnimationController : public RefCounted<ScriptedAnimationController>
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
- : public DisplayRefreshMonitorClient
+ , public DisplayRefreshMonitorClient
#endif
{
-WTF_MAKE_NONCOPYABLE(ScriptedAnimationController);
public:
- static PassOwnPtr<ScriptedAnimationController> create(Document* document, PlatformDisplayID displayID)
+ static PassRefPtr<ScriptedAnimationController> create(Document* document, PlatformDisplayID displayID)
{
- return adoptPtr(new ScriptedAnimationController(document, displayID));
+ return adoptRef(new ScriptedAnimationController(document, displayID));
}
+ ~ScriptedAnimationController();
+ void clearDocumentPointer() { m_document = 0; }
typedef int CallbackId;
@@ -70,7 +70,7 @@
void windowScreenDidChange(PlatformDisplayID);
private:
- explicit ScriptedAnimationController(Document*, PlatformDisplayID);
+ ScriptedAnimationController(Document*, PlatformDisplayID);
typedef Vector<RefPtr<RequestAnimationFrameCallback> > CallbackList;
CallbackList m_callbacks;
diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
index 876ea10..547a1c1 100644
--- a/Source/WebCore/page/FrameView.cpp
+++ b/Source/WebCore/page/FrameView.cpp
@@ -2085,8 +2085,13 @@
#if ENABLE(REQUEST_ANIMATION_FRAME)
void FrameView::serviceScriptedAnimations(DOMTimeStamp time)
{
+ Vector<RefPtr<Document> > documents;
+
for (Frame* frame = m_frame.get(); frame; frame = frame->tree()->traverseNext())
- frame->document()->serviceScriptedAnimations(time);
+ documents.append(frame->document());
+
+ for (size_t i = 0; i < documents.size(); ++i)
+ documents[i]->serviceScriptedAnimations(time);
}
#endif
