1681829 - about:restartrequired pages don't get restored properly from sessionstore now

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect)

Description

[Daniel Holbert [:dholbert]]

Rough STR (just based on my usage, not sure exactly):

1. Have Firefox configured to "restore previous session" at startup, in your Firefox about:preferences page.
2. Be running an out-of-date version of Nightly (e.g. yesterday's nightly).
3. Start another instance of Firefox in the background with a fresh profile:

 mkdir /tmp/foo; firefox -no-remote -profile /tmp/foo

4. Allow a Firefox update to install (not sure exactly how/when this happens)
5. Navigate around / open tabs / etc. in your original instance of Firefox, until one of them shows an about:restartrequired page ("Sorry. We just need to do one small thing to keep going.")
6. Restart Firefox.

EXPECTED RESULTS:
The about:restartrequired tab should now load the actual content that it was trying to load.

ACTUAL RESULTS:
The about:restartrequired page gets restored still showing the error page (even though now there's no update staged & no issues that require a restart). The URL bar for this tab now shows something like about:restartrequired?e=restartrequired&u=about%3Ablank&c=UTF-8&d=%20, where the "u=..." contains a URL-encoded version of the original URL.

I think this is a regression - I perform something like the STR (and hit this restartrequired page) fairly frequently, and I only started noticing this tab-failing-to-restore issue in the last month or so.

Comment 1

[:gerard-majax]

Attachment: Bug 1681829 - Block about:restartrequired from being restored during SessionRestore r?#sessionstore-reviewers!

Comment 2

[:gerard-majax]

My current patch would make it an empty tab, I guess we need to play with entries ?

Comment 3

[Dão Gottwald [:dao]]

(In reply to :gerard-majax from comment #2)

My current patch would make it an empty tab, I guess we need to play with entries ?

Yeah...

Comment 4

[:gerard-majax]

Do you mind sharing feedback on the current patch? Is this the right approach or should it be done differently ?

Comment 5

[Dão Gottwald [:dao]]

Thanks, I've responded in phabricator.

Comment 6

[Max Christian Pohle [:/dev/max]]

Hi! Please allow me first to ask if you are still actively working on the patch for this bug, and secondly to link to

Bug 1910115 - about:tabcrashed pages are restored as about:blank

This begs the question, should specific about pages be restored instead of listing individual pages that should not be restored?

Comment 7

[:gerard-majax]

(In reply to Max Christian Pohle [:/dev/max] from comment #6)

Hi! Please allow me first to ask if you are still actively working on the patch for this bug, and secondly to link to

Bug 1910115 - about:tabcrashed pages are restored as about:blank

This begs the question, should specific about pages be restored instead of listing individual pages that should not be restored?

There's an attached patch I updated the other day that should answer your question. For the rest I'm not qualified to answer

Comment 8

[Daniel Holbert [:dholbert]]

(In reply to Max Christian Pohle [:/dev/max] from comment #6)

This begs the question, should specific about pages be restored instead of listing individual pages that should not be restored?

In general, all pages should be restored, whether they're about: pages or not. The point of session-restore is to get Firefox back to the exact state where it was before the restart (or the crash, or whatever).

There are a few cases of about: pages that reflect a "whoops, we couldn't load the page for whatever reason" status - those include about:restartrequired, about:tabcrashed, and perhaps the various flavors of about:neterror. Those do merit special handling to do the right thing, depending on reason for the failure, and it seems entirely reasonable to give them special handling. I think such pages are the exception rather than the rule for about: pages, though.

Comment 9

[Daniel Holbert [:dholbert]]

(Also, FWIW, I don't think I've actually hit this bug for quite a while - not sure if that's because my habits have changed or if Firefox has gotten better about not needing to show about:restartrequired, but I suspect it's the latter. Still good to make things even-more-robust where we can, so don't take this as any discouragement on work here - just wanted to mention the difference-in-observed-results for the record, since the necessary STR here might have changed, if there are even STR that can be used to easily trigger issues here at this point.)

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3ba088fe491
Block about:restartrequired from being restored during SessionRestore r=sessionstore-reviewers,sfoster

Comment 11

[Iulian Moraru]

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