1098240 - Use hw layers in tab strip animations

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Lucas Rocha (:lucasr)]

Every little helps.

Comment 1

[Lucas Rocha (:lucasr)]

Attachment: Use hw layers in tab strip animations (r=mcomella)

Comment 2

[Lucas Rocha (:lucasr)]

Attachment: Remove obsolete comment (r=mcomella)

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8522153 [details] [diff] [review]
Use hw layers in tab strip animations (r=mcomella)

Review of attachment 8522153 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/tabs/TabStripView.java
@@ +323,5 @@
> +            setLayerType(View.LAYER_TYPE_HARDWARE);
> +        }
> +
> +        @Override
> +        public void onAnimationEnd(Animator animation) {

nit: You could add a comment here (or in onAnimationCancel) that this is also called when onAnimationCancel is, to save the time of future travellers who are unfamiliar with the framework from having to read the source (e.g. me).

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8522154 [details] [diff] [review]
Remove obsolete comment (r=mcomella)

Review of attachment 8522154 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/tabs/TabStripView.java
@@ -123,5 @@
>                  final int childCount = getChildCount();
>                  for (int i = removedPosition - firstPosition; i < childCount; i++) {
>                      final View child = getChildAt(i);
>  
> -                    // TODO: optimize with Valueresolver

Can't seem to find out what a Valueresolver is so why is this out-of-date? What would a Valueresolver do?

Comment 5

[Lucas Rocha (:lucasr)]

(In reply to Michael Comella (:mcomella) from comment #4)
> Comment on attachment 8522154 [details] [diff] [review]
> Remove obsolete comment (r=mcomella)
> 
> Review of attachment 8522154 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/tabs/TabStripView.java
> @@ -123,5 @@
> >                  final int childCount = getChildCount();
> >                  for (int i = removedPosition - firstPosition; i < childCount; i++) {
> >                      final View child = getChildAt(i);
> >  
> > -                    // TODO: optimize with Valueresolver
> 
> Can't seem to find out what a Valueresolver is so why is this out-of-date?
> What would a Valueresolver do?

I actually meant PropertyValuesHolder :-) It's only useful when you're animating more than on property in the same object, which is not necessary here.

Comment 6

[Lucas Rocha (:lucasr)]

(In reply to Michael Comella (:mcomella) from comment #3)
> Comment on attachment 8522153 [details] [diff] [review]
> Use hw layers in tab strip animations (r=mcomella)
> 
> Review of attachment 8522153 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/tabs/TabStripView.java
> @@ +323,5 @@
> > +            setLayerType(View.LAYER_TYPE_HARDWARE);
> > +        }
> > +
> > +        @Override
> > +        public void onAnimationEnd(Animator animation) {
> 
> nit: You could add a comment here (or in onAnimationCancel) that this is
> also called when onAnimationCancel is, to save the time of future travellers
> who are unfamiliar with the framework from having to read the source (e.g.
> me).

Ok, done :-)

Comment 7

[Lucas Rocha (:lucasr)]

https://hg.mozilla.org/integration/fx-team/rev/1a271fff9c6a
https://hg.mozilla.org/integration/fx-team/rev/e66f10fd8921

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1a271fff9c6a
https://hg.mozilla.org/mozilla-central/rev/e66f10fd8921