When updating animations, we shouldn't unnecessarily clobber the "wins in
cascade" state of their properties since this can lead to unnecessary restyles
when we then decide we need to update the cascade.
This flag is no longer needed because in bug 1232563 we introduced a more
thorough optimization that detects when the animation is not changing by
comparing the progress value between samples and avoids requesting restyles
when it does not change.
Now that we track whether or not animations are up to date using the hashset in
EffectCompositor, we can remove the mStyleRuleRefreshTime flag that is, as of
part 5 of this patch series, now only used for detecting whether or not
animations are up to date.
In order to preserve the existing behavior of FlushAnimations, however, this
patch temporarily introduces a method to indicate if there are throttled
animations or not.
It might not be obvious that FlushAnimations is only concerned with throttled
animations due to its name. FlushAnimations is simply intended to post
animation restyles for out-of-date animations. Any animations that are *not*
throttled will either be up to date, or we will have already posted an
animation restyle so we only need to consider throttled animations in this case.
This is needed in order to support script-generated animations since they do not
belong to any AnimationCollection.
This patch adopts the naming "animation rule" over "style rule". Currently we
are inconsistent about this (e.g. GetAnimationRule vs EnsureStyleRuleFor).
We don't do a mass rename here but just a few places near where we're touching.
Many of the other references to "style rule" will be revised in this bug or
related bugs so we can fix those references when we come to them.
--HG--
extra : rebase_source : e1f824029b39960915e056328447de256b6c1c6d
This patch also simplifies this logic by simply always looking for overrides of
'transform' and 'opacity'.
--HG--
extra : rebase_source : d1e432e629e2b97651f14c784f97c03f55d217be
Without this fix, mIsRunningOnCompositor will be unpredictable in
MutationObserver callbacks.
For example:
mIsRunningOnCompositor will be false if the micro task for
the MutationObserver is processed before building display list.
mIsRunningOnCompositor will be true if there is no room to process
the micro task before building display list.
This is to align with the existing GetAnimationCollection method that takes
a frame. Also, by making this name more specific hopefully it will be used less
since we are trying to move as much code as possible over to using EffectSet
instead of AnimationCollection.
This isn't needed today, but it makes more sense, and if we ever gave
the cache a longer lifetime, it would be needed, since the
nsCSSKeyframeRule can maintain its identity across style changes whereas
a matched Declaration cannot.
--HG--
extra : commitid : LPoMlq8m1TH
This is the key change in this patch series; it changes the object we
use for style data (currently nsIStyleRule) identity. It allows
removing some hacks we have to deal with that for StyleRule, and avoids
having to write similar hacks for nsCSSKeyframeRule and nsCSSPageRule
(which are broken without this).
I confirmed locally that it is this patch that fixes both of the todo_is
mochitests, by building and testing with the patch queue through patch
11, and again through patch 12.
--HG--
extra : commitid : AzgBp6KfPhJ
The current location of the assertion will stop being called in patch 12
and will go away in patch 15; the new location is valid both before and
after patch 12.
--HG--
extra : commitid : 8wH1hXHKWU5
This probably should have been done before, but prior to this patch
series, dynamic changes of the declarations on these rules were broken
due to rule immutability violations; now that is no longer the case, but
to benefit from that, I believe we actually need to mark the
declarations as immutable once matched so that dynamic changes will
trigger construction of a new declaration (which thus has a new
nsIStyleRule identity).
--HG--
extra : commitid : 8IsYBd67qQr
Do some minor revisions in struct ComputedTiming.
1. Use Nullable<double> mProgress, so remove the static const kNullProgress.
The generated ComputedTimingProperties dictionary uses "Nullable" variable,
so we replace the origin type in ComputedTiming to make it more consistent
with that in ComputedTimingProperties dictionary.
2. Use scoped enums for AnimationPhase.
--HG--
extra : rebase_source : 31280c867a30e7bcdcfe831cbc72ca08c8ddc762
The bulk of this commit was generated with a script, executed at the top
level of a typical source code checkout. The only non-machine-generated
part was modifying MFBT's moz.build to reflect the new naming.
CLOSED TREE makes big refactorings like this a piece of cake.
# The main substitution.
find . -name '*.cpp' -o -name '*.cc' -o -name '*.h' -o -name '*.mm' -o -name '*.idl'| \
xargs perl -p -i -e '
s/nsRefPtr\.h/RefPtr\.h/g; # handle includes
s/nsRefPtr ?</RefPtr</g; # handle declarations and variables
'
# Handle a special friend declaration in gfx/layers/AtomicRefCountedWithFinalize.h.
perl -p -i -e 's/::nsRefPtr;/::RefPtr;/' gfx/layers/AtomicRefCountedWithFinalize.h
# Handle nsRefPtr.h itself, a couple places that define constructors
# from nsRefPtr, and code generators specially. We do this here, rather
# than indiscriminantly s/nsRefPtr/RefPtr/, because that would rename
# things like nsRefPtrHashtable.
perl -p -i -e 's/nsRefPtr/RefPtr/g' \
mfbt/nsRefPtr.h \
xpcom/glue/nsCOMPtr.h \
xpcom/base/OwningNonNull.h \
ipc/ipdl/ipdl/lower.py \
ipc/ipdl/ipdl/builtin.py \
dom/bindings/Codegen.py \
python/lldbutils/lldbutils/utils.py
# In our indiscriminate substitution above, we renamed
# nsRefPtrGetterAddRefs, the class behind getter_AddRefs. Fix that up.
find . -name '*.cpp' -o -name '*.h' -o -name '*.idl' | \
xargs perl -p -i -e 's/nsRefPtrGetterAddRefs/RefPtrGetterAddRefs/g'
if [ -d .git ]; then
git mv mfbt/nsRefPtr.h mfbt/RefPtr.h
else
hg mv mfbt/nsRefPtr.h mfbt/RefPtr.h
fi
--HG--
rename : mfbt/nsRefPtr.h => mfbt/RefPtr.h
This patch renames AnimationCollection::mNeedsRefreshes to indicate that it
no longer has any relationship to whether or not we observe the refresh driver.
We need to do this so effects can query their owning animation for the current
time and avoid falling out of sync. Furthermore, this pointer is needed
for a number of other bugs (e.g. bug 1166500 comment 12, or bug 1190235)
anyway.
In order to sort between events that have the same timestamp we use the
sort order of the corresponding animations so we need to store a pointer
to the animation along with the event.
--HG--
extra : rebase_source : 2767157135abd5a094d856410cd9c70e46a33b68
The elapsedTime member reported on AnimationEvents measures the time from
the *end* of the delay phase (i.e. the beginning of the active interval) to
when the event occurred. However, the AnimationTimeToTimeStamp method
introduced in the previous patch expects a time relative to the animation's
start time (i.e. the *start* of the delay phase). This patch adds a method
that performs the necessary conversion from an elapsedTime to an animation
time before calling AnimationTimeToTimeStamp. It also provides extra handling
for cases such as when the animation's start time has not yet been resolved or
when animation effect has disappeared.
--HG--
extra : rebase_source : 594f3f12bab3febe592d09b57be72b2ec09b47cf
This patch lines up the parameters of AnimationEventInfo and
TransitionEventInfo constructors so that they are more logical and consistent.
Specifically, it groups the element and pseudo type together since they
form a logical pair denoting the event target. For AnimationEventInfo this
patch also places the type of event before the common event parameters since
the event type seems to be more significant.
This patch also performs some miscelleaneous housekeeping: removing some
unnecessary namespace prefixes, whitespace fixes, and making
TransitionEventInfo use the same concrete type to store the target element
as AnimationEventInfo (dom::Element instead of nsIContent).
--HG--
extra : rebase_source : ce6935f74f31dffadce4d0e7d4fa8859ec213740
Currently we define a helper method, InitialAdvance, on KeyframeEffectReadOnly.
However, this method is only used for filling out the elapsedTime member of
AnimationEvents (which are generated by CSS animations). This patch moves this
method to CSSAnimation since it is unneeded for other types of Animations.
--HG--
extra : rebase_source : 9ab3b81a8272c004aabf26fea557c9a2f5d76caf
The Web Animations specification has replaced the term "sequence number" with
references to a global animation list. This patch applies similar naming
to our animation structures.
As well as ensuring that we don't create animations for elements that are not
part of the document tree, this test also adjusts the assertion that checks
this in the following ways:
* Calls GetComposedDoc() instead of GetCrossShadowCurrentDoc() since the
latter is deprecated.
* Moves it from RequestRestyle to FlushAnimations since, depending on how we
refactor this code in the future, it's possible we might end up calling
RequestRestyle even for animations on elements that have been removed from the
document but we shouldn't call FlushAnimations in this case.
We currently determine if we need refresh driver ticks when composing style
but sometimes we might not need ticks for composing style but we might need
one more tick in order to queue a final end event. Currently, this doesn't
seem to be a problem because FlushAnimations calls Animation::Tick where we
queue up events. When we remove the call to Animation::Tick from
FlushAnimations in order to make FlushAnimations purely responsible for
posting restyles, however, we will create a situation where we might mark an
animation collection as no longer needing refreshes and not simultaneously
queueing the corresponding event. If another animation collection is deleted in
the meantime we may trigger the code that causes us to disassociate from the
refresh driver and the corresponding event will never be dispatched.
Long-term (bug 1195180) we will check if it we can stop observing the refresh
driver and queue events in the same step. Until then, this patch adds a method
to detect this particular situation and uses it to avoid unregistering from
the refresh driver while we still have end events to queue.
In bug 1195180 we plan to tick animations from their timeline where they
are stored in a hashtable. As a result, we will not visit them in order of
their associated target element (indeed, part of the reason we are doing
this is to support animations that do not have, or even have multiple target
elements).
The current animation mutation observer batching mechanism, however, assumes
that we visit each target element in turn and make all the necessary work at
once. In order to support visiting animations in a potentially random order
this patch reworks the animation mutation observer batching mechanism so that
it can support batching multiple elements at once.
Now KeyframeEffect.SetTiming() updates the owning animation timing and relavance, so
we don't need to call each methods respectively for the animation any more.
When updating the cascade results between transitions and animations, if we
detect a change we force an update by taking the following steps:
a. Updating the animation generation on the restyle manager
b. Updating the animation generation on the collection
c. Iterating over all the properties animated by the collection and, for
each property that we can animate on the compositor, posting a restyle
event with the appropriate change hint (nsChangeHint_UpdateTransformLayer
or nsChangeHint_UpdateTransformOpacity)
d. Marking the collection as needing refreshes
e. Clearing the style rule refresh time so we generate a new style rule in
EnsureStyleRuleFor
As it turns out, the newly-added
AnimationCollection::RequestRestyle(RestyleType::Layer) already performs a, b,
d, and e. It also:
* Ensures we are observing the refresh driver if need be (should have no effect
in this case)
* Clears the last animation style update time on the pres context so that
subsequent calls to FlushPendingNotifications will update animation style
(it seems like we probably should have been doing this for changes to cascade
results anyway)
* Posts a restyle event with restyle hint eRestyle_CSSTransitions or
eRestyle_CSSAnimations
* Marks the document as needing a style flush (irrelevant since posting
a restyle event does this anyway)
The only missing piece that would prevent using RequestRestyle in place of this
code when updating cascade results is (c) from the list above. However, (c)
should not be necessary since ElementRestyler::AddLayerChangesForAnimation()
explicitly checks for out-of-date layer animation generation numbers and adds
the appropriate change hints (nsChangeHint_UpdateTransformLayer etc.) to the
change list.
EnsureStyleRuleFor contains logic for performing throttled updates to the style
rule but it is only used in one case: inside
nsTransitionManager::UpdateCascadeResults to determine what properties are
being animated by CSS animations.
We would like to remove throttling logic from EnsureStyleRuleFor altogether but
if that one case where it is currently used is run on every tick then removing
this logic could effectively mean we end up updating the style rule on every
tick. Fortunately nsTransitionManager::UpdateCascadeResults is only called
in the following cases:
1. From nsTransitionManager::StyleContextChanged (via
TransitionManager::UpdateCascadeResultsWithTransitions), when we are
processing style changes for transitions.
2. From AnimationCollection::EnsureStyleRuleFor (via
nsAnimationManager::MaybeUpdateCascadeResults and
nsTransitionManager::UpdateCascadeResultsWithAnimations), when we are
updating the animation style rule from CSS animations.
3. From nsAnimationManager::CheckAnimationRule (via
TransitionManager::UpdateCascadeResultsWithAnimationsToBeDestroyed), when
we are processing style changes for CSS animations.
None of these things should be happenning on a regular throttle-able tick so by
removing this logic we shouldn't be causing any additional work.
I have verified, using a test case that combines transitions and animations on
the same property, that we have the same behavior with regard to calling
EnsureStyleRuleFor both before and after this patch (specifically we avoid
calling it altogether while running only the transition but when the animation
starts and clobbers the transition we end up calling EnsureStyleRuleFor once on
each tick).
We want to move the newly-introduced RequestRestyle call from FlushAnimations
to Animation::Tick. However, nsAnimationManager::CheckAnimationRule calls
Animation::Tick so this would cause us to start posting animation restyles
within a restyle.
Typically, Animations have an effect (currently there is only one type of
effect: KeyframeEffectReadOnly) and when there is any change in timing they
pass it down to their effect. However, the Animation is dependent on the
duration of the effect for determining if it is "finished" or not. As a result,
when an effect's timing changes, the owning Animation needs to know.
(The way this *should* work is that effects should tell their animation or
trigger some chain of events that causes animation's to update themselves.
However, the current implementation of effects is fairly primitive and does
not do this or even have a reference to the owning Animation. When we
implement the script API for updating the timing properties of effects we will
have to fix this but for now it is up to code in layout/style to update the
Animation when it touches the corresponding effect's timing.)
nsAnimationManager::CheckAnimationRule currently does this by calling
Animation::Tick() which ensures the Animation's finished state is updated
accordingly.
Ultimately we want to ensure that Animation::Tick is called exactly once per
frame (and at the appropriate point in that frame) so we'd like to remove this
call from CheckAnimationRule.
This patch achieves that by:
* Making Animation::SetEffect update the animation's timing - this is necessary
for animations that are created by CheckAnimationRule and will be
necessary when once we make Animation.effect writeable from script anyway.
* Calling Animation::SetEffect even for the case when we are updating the
existing effect.
Another side-effect of calling Animation::Tick within
nsAnimationManager::CheckAnimationRule is that CSSAnimation::Tick queues
events. There are some tests (e.g. layout/style/test/test_animations.html) that
assume that animationstart events are dispatched immediately when new
animations are created. That will change with bug 1134163 but for now we
should maintain this existing behavior since changing this might introduce
compatibility issues that are best dealt with as a separate bug rather than
blocking this refactoring. To that end, this patch also explicitly queues
animationstart events for newly-created animations.
nsTransitionManager::WillRefresh and nsAnimationManager::WillRefresh are now
identical and all methods they call exist on CommonAnimationManager so we
can unify them there.
The implementations of FlushAnimations and FlushTransitions should now be all
but equivalent so this patch combines them into a single implementation on
CommonAnimationManager.
Regarding some of the minor differences between the two methods:
* The combined implementation drops the check for an empty list of collections
found only in FlushTransitions. This seems like a very minor optimization
that could possibly cause us to fail to unregister from the refresh driver
if we forgot to do so when removing the last collection.
* The combined implementation uses the loop implementation from FlushAnimations
since it is more compact.
This patch also removes the extra nested scope since it doesn't seem necessary.
This patch moves the additional checks (beyond those of Animation::CanThrottle)
from FlushAnimations/FlushTransitions to AnimationCollection::RequestRestyle.
These checks are on a per-collection basis hence it makes sense for the
collection to perform them. This also moves logic out of the managers which is
needed if we want to support script-based animations without introducing another
manager.
Ultimately we want to move throttling logic to AnimationCollection and
Animation::Tick (and later to KeyframeEffect::SetParentTime). This is so that
we can support script-generated animations without having to introduce yet
another manager.
To that end this patch introduces a method on AnimationCollection that can be
called from Animation::Tick to perform the necessary notifications needed to
update style.
Later in this patch series we will extend RequestRestyle to incorporate more of
the throttling logic and further extend it to cover some of the other
notifications such as updating layers.
This patch tracks whether or not we have already posted a restyle for animation
to avoid making redundant calls. Calls to nsIDocument::SetNeedStyleFlush are
cheap and more difficult to detect when they have completed so we don't filter
redundant calls in the Restyle::Throttled case.
If mHasPendingAnimationRestyle is set and AnimationCommon::EnsureStyleRuleFor
is *never* called then we could arrive at situation where we fail to make post
further restyles for animation.
I have verified that if we fail to reset mHasPendingAnimationRestyle at the
appropriate point (e.g. resetting it at the end of EnsureStyleRuleFor *after*
the early-returns) then a number of existing tests fail.
Furthermore, I have observed that it is reset by the beginning of each tick
in almost every case except for a few instances of browser mochitests such as
browser/components/customizableui/test/browser_1007336_lwthemes_in_customize_mode.js.
In this case, during the async cleanup of the test, we have an opacity
transition on a vbox element that becomes display:none and appears to be skipped
during restyling. However, even in this case, EnsureStyleRuleFor is called
within one or at most two ticks and mHasPendingAnimationRestyle flag is cleared
(i.e. it does not get stuck).
Brian Birtles
Hiroyuki Ikezoe
L. David Baron
Boris Chiou
Nathan Froyd
Masayuki Nakano