None of the consumer need to mutate styles, and this saves some ugly
const_casting on the next patch.
Doesn't change behavior.
Differential Revision: https://phabricator.services.mozilla.com/D147555
The test-case in the bug does something interesting, where it causes a
transition on the parent by removing a CSS rule, and that causes us to
transition text-underline-offset on our ::marker, via the magic of
font-size-relative properties.
text-underline-offset, while it gets inherited from ::marker, is not a
valid CSS property to specify on marker per spec, so we trim it here:
https://searchfox.org/mozilla-central/rev/899bbd9e5a0d6de9bb9f068c48b1445c7905d9cf/servo/ports/geckolib/glue.rs#5709-5712
And that causes us to create a transition with an empty effect and
everything goes downhill from here.
For now, just bail out in a nicer way than we were doing. I still need
to look into whether we should handle inherited transitions differently
from non-inherited one in this case...
I think our behavior after this patch would be correct for the test-case
(because text-underline-offset would transition on the parent and
::marker would inherit it). If you specify transition only on the marker
we'd refuse to transition (which I guess it is somewhat of a sensible
behavior).
Differential Revision: https://phabricator.services.mozilla.com/D105124
The machinery to report janked animations is;
1) Store the partial pre-rendered animation id and the Animation object in a
hashtable in LayerManager
2) Store the animation id in the Animation object as well
3) When we detect jank, we send the animation id to the main-thread via an IPC
call
4) Find the Animation object with the id in the hashtable and update the
Animaiton
5) Whenever the partial pre-rendered Animation stop running on the compositor
i.e. the Animation finished normally, the Animation's target element is
changed, etc. etc., remove the Animation from the hashtable
Depends on D75731
Differential Revision: https://phabricator.services.mozilla.com/D75732
The machinery to report janked animations is;
1) Store the partial pre-rendered animation id and the Animation object in a
hashtable in LayerManager
2) Store the animation id in the Animation object as well
3) When we detect jank, we send the animation id to the main-thread via an IPC
call
4) Find the Animation object with the id in the hashtable and update the
Animaiton
5) Whenever the partial pre-rendered Animation stop running on the compositor
i.e. the Animation finished normally, the Animation's target element is
changed, etc. etc., remove the Animation from the hashtable
Differential Revision: https://phabricator.services.mozilla.com/D75732
We instantiate this class in various binding methods. Future patches will make
use of it to throw errors.
Differential Revision: https://phabricator.services.mozilla.com/D64883
--HG--
extra : moz-landing-system : lando
We instantiate this class in various binding methods. Future patches will make
use of it to throw errors.
Differential Revision: https://phabricator.services.mozilla.com/D64883
--HG--
extra : moz-landing-system : lando
This is needed so that later we can make the effect of transitions replaceable.
Note that the test added in this patch will fail without the code changes in
this patch.
Differential Revision: https://phabricator.services.mozilla.com/D64519
--HG--
rename : layout/style/test/test_transitions_replacement_on_busy_frame.html => layout/style/test/test_transitions_replacement_with_setKeyframes.html
extra : moz-landing-system : lando
This is needed so that later we can make the effect of transitions replaceable.
Note that the test added in this patch will fail without the code changes in
this patch.
Differential Revision: https://phabricator.services.mozilla.com/D64519
--HG--
rename : layout/style/test/test_transitions_replacement_on_busy_frame.html => layout/style/test/test_transitions_replacement_with_setKeyframes.html
extra : moz-landing-system : lando
I hope you can help me figure out writing tests for this and figuring whether
the reasoning here is correct / I'm missing something else :)
Differential Revision: https://phabricator.services.mozilla.com/D64442
--HG--
extra : moz-landing-system : lando
Replace ElementOrCSSPseudoElement with Element and add PseudoElement (which is
a DOMString) into KeyframeAnimationOptions and KeyframeEffect.
Differential Revision: https://phabricator.services.mozilla.com/D62667
--HG--
extra : moz-landing-system : lando
In order to store the different combinations of (Element, PsuedoStyleType)
pairs, including (nullptr, ::before/::after), we drop Maybe<> and use
OwningAnimationTarget directly.
Differential Revision: https://phabricator.services.mozilla.com/D62666
--HG--
extra : moz-landing-system : lando
Initially this was going to be a simple cleanup: Remove some useless namespaces
here and there and so on, remove `using` statements from the header and so on.
But unfortunately, DOMIntersectionObserver.h (which is included in Element.h,
unnecessarily) ended up exposing `Element` unnamespaced to a lot of code, so I
had to fix that.
Differential Revision: https://phabricator.services.mozilla.com/D55316
--HG--
extra : moz-landing-system : lando
We move the check of important rule and animation level into
KeyframeEffect::ShouldBlockAsyncTransformAnimations(), and add a new warning
for it.
Note:
1. ShouldBlockAsyncTransformAnimations() only cares about transforms. And
for other compositor animation properties, we count on
HasEffectiveAnimationOfPropertySet() (in IsMatchForCompositor()).
2. If we check the important rules in both
EffectCompositor::HasAnimationsForCompositor() and
ActiveLayerTracker::IsTransformMaybeAnimated(), we may get the incorrect
animation warnings (i.e. TransformFrameInactive). In most cases, we
check these two functions together, so perhaps move the check of important
rules outside HasEffectiveAnimationOfPropertySet() is fine.
Besides, ActiveLayerTracker just tracks if there is a style change on this
property (or display item) on the active layers, so should be OK to not
check important rules in it.
So IsMatchForCompositor() should check all transform-like properties,
instead of each one, to get the correct result. (That's why we have to
refactor KeyframeEffect::GetPropertiesForCompositor() as well.)
Differential Revision: https://phabricator.services.mozilla.com/D34432
--HG--
extra : moz-landing-system : lando
Currently we avoid posting animation restyles when unbinding an element by
removing the element from the document before deleting its animation
collections. As a result, when canceled animations go to post a restyle, they
can't find a pres context and give up posting a restyle.
However, this is problematic for two reasons:
* It means we can't remove such canceled animations from the
PendingAnimationTracker if they are present there (i.e. it regresses the fix
from bug 1223445).
* It means we can't post cancel events for such animations/transitions since we
can't lookup the appropriate AnimationEventDispatcher.
In the next patch in this series we will change that order to fix the above
problems but before we do that, we need to introduce another mechanism to make
sure that we don't post restyles when unbinding an element or else we will
regress bug 1396041.
This patch does that by introducing a flag which causes us to not post restyles
when we are doing DOM surgery. For all other cases we actually _do_ need to post
restyles in order to update the style correctly.
Without this patch, layout/style/crashtests/1396041.html would fail after
applying the next patch in this series.
Differential Revision: https://phabricator.services.mozilla.com/D28021
--HG--
extra : moz-landing-system : lando
This makes `Document::GetShell()` return `PresShell*` instead of `nsIPresShell`.
Additonally, "shell" is unclear ("docshell" vs. "presshell"). Therefore, this
also renames `Document::GetShell()` to `Document::GetPresShell()`.
Similarly, some other method names of `Document` are also renamed from
`*Shell*` to `*PresShell*`.
Differential Revision: https://phabricator.services.mozilla.com/D25338
--HG--
extra : moz-landing-system : lando
This makes `Document::GetShell()` return `PresShell*` instead of `nsIPresShell`.
Additonally, "shell" is unclear ("docshell" vs. "presshell"). Therefore, this
also renames `Document::GetShell()` to `Document::GetPresShell()`.
Similarly, some other method names of `Document` are also renamed from
`*Shell*` to `*PresShell*`.
Differential Revision: https://phabricator.services.mozilla.com/D25338
--HG--
extra : moz-landing-system : lando
We want to set the performance warning by a property set, so update it.
Besides, add more tests for individual transforms (translate, rotate,
scale).
Differential Revision: https://phabricator.services.mozilla.com/D19633
--HG--
extra : moz-landing-system : lando
It took me a long time to understand why
KeyframeEffect::HasEffectiveAnimationOfPropertySet behaved so differently to
KeyframeEffect::HasAnimationOfPropertySet. This patch attempts to clarify that
while making KeyframeEffect::HasEffectiveAnimationOnPropertySet a little more
generally useful. This will allow us to tidy up the various animation checks in
nsLayoutUtils later in this patch series.
Ultimately, however, we should make this check part of the regular compositor
animation vetting machinery in bug 1534884. That should remove a number of
inconsistencies such that we don't need the extended comments added in this
patch.
Differential Revision: https://phabricator.services.mozilla.com/D23281
--HG--
extra : moz-landing-system : lando
Since bug 1524480 we set the NS_FRAME_MAY_BE_TRANSFORMED frame bit when needed
in RestyleManager::ProcessRestyledFrames so that it is now redundant to also set
it from KeyframeEffect.
Furthermore, setting frame bits from KeyframeEffect is a little fragile since it
depends on the life cycle of the KeyframeEffect which is independent of the
nsFrame. If we can avoid doing that, we probably should.
Differential Revision: https://phabricator.services.mozilla.com/D21885
--HG--
extra : moz-landing-system : lando
We use DisplayItemType as the input of HasAnimationsForCompositor, and
nsCSSPropertyIDSet as the input of GetAnimationsForCompositor.
The caller of HasAnimationsForCompositor just wants to check if there is
any compositor animation for a display item, so we can replace it by the
display item, and get the properties from this display item.
However, the caller of GetAnimationsForCompositor may use a subset of
transform-like properties for getting scale factors, or use all the
transform-like properties for sending all transform animations to the
compositor thread.
Depends on D19630
Differential Revision: https://phabricator.services.mozilla.com/D19628
--HG--
extra : moz-landing-system : lando
FrameLayerBuilder needs to clear this flag by DisplayItemType, so we add
a new function for it.
Differential Revision: https://phabricator.services.mozilla.com/D19630
--HG--
extra : moz-landing-system : lando
Let ActiveLayerTracker track individual transforms. (Will add
motion-path in the future.)
Besides, using a property set for transform and opacity is more efficient,
so let's change it. For background position, we use a different code path,
so we can have more restrictions in IsStyleAnimated.
Differential Revision: https://phabricator.services.mozilla.com/D19631
--HG--
extra : moz-landing-system : lando
nsIFrame::BuildDisplayListForStackingContext() will check the existence
of transform animations, so we need to update
nsLayoutUtils::HasAnimationsOfPoperty(). However, checking only
eCSSProperty_transform is not enough. We have to check all the transform-like
properties. Therefore, we update these functions to accept a property
set as the argument, and pass a collection of transform-like properties
into them.
Differential Revision: https://phabricator.services.mozilla.com/D20412
--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
A forthcoming spec change will require that Animatable.animate() and other
methods that mutate animations do not flush style:
https://github.com/w3c/csswg-drafts/issues/3613
Bug 1525809 will add web-platform-tests for this change once it is made (and
tweak the behavior introduced in this patch if necessary).
Currently Firefox and WebKit will flush styles when calling
Animatable.animate(). This is undesirable since this method will _also_
invalidate style. As a result, if content triggers multiple animations in
a single animation frame, it will restyle every time it creates an animation.
This patch removes the style flush from a number of these methods.
In general the style flush is not necessary. For example, we don't need to flush
style before getting the computed style to pass to UpdateProperties. That's
because if there are pending style changes, then UpdateProperties will be called
with the updated style once they are applied anyway. Flushing style first means
that we may end up resolving style twice, when once would be sufficient.
For GetKeyframes() however, when used on a CSS animation, it should return
the most up-to-date style so for that call site we *do* want to flush style.
The test case added in this patch will fail without the code changes in the
patch. Specifically, we will observe 10 non-animation restyles (from the
5 animations) if we flush styles from SetKeyframes.
Differential Revision: https://phabricator.services.mozilla.com/D18916
--HG--
extra : moz-landing-system : lando
Without this fix, various tests in
dom/animation/tests/mozilla/test_restyles.html will fail once we remove the
style flush from KeyframeEffect::SetKeyframes. That is because we will fail to
set mCumulativeChangeHint correctly when we initially set up the keyframe's
properties and then never update it since UpdateProperties will return early
when it determines the properties have not changed.
Differential Revision: https://phabricator.services.mozilla.com/D18914
--HG--
extra : moz-landing-system : lando
Summary: Really sorry for the size of the patch. It's mostly automatic
s/nsIDocument/Document/ but I had to fix up in a bunch of places manually to
add the right namespacing and such.
Overall it's not a very interesting patch I think.
nsDocument.cpp turns into Document.cpp, nsIDocument.h into Document.h and
nsIDocumentInlines.h into DocumentInlines.h.
I also changed a bunch of nsCOMPtr usage to RefPtr, but not all of it.
While fixing up some of the bits I also removed some unneeded OwnerDoc() null
checks and such, but I didn't do anything riskier than that.
Now we no longer update the corresponding display items for the animations that
are prevented from running on the compositor if the animations themselves don't
generate any change hints, e.g the same value is specified in both 'from' and
'to' keyframes. So that we can enable the reftests that we had been suffering
from continuous MozAfterPaint events.
Depends on D12397
Differential Revision: https://phabricator.services.mozilla.com/D12369
--HG--
extra : moz-landing-system : lando
So that we can use for KeyframeEffect::GetPropertiesForCompositor().
Depends on D12396
Differential Revision: https://phabricator.services.mozilla.com/D12397
--HG--
extra : moz-landing-system : lando
Emilio Cobos Álvarez
Simon Giesecke
Hiroyuki Ikezoe
Brindusan Cristian
Emily McDonough
Boris Zbarsky
Arthur Iakab
Brian Birtles
Oana Pop Rus
Boris Chiou
Barret Rennie
Masayuki Nakano
Dorel Luca
Sylvestre Ledru
Hiroyuki Ikezoe