775211 - Custom menu is a tad slower to expand on initial invoke in Jellybean (Android 4.1)

Status: VERIFIED FIXED

Description

[Aaron Train [:aaronmt]]

Load up a new profile of Fennec on Jellybean 4.1.x and open the custom menu - in comparison to ICS it 'inflates' a tad slower the first time. 

Tested via:

--
Nightly (07/18)
Galaxy Nexus (Android 4.1.1)

Comment 1

[Sriram Ramasubramanian [:sriram]]

Created attachment 645548 [details] [diff] [review]
Patch

The problem was depending on child's height -- which is 0 until it is shown for the first time -- which caused the problem.
Hence the ScrollView measured a 0, and then the actual height -- showing the flickering.
Instead, this does an AT_MOST call on preferred height of 75% of screen height. This fixes the problem.

Comment 3

[Alex Keybl [:akeybl]]

Moving the tracking flags over from bug 767668.

Comment 4

[Matt Brubeck (:mbrubeck)]

Comment on attachment 645548 [details] [diff] [review]
Patch

Nice!

>         protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
>+            super.onMeasure(widthMeasureSpec, heightMeasureSpec);
> 
>             DisplayMetrics metrics = new DisplayMetrics();
>             ((Activity) GeckoApp.mAppContext).getWindowManager().getDefaultDisplay().getMetrics(metrics);
> 
>             // heightPixels changes during rotation.
>+            int restrictedHeightSpec = MeasureSpec.makeMeasureSpec((int) (0.75 * metrics.heightPixels), MeasureSpec.AT_MOST);
> 
>             super.onMeasure(widthMeasureSpec, restrictedHeightSpec);
>         }

We're calling super.onMeasure twice now.  Is the first call still useful, or should it be removed?

Comment 5

[Sriram Ramasubramanian [:sriram]]

The first call is still useful. I still removing it and it failed.
So (from my understanding), the first call makes the size to be 100% of its children (which may fill the entire screen). The second call restricts the height to 75%.
We do super.onMeasure() which takes care of setMeasuredDimension().

Comment 6

[Sriram Ramasubramanian [:sriram]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/9f8f2b6e2f8b

Comment 7

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 645548 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): -
User impact if declined: The menu will take time to show in JB. HTC will have visual glitches.
Testing completed (on m-c, etc.): Landed in m-i on 07/24
Risk to taking this patch (and alternatives if risky): None.
String or UUID changes made by this patch: None.

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/9f8f2b6e2f8b

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 645548 [details] [diff] [review]
Patch

no risk, mobile only - approving.

Comment 10

[Sriram Ramasubramanian [:sriram]]

Pushed to aurora:
https://hg.mozilla.org/releases/mozilla-aurora/rev/540cd72d8445

Comment 11

[Sriram Ramasubramanian [:sriram]]

Pushed to beta:
https://hg.mozilla.org/releases/mozilla-beta/rev/686479fc0e26

Comment 12

[Daniel Asarnow]

I am still seeing bug 767668 (duplicate of this bug) in Beta. Did the beta update of 7/27 include Sriram's fixes?

Comment 13

[Matt Brubeck (:mbrubeck)]

(In reply to Daniel Asarnow from comment #12)
> I am still seeing bug 767668 (duplicate of this bug) in Beta. Did the beta
> update of 7/27 include Sriram's fixes?

No, it didn't.  This fix will be included in the next beta update (probably later this week).