Closed Bug 1703556 Opened 1 month ago Closed 9 days ago

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

Categories

(Firefox :: Session Restore, defect)

defect

Tracking

()

RESOLVED FIXED
90 Branch
Fission Milestone M7a
Tracking Status
firefox90 --- fixed

People

(Reporter: kashav, Assigned: kashav)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 5 obsolete files)

This is still failing.

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.

Assignee: nobody → kmadan
Status: NEW → ASSIGNED
See Also: → 1698078

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

Also replaces a ContentTask with a SpecialPowers task.

See Also: → 1705716

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.

Attachment #9214150 - Attachment is obsolete: true

Depends on D112885

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

See Also: → 1696548
See Also: → 1696137
Attachment #9217290 - Attachment is obsolete: true

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.

Attachment #9217291 - Attachment is obsolete: true
Attachment #9217289 - Attachment description: Bug 1703556 - Only fire SSWindowRestored after all tabs have finished restoring, r?nika → Bug 1703556 - Add an "all tabs restored" event, r?nika

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

Depends on D112885

Attachment #9217289 - Attachment is obsolete: true
Attachment #9218427 - Attachment is obsolete: true

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.

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.

Pushed by kmadan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cb50afcc6165
Wait for individual SSTabRestored events before proceeding, r=annyG
Status: ASSIGNED → RESOLVED
Closed: 9 days ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch
You need to log in before you can comment on or make changes to this bug.