1166452 - Add an even-more-delayed startup event

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

Currently we have the browser-delayed-startup-finished event for doing "delayed" initialization. This event is fired in response to DOMContentLoaded, though, which is still pretty early in the page load. We should add a new even later event to do some of the things that get done here (probably connected to the document 'load' event).

Comment 1

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

(In reply to James Willcox (:snorp) ([e-mail]) from comment #0)
> Currently we have the browser-delayed-startup-finished event for doing
> "delayed" initialization.

If this is the delayed-startup event used in testing (and afaik only in testing), is there any reason we shouldn't just move that event to fire at this later point, rather than adding another event?

Note that we expect the browser to open about:home on startup (on all tests?) which may not fire the same events as a regular page-load.

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

We do have some delayed initialization in JS [1] and Java [2], outside of tests.

[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js?rev=4fb7ff694bf5#378
[2] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java?rev=4fb7ff694bf5#1802

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

As Jim said, we do several things there where the event is emitted -- but not actually in response to the browser-delayed-startup-finished event. I guess we wouldn't necessarily have to add a new 'more delayed' event, but we do need to init some of those things later.

Comment 4

[Wesley Johnston (:wesj)]

I wanted that event to fire after the first page was shown to the user. So if there's a good event after DOMContentLoaded I think we should just use it.

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Wesley Johnston (:wesj) from comment #4)
> I wanted that event to fire after the first page was shown to the user. So
> if there's a good event after DOMContentLoaded I think we should just use it.

I think just 'load' would be an improvement at least. DOMContentLoaded is, I believe, fired after body parsing is complete.

Comment 6

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

'pageshow' might fire a little after 'load'

I would be OK with adding a "browser-initial-pageload-finished" notification. We also support a "sessionstore-windows-restored", like desktop. I have never tested as to when it fires wrt other notifications:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/SessionStore.js#157
http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/SessionStore.js#1224

Comment 7

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Add DelayedInit module for Fennec startup (v1)

This patch adds a DelayedInit module for performing delayed initializations. There are more descriptions inside DelayedInit.jsm

Comment 8

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Convert delayed startup to using DelayedInit (v1)

This patch converts the current delayed startup block to using DelayedInit. Instead of having a big block of code that runs on DOMContentLoaded, we will have a series of initializers that are run more granularly and will run only when the message loop is idle.

Comment 9

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

Comment on attachment 8614265 [details] [diff] [review]
Convert delayed startup to using DelayedInit (v1)

Have you pushed this to Try for autophone? I am wondering if some of these changes are too aggressive. The GMPInstallManager, for example. It could get inited too early. We wait 60 seconds currently.

Comment 10

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

Comment on attachment 8614263 [details] [diff] [review]
Add DelayedInit module for Fennec startup (v1)

This looks good. Let's see what might need tweaks as we start to use it in the app.

Comment 11

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Convert delayed startup to using DelayedInit (v2)

I made several autophone runs but the results were a bit inconclusive -- some numbers were down but some others were up.

So while I investigate the results more, I made this patch a lot more conservative -- this new patch only converts the existing delayed startup tasks to using DelayedInit. It should still give us some wins. I moved the delayed startup block to the bottom of the function to make it more clear that it will run later.

As for the GMP installer, I think we can get rid of the minute-long wait, because DelayedInit will only run it on idle anyways, so the impact is small.

Comment 12

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

Comment on attachment 8616066 [details] [diff] [review]
Convert delayed startup to using DelayedInit (v2)

Looks safe enough to try out. Nice work.

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f2535394c205
https://hg.mozilla.org/integration/mozilla-inbound/rev/f84de1ca1d11

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f2535394c205
https://hg.mozilla.org/mozilla-central/rev/f84de1ca1d11

Comment 15

[Bob Clary [:bc] (inactive)]

FYI, win on throbber stop especially noticeable in twitter and nytimes.

http://phonedash.mozilla.org/#/org.mozilla.fennec/throbberstop/local-twitter/norejected/2015-06-09/2015-06-09/notcached/errorbars/standarderror/notry

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=acd94173f7e1&tochange=f84de1ca1d11

either due to Bug 1166452 - Add an even-more-delayed startup event or Bug 1166309 - java.lang.UnsatisfiedLinkError: nativeAllocateDirectBuffer when using standalone GeckoView