Diff
Modified: trunk/LayoutTests/ChangeLog (102404 => 102405)
--- trunk/LayoutTests/ChangeLog 2011-12-09 01:31:12 UTC (rev 102404)
+++ trunk/LayoutTests/ChangeLog 2011-12-09 01:33:41 UTC (rev 102405)
@@ -1,3 +1,13 @@
+2011-12-08 James Robinson <jam...@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 <jac...@adobe.com>
Convert some fast/regions pixel tests to reftests
Added: trunk/LayoutTests/fast/animation/request-animation-frame-detach-element-expected.txt (0 => 102405)
--- trunk/LayoutTests/fast/animation/request-animation-frame-detach-element-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/animation/request-animation-frame-detach-element-expected.txt 2011-12-09 01:33:41 UTC (rev 102405)
@@ -0,0 +1 @@
+Test passes is there is no crash.
Property changes on: trunk/LayoutTests/fast/animation/request-animation-frame-detach-element-expected.txt
___________________________________________________________________
Added: svn:eol-style
Added: trunk/LayoutTests/fast/animation/request-animation-frame-detach-element.html (0 => 102405)
--- trunk/LayoutTests/fast/animation/request-animation-frame-detach-element.html (rev 0)
+++ trunk/LayoutTests/fast/animation/request-animation-frame-detach-element.html 2011-12-09 01:33:41 UTC (rev 102405)
@@ -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>
Property changes on: trunk/LayoutTests/fast/animation/request-animation-frame-detach-element.html
___________________________________________________________________
Added: svn:eol-style
Modified: trunk/Source/WebCore/ChangeLog (102404 => 102405)
--- trunk/Source/WebCore/ChangeLog 2011-12-09 01:31:12 UTC (rev 102404)
+++ trunk/Source/WebCore/ChangeLog 2011-12-09 01:33:41 UTC (rev 102405)
@@ -1,3 +1,28 @@
+2011-12-08 James Robinson <jam...@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_zh...@apple.com>
Use bitfield for bool data members in BitmapImage.
Modified: trunk/Source/WebCore/dom/Document.cpp (102404 => 102405)
--- trunk/Source/WebCore/dom/Document.cpp 2011-12-09 01:31:12 UTC (rev 102404)
+++ trunk/Source/WebCore/dom/Document.cpp 2011-12-09 01:33:41 UTC (rev 102405)
@@ -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();
Modified: trunk/Source/WebCore/dom/Document.h (102404 => 102405)
--- trunk/Source/WebCore/dom/Document.h 2011-12-09 01:31:12 UTC (rev 102404)
+++ trunk/Source/WebCore/dom/Document.h 2011-12-09 01:33:41 UTC (rev 102405)
@@ -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;
Modified: trunk/Source/WebCore/dom/ScriptedAnimationController.cpp (102404 => 102405)
--- trunk/Source/WebCore/dom/ScriptedAnimationController.cpp 2011-12-09 01:31:12 UTC (rev 102404)
+++ trunk/Source/WebCore/dom/ScriptedAnimationController.cpp 2011-12-09 01:33:41 UTC (rev 102405)
@@ -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) {
Modified: trunk/Source/WebCore/dom/ScriptedAnimationController.h (102404 => 102405)
--- trunk/Source/WebCore/dom/ScriptedAnimationController.h 2011-12-09 01:31:12 UTC (rev 102404)
+++ trunk/Source/WebCore/dom/ScriptedAnimationController.h 2011-12-09 01:33:41 UTC (rev 102405)
@@ -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;
Modified: trunk/Source/WebCore/page/FrameView.cpp (102404 => 102405)
--- trunk/Source/WebCore/page/FrameView.cpp 2011-12-09 01:31:12 UTC (rev 102404)
+++ trunk/Source/WebCore/page/FrameView.cpp 2011-12-09 01:33:41 UTC (rev 102405)
@@ -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
