There are many bugs regarding our use of EffectSet::GetEffectSet(nsIFrame*)
because the intention of the caller is not clear. If it is called for the
primary frame of display:table content do we expect it to get the EffectSet
associated with the style frame or not? Generally it depends on if we are
looking for transform animations or not.
Rather than inspecting each call site and making it choose the appropriate frame
to use, this patch introduces a new method to EffectSet to get the appropriate
EffectSet based on the properties the caller is interested in.
This patch also uses this function in FindAnimationsForCompositor which in turns
fixes the glitching observed on Tumblr that arose since a number of places in
our display list code were passing the style frame to
EffectCompositor::HasAnimationsForCompositor.
Over the remainder of this patch series we will convert more callers of
EffectSet::GetEffectSet(nsIFrame*) to this new method before renaming
EffectSet::GetEffectSet to GetEffectSetForStyleFrame to make clear how the
method is intended to work.
Differential Revision: https://phabricator.services.mozilla.com/D23282
--HG--
extra : moz-landing-system : lando
This is more consistent with what the Rust bits of the style system do, and
removes a pointer from ComputedStyle which is always nice.
This also aligns the Rust bits with the C++ bits re. not treating xul pseudos as
anonymous boxes. See the comment in nsTreeStyleCache.cpp regarding those.
Can't wait for XUL trees to die.
Depends on D19001
Differential Revision: https://phabricator.services.mozilla.com/D19002
--HG--
extra : moz-landing-system : lando
As for removing an entry, EnsureRemoved is equal to what Contains && RemoveEntry
do, but for consistency we use EnsureRemoved here.
MozReview-Commit-ID: 9qE3YtvmwC8
--HG--
extra : rebase_source : 1681194cd8b9700d46a07a502f7d2f15580918aa
This patch is an automatic replacement of s/NS_NOTREACHED/MOZ_ASSERT_UNREACHABLE/. Reindenting long lines and whitespace fixups follow in patch 6b.
MozReview-Commit-ID: 5UQVHElSpCr
--HG--
extra : rebase_source : 4c1b2fc32b269342f07639266b64941e2270e9c4
extra : source : 907543f6eae716f23a6de52b1ffb1c82908d158a
(Path is actually r=froydnj.)
Bug 1400459 devirtualized nsIAtom so that it is no longer a subclass of
nsISupports. This means that nsAtom is now a better name for it than nsIAtom.
MozReview-Commit-ID: 91U22X2NydP
--HG--
rename : xpcom/ds/nsIAtom.h => xpcom/ds/nsAtom.h
extra : rebase_source : ac3e904a21b8b48e74534fff964f1623ee937c67
We can early return to call MayHaveAnimations() (and it should be fast) since
we set the flag in EffectSet::GetOrCreateEffectSet(),
MozReview-Commit-ID: 2UfWVVi6nY5
--HG--
extra : rebase_source : 97e98bf1fb9e725a107ed3200b95921375bfb637
We need to call GetEffectSet() from Servo code, which passes an
immutable reference to GeckoElement, so it's better to make dom::Element
const.
MozReview-Commit-ID: GqQEB7BwkJA
--HG--
extra : rebase_source : 4b025f56949d8327781be26d1607e6d9f4668f2f
Before this patch, we could't use EffectSet::GetEffectSet(nsIFrame*) until
the target content associated with the nsIFrame has a primary frame since
nsLayoutUtils::GetStyleFrame(nsIContent*) needs the primary frame.
In this patch, StyleContext()->GetPseudoType() is used for obtaining
CSSPseudoElementType instread of content->NodeInfo()->NameAtom().
As a result, we don't need to care about whether the content has a
primary frame or not.
This means that we won't associate animations with additional frames.
In this case, this fixes associating off-main-thread animations with a
table outer frame, when they should have been associated only with the
table frame.
Locally, the test fails without the patch (with opacity in the test
being 0.36 instead of the expected 0.6), and passes with the patch.
(Opacity 0.36 gives a color of rgb(163,163,255), whereas 0.6 gives
rgb(102,102,255).)
--HG--
extra : commitid : 7wtkIDLDHBF
There are three situations when the cascade results of effects needs to be
updated.
1. The sets of effects (animations) has changed.
2. One or more effects have changed their "in effect" status.
3. Other style properties affecting the element have changing meaning that
animations applied at the animations-level of the cascade may now be
overridden or become active again.
We want to detect these situations so we can avoid updating the cascade when
none of these possibilities exist.
Currently we handle case 1 by calling UpdateCascadeResults at the appropriate
point in nsAnimationManager and nsTransitionManager when we build
animations/transtiions.
Case 2 only affects animations (since whether transitions are in effect or not
makes no difference to the cascade--they have a lower "composite order" than
animations and never overlap with each other so they can't override anything).
As a result, we handle it by adding a flag to CSSAnimation to track when an
animation was in effect last time we checked or not.
For case 3, we take care to call UpdateCascadeResults when the style context
changed in nsAnimationManager::CheckAnimationRule (called from
nsStyleSet::GetContext).
We want to generalize this detection to handle script-generated animations too.
In order to do that this patch introduces a flag to EffectSet that we will use
to mark when the cascade needs to be updated in cases 1 and 2. This patch also
sets the flag when we detect case 1. A subsequent patch sets the flag for
case 2.
Case 3 is more difficult to detect and so we simply maintain the existing
behavior of making nsAnimationManager::CheckAnimationRule unconditionally
update the cascade without checking if the "needs update" flag is set.
--HG--
extra : rebase_source : fc56b1bb5a98ae78b93a179c7a3b8af4724a06a1
This is so that when we have code like:
elem.animate({ opacity: 0 }, 1000)
the resulting Animation object is kept alive by |elem| based on the following
ownership chain:
elem --(strong)--> KeyframeEffectReadOnly --(strong)--> Animation
Now, there is an ownership cycle introduced here because KeyframeEffectReadOnly
objects also store owning references to their target elements. This is broken
when the Animation finishes (if it does not fill forwards) or is cancelled
since either event will trigger a call to
KeyframeEffectReadOnly::UpdateTargetRegistration.
If the Animation fills forwards, the resource will not be released until
it is cancelled. For Animations corresponding to CSS Animations / CSS
Transitions this happens when the Element is unbound or when the corresponding
style property is updated causing the animation to be replaced or removed.
For the general case of script-generated animations, however, this cycle won't
be broken until the Element is unbound and all external references to the
Animation or KeyframeEffectReadOnly are dropped.
It's unfortunate that we can't more aggressively prune these objects but it's
what the spec currently says. I've posted to the mailing list[1] about this but
have yet to find a good solution.
[1] https://lists.w3.org/Archives/Public/public-fx/2015OctDec/0029.html