1703556 - Fix browser/components/sessionstore/test/browser_windowStateContainer.js for SHIP

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[u608768]

This is still failing.

Comment 1

[u608768]

This fails locally very intermittently. When it does timeout, it hangs in the ContentTask with i = 3, similar to bug 1698078. Changing the ContentTask to SpecialPowers results in an intermittent "Actor 'SpecialPowers' destroyed before query 'Spawn' was resolved". Not really sure what's going on yet.

Comment 2

[u608768]

Adding a tab flush prior to spawning the content task appears to fix things.

Comment 3

[u608768]

Attachment: Bug 1703556 - Flush the browser's state before checking its userContextId, r?farre

Also replaces a ContentTask with a SpecialPowers task.

Comment 4

[u608768]

This is still failing test-verify and I'm not completely sure why. It's only hanging for i = 2 or i = 3 in the ContentTask, so I think there's some weird interaction between SessionStore.setWindowState(), the XULFrameLoaderCreated event, and the ContentTask frame script.

Comment 5

[u608768]

Attachment: Bug 1703556 - Add an "all tabs restored" event, r?nika

Comment 6

[u608768]

Attachment: Bug 1703556 - Adjust some tests to work with the new SSWindowRestored timing, r?nika

Depends on D112885

Comment 7

[u608768]

Attachment: Bug 1703556 - Update browser_reopen_all_windows.js to work with the new "sessionstore-single-window-restored" timing, r?annyG

We can no longer begin waiting for the SSTabRestored events after receiving
the "sessionstore-single-window-restored" notification, since the notification
is now delayed until all tabs have actually restored.

This also needed to change the test to manually run the "REstore All Windows"
code (instead of clicking the button), since we need references to the windows
to setup listeners before the tab restoration can start.

Depends on D112886

Comment 8

[Phabricator Automation]

Comment on attachment 9217291 [details]
Bug 1703556 - Update browser_reopen_all_windows.js to work with the new "sessionstore-single-window-restored" timing, r?annyG

Revision D112887 was moved to bug 1696137. Setting attachment 9217291 [details] to obsolete.

Comment 9

[u608768]

Attachment: Bug 1703556 - Update browser_windowStateContainer.js to use SSAllTabsRestored, r?nika

We should only proceed after all tabs have loaded their new window state.

Depends on D112885

Comment 10

[u608768]

Attachment: Bug 1703556 - Wait for individual SSTabRestored events before proceeding, r?annyG

Awaiting setWindowState doesn't actually work here, since we fire the
SSWindowRestored event before the tabs finish restoring (bug 1709411 to rename
the event). It works to just wait for the individual SSTabRestored events, since
we'll only fire those after each tab is restored.

Comment 11

[u608768]

To summarize what happened here:

1. We initially thought we'd delay sending SSWindowRestored (and the other related notifications, events, and telemetry pings) in SessionStore.restoreWindow() until after all tabs have finished restoring.
2. We found that too much relies on the current timing of those events, so decided it'd make more sense to just introduce a new event instead of delaying existing events.
3. I realized it was simpler to just adjust the test to wait for individual SSTabRestored events, which essentially accomplishes that same thing.

We can look into adding the new event from #2 if this becomes a recurring problem.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb50afcc6165
Wait for individual SSTabRestored events before proceeding, r=annyG

Comment 13

[Sandor Molnar[:smolnar]]

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