1412716 - Drop KeyframeEffectReadOnly::mIsComposingStyle flag

Status: RESOLVED FIXED

(Core :: DOM: Animation, enhancement, P3)

Description

[Hiroyuki Ikezoe (:hiro)]

Once we remove old gecko style system in bug 1395112, we don't need to worry about this recursive calls for ComposeStyle().

Comment 1

[Hiroyuki Ikezoe (:hiro)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=269fa7e3c071c9887c92f3ce3dc2915e0ef8053d

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1412716 - Rewrite a comment without using |aStyle|.

Review commit: https://reviewboard.mozilla.org/r/235062/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/235062/

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1412716 - De-templatize Animation::ComposeStyle and relevant stuff.

Review commit: https://reviewboard.mozilla.org/r/235064/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/235064/

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1412716 - Drop KeyframeEffectReadOnly::mIsComposingStyle.

The flag was introduced in bug 1322291 to prevent recursive calls of
KeyframeEffectReadOnly::ComposeStyle() on the old style system.  On the
new style system we never call the function recursively.

Review commit: https://reviewboard.mozilla.org/r/235066/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/235066/

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1412716 - De-templatize EffectCompositor::UpdateEffectProperties and relevant stuff.

Review commit: https://reviewboard.mozilla.org/r/235068/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/235068/

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1412716 - De-templatize functions used for in nsAnimationManager.

Review commit: https://reviewboard.mozilla.org/r/235070/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/235070/

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1412716 - De-templatize functions in nsTransitionManager.

Review commit: https://reviewboard.mozilla.org/r/235072/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/235072/

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1412716 - Drop redundant forward declarations of ComputedStyle.

Review commit: https://reviewboard.mozilla.org/r/235074/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/235074/

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1412716 - Drop forward declarations of GeckoComputedStyle.

Review commit: https://reviewboard.mozilla.org/r/235076/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/235076/

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8966372 [details]
Bug 1412716 - Rewrite a comment without using |aStyle|.

https://reviewboard.mozilla.org/r/235062/#review240764

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8966373 [details]
Bug 1412716 - De-templatize Animation::ComposeStyle and relevant stuff.

https://reviewboard.mozilla.org/r/235064/#review240766

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8966374 [details]
Bug 1412716 - Drop KeyframeEffectReadOnly::mIsComposingStyle.

https://reviewboard.mozilla.org/r/235066/#review240768

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8966375 [details]
Bug 1412716 - De-templatize EffectCompositor::UpdateEffectProperties and relevant stuff.

https://reviewboard.mozilla.org/r/235068/#review240770

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8966376 [details]
Bug 1412716 - De-templatize functions used for in nsAnimationManager.

https://reviewboard.mozilla.org/r/235070/#review240772

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8966377 [details]
Bug 1412716 - De-templatize functions in nsTransitionManager.

https://reviewboard.mozilla.org/r/235072/#review240774

::: layout/style/nsTransitionManager.h:353
(Diff revision 1)
> -                               const nsStyleDisplay& aStyleDisplay,
> +                                    const nsStyleDisplay& aStyleDisplay,
> -                               uint32_t transitionIdx,
> +                                    uint32_t transitionIdx,
> -                               mozilla::dom::Element* aElement,
> +                                    mozilla::dom::Element* aElement,
> -                               mozilla::CSSPseudoElementType aPseudoType,
> +                                    mozilla::CSSPseudoElementType aPseudoType,
> -                               CSSTransitionCollection*& aElementTransitions,
> +                                    CSSTransitionCollection*& aElementTransitions,
> -                               StyleType aOldStyle,
> +                                    const mozilla::ComputedStyle* aOldStyle,

If you want to use const references here and in relevant places, that'd be awesome :)

Comment 16

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8966378 [details]
Bug 1412716 - Drop redundant forward declarations of ComputedStyle.

https://reviewboard.mozilla.org/r/235074/#review240776

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8966379 [details]
Bug 1412716 - Drop forward declarations of GeckoComputedStyle.

https://reviewboard.mozilla.org/r/235076/#review240778

Comment 18

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8966377 [details]
Bug 1412716 - De-templatize functions in nsTransitionManager.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/235072/diff/1-2/

Comment 19

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8966378 [details]
Bug 1412716 - Drop redundant forward declarations of ComputedStyle.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/235074/diff/1-2/

Comment 20

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8966379 [details]
Bug 1412716 - Drop forward declarations of GeckoComputedStyle.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/235076/diff/1-2/

Comment 21

[Hiroyuki Ikezoe (:hiro)]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #15)
> Comment on attachment 8966377 [details]
> Bug 1412716 - De-templatize functions in nsTransitionManager.
> 
> https://reviewboard.mozilla.org/r/235072/#review240774
> 
> ::: layout/style/nsTransitionManager.h:353
> (Diff revision 1)
> > -                               const nsStyleDisplay& aStyleDisplay,
> > +                                    const nsStyleDisplay& aStyleDisplay,
> > -                               uint32_t transitionIdx,
> > +                                    uint32_t transitionIdx,
> > -                               mozilla::dom::Element* aElement,
> > +                                    mozilla::dom::Element* aElement,
> > -                               mozilla::CSSPseudoElementType aPseudoType,
> > +                                    mozilla::CSSPseudoElementType aPseudoType,
> > -                               CSSTransitionCollection*& aElementTransitions,
> > +                                    CSSTransitionCollection*& aElementTransitions,
> > -                               StyleType aOldStyle,
> > +                                    const mozilla::ComputedStyle* aOldStyle,
> 
> If you want to use const references here and in relevant places, that'd be
> awesome :)

Done. :)

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/10477098a165
Rewrite a comment without using |aStyle|. r=emilio
https://hg.mozilla.org/integration/autoland/rev/9a88cf9d4308
De-templatize Animation::ComposeStyle and relevant stuff. r=emilio
https://hg.mozilla.org/integration/autoland/rev/607f333a9cd7
Drop KeyframeEffectReadOnly::mIsComposingStyle. r=emilio
https://hg.mozilla.org/integration/autoland/rev/187de5006e65
De-templatize EffectCompositor::UpdateEffectProperties and relevant stuff. r=emilio
https://hg.mozilla.org/integration/autoland/rev/471f6c02fe7e
De-templatize functions used for in nsAnimationManager. r=emilio
https://hg.mozilla.org/integration/autoland/rev/75c4a4f95797
De-templatize functions in nsTransitionManager. r=emilio
https://hg.mozilla.org/integration/autoland/rev/3da37134c45a
Drop redundant forward declarations of ComputedStyle. r=emilio
https://hg.mozilla.org/integration/autoland/rev/426b3bc75674
Drop forward declarations of GeckoComputedStyle. r=emilio

Comment 23

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/10477098a165
https://hg.mozilla.org/mozilla-central/rev/9a88cf9d4308
https://hg.mozilla.org/mozilla-central/rev/607f333a9cd7
https://hg.mozilla.org/mozilla-central/rev/187de5006e65
https://hg.mozilla.org/mozilla-central/rev/471f6c02fe7e
https://hg.mozilla.org/mozilla-central/rev/75c4a4f95797
https://hg.mozilla.org/mozilla-central/rev/3da37134c45a
https://hg.mozilla.org/mozilla-central/rev/426b3bc75674