553691 - Stop using setTimeout to initialize the browser tools panel

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

We use setTimeout to delay-init the browser tools panel (prefs, add-ons, etc) at startup. We should just wait for the first web content page to load, signalling that the UI has finished loading.

This patch does that.

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 433630 [details] [diff] [review]
patch

>diff --git a/chrome/content/browser-ui.js b/chrome/content/browser-ui.js
>--- a/chrome/content/browser-ui.js
>+++ b/chrome/content/browser-ui.js
>@@ -406,24 +406,29 @@ var BrowserUI = {
>     // listening mousedown for automatically dismiss some popups (e.g. larry)
>     window.addEventListener("mousedown", this, true);
> 
>     // listening escape to dismiss dialog on VK_ESCAPE
>     window.addEventListener("keypress", this, true);
> 
>     // Push the panel initialization out of the startup path
>     // (Using a timeout because we have no good way to delay-init [Bug 535366])

Maybe you should replace "timeout" by something else in this comment.

>-    setTimeout(function() {
>+    browsers.addEventListener("load", function() {
>+      // We only want to delay one time
>+      browsers.removeEventListener("load", arguments.callee, false);

Hey, I've never thought of doing it like that, nice trick!

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
http://hg.mozilla.org/mobile-browser/rev/c6e8869f8618

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Looks like this change moved enough code execution out of the startup path to make a difference:
Ts before: ~12 sec N810, ~7 sec N900
Ts after: ~9.5 sec N810, ~5.5 sec N900

Txul/Twinopen also decreased