Open Bug 1730026 Opened 3 years ago Updated 2 years ago

geckoview.js runs some global initializers repeatedly instead of once

Categories

(GeckoView :: General, defect, P3)

Unspecified
All

Tracking

(Not tracked)

People

(Reporter: robwu, Unassigned)

References

Details

(Keywords: good-first-bug)

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
  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
    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).

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.

Priority: -- → P3
Severity: -- → S3
You need to log in before you can comment on or make changes to this bug.