1730026 - geckoview.js runs some global initializers repeatedly instead of once

Status: NEW

(GeckoView :: General, defect, P3)

Description

[Rob Wu [:robwu]]

geckoview.js has logic that is an inaccurate mirror of desktop's BrowserGlue.jsm + browser.js.
The most significant issue is that it runs some logic multiple times when they should be run once. Below I've listed relevant calls and their order of desktop vs mobile, and a summary of the differences and impact.

Desktop browser:

1. browser.js's onLoad notifies "browser-window-before-show" observers, which triggers SessionStore.jsm's onBeforeBrowserWindowShown
   • onBeforeBrowserWindowShown awaits the first "browser-delayed-startup-finished", restores windows and ultimately triggers "sessionstore-windows-restored" (once).
     • "sessionstore-windows-restored" runs once globally and is handled by BrowserGlue's _onWindowsRestored, which:
       1. Calls _scheduleStartupIdleTasks, which schedules (many) idle tasks, mostly without deadlines (the last of which is triggering remote-startup-requested and marionette-startup-requested).
       2. And schedules an idle timer with a 20 sec deadline to call _scheduleBestEffortUserIdleTasks, which schedules some idle tasks (including initialization of RemoteSettings and RemoteSecuritySettings).
2. browser.js's onLoad registers a MozAfterPaint listener that calls _delayedStartup.
   1. _delayedStartup runs per-window initialization methods.
   2. _delayedStartup chains a callback to the SessionStore.promiseAllWindowsRestored (a global promise that resolves right after sessionstore-windows-restored) and then schedules per-window idle tasks (mostly without deadlines) in _schedulePerWindowIdleTask
      • _schedulePerWindowIdleTask's last task is triggering browser-idle-startup-tasks-finished.
   3. _delayedStartup ultimately triggers browser-delayed-startup-finished (without waiting for the above to complete).

Summarized, the order of the tasks/notifications is:

1. Per-window initialization
2. Per-window browser-delayed-startup-finished
3. Once, sessionstore-windows-restored
4. Once, BrowserGlue's _scheduleStartupIdleTasks
5. Per-window idle tasks without deadline
6. Per-window browser-idle-startup-tasks-finished
7. **Once, BrowserGlue's _scheduleBestEffortUserIdleTasks (with a 20 second deadline; long enough to likely be run after tasks without deadline in practice).

GeckoView (geckoview.js) has similar logic, but with some crucial differences:

1. Per-window initialization (i.e loading of geckoview.js) (note on GeckoView a window has exactly one tab), which schedules the following as idle tasks with a 15 second deadline.
2. Per-window browser-delayed-startup-finished
3. Per-window extensions-late-startup (instead of browser-delayed-startup-finished) (used in ExtensionParent), introduced in bug 1621503
4. Per-window tasks that are the equivalent of _scheduleBestEffortUserIdleTasks (= RemoteSecuritySettings.init() + SafeBrowsing.init()).
5. (no other per-window tasks, would be the same as 4)
6. Per-window browser-idle-startup-tasks-finished
7. Per-window equivalent of _scheduleStartupIdleTasks (remote-startup-requested and marionette-startup-requested)

The significant differences are:

• extensions-late-startup is called multiple times (in practice this doesn't matter, there is a small chance of affecting xpcshell tests that use ExtensionParent._resetStartupPromises).
• The order of steps 5 and 7 are swapped (_scheduleStartupIdleTasks and _scheduleBestEffortUserIdleTasks).
• The idle tasks (step 5 and 7) run multiple times (once per window) instead of once globally.

One concrete result of this bug is that RemoteSecuritySettings.init may be called repeatedly, and unnecessarily invoke the updateCertBlocklistener more than once (because another listener is registered for each call - although the RemoteSettings constructor returns the same object for a given name, its on method does not check for duplicates before registering the listener).

Comment 1

[[ex-Mozilla] Agi Sferro | :agi]

Thank you Rob.

We also have code to track the first initalization here: https://searchfox.org/mozilla-central/source/mobile/android/modules/geckoview/GeckoViewTelemetry.jsm#10 we should probably generalize that and move all the initialize-once method calls there.

Comment 2

[Rob Wu [:robwu]]

(In reply to [ex-Mozilla] Agi Sferro | :agi from comment #1)

We also have code to track the first initalization here: https://searchfox.org/mozilla-central/source/mobile/android/modules/geckoview/GeckoViewTelemetry.jsm#10 we should probably generalize that and move all the initialize-once method calls there.

The current version of that code is now at https://searchfox.org/mozilla-central/rev/ae3df68e9ba2faeaf76445a7650e4e742eb7b4e7/mobile/android/modules/geckoview/GeckoViewTelemetry.sys.mjs#10

The entry point to that is geckoview.js, comparable to the rest of the logic in this bug (https://searchfox.org/mozilla-central/rev/ae3df68e9ba2faeaf76445a7650e4e742eb7b4e7/mobile/android/chrome/geckoview/geckoview.js#950).

NOTE: All initialization logic here assumes the creation of a geckoview window as the first step towards initialization. As bug 1870760 shows, Gecko may already be relevant even before any tab/window has been created. That implies that these callers should either ensure that geckoview.xhtml is loaded, or the implementation should have an "initialization" point other than "when the first window is opened".