Log Message
Large negative animation-delays may not work depending on machine uptime https://bugs.webkit.org/show_bug.cgi?id=166962 <rdar://problem/30091526>
Reviewed by Tim Horton. Source/WebCore: If you set a really negative animation delay, it would cause AnimationBase::m_startTime to become negative, because the delay value was "bigger" than monotonicallyIncreasingTime. However, the state machine was using -1 to mean that the start time hadn't yet been set. Classic cmarrin! Replace all the special values with std::optional, and use nullopt to mean the value doesn't exist yet. Test: animations/large-negative-delay.html * page/animation/AnimationBase.cpp: (WebCore::AnimationBase::updateStateMachine): (WebCore::AnimationBase::fireAnimationEventsIfNeeded): (WebCore::AnimationBase::getTimeToNextEvent): (WebCore::AnimationBase::freezeAtTime): (WebCore::AnimationBase::getElapsedTime): * page/animation/AnimationBase.h: Use std::optional. (WebCore::AnimationBase::paused): LayoutTests: * animations/large-negative-delay-expected.txt: Added. * animations/large-negative-delay.html: Added.
Modified Paths
- trunk/LayoutTests/ChangeLog
- trunk/Source/WebCore/ChangeLog
- trunk/Source/WebCore/page/animation/AnimationBase.cpp
- trunk/Source/WebCore/page/animation/AnimationBase.h
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (215351 => 215352)
--- trunk/LayoutTests/ChangeLog 2017-04-14 00:09:08 UTC (rev 215351)
+++ trunk/LayoutTests/ChangeLog 2017-04-14 01:56:35 UTC (rev 215352)
@@ -1,3 +1,14 @@
+2017-04-13 Dean Jackson <d...@apple.com>
+
+ Large negative animation-delays may not work depending on machine uptime
+ https://bugs.webkit.org/show_bug.cgi?id=166962
+ <rdar://problem/30091526>
+
+ Reviewed by Tim Horton.
+
+ * animations/large-negative-delay-expected.txt: Added.
+ * animations/large-negative-delay.html: Added.
+
2017-04-13 Andy VanWagoner <thetalecraf...@gmail.com>
Change Intl prototypes to plain objects
Added: trunk/LayoutTests/animations/large-negative-delay-expected.txt (0 => 215352)
--- trunk/LayoutTests/animations/large-negative-delay-expected.txt (rev 0)
+++ trunk/LayoutTests/animations/large-negative-delay-expected.txt 2017-04-14 01:56:35 UTC (rev 215352)
@@ -0,0 +1,3 @@
+PASS: Saw iteration event. The animation did start.
+PASS: Elapsed time is large. The animation thinks it started a long time ago.
+
Property changes on: trunk/LayoutTests/animations/large-negative-delay-expected.txt
___________________________________________________________________
Added: svn:eol-style
Added: svn:keywords
Added: svn:mime-type
Added: trunk/LayoutTests/animations/large-negative-delay.html (0 => 215352)
--- trunk/LayoutTests/animations/large-negative-delay.html (rev 0)
+++ trunk/LayoutTests/animations/large-negative-delay.html 2017-04-14 01:56:35 UTC (rev 215352)
@@ -0,0 +1,44 @@
+<style>
+#box {
+ position: relative;
+ height: 100px;
+ width: 100px;
+ background-color: blue;
+ animation-name: move;
+ animation-duration: 0.2s;
+ animation-iteration-count: infinite;
+ animation-timing-function: linear;
+ animation-delay: -999999s;
+}
+
+@keyframes move {
+ 0% { left: 0; }
+ 100% { left: 300px; }
+}
+</style>
+<script>
+if (window.testRunner) {
+ testRunner.waitUntilDone();
+ testRunner.dumpAsText();
+}
+
+function output(msg) {
+ document.getElementById('result').innerHTML += `${msg}<br>`;
+}
+
+function iterated(event) {
+ output("PASS: Saw iteration event. The animation did start.");
+
+ if (event.elapsedTime > 1000)
+ output("PASS: Elapsed time is large. The animation thinks it started a long time ago.");
+ else
+ output("FAIL: Elapsed time wasn't big enough.");
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+}
+
+window.addEventListener("animationiteration", iterated, { once: true });
+</script>
+<div id="box"></div>
+<div id="result"></div>
Property changes on: trunk/LayoutTests/animations/large-negative-delay.html
___________________________________________________________________
Added: svn:eol-style
Added: svn:keywords
Added: svn:mime-type
Modified: trunk/Source/WebCore/ChangeLog (215351 => 215352)
--- trunk/Source/WebCore/ChangeLog 2017-04-14 00:09:08 UTC (rev 215351)
+++ trunk/Source/WebCore/ChangeLog 2017-04-14 01:56:35 UTC (rev 215352)
@@ -1,3 +1,32 @@
+2017-04-13 Dean Jackson <d...@apple.com>
+
+ Large negative animation-delays may not work depending on machine uptime
+ https://bugs.webkit.org/show_bug.cgi?id=166962
+ <rdar://problem/30091526>
+
+ Reviewed by Tim Horton.
+
+ If you set a really negative animation delay, it would cause
+ AnimationBase::m_startTime to become negative, because the delay
+ value was "bigger" than monotonicallyIncreasingTime.
+
+ However, the state machine was using -1 to mean that the start time
+ hadn't yet been set. Classic cmarrin!
+
+ Replace all the special values with std::optional, and use nullopt
+ to mean the value doesn't exist yet.
+
+ Test: animations/large-negative-delay.html
+
+ * page/animation/AnimationBase.cpp:
+ (WebCore::AnimationBase::updateStateMachine):
+ (WebCore::AnimationBase::fireAnimationEventsIfNeeded):
+ (WebCore::AnimationBase::getTimeToNextEvent):
+ (WebCore::AnimationBase::freezeAtTime):
+ (WebCore::AnimationBase::getElapsedTime):
+ * page/animation/AnimationBase.h: Use std::optional.
+ (WebCore::AnimationBase::paused):
+
2017-04-13 Alex Christensen <achristen...@webkit.org>
Remove unused SharedBuffer constructor
Modified: trunk/Source/WebCore/page/animation/AnimationBase.cpp (215351 => 215352)
--- trunk/Source/WebCore/page/animation/AnimationBase.cpp 2017-04-14 00:09:08 UTC (rev 215351)
+++ trunk/Source/WebCore/page/animation/AnimationBase.cpp 2017-04-14 01:56:35 UTC (rev 215352)
@@ -163,10 +163,10 @@
m_compositeAnimation->animationController().removeFromAnimationsWaitingForStyle(this);
LOG(Animations, "%p AnimationState %s -> New", this, nameForState(m_animationState));
m_animationState = AnimationState::New;
- m_startTime = 0;
- m_pauseTime = -1;
+ m_startTime = std::nullopt;
+ m_pauseTime = std::nullopt;
m_requestedStartTime = 0;
- m_nextIterationDuration = -1;
+ m_nextIterationDuration = std::nullopt;
endAnimation();
return;
}
@@ -176,10 +176,10 @@
m_compositeAnimation->animationController().removeFromAnimationsWaitingForStyle(this);
LOG(Animations, "%p AnimationState %s -> New", this, nameForState(m_animationState));
m_animationState = AnimationState::New;
- m_startTime = 0;
- m_pauseTime = -1;
+ m_startTime = std::nullopt;
+ m_pauseTime = std::nullopt;
m_requestedStartTime = 0;
- m_nextIterationDuration = -1;
+ m_nextIterationDuration = std::nullopt;
endAnimation();
if (!paused())
@@ -209,7 +209,7 @@
if (input == AnimationStateInput::ResumeOverride) {
if (m_animationState == AnimationState::Looping || m_animationState == AnimationState::Ending) {
// Start the animation
- startAnimation(beginAnimationUpdateTime() - m_startTime);
+ startAnimation(beginAnimationUpdateTime() - m_startTime.value_or(0));
}
return;
}
@@ -227,7 +227,7 @@
// We are pausing before we even started.
LOG(Animations, "%p AnimationState %s -> AnimationState::PausedNew", this, nameForState(m_animationState));
m_animationState = AnimationState::PausedNew;
- m_pauseTime = 0;
+ m_pauseTime = std::nullopt;
}
#if ENABLE(CSS_ANIMATIONS_LEVEL_2)
@@ -299,11 +299,11 @@
LOG(Animations, "%p AnimationState %s -> StartTimeSet (time is %f)", this, nameForState(m_animationState), param);
// We have a start time, set it, unless the startTime is already set
- if (m_startTime <= 0) {
+ if (!m_startTime) {
m_startTime = param;
// If the value for 'animation-delay' is negative then the animation appears to have started in the past.
if (m_animation->delay() < 0)
- m_startTime += m_animation->delay();
+ m_startTime = m_startTime.value() + m_animation->delay();
}
// Now that we know the start time, fire the start event.
@@ -319,7 +319,7 @@
// We are pausing while waiting for a start response. Cancel the animation and wait. When
// we unpause, we will act as though the start timer just fired
m_pauseTime = beginAnimationUpdateTime();
- pauseAnimation(beginAnimationUpdateTime() - m_startTime);
+ pauseAnimation(beginAnimationUpdateTime() - m_startTime.value_or(0));
LOG(Animations, "%p AnimationState %s -> PausedWaitResponse", this, nameForState(m_animationState));
m_animationState = AnimationState::PausedWaitResponse;
}
@@ -339,7 +339,7 @@
} else {
// We are pausing while running. Cancel the animation and wait
m_pauseTime = beginAnimationUpdateTime();
- pauseAnimation(beginAnimationUpdateTime() - m_startTime);
+ pauseAnimation(beginAnimationUpdateTime() - m_startTime.value_or(0));
LOG(Animations, "%p AnimationState %s -> PausedRun", this, nameForState(m_animationState));
m_animationState = AnimationState::PausedRun;
}
@@ -371,7 +371,7 @@
} else {
// We are pausing while running. Cancel the animation and wait
m_pauseTime = beginAnimationUpdateTime();
- pauseAnimation(beginAnimationUpdateTime() - m_startTime);
+ pauseAnimation(beginAnimationUpdateTime() - m_startTime.value_or(0));
LOG(Animations, "%p AnimationState %s -> PausedRun", this, nameForState(m_animationState));
m_animationState = AnimationState::PausedRun;
}
@@ -381,8 +381,8 @@
ASSERT(input == AnimationStateInput::PlayStateRunning);
ASSERT(paused());
// Update the times
- m_startTime += beginAnimationUpdateTime() - m_pauseTime;
- m_pauseTime = -1;
+ m_startTime = m_startTime.value() + beginAnimationUpdateTime() - m_pauseTime.value_or(0);
+ m_pauseTime = std::nullopt;
// we were waiting for the start timer to fire, go back and wait again
LOG(Animations, "%p AnimationState %s -> New", this, nameForState(m_animationState));
@@ -406,7 +406,7 @@
// to start, so jump back to the New state and reset.
LOG(Animations, "%p AnimationState %s -> AnimationState::New", this, nameForState(m_animationState));
m_animationState = AnimationState::New;
- m_pauseTime = -1;
+ m_pauseTime = std::nullopt;
updateStateMachine(input, param);
break;
}
@@ -413,11 +413,11 @@
// Update the times
if (m_animationState == AnimationState::PausedRun)
- m_startTime += beginAnimationUpdateTime() - m_pauseTime;
+ m_startTime = m_startTime.value() + beginAnimationUpdateTime() - m_pauseTime.value_or(0);
else
m_startTime = 0;
- m_pauseTime = -1;
+ m_pauseTime = std::nullopt;
if (m_animationState == AnimationState::PausedWaitStyleAvailable) {
LOG(Animations, "%p AnimationState %s -> StartWaitStyleAvailable", this, nameForState(m_animationState));
@@ -436,7 +436,7 @@
updateStateMachine(AnimationStateInput::StartTimeSet, beginAnimationUpdateTime());
m_isAccelerated = true;
} else {
- bool started = startAnimation(beginAnimationUpdateTime() - m_startTime);
+ bool started = startAnimation(beginAnimationUpdateTime() - m_startTime.value_or(0));
m_compositeAnimation->animationController().addToAnimationsWaitingForStartTimeResponse(this, started);
m_isAccelerated = started;
}
@@ -451,9 +451,9 @@
// We ignore the start time and just move into the paused-run state.
LOG(Animations, "%p AnimationState %s -> PausedRun (time is %f)", this, nameForState(m_animationState), param);
m_animationState = AnimationState::PausedRun;
- ASSERT(m_startTime == 0);
+ ASSERT(!m_startTime);
m_startTime = param;
- m_pauseTime += m_startTime;
+ m_pauseTime = m_pauseTime.value_or(0) + param;
break;
}
@@ -506,7 +506,7 @@
return;
}
- double elapsedDuration = beginAnimationUpdateTime() - m_startTime;
+ double elapsedDuration = beginAnimationUpdateTime() - m_startTime.value_or(0);
#if ENABLE(CSS_ANIMATIONS_LEVEL_2)
// If we are a triggered animation that depends on scroll, our elapsed
// time is determined by the scroll position.
@@ -520,7 +520,7 @@
elapsedDuration = std::max(elapsedDuration, 0.0);
// Check for end timeout
- if (m_totalDuration >= 0 && elapsedDuration >= m_totalDuration) {
+ if (m_totalDuration && elapsedDuration >= m_totalDuration.value()) {
// We may still be in AnimationState::Looping if we've managed to skip a
// whole iteration, in which case we should jump to the end state.
LOG(Animations, "%p AnimationState %s -> Ending", this, nameForState(m_animationState));
@@ -527,10 +527,10 @@
m_animationState = AnimationState::Ending;
// Fire an end event
- updateStateMachine(AnimationStateInput::EndTimerFired, m_totalDuration);
+ updateStateMachine(AnimationStateInput::EndTimerFired, m_totalDuration.value());
} else {
// Check for iteration timeout
- if (m_nextIterationDuration < 0) {
+ if (!m_nextIterationDuration) {
// Hasn't been set yet, set it
double durationLeft = m_animation->duration() - fmod(elapsedDuration, m_animation->duration());
m_nextIterationDuration = elapsedDuration + durationLeft;
@@ -538,7 +538,7 @@
if (elapsedDuration >= m_nextIterationDuration) {
// Set to the next iteration
- double previous = m_nextIterationDuration;
+ double previous = m_nextIterationDuration.value();
double durationLeft = m_animation->duration() - fmod(elapsedDuration, m_animation->duration());
m_nextIterationDuration = elapsedDuration + durationLeft;
@@ -678,16 +678,16 @@
void AnimationBase::getTimeToNextEvent(Seconds& time, bool& isLooping) const
{
// Decide when the end or loop event needs to fire
- const double elapsedDuration = std::max(beginAnimationUpdateTime() - m_startTime, 0.0);
+ const double elapsedDuration = std::max(beginAnimationUpdateTime() - m_startTime.value_or(0), 0.0);
double durationLeft = 0;
- double nextIterationTime = m_totalDuration;
+ double nextIterationTime = m_totalDuration.value_or(0);
- if (m_totalDuration < 0 || elapsedDuration < m_totalDuration) {
+ if (!m_totalDuration || elapsedDuration < m_totalDuration.value()) {
durationLeft = m_animation->duration() > 0 ? (m_animation->duration() - fmod(elapsedDuration, m_animation->duration())) : 0;
nextIterationTime = elapsedDuration + durationLeft;
}
- if (m_totalDuration < 0 || nextIterationTime < m_totalDuration) {
+ if (!m_totalDuration || nextIterationTime < m_totalDuration.value()) {
// We are not at the end yet
ASSERT(nextIterationTime > 0);
isLooping = true;
@@ -722,12 +722,12 @@
ASSERT(m_startTime); // If m_startTime is zero, we haven't started yet, so we'll get a bad pause time.
if (t <= m_animation->delay())
- m_pauseTime = m_startTime;
+ m_pauseTime = m_startTime.value_or(0);
else
- m_pauseTime = m_startTime + t - m_animation->delay();
+ m_pauseTime = m_startTime.value_or(0) + t - m_animation->delay();
if (m_object && m_object->isComposited())
- downcast<RenderBoxModelObject>(*m_object).suspendAnimations(m_pauseTime);
+ downcast<RenderBoxModelObject>(*m_object).suspendAnimations(m_pauseTime.value());
}
double AnimationBase::beginAnimationUpdateTime() const
@@ -758,16 +758,16 @@
if (paused()) {
double delayOffset = (!m_startTime && m_animation->delay() < 0) ? m_animation->delay() : 0;
- return m_pauseTime - m_startTime - delayOffset;
+ return m_pauseTime.value_or(0) - m_startTime.value_or(0) - delayOffset;
}
- if (m_startTime <= 0)
+ if (!m_startTime)
return 0;
if (postActive() || fillingForwards())
- return m_totalDuration;
+ return m_totalDuration.value_or(0);
- return beginAnimationUpdateTime() - m_startTime;
+ return beginAnimationUpdateTime() - m_startTime.value_or(0);
}
void AnimationBase::setElapsedTime(double time)
Modified: trunk/Source/WebCore/page/animation/AnimationBase.h (215351 => 215352)
--- trunk/Source/WebCore/page/animation/AnimationBase.h 2017-04-14 00:09:08 UTC (rev 215351)
+++ trunk/Source/WebCore/page/animation/AnimationBase.h 2017-04-14 01:56:35 UTC (rev 215352)
@@ -121,7 +121,7 @@
bool fillingForwards() const { return m_animationState == AnimationState::FillingForwards; }
bool active() const { return !postActive() && !preActive(); }
bool running() const { return !isNew() && !postActive(); }
- bool paused() const { return m_pauseTime >= 0 || m_animationState == AnimationState::PausedNew; }
+ bool paused() const { return m_pauseTime || m_animationState == AnimationState::PausedNew; }
bool inPausedState() const { return m_animationState >= AnimationState::PausedNew && m_animationState <= AnimationState::PausedRun; }
bool isNew() const { return m_animationState == AnimationState::New || m_animationState == AnimationState::PausedNew; }
bool waitingForStartTime() const { return m_animationState == AnimationState::StartWaitResponse; }
@@ -244,11 +244,11 @@
CompositeAnimation* m_compositeAnimation; // Ideally this would be a reference, but it has to be cleared if an animation is destroyed inside an event callback.
Ref<Animation> m_animation;
- double m_startTime { 0 };
- double m_pauseTime { -1 };
+ std::optional<double> m_startTime;
+ std::optional<double> m_pauseTime;
double m_requestedStartTime { 0 };
- double m_totalDuration { -1 };
- double m_nextIterationDuration { -1 };
+ std::optional<double> m_totalDuration;
+ std::optional<double> m_nextIterationDuration;
AnimationState m_animationState { AnimationState::New };
bool m_isAccelerated { false };
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes
