Animation::Cancel calls UpdateTiming() which in turns runs the procedure to
update the finished state. However, the spec[1] doesn't require that.
Furthermore, calling UpdateTiming here hides the fact that we end up triggering
a restyle.
It would be better to move the parts of UpdateTiming we require into Cancel
itself so that we align better with the spec and to make it a bit more clear
what side-effects of UpdateTiming we actually rely on.
[1] https://drafts.csswg.org/web-animations-1/#cancel-an-animation
Differential Revision: https://phabricator.services.mozilla.com/D28020
CancelNoUpdate actually can and does trigger restyles via its call to
KeyframeEffect::NotifyAnimationTimingUpdated so at very least its name is wrong.
Furthermore, we actually want canceling to trigger restyles in most cases since
when an animation is canceled we need to trigger a subsequent restyle to apply
the (no-longer-animated) result.
This wasn't necessary when CancelNoUpdate was first introduced but since then we
have introduced the Servo style engine where we use a separate traversal to
apply the result from creating/deleting/modifying animations.
This change will mean that we now trigger a "layer" restyle when canceling an
animation when we previously didn't. That, however, seems more correct if
anything.
This patch also makes CancelFromStyle no longer virtual since it doesn't seem
necessary anymore (perhaps because we now point to the concrete type:
CSSAnimation/CSSTransition from nsAnimationManager/nsTransitionManager whereas
previously we didn't).
Differential Revision: https://phabricator.services.mozilla.com/D28019
A lot of files include `nsIPresShell.h` even though currently they don't
need it. This patch removes the unnecessary inclusions.
Differential Revision: https://phabricator.services.mozilla.com/D25744
A lot of files include `nsIPresShell.h` even though currently they don't
need it. This patch removes the unnecessary inclusions.
Differential Revision: https://phabricator.services.mozilla.com/D25744
If `Document::GetShell()` returns `PresShell*` rather than `nsIPresShell`, it's
a good step to deCOMTaminate `PresShell`.
This patch makes `Document.h` stop including `nsIPresShell.h` since
`nsIPresShell.h` includes `Document.h` indirectly and that causes bustage
when we make `Document::GetShell()` return `PresShell*`.
Differential Revision: https://phabricator.services.mozilla.com/D25332
If `Document::GetShell()` returns `PresShell*` rather than `nsIPresShell`, it's
a good step to deCOMTaminate `PresShell`.
This patch makes `Document.h` stop including `nsIPresShell.h` since
`nsIPresShell.h` includes `Document.h` indirectly and that causes bustage
when we make `Document::GetShell()` return `PresShell*`.
Differential Revision: https://phabricator.services.mozilla.com/D25332
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
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
Bug 1517241 renamed nsIDocument to mozilla::dom::Document but unfortunately in
the process it messed up the ordering of includes which, according to the coding
style[1], should be alphabetically sorted.
Also, in TimingParams.cpp it didn't add the dom::* prefix so when the unified
build chunking changes, if the "using namespace mozilla::dom" declaration
disappears from the chunk, it will fail to build.
[1] https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#CC_practices
Depends on D15902
Differential Revision: https://phabricator.services.mozilla.com/D15903
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.
As discussed here:
https://github.com/w3c/csswg-drafts/issues/2691
We have a similar check in SetCurrentTime (with the exception that, according to
the spec, this behavior applies to either play OR pause pending, instead of just
pause-pending) so this patch tries to match the comment and format of that
check.
Differential Revision: https://phabricator.services.mozilla.com/D2410
When the pending animation having no target element sets a new effect having
a target element associated with a document, PendingAnimationTracker has to
start tracking the animation regardless of mPendingReadyTime.
MozReview-Commit-ID: DxmbXtLhjCT
Before this change, the test in this commit fails. The received events order
is;
1) cancel
2) transitioncancel
3) transitionstart
4) finish
MozReview-Commit-ID: 8liTFXime6e
Same approach as the other bug, mostly replacing automatically by removing
'using mozilla::Forward;' and then:
s/mozilla::Forward/std::forward/
s/Forward</std::forward</
The only file that required manual fixup was TestTreeTraversal.cpp, which had
a class called TestNodeForward with template parameters :)
MozReview-Commit-ID: A88qFG5AccP
Animation::FlushStyle() gets called only for CSS animations/transitions'
playState changes in JS or ready Promise for CSS animations. In either case
throttled animation state, which is, to be precise, transformed position or
opacity value on the compositor, doesn't affect those results.
The first test case for CSS animations and the first test case for CSS
transitions in this patch fail without this fix.
MozReview-Commit-ID: EVym4qputL4
This pref was introduced in case we encountered compatibility issues from
changing the return value of Animation.playState (bug 1412765). Now that the
change to Animation.playState has shipped to release channel without any known
problems we should drop this pref.
MozReview-Commit-ID: CwMWRRtIf6u
Now AnimationEventDispatcher ensures that the refresh driver's next tick
happens for cancel event, so we don't need to re-observe the timeline
(it happens in UpdateTiming) once after removing the animation from the
timeline.
MozReview-Commit-ID: 7ivclmYIkPa
Now AnimationEventDispatcher ensures that the refresh driver's next tick
happens for cancel event, so we don't need to re-observe the timeline
(it happens in UpdateTiming) once after removing the animation from the
timeline.
MozReview-Commit-ID: 7ivclmYIkPa
This reflects the change made to the Web Animations specification in:
9e2053f5531c3415f4cc
(I got it wrong the first time. The second commit fixes the first.)
And discussed in:
https://github.com/w3c/web-animations/issues/196
In summary, we are splitting the "pending" play state out into a separate
boolean member so that it is possible to distinguish between "play-pending" and
"pause-pending" and because most of the time when you check for
animation.playState === 'running' you also really want to include play-pending
animations.
MozReview-Commit-ID: IJSNoZTKW2I
