Skip to content

GitLab

T
tangled
Commit 8179fe41 authored by dstockwell@chromium.org's avatar dstockwell@chromium.org
Browse files
Options

Web Animations: Timeline should not advance during task execution 

Previously the animation clock was only frozen during animation frame
callbacks. This meant that the timeline was able to advance during other
tasks.

This patch allows the clock to advance once per task, either to the
compositor supplied frame start time or to an approximation of the next
expected frame time.

http://dev.w3.org/fxtf/web-animations/#script-execution-and-live-updates-to-the-model

This is a reland of r173583[1], the previous patch hit added assertions
due to double rounding issues, the new approach avoids these. Also ensures
that other unit tests see a non-zero currentTime (seen in some webkit_unit_test
runs on Android).

[1] https://src.chromium.org/viewvc/blink?view=rev&revision=173583
BUG=367903
TBR=abarth

Review URL: https://codereview.chromium.org/275753002

git-svn-id: svn://svn.chromium.org/blink/trunk@173729 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 2a0e1f04
Hide whitespace changes
Inline Side-by-side
Showing with 173 additions and 78 deletions
third_party/WebKit/LayoutTests/TestExpectations
View file @ 8179fe41
...@@ -978,8 +978,6 @@ crbug.com/371651 svg/text/text-text-05-t.svg [ Failure ] ...@@ -978,8 +978,6 @@ crbug.com/371651 svg/text/text-text-05-t.svg [ Failure ]
crbug.com/371654 fast/html/imports/import-custom-element-dup-resolve.html [ Pass Failure ] crbug.com/371654 fast/html/imports/import-custom-element-dup-resolve.html [ Pass Failure ]
crbug.com/367903 web-animations-api/timeline-time.html [ Pass Failure ]
# Started failing with v8 roll in chromium r267851. # Started failing with v8 roll in chromium r267851.
crbug.com/369600 fast/events/onerror-no-constructor.html [ Failure ] crbug.com/369600 fast/events/onerror-no-constructor.html [ Failure ]
......
third_party/WebKit/LayoutTests/web-animations-api/element-animate-position-crash.html
View file @ 8179fe41
...@@ -12,6 +12,7 @@ if (window.testRunner) { ...@@ -12,6 +12,7 @@ if (window.testRunner) {
testRunner.waitUntilDone(); testRunner.waitUntilDone();
testRunner.dumpAsText(); testRunner.dumpAsText();
requestAnimationFrame(function() { requestAnimationFrame(function() {
document.documentElement.textContent = 'PASS';
testRunner.notifyDone(); testRunner.notifyDone();
}); });
} }
......
third_party/WebKit/LayoutTests/web-animations-api/timeline-time.html
View file @ 8179fe41
...@@ -2,25 +2,45 @@ ...@@ -2,25 +2,45 @@
<script src="../resources/testharness.js"></script> <script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script> <script src="../resources/testharnessreport.js"></script>
<body>
<div id='element'></div>
</body>
<script> <script>
var element = document.getElementById('element');
var duration = 1;
var animation = new Animation(element,
[{opacity: '1', offset: 0},
{opacity: '0', offset: 1}],
duration);
test(function() { test(function() {
var startTime = document.timeline.currentTime; var startTime = document.timeline.currentTime;
var player = document.timeline.play(animation); assert_greater_than_equal(startTime, 0);
player.finish(); var start = performance.now();
var finishTime = document.timeline.currentTime; while (performance.now() < start + 250)
assert_greater_than_equal(startTime, 0); /* wait */;
assert_equals(startTime, finishTime); assert_equals(document.timeline.currentTime, startTime);
}, 'document.timeline.currentTime should not change within a script block.'); }, 'document.timeline.currentTime should not change within a script block.');
(function() {
var timeoutTest = async_test('document.timeline.currentTime time should change after a script timeout');
var startTime = document.timeline.currentTime;
setTimeout(function() {
timeoutTest.step(function() {
assert_greater_than(document.timeline.currentTime, startTime);
});
timeoutTest.done();
}, 100);
})();
(function() {
var rafTest = async_test('document.timeline.currentTime time should be the same for all RAF callbacks in a frame');
var startTime = document.timeline.currentTime;
var firstRafTime;
requestAnimationFrame(function() {
rafTest.step(function() {
assert_greater_than_equal(document.timeline.currentTime, startTime);
firstRafTime = document.timeline.currentTime;
});
});
requestAnimationFrame(function() {
rafTest.step(function() {
assert_equals(document.timeline.currentTime, firstRafTime);
});
rafTest.done();
});
})();
</script> </script>
third_party/WebKit/Source/core/animation/AnimationClock.cpp
View file @ 8179fe41
...@@ -28,26 +28,54 @@ ...@@ -28,26 +28,54 @@
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/ */
#include <math.h>
#include "config.h" #include "config.h"
#include "core/animation/AnimationClock.h" #include "core/animation/AnimationClock.h"
#include "wtf/CurrentTime.h"
namespace {
// FIXME: This is an approximation of time between frames, used when
// ticking the animation clock outside of animation frame callbacks.
// Ideally this would be generated by the compositor.
const double approximateFrameTime = 1 / 60.0;
}
namespace WebCore { namespace WebCore {
unsigned AnimationClock::s_currentTask = 0;
void AnimationClock::updateTime(double time) void AnimationClock::updateTime(double time)
{ {
if (time > m_time) if (time > m_time)
m_time = time; m_time = time;
m_frozen = true; m_currentTask = s_currentTask;
} }
double AnimationClock::currentTime() double AnimationClock::currentTime()
{ {
if (!m_frozen) { if (m_currentTask != s_currentTask) {
double newTime = m_monotonicallyIncreasingTime(); const double currentTime = m_monotonicallyIncreasingTime();
if (newTime >= m_time + minTimeBeforeUnsynchronizedAnimationClockTick) if (m_time < currentTime) {
m_time = newTime; // Advance to the first estimated frame after the current time.
const double frameShift = fmod(currentTime - m_time, approximateFrameTime);
const double newTime = currentTime + approximateFrameTime - frameShift;
ASSERT(newTime > currentTime);
ASSERT(newTime <= currentTime + approximateFrameTime);
updateTime(newTime);
} else {
m_currentTask = s_currentTask;
}
} }
return m_time; return m_time;
} }
void AnimationClock::resetTimeForTesting()
{
m_time = 0;
m_currentTask = 0;
s_currentTask = 0;
}
} }
third_party/WebKit/Source/core/animation/AnimationClock.h
View file @ 8179fe41
...@@ -31,40 +31,34 @@ ...@@ -31,40 +31,34 @@
#ifndef AnimationClock_h #ifndef AnimationClock_h
#define AnimationClock_h #define AnimationClock_h
#include <limits>
#include "wtf/CurrentTime.h" #include "wtf/CurrentTime.h"
#include "wtf/PassOwnPtr.h" #include "wtf/PassOwnPtr.h"
#include "wtf/Noncopyable.h"
namespace WebCore { namespace WebCore {
// FIXME: This value is used to suppress updates when time is required outside of a frame.
// The purpose of allowing the clock to update during such periods is to allow animations
// to have an appropriate start time and for getComputedStyle to attempt to catch-up to a
// compositor animation. However a more accurate system might be to attempt to phase-lock
// with the frame clock.
const double minTimeBeforeUnsynchronizedAnimationClockTick = 0.005;
class AnimationClock { class AnimationClock {
WTF_MAKE_NONCOPYABLE(AnimationClock);
public: public:
static PassOwnPtr<AnimationClock> create(WTF::TimeFunction monotonicallyIncreasingTime = WTF::monotonicallyIncreasingTime) explicit AnimationClock(WTF::TimeFunction monotonicallyIncreasingTime = WTF::monotonicallyIncreasingTime)
: m_monotonicallyIncreasingTime(monotonicallyIncreasingTime)
, m_time(0)
, m_currentTask(std::numeric_limits<unsigned>::max())
{ {
return adoptPtr(new AnimationClock(monotonicallyIncreasingTime));
} }
void updateTime(double time); void updateTime(double time);
double currentTime(); double currentTime();
void unfreeze() { m_frozen = false; } void resetTimeForTesting();
void resetTimeForTesting() { m_time = 0; m_frozen = true; }
static void notifyTaskStart() { ++s_currentTask; }
private: private:
AnimationClock(WTF::TimeFunction monotonicallyIncreasingTime)
: m_monotonicallyIncreasingTime(monotonicallyIncreasingTime)
, m_time(0)
, m_frozen(false)
{
}
WTF::TimeFunction m_monotonicallyIncreasingTime; WTF::TimeFunction m_monotonicallyIncreasingTime;
double m_time; double m_time;
bool m_frozen; unsigned m_currentTask;
static unsigned s_currentTask;
}; };
} // namespace WebCore } // namespace WebCore
......
third_party/WebKit/Source/core/animation/AnimationClockTest.cpp
View file @ 8179fe41
...@@ -39,11 +39,15 @@ using namespace WebCore; ...@@ -39,11 +39,15 @@ using namespace WebCore;
namespace { namespace {
class AnimationAnimationClockTest : public ::testing::Test { class AnimationAnimationClockTest : public ::testing::Test {
public:
AnimationAnimationClockTest()
: animationClock(mockTimeFunction)
{}
protected: protected:
virtual void SetUp() virtual void SetUp()
{ {
animationClock = AnimationClock::create(mockTimeFunction); mockTime = 0;
mockTime = 200; animationClock.resetTimeForTesting();
} }
static double mockTimeFunction() static double mockTimeFunction()
...@@ -52,36 +56,88 @@ protected: ...@@ -52,36 +56,88 @@ protected:
} }
static double mockTime; static double mockTime;
OwnPtr<AnimationClock> animationClock; AnimationClock animationClock;
}; };
double AnimationAnimationClockTest::mockTime; double AnimationAnimationClockTest::mockTime;
TEST_F(AnimationAnimationClockTest, CurrentTime) TEST_F(AnimationAnimationClockTest, TimeIsGreaterThanZeroForUnitTests)
{ {
// Current time should not advance until minTimeBeforeUnsynchronizedTick has elapsed AnimationClock clock;
EXPECT_EQ(200, animationClock->currentTime()); // unit tests outside core/animation shouldn't need to do anything to get
mockTime = 200 + minTimeBeforeUnsynchronizedAnimationClockTick / 2.0; // a non-zero currentTime().
EXPECT_EQ(200, animationClock->currentTime()); EXPECT_GT(clock.currentTime(), 0);
}
TEST_F(AnimationAnimationClockTest, TimeDoesNotChange)
{
animationClock.updateTime(100);
EXPECT_EQ(100, animationClock.currentTime());
EXPECT_EQ(100, animationClock.currentTime());
}
TEST_F(AnimationAnimationClockTest, TimeAdvancesWhenUpdated)
{
animationClock.updateTime(100);
EXPECT_EQ(100, animationClock.currentTime());
mockTime = 200 + minTimeBeforeUnsynchronizedAnimationClockTick; animationClock.updateTime(200);
EXPECT_EQ(mockTime, animationClock->currentTime()); EXPECT_EQ(200, animationClock.currentTime());
} }
TEST_F(AnimationAnimationClockTest, UpdateTime) TEST_F(AnimationAnimationClockTest, TimeAdvancesToTaskTime)
{ {
animationClock->updateTime(100); animationClock.updateTime(100);
EXPECT_EQ(100, animationClock->currentTime()); EXPECT_EQ(100, animationClock.currentTime());
mockTime = 200;
EXPECT_EQ(100, animationClock->currentTime()); mockTime = 150;
AnimationClock::notifyTaskStart();
animationClock->unfreeze(); EXPECT_GE(animationClock.currentTime(), mockTime);
EXPECT_EQ(200, animationClock->currentTime()); }
animationClock->updateTime(300); TEST_F(AnimationAnimationClockTest, TimeAdvancesToTaskTimeOnlyWhenRequired)
EXPECT_EQ(300, animationClock->currentTime()); {
mockTime = 400; animationClock.updateTime(100);
EXPECT_EQ(300, animationClock->currentTime()); EXPECT_EQ(100, animationClock.currentTime());
AnimationClock::notifyTaskStart();
animationClock.updateTime(125);
EXPECT_EQ(125, animationClock.currentTime());
}
TEST_F(AnimationAnimationClockTest, UpdateTimeIsMonotonic)
{
animationClock.updateTime(100);
EXPECT_EQ(100, animationClock.currentTime());
// Update can't go backwards.
animationClock.updateTime(50);
EXPECT_EQ(100, animationClock.currentTime());
mockTime = 50;
AnimationClock::notifyTaskStart();
EXPECT_EQ(100, animationClock.currentTime());
mockTime = 150;
AnimationClock::notifyTaskStart();
EXPECT_GE(animationClock.currentTime(), mockTime);
// Update can't go backwards after advance to estimate.
animationClock.updateTime(100);
EXPECT_GE(animationClock.currentTime(), mockTime);
}
TEST_F(AnimationAnimationClockTest, CurrentTimeUpdatesTask)
{
animationClock.updateTime(100);
EXPECT_EQ(100, animationClock.currentTime());
mockTime = 100;
AnimationClock::notifyTaskStart();
EXPECT_EQ(100, animationClock.currentTime());
mockTime = 150;
EXPECT_EQ(100, animationClock.currentTime());
} }
} }
third_party/WebKit/Source/core/animation/DocumentAnimations.cpp
View file @ 8179fe41
...@@ -92,8 +92,6 @@ void DocumentAnimations::startPendingAnimations(Document& document) ...@@ -92,8 +92,6 @@ void DocumentAnimations::startPendingAnimations(Document& document)
ASSERT(document.view()); ASSERT(document.view());
document.view()->scheduleAnimation(); document.view()->scheduleAnimation();
} }
document.animationClock().unfreeze();
} }
} // namespace WebCore } // namespace WebCore
third_party/WebKit/Source/core/animation/css/TransitionTimeline.cpp
View file @ 8179fe41
...@@ -45,7 +45,6 @@ TransitionTimeline::TransitionTimeline(Document* document, PassOwnPtr<PlatformTi ...@@ -45,7 +45,6 @@ TransitionTimeline::TransitionTimeline(Document* document, PassOwnPtr<PlatformTi
: DocumentTimeline(document, timing) : DocumentTimeline(document, timing)
{ {
setZeroTime(document->animationClock().currentTime()); setZeroTime(document->animationClock().currentTime());
document->animationClock().unfreeze();
} }
} // namespace WebCore } // namespace WebCore
third_party/WebKit/Source/core/dom/Document.cpp
View file @ 8179fe41
...@@ -42,7 +42,6 @@ ...@@ -42,7 +42,6 @@
#include "bindings/v8/ExceptionStatePlaceholder.h" #include "bindings/v8/ExceptionStatePlaceholder.h"
#include "bindings/v8/ScriptController.h" #include "bindings/v8/ScriptController.h"
#include "core/accessibility/AXObjectCache.h" #include "core/accessibility/AXObjectCache.h"
#include "core/animation/AnimationClock.h"
#include "core/animation/DocumentAnimations.h" #include "core/animation/DocumentAnimations.h"
#include "core/animation/DocumentTimeline.h" #include "core/animation/DocumentTimeline.h"
#include "core/animation/css/TransitionTimeline.h" #include "core/animation/css/TransitionTimeline.h"
...@@ -476,7 +475,6 @@ Document::Document(const DocumentInit& initializer, DocumentClassFlags documentC ...@@ -476,7 +475,6 @@ Document::Document(const DocumentInit& initializer, DocumentClassFlags documentC
#ifndef NDEBUG #ifndef NDEBUG
, m_didDispatchViewportPropertiesChanged(false) , m_didDispatchViewportPropertiesChanged(false)
#endif #endif
, m_animationClock(AnimationClock::create())
, m_timeline(DocumentTimeline::create(this)) , m_timeline(DocumentTimeline::create(this))
, m_transitionTimeline(TransitionTimeline::create(this)) , m_transitionTimeline(TransitionTimeline::create(this))
, m_templateDocumentHost(nullptr) , m_templateDocumentHost(nullptr)
......
third_party/WebKit/Source/core/dom/Document.h
View file @ 8179fe41
...@@ -30,6 +30,7 @@ ...@@ -30,6 +30,7 @@
#include "bindings/v8/ExceptionStatePlaceholder.h" #include "bindings/v8/ExceptionStatePlaceholder.h"
#include "bindings/v8/ScriptValue.h" #include "bindings/v8/ScriptValue.h"
#include "core/animation/AnimationClock.h"
#include "core/animation/CompositorPendingAnimations.h" #include "core/animation/CompositorPendingAnimations.h"
#include "core/dom/ContainerNode.h" #include "core/dom/ContainerNode.h"
#include "core/dom/DocumentEncodingData.h" #include "core/dom/DocumentEncodingData.h"
...@@ -64,7 +65,6 @@ ...@@ -64,7 +65,6 @@
namespace WebCore { namespace WebCore {
class AXObjectCache; class AXObjectCache;
class AnimationClock;
class Attr; class Attr;
class CDATASection; class CDATASection;
class CSSFontSelector; class CSSFontSelector;
...@@ -1022,7 +1022,7 @@ public: ...@@ -1022,7 +1022,7 @@ public:
// Return a Locale for the default locale if the argument is null or empty. // Return a Locale for the default locale if the argument is null or empty.
Locale& getCachedLocale(const AtomicString& locale = nullAtom); Locale& getCachedLocale(const AtomicString& locale = nullAtom);
AnimationClock& animationClock() { return *m_animationClock; } AnimationClock& animationClock() { return m_animationClock; }
DocumentTimeline& timeline() const { return *m_timeline; } DocumentTimeline& timeline() const { return *m_timeline; }
DocumentTimeline& transitionTimeline() const { return *m_transitionTimeline; } DocumentTimeline& transitionTimeline() const { return *m_transitionTimeline; }
CompositorPendingAnimations& compositorPendingAnimations() { return m_compositorPendingAnimations; } CompositorPendingAnimations& compositorPendingAnimations() { return m_compositorPendingAnimations; }
...@@ -1369,7 +1369,7 @@ private: ...@@ -1369,7 +1369,7 @@ private:
typedef HashMap<AtomicString, OwnPtr<Locale> > LocaleIdentifierToLocaleMap; typedef HashMap<AtomicString, OwnPtr<Locale> > LocaleIdentifierToLocaleMap;
LocaleIdentifierToLocaleMap m_localeCache; LocaleIdentifierToLocaleMap m_localeCache;
OwnPtr<AnimationClock> m_animationClock; AnimationClock m_animationClock;
RefPtr<DocumentTimeline> m_timeline; RefPtr<DocumentTimeline> m_timeline;
RefPtr<DocumentTimeline> m_transitionTimeline; RefPtr<DocumentTimeline> m_transitionTimeline;
CompositorPendingAnimations m_compositorPendingAnimations; CompositorPendingAnimations m_compositorPendingAnimations;
......
third_party/WebKit/Source/web/WebKit.cpp
View file @ 8179fe41
...@@ -35,6 +35,7 @@ ...@@ -35,6 +35,7 @@
#include "bindings/v8/V8Binding.h" #include "bindings/v8/V8Binding.h"
#include "bindings/v8/V8Initializer.h" #include "bindings/v8/V8Initializer.h"
#include "core/Init.h" #include "core/Init.h"
#include "core/animation/AnimationClock.h"
#include "core/dom/Microtask.h" #include "core/dom/Microtask.h"
#include "core/frame/Settings.h" #include "core/frame/Settings.h"
#include "core/page/Page.h" #include "core/page/Page.h"
...@@ -66,8 +67,11 @@ namespace { ...@@ -66,8 +67,11 @@ namespace {
class EndOfTaskRunner : public WebThread::TaskObserver { class EndOfTaskRunner : public WebThread::TaskObserver {
public: public:
virtual void willProcessTask() { } virtual void willProcessTask() OVERRIDE
virtual void didProcessTask() {
WebCore::AnimationClock::notifyTaskStart();
}
virtual void didProcessTask() OVERRIDE
{ {
WebCore::Microtask::performCheckpoint(); WebCore::Microtask::performCheckpoint();
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment