Diff
Modified: trunk/LayoutTests/ChangeLog (113572 => 113573)
--- trunk/LayoutTests/ChangeLog 2012-04-09 10:58:06 UTC (rev 113572)
+++ trunk/LayoutTests/ChangeLog 2012-04-09 11:42:18 UTC (rev 113573)
@@ -1,3 +1,16 @@
+2012-04-09 James Robinson <[email protected]>
+
+ Remove partially implemented per-Element visibility checks from requestAnimationFrame logic
+ https://bugs.webkit.org/show_bug.cgi?id=74232
+
+ Reviewed by Dean Jackson.
+
+ Remove tests for removed functionality.
+
+ * fast/animation/request-animation-frame-display-expected.txt: Removed.
+ * fast/animation/request-animation-frame-display.html: Removed.
+ * fast/animation/script-tests/request-animation-frame-display.js: Removed.
+
2012-04-09 Ami Fischman <[email protected]>
Layout Test http/tests/media/media-document.html is flaky
Deleted: trunk/LayoutTests/fast/animation/request-animation-frame-display-expected.txt (113572 => 113573)
--- trunk/LayoutTests/fast/animation/request-animation-frame-display-expected.txt 2012-04-09 10:58:06 UTC (rev 113572)
+++ trunk/LayoutTests/fast/animation/request-animation-frame-display-expected.txt 2012-04-09 11:42:18 UTC (rev 113573)
@@ -1,10 +0,0 @@
-Tests requestAnimationFrame callback handling of display: property changed within another callback
-
-On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-
-
-PASS callbackInvokedOnA is true
-PASS successfullyParsed is true
-
-TEST COMPLETE
-
Deleted: trunk/LayoutTests/fast/animation/request-animation-frame-display.html (113572 => 113573)
--- trunk/LayoutTests/fast/animation/request-animation-frame-display.html 2012-04-09 10:58:06 UTC (rev 113572)
+++ trunk/LayoutTests/fast/animation/request-animation-frame-display.html 2012-04-09 11:42:18 UTC (rev 113573)
@@ -1,11 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<script src=""
-</head>
-<body>
-<span id="e"></span>
-<span id="f"></span>
-<script src=""
-</body>
-</html>
Deleted: trunk/LayoutTests/fast/animation/script-tests/request-animation-frame-display.js (113572 => 113573)
--- trunk/LayoutTests/fast/animation/script-tests/request-animation-frame-display.js 2012-04-09 10:58:06 UTC (rev 113572)
+++ trunk/LayoutTests/fast/animation/script-tests/request-animation-frame-display.js 2012-04-09 11:42:18 UTC (rev 113573)
@@ -1,29 +0,0 @@
-description("Tests requestAnimationFrame callback handling of display: property changed within another callback");
-
-var e = document.getElementById("e");
-e.style.display="none";
-var callbackInvokedOnA=false;
-window.webkitRequestAnimationFrame(function() {
- callbackInvokedOnA=true;
-}, e);
-
-var f = document.getElementById("f");
-window.webkitRequestAnimationFrame(function() {
- e.style.display="";
-}, f);
-
-if (window.layoutTestController)
- layoutTestController.display();
-
-setTimeout(function() {
- shouldBeTrue("callbackInvokedOnA");
-}, 100);
-
-if (window.layoutTestController)
- layoutTestController.waitUntilDone();
-
-setTimeout(function() {
- isSuccessfullyParsed();
- if (window.layoutTestController)
- layoutTestController.notifyDone();
-}, 200);
Modified: trunk/Source/WebCore/ChangeLog (113572 => 113573)
--- trunk/Source/WebCore/ChangeLog 2012-04-09 10:58:06 UTC (rev 113572)
+++ trunk/Source/WebCore/ChangeLog 2012-04-09 11:42:18 UTC (rev 113573)
@@ -1,3 +1,36 @@
+2012-04-09 James Robinson <[email protected]>
+
+ Remove partially implemented per-Element visibility checks from requestAnimationFrame logic
+ https://bugs.webkit.org/show_bug.cgi?id=74232
+
+ Reviewed by Dean Jackson.
+
+ The initial requestAnimationFrame implementation had an Element parameter as the second argument to the
+ function. This element was intended to convey the element associated with the animation so that when the element
+ was not visible the animation callback would not be run. The checked in implementation does a very limited check
+ - testing for display:none and being detached from the tree - but does it in a way that does not work correctly
+ if an element's visibility is manipulated by a callback running from a different document. It also adds
+ significant complexity to the code, making it less hackable and easy to introduce subtle security bugs or
+ infinite loops.
+
+ This patch removes the parameter. Since it has always been marked optional, there is no web compat risk.
+
+ If this functionality is added back in the future it needs to be implemented in a way that considers all
+ callbacks within a Page and not only those within a single Document.
+
+ * dom/Document.cpp:
+ (WebCore::Document::webkitRequestAnimationFrame):
+ * dom/Document.h:
+ * dom/RequestAnimationFrameCallback.h:
+ * dom/ScriptedAnimationController.cpp:
+ (WebCore::ScriptedAnimationController::registerCallback):
+ (WebCore::ScriptedAnimationController::serviceScriptedAnimations):
+ * dom/ScriptedAnimationController.h:
+ * page/DOMWindow.cpp:
+ (WebCore::DOMWindow::webkitRequestAnimationFrame):
+ * page/DOMWindow.h:
+ * page/DOMWindow.idl:
+
2012-04-09 Chris Guan <[email protected]>
[Blackberry] m_isRequestedByPlugin should be copied in ResourceRequest
Modified: trunk/Source/WebCore/dom/Document.cpp (113572 => 113573)
--- trunk/Source/WebCore/dom/Document.cpp 2012-04-09 10:58:06 UTC (rev 113572)
+++ trunk/Source/WebCore/dom/Document.cpp 2012-04-09 11:42:18 UTC (rev 113573)
@@ -5643,7 +5643,7 @@
}
#if ENABLE(REQUEST_ANIMATION_FRAME)
-int Document::webkitRequestAnimationFrame(PassRefPtr<RequestAnimationFrameCallback> callback, Element* animationElement)
+int Document::webkitRequestAnimationFrame(PassRefPtr<RequestAnimationFrameCallback> callback)
{
if (!m_scriptedAnimationController) {
#if USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
@@ -5658,7 +5658,7 @@
m_scriptedAnimationController->suspend();
}
- return m_scriptedAnimationController->registerCallback(callback, animationElement);
+ return m_scriptedAnimationController->registerCallback(callback);
}
void Document::webkitCancelAnimationFrame(int id)
Modified: trunk/Source/WebCore/dom/Document.h (113572 => 113573)
--- trunk/Source/WebCore/dom/Document.h 2012-04-09 10:58:06 UTC (rev 113572)
+++ trunk/Source/WebCore/dom/Document.h 2012-04-09 11:42:18 UTC (rev 113573)
@@ -1106,7 +1106,7 @@
const DocumentTiming* timing() const { return &m_documentTiming; }
#if ENABLE(REQUEST_ANIMATION_FRAME)
- int webkitRequestAnimationFrame(PassRefPtr<RequestAnimationFrameCallback>, Element*);
+ int webkitRequestAnimationFrame(PassRefPtr<RequestAnimationFrameCallback>);
void webkitCancelAnimationFrame(int id);
void serviceScriptedAnimations(DOMTimeStamp);
#endif
Modified: trunk/Source/WebCore/dom/RequestAnimationFrameCallback.h (113572 => 113573)
--- trunk/Source/WebCore/dom/RequestAnimationFrameCallback.h 2012-04-09 10:58:06 UTC (rev 113572)
+++ trunk/Source/WebCore/dom/RequestAnimationFrameCallback.h 2012-04-09 11:42:18 UTC (rev 113573)
@@ -31,8 +31,6 @@
#ifndef RequestAnimationFrameCallback_h
#define RequestAnimationFrameCallback_h
-#include "Element.h"
-#include <wtf/PassRefPtr.h>
#include <wtf/RefCounted.h>
namespace WebCore {
@@ -42,7 +40,6 @@
virtual ~RequestAnimationFrameCallback() { }
virtual bool handleEvent(DOMTimeStamp) = 0;
- RefPtr<Element> m_element;
int m_id;
bool m_firedOrCancelled;
};
Modified: trunk/Source/WebCore/dom/ScriptedAnimationController.cpp (113572 => 113573)
--- trunk/Source/WebCore/dom/ScriptedAnimationController.cpp 2012-04-09 10:58:06 UTC (rev 113572)
+++ trunk/Source/WebCore/dom/ScriptedAnimationController.cpp 2012-04-09 11:42:18 UTC (rev 113573)
@@ -29,7 +29,6 @@
#if ENABLE(REQUEST_ANIMATION_FRAME)
#include "Document.h"
-#include "Element.h"
#include "FrameView.h"
#include "InspectorInstrumentation.h"
#include "RequestAnimationFrameCallback.h"
@@ -81,12 +80,11 @@
scheduleAnimation();
}
-ScriptedAnimationController::CallbackId ScriptedAnimationController::registerCallback(PassRefPtr<RequestAnimationFrameCallback> callback, Element* animationElement)
+ScriptedAnimationController::CallbackId ScriptedAnimationController::registerCallback(PassRefPtr<RequestAnimationFrameCallback> callback)
{
ScriptedAnimationController::CallbackId id = m_nextCallbackId++;
callback->m_firedOrCancelled = false;
callback->m_id = id;
- callback->m_element = animationElement;
m_callbacks.append(callback);
InspectorInstrumentation::didRequestAnimationFrame(m_document, id);
@@ -112,48 +110,24 @@
{
if (!m_callbacks.size() || m_suspendCount)
return;
- // We want to run the callback for all elements in the document that have registered
- // for a callback and that are visible. Running the callbacks can cause new callbacks
- // to be registered, existing callbacks to be cancelled, and elements to gain or lose
- // visibility so this code has to iterate carefully.
- // FIXME: Currently, this code doesn't do any visibility tests beyond checking display:
-
// First, generate a list of callbacks to consider. Callbacks registered from this point
// on are considered only for the "next" frame, not this one.
CallbackList callbacks(m_callbacks);
- // Firing the callback may cause the visibility of other elements to change. To avoid
- // 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();
- for (size_t i = 0; i < callbacks.size(); ++i) {
- RequestAnimationFrameCallback* callback = callbacks[i].get();
- if (!callback->m_firedOrCancelled && (!callback->m_element || callback->m_element->renderer())) {
- callback->m_firedOrCancelled = true;
- InspectorInstrumentationCookie cookie = InspectorInstrumentation::willFireAnimationFrame(m_document, callback->m_id);
- callback->handleEvent(time);
- InspectorInstrumentation::didFireAnimationFrame(cookie);
- firedCallback = true;
- callbacks.remove(i);
- break;
- }
+ for (size_t i = 0; i < callbacks.size(); ++i) {
+ RequestAnimationFrameCallback* callback = callbacks[i].get();
+ if (!callback->m_firedOrCancelled) {
+ callback->m_firedOrCancelled = true;
+ InspectorInstrumentationCookie cookie = InspectorInstrumentation::willFireAnimationFrame(m_document, callback->m_id);
+ callback->handleEvent(time);
+ InspectorInstrumentation::didFireAnimationFrame(cookie);
}
- } while (firedCallback);
+ }
// Remove any callbacks we fired from the list of pending callbacks.
for (size_t i = 0; i < m_callbacks.size();) {
Modified: trunk/Source/WebCore/dom/ScriptedAnimationController.h (113572 => 113573)
--- trunk/Source/WebCore/dom/ScriptedAnimationController.h 2012-04-09 10:58:06 UTC (rev 113572)
+++ trunk/Source/WebCore/dom/ScriptedAnimationController.h 2012-04-09 11:42:18 UTC (rev 113573)
@@ -42,7 +42,6 @@
namespace WebCore {
class Document;
-class Element;
class RequestAnimationFrameCallback;
class ScriptedAnimationController : public RefCounted<ScriptedAnimationController>
@@ -60,7 +59,7 @@
typedef int CallbackId;
- CallbackId registerCallback(PassRefPtr<RequestAnimationFrameCallback>, Element*);
+ CallbackId registerCallback(PassRefPtr<RequestAnimationFrameCallback>);
void cancelCallback(CallbackId);
void serviceScriptedAnimations(DOMTimeStamp);
Modified: trunk/Source/WebCore/page/DOMWindow.cpp (113572 => 113573)
--- trunk/Source/WebCore/page/DOMWindow.cpp 2012-04-09 10:58:06 UTC (rev 113572)
+++ trunk/Source/WebCore/page/DOMWindow.cpp 2012-04-09 11:42:18 UTC (rev 113573)
@@ -1485,10 +1485,10 @@
}
#if ENABLE(REQUEST_ANIMATION_FRAME)
-int DOMWindow::webkitRequestAnimationFrame(PassRefPtr<RequestAnimationFrameCallback> callback, Element* e)
+int DOMWindow::webkitRequestAnimationFrame(PassRefPtr<RequestAnimationFrameCallback> callback)
{
if (Document* d = document())
- return d->webkitRequestAnimationFrame(callback, e);
+ return d->webkitRequestAnimationFrame(callback);
return 0;
}
Modified: trunk/Source/WebCore/page/DOMWindow.h (113572 => 113573)
--- trunk/Source/WebCore/page/DOMWindow.h 2012-04-09 10:58:06 UTC (rev 113572)
+++ trunk/Source/WebCore/page/DOMWindow.h 2012-04-09 11:42:18 UTC (rev 113573)
@@ -252,7 +252,7 @@
// WebKit animation extensions
#if ENABLE(REQUEST_ANIMATION_FRAME)
- int webkitRequestAnimationFrame(PassRefPtr<RequestAnimationFrameCallback>, Element*);
+ int webkitRequestAnimationFrame(PassRefPtr<RequestAnimationFrameCallback>);
void webkitCancelAnimationFrame(int id);
void webkitCancelRequestAnimationFrame(int id) { webkitCancelAnimationFrame(id); }
#endif
Modified: trunk/Source/WebCore/page/DOMWindow.idl (113572 => 113573)
--- trunk/Source/WebCore/page/DOMWindow.idl 2012-04-09 10:58:06 UTC (rev 113572)
+++ trunk/Source/WebCore/page/DOMWindow.idl 2012-04-09 11:42:18 UTC (rev 113573)
@@ -213,7 +213,7 @@
#if defined(ENABLE_REQUEST_ANIMATION_FRAME)
// WebKit animation extensions, being standardized in the WebPerf WG
- long webkitRequestAnimationFrame(in [Callback] RequestAnimationFrameCallback callback, in [Optional=DefaultIsUndefined] Element element);
+ long webkitRequestAnimationFrame(in [Callback] RequestAnimationFrameCallback callback);
void webkitCancelAnimationFrame(in long id);
void webkitCancelRequestAnimationFrame(in long id); // This is a deprecated alias for webkitCancelAnimationFrame(). Remove this when removing vendor prefix.
#endif
