Bug 1195180 part 7 - Store animations in an array; r=heycam

Currently AnimationTimeline stores animations in a hashmap which means that
when we go to iterate over those animations to tick them we will visit them
in an order that is non-deterministic.

Although many of the observable effects of ticking an animation (e.g. CSS
animation/transition events, mutation observer events) are later sorted so that
the result does not depend on the order in which animations are ticked, this is
not true for in all cases. In particular, the order in which Animation.finished
promises are resolved will vary depending on the order in which animations are
ticked. Likewise, for Animation finish events.

Furthermore, it seems generally desirable to have a deterministic order for
visiting animations in order to aid reproducing bugs.

To achieve this, this patch switches the storage of animations in
AnimationTimeline to use an array instead. However, when adding animations
we need to determine if the animation to add already exists. To this end we
also maintain a hashmap of the animations so we can quickly determine if
the animation to add is a duplicate or not.
This commit is contained in:
Brian Birtles 2015-09-28 12:38:41 +09:00
parent fc7aec7e33
commit 62b28f9716
3 changed files with 28 additions and 11 deletions

View File

@ -10,7 +10,8 @@
namespace mozilla {
namespace dom {
NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(AnimationTimeline, mWindow, mAnimations)
NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(AnimationTimeline, mWindow,
mAnimationOrder)
NS_IMPL_CYCLE_COLLECTING_ADDREF(AnimationTimeline)
NS_IMPL_CYCLE_COLLECTING_RELEASE(AnimationTimeline)
@ -31,8 +32,9 @@ AnimationTimeline::GetAnimations(AnimationSequence& aAnimations)
}
}
for (auto iter = mAnimations.Iter(); !iter.Done(); iter.Next()) {
Animation* animation = iter.Get()->GetKey();
aAnimations.SetCapacity(mAnimationOrder.Length());
for (Animation* animation : mAnimationOrder) {
// Skip animations which are no longer relevant or which have been
// associated with another timeline. These animations will be removed
@ -63,7 +65,12 @@ AnimationTimeline::GetAnimations(AnimationSequence& aAnimations)
void
AnimationTimeline::NotifyAnimationUpdated(Animation& aAnimation)
{
if (mAnimations.Contains(&aAnimation)) {
return;
}
mAnimations.PutEntry(&aAnimation);
mAnimationOrder.AppendElement(&aAnimation);
}
} // namespace dom

View File

@ -95,8 +95,17 @@ protected:
nsCOMPtr<nsIGlobalObject> mWindow;
// Animations observing this timeline
typedef nsTHashtable<nsRefPtrHashKey<dom::Animation>> AnimationSet;
AnimationSet mAnimations;
//
// We store them in (a) a hashset for quick lookup, and (b) an array
// to maintain a fixed sampling order.
//
// The array keeps a strong reference to each animation in order
// to save some addref/release traffic and because we never dereference
// the pointers in the hashset.
typedef nsTHashtable<nsPtrHashKey<dom::Animation>> AnimationSet;
typedef nsTArray<nsRefPtr<dom::Animation>> AnimationArray;
AnimationSet mAnimations;
AnimationArray mAnimationOrder;
};
} // namespace dom

View File

@ -111,20 +111,21 @@ DocumentTimeline::WillRefresh(mozilla::TimeStamp aTime)
MOZ_ASSERT(mIsObservingRefreshDriver);
bool needsTicks = false;
AnimationArray animationsToKeep(mAnimationOrder.Length());
for (auto iter = mAnimations.Iter(); !iter.Done(); iter.Next()) {
Animation* animation = iter.Get()->GetKey();
// Drop any animations which no longer need to be tracked by this timeline.
for (Animation* animation : mAnimationOrder) {
if (animation->GetTimeline() != this ||
(!animation->IsRelevant() && !animation->NeedsTicks())) {
iter.Remove();
mAnimations.RemoveEntry(animation);
continue;
}
needsTicks |= animation->NeedsTicks();
animationsToKeep.AppendElement(animation);
}
mAnimationOrder.SwapElements(animationsToKeep);
if (!needsTicks) {
// If another refresh driver observer destroys the nsPresContext,
// nsRefreshDriver will detect it and we won't be called.
@ -142,7 +143,7 @@ DocumentTimeline::NotifyRefreshDriverCreated(nsRefreshDriver* aDriver)
"Timeline should not be observing the refresh driver before"
" it is created");
if (mAnimations.Count()) {
if (!mAnimationOrder.IsEmpty()) {
aDriver->AddRefreshObserver(this, Flush_Style);
mIsObservingRefreshDriver = true;
}