2034633 - Improve when SidebarController's _initDeferred promise gets resolved to avoid races.

Status: RESOLVED FIXED

(Firefox :: Sidebar, task, P3)

Description

[Sam Foster [:sfoster] (he/him)]

In investigating bug 2021526, I spotted that the promiseInitialized used extensively in both the marionette and browser-mochitests gets resolved before the initialization is truly complete. We register a call to this.updateUIState(backupState) with requestIdleCallback which means it executes after promiseInitialized becomes true. This seems likely to race in an automated test scenario.

Fixing the race does cause some other test fallout though, so rather than fix one of the existing intermittent bugs and bring along a load of other unrelated test changes, I think it makes sense to fix it in its own bug and resolve any of those other bugs as needed.

Comment 1

[Sam Foster [:sfoster] (he/him)]

There's a number of scenarios which sidebar initialization needs to handle, with both values of sidebar.revamp

• At browser startup, no session restore: we need to fallback to the backup/default sidebar state which is managed in sidebar.backupState
• At browser startup, sidebar state restored from a previous session: session restore in the startup path is squiggly as there's a setTimeout 0 in there and different cases such as restoring a previous state into an already-open window.
• A single-tab window e..g taskbar tab or a popup where we always want to hide the sidebar intially
• A window opened by another window where we want to adopt/inherit the sidebar state from the opener.

The variability of the timing in each of these scenarios had lead us to using an requestIdleCallaback to finalize sidebar state. I.e. whatever needed to happen, hopefully its happened by now. But it turns out we can handle each of these cases a bit more deterministically and make promiseInitialized a better promise we can build on both in sidebar interactions and tests.

Comment 2

[Sam Foster [:sfoster] (he/him)]

Attachment: Bug 2034633 - Rearrange Sidebar initialization to resolve promiseInitialized reliably and earlier. r?#sidebar-reviewers,Gijs

• Make promiseInitialized a more stable promise that the sidebar has its initial state. initialized means:
  • its not in-flight and can be updated
  • is ready to interact with
  • can be interrogated from tests
• Add a willRestoreWindow method to SessionStore to allow SidebarController to know when session state should be treated as in-flux for a given window

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/c925a592c995
https://hg.mozilla.org/integration/autoland/rev/cd4438d92fcd
Rearrange Sidebar initialization to resolve promiseInitialized reliably and earlier. r=Gijs,kcochrane,nsharpley

Comment 4

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/cd4438d92fcd