2070404 - Record telemetry when restoring into a mismatched window

Status: RESOLVED FIXED

(Firefox :: Session Restore, task, P1)

Description

[Stephen Thompson [:sthompson]]

Bug 2065228 fixes SessionStore.restoreLastSession() so that we don't attempt to restore a saved window state into an existing window unless the existing window already has all of the window features declared by the saved window state.

However, this does not address the more general SessionStore.#restoreWindows() path that runs in more scenarios. I'm less sure about what might break if I try to prevent any attempts to reuse existing windows, even if they are not in the right state. I figure that adding telemetry might help assess:

• which paths through the code can lead to this problem
• which types of windows/window features might be affected
• how many people might be affected
• what the impact might be

Comment 1

[Stephen Thompson [:sthompson]]

Attachment: Bug 2070404 - add telemetry about restoring into mismatched windows r=#sessionstore-reviewers

This patch records when SessionRestore.#restoreWindows was asked to restore state into a window that has immutable window features. In these cases, affected windows won't be fully restored to the desired state, so there may be bugs or other issues that result. This patch only provides a warning log and records the window features so that we can learn more about any code paths or window features need to be updated before we can fully prevent SessionStore from reusing unsuitable windows.

After bug 2060396 and bug 2064683, it's no longer possible to change some window features on existing windows. Manual session restore ("restore last session") has long had additional logic to avoid unsuitable existing windows to restore into, and the previous patch in this stack hardens that code.

However, that code path is separate from the general "restore windows" path. The "restore windows" path seems like it has guaranteed to callers that SessionStore will make a window fully conform to a session state. Even though we can't guarantee that anymore, I'm reluctant to make a broad change here because I don't fully understand all of the potential impacts.

Comment 2

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

Adding to bug 2066295's list of telemetry improvements for session restore.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/a80e754e53b3
https://hg.mozilla.org/integration/autoland/rev/e8d1e15ed085
add telemetry about restoring into mismatched windows r=sfoster

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/b60f46722863
https://hg.mozilla.org/integration/autoland/rev/06fb4888f5b1
Revert "Bug 2070404, Bug 2066315, Bug 2065228 - add telemetry about restoring into mismatched windows r=sfoster" for causing debug marionette failures at sessionstore

Comment 5

[Silaghi Andreea]

Backed out for causing debug marionette failures at sessionstore
Backout Link
Push with failures
Failure Log
Failure line TEST-UNEXPECTED-ERROR | browser/components/sessionstore/test/marionette/test_close_last_nonpopup_window.py TestCloseLastNonPopupWindow.test_close_last_nonpopup_window | TypeError: SessionStoreTestCase.open_window_with_extra_options() missing 1 required positional argument: 'extra_options'

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/3b7936ef518b
https://hg.mozilla.org/integration/autoland/rev/1b9f1b42de7b
add telemetry about restoring into mismatched windows r=sfoster

Comment 7

[Silaghi Andreea]

https://hg.mozilla.org/mozilla-central/rev/1b9f1b42de7b