PreferencesView._delayedInit is not delayed enough

VERIFIED FIXED

Status

Firefox for Android Graveyard
General
VERIFIED FIXED
8 years ago
7 years ago

People

(Reporter: mbrubeck, Assigned: mbrubeck)

Tracking

({perf})

Details

(Whiteboard: [fennec-checkin-postb4])

Attachments

(1 attachment)

(Assignee)

Description

8 years ago
We wait for a "select" event on the preference panel to initialize some of the prefs UI.  However, during browser startup, a getBoundingClientRect call causes the panel's XBL constructors to run, and the <menulist> constructors cause "select" events that bubble up to the panel, causing the initialization to run early.

Because the prefs panel is initially selected, it doesn't actually fire any event when you first open the panel.  So we can't just filter the events; we'll need to add a new "panel open" event instead.
I assume the getBoundingClientRect call happens after we make the browser tools panels hidden=false (id="panel-container").

Looking at PreferencesView._delayedInit, we don't seem to be doing too much work in there. It could be worse. I am concerned that the XBL constructors are firing while the panel container is still supposed to be hidden. That would be much worse since the <setting>s are slow to init and do hurt Ts. Which is why we made them hidden before.

One simple idea could be to make the <richlistbox id="prefs-list"> hidden by default too. Then in PreferencesView._delayedInit, we could make it hidden=false

Hopefully, the hidden list would not get any XBL initialized.
(Assignee)

Comment 2

8 years ago
Created attachment 505946 [details] [diff] [review]
patch

I verified that the other panel controllers don't suffer from this bug (they're protected from it because their panels aren't selected during startup).

(In reply to comment #1)
> I assume the getBoundingClientRect call happens after we make the browser tools
> panels hidden=false (id="panel-container").

Looks like that's correct.  _delayedInit is not called until after UIReadyDelayed is fired, so this is out of the path of the first page load (but still earlier than it could be).

> Looking at PreferencesView._delayedInit, we don't seem to be doing too much
> work in there. It could be worse.

Yeah, _delayedInit takes only 7ms on my Galaxy Tab so the impact is pretty small.  (I could imagine us doing more work there in the future, though.)

> I am concerned that the XBL constructors are
> firing while the panel container is still supposed to be hidden. That would be
> much worse since the <setting>s are slow to init and do hurt Ts.

Fortunately, this does not seem to be the case.
Attachment #505946 - Flags: review?(mark.finkle)
Comment on attachment 505946 [details] [diff] [review]
patch

nice
Attachment #505946 - Flags: review?(mark.finkle) → review+
Whiteboard: [fennec-checkin-postb4]
(Assignee)

Comment 4

8 years ago
http://hg.mozilla.org/mobile-browser/rev/090018acbd6f
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Updated

8 years ago
Depends on: 633497

Comment 5

7 years ago
Is this verified?
Mozilla/5.0 (Android; Linux armv71; rv6.0a1) Gecko/20110506 Firefox/6.0a1 Fennec/6.0a1
Device: Droid 2 
OS: Android 2.2

Mozilla/5.0 (Android; Linux armv71; rv6.0a1) Gecko/20110506 Firefox/6.0a1 Fennec/6.0a1
Device: Thunderbolt
OS: Android 2.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.