1624044 - "Restore Previous Session" item is always greyed out after landing patch from bug #1622195

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect, P1)

Description

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

STR:

1. Open few tabs and/or windows
2. Close them by selecting "Exit" item in "hamburger" menu or by pressing Ctrl+Shift+Q keyboard shortcut
3. Restore tabs and/or windows by selecting "Restore Previous Session" item in "hamburger" menu

and see that you can't, because it's always greyed out.

Recent regression, maybe it could be because of bug #1507287.

@ Alphan Chen [:alchen] - Any ideas or it's missed call?

Comment 1

[Alphan Chen [:alchen]]

I cannot reproduce on Linux and Windows 10.
But I can found the symptom on OSX.

Comment 2

[Alphan Chen [:alchen]]

Now I use the central branch to build FF and reproduce the symptom locally.
I don't find the symptom in the local build based on my commit both on Ubuntu and OSX.

commit f22530009d43d3c31217de5f7d2093aafcc338af
Author: Alphan Chen <[e-mail]>
Date: Thu Mar 19 14:31:52 2020 +0000
Bug 1507287 - Make sessionRestore work with session history living in the parent process. r=peterv,mikedeboer

Comment 3

[Alphan Chen [:alchen]]

(In reply to Alphan Chen [:alchen] from comment #1)

I cannot reproduce on Linux and Windows 10.
But I can found the symptom on OSX.

Actually, I cannot reproduce the symptom consistently.
Need a solid STR.

Comment 4

[Alphan Chen [:alchen]]

I found that we need to open a new profile to reproduce the symptom consistently.
From my experiment, the regression is caused by Bug 1622195.
I can reproduce the symptom on the latest central and revert the commit can fix the problem.

Hello Harry,
could you check this problem?

Comment 5

[Alice0775 White]

(In reply to Alphan Chen [:alchen] from comment #4)

I found that we need to open a new profile to reproduce the symptom consistently.
From my experiment, the regression is caused by Bug 1622195.
I can reproduce the symptom on the latest central and revert the commit can fix the problem.

Hello Harry,
could you check this problem?

I can reproduce the issue with new profile Nightly76.0a1 Windows10. But, it is intermittent.
However, once this happens, this problem may be persistence.

I found a more solid str:

1. Start Nightly with new profile and open several tabs. Exit browser.
2. Start Nightly with -jsconsole option
3. Click "Not now" button to close the "Default Browser" dialog
4. Observe "Restore Previous Session" item in Hamburger menu/Menu bar

Comment 6

[Harry Twyford]

Thank you all for the investigation and STR. I'm able to reproduce. The issue appears to be related to bug 1622195 adding a new export. While the export works, clearly it has side effects elsewhere. I'll work on this.

Also, I'm going to simplify the title a little seeing as Ryan added tracking flags.

Comment 7

[Harry Twyford]

The issue is actually at this line, which checks SessionStartup. The call to willCheckDefaultBrowser() (containing the check) added in bug 1622195 happens earlier than any call that existed before. I might be accessing SessionStartup too soon. Bug 1369456 deals with the timing of the initialization of SessionStartup, so I'll look through that bug.

Comment 8

[Harry Twyford]

Attachment: Bug 1624044 - Wait for SessionStartup initialization before using it in willCheckDefaultBrowser.

The sessionType getter in SessionStartup sets its internal state if there isn't already one. Since the willCheckDefaultBrowser call added in bug 1622195 happens very early, SessionStartup isn't initialized yet so its state is set to "never restore". There's no clean solution here, as far as I can tell. Other solutions I considered:

• Adding an intialization parameter to UrlbarProviderSearchTips and not setting it until after SessionStartup is initialized. This means that UrlbarProviderSearchTips is initialized later. As a result, it does not show the about:newtab Search Tip when a new window is opened, but rather on the next open newtab. Also this issue would come back if any early consumers of willCheckDefaultBrowser are added.

• Pushing the initialization of SessionStartup forward. This would regress the performance gains from bug 1369456.

I think the false positive introduced in this patch is preferable to those two options. Future consumers of willCheckDefaultBrowser will presumably use it to disable behaviour that might get in the way of a session restore or browser startup. A false positive would disable the behaviour even though there's no session startup happening. This seems like an acceptable tradeoff.

EDIT: To be clear, this false positive does not affect the actual default browser check, just the preemptive willCheckDefaultBrowser function, which right now is only used by UrlbarProviderSearchTips.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/53361841567a
Wait for SessionStartup initialization before using it in willCheckDefaultBrowser. r=dao

Comment 11

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/53361841567a

Comment 12

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

I'm confirming that bug is fixed, starting in Mozilla Firefox Nightly 76.0a1 (2020-03-25), so I'm marking this bug as VERIFIED.
Thank you very much everyone! \o/

Comment 13

[Julien Cristau [:jcristau] (back August 18)]

Comment on attachment 9135177 [details]
Bug 1624044 - Wait for SessionStartup initialization before using it in willCheckDefaultBrowser.

companion for bug 1622195, approved for 75 rc1

Comment 14

[Julien Cristau [:jcristau] (back August 18)]

https://hg.mozilla.org/releases/mozilla-release/rev/5b9178593cf6

Comment 15

[Camelia Badau [:cbadau], Desktop Test Engineering]

Reproduced the issue on Firefox Nightly 76.0a1 (2020-03-22) using the steps from comment 5.
Verified fixed on Windows 10 x64, Ubuntu 18.04 x64 and macOS 10.14 using Firefox 75 RC (buildID: 20200331175109).