Now that we don't actually start pending animations until the following refresh
driver tick we no longer need to be able to fast-forward the AnimationTimeline
between ticks.
When a player is made pending, we rely on it being added to a pending player
tracker that will eventually start the player. However, there are a few
situations where this might not happen. For example, we can't find a pending
player tracker (e.g. there's no source content or the source content isn't
attached to a document), or the pending player tracker disappeared.
In these cases we still want to ensure that such a player does actually get
started. This patch attempts to detect such situations and start players
accordingly.
There are, unfortunately, currently no tests for this. I have been unsuccessful
in recreating any of the situations these tests are supposed to cover.
This patch switches on the new, "actually start the player in the next refresh
driver tick" behavior. It updates PendingPlayerTracker, adding
a StartPendingPlayersOnNextTick method which calls the appropriate method on
AnimationPlayer. The existing StartPendingPlayers is renamed to
StartPendingPlayersNow and is used for testing only.
Furthermore, since we now expect AnimationPlayer::StartOnNextTick to be
functional, AnimationPlayer::DoPlay is updated to use it when there is no
document available. This should make playing an animation player always
asynchronous, that is, always transition to the pending state temporarily
(unless we are already playing).
Earlier in this patch series we added AnimationPlayer::StartOnNextTick which
takes a ready time parameter expressed in timeline time. In order to call this
method when painting finishes we need to convert the TimeStamp recorded when
painting finished to a timeline time. However, when the timeline is driven by
a refresh driver under test control we can no longer meaningfully do this
conversion since there is no correspondence between the notion of time used to
record the time when painting finished (wallclock time) and the notion of time
used by the timeline (which has been arbitrarily adjusted by test code).
We need a way to detect this situation so that we know not to call
ToTimelineTime in that case.
Alternatively, we could make ToTimelineTime automatically return a null value
when its refresh driver is under test control. However, in this situation
ToTimelineTime can still actually be used to convert a TimeStamp to a timeline
time as long as the TimeStamp is from the same refresh driver. Indeed,
GetCurrentTime does exactly that. So if we were to go down that route we would
have to provide a way for GetCurrentTime to work around that restriction.
For now, this patch puts the onus on the caller of ToTimelineTime to check if
the timeline is under test control first (unless they are passing a TimeStamp
from the same refresh driver, in which case there is no need to check).
This patch makes AnimationPlayer act on requests to StartOnNextTick by checking
for mPendingReadyTime on each tick.
We also check that the ready time is not in the future since currently it might
be possible that we get multiple calls to AnimationPlayer::Tick within a single
refresh driver tick.
Note that this patch shouldn't actually produce any observable change yet,
however, since we don't call StartOnNextTick anywhere.
Adds a method that schedules an animation player to begin on the next tick using
the supplied time as the start time.
We don't call this yet, however, but simply add the method and the
mPendingReadyTime member it sets.
In addition to AnimationPlayer::StartNow, this patch series also makes
AnimationPlayer::Tick start animations.
Since these methods will share a lot of code we first factor out a common
ResumeAt method to encapsulate the common code.
In this patch series we adjust the behavior of animation starting so that the
animation does not actually start until the following refresh driver tick. This
requires some tweaks to tests to ensure they continue to pass.
Turns out there was a code path that resulted in attempting to acquire a lock
on the DataStorage mutex when one had already been acquired, resulting in
deadlock. This fixes it.
clang emits the following warning on this code:
warning: field 'gc' is uninitialized when used here [-Wuninitialized]
The warning is not an indication of a real bug since we're just taking the store
buffer's address, but we may as well silence it.
Recent clang emits the following warning (which is treated as an error) on this code:
error: implicit conversion of nullptr constant to 'bool' [-Werror,-Wnull-conversion]
