806180 - "Restore Previous Sessions" missing from start page until reload of the page on Linux

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Bill Gianopoulos [:WG9s]]

For the last few days, I have noticed the "Restore Previous Session" button does not appear on the start page under Linux, even with a clean profile.

I do not see this issue under Windows.

I suspect bug 715402 as the cause.

Comment 1

[Bill Gianopoulos [:WG9s]]

After backing out the fix for bug 715402, I am no longer able to reproduce this issue.

Comment 2

[Dão Gottwald [:dao]]

(In reply to Bill Gianopoulos [:WG9s] from comment #0)
> I do not see this issue under Windows.

Same here.

Comment 3

[Bill Gianopoulos [:WG9s]]

and what is up with the new lame blocking flags so that most of us contributors can't figure out how to set blocking flags anymore?????

Comment 4

[Bill Gianopoulos [:WG9s]]

(In reply to Bill Gianopoulos [:WG9s] from comment #3)
> and what is up with the new lame blocking flags so that most of us
> contributors can't figure out how to set blocking flags anymore?????

OK maybe tracking and affected is easier it is just the kilimanjaro and basecamp that i could not decipher.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

I imagine there are probably race conditions here that affect the reproducibility of this.

IIRC the button on about:home only appears when sessionstore.canRestoreLastSession is true, which can only occur after we've initialized sessionstore, which can sometimes happen in delayedStartup (see bug 654388 for a related issue). So it seems likely that bug 715402 changed timing such that we're now losing that race.

Comment 6

[Dão Gottwald [:dao]]

Attachment: patch

Comment 7

[Dão Gottwald [:dao]]

Loading the home page in _delayedStartup (bug 756313) would fix this too, although it's not quite clear to me that we'll want to do this.

Comment 8

[Bill Gianopoulos [:WG9s]]

(In reply to Dão Gottwald [:dao] from comment #6)
> Created attachment 676804 [details] [diff] [review]
> patch

This patch fixes the issue for me.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 676804 [details] [diff] [review]
patch

This will push session restore initialization into the startup path for some users, and undo most of the benefits of bug 715402 for them. A better solution that would avoid that problem is having BrowserOnAboutPageLoad check the session store service's initialization state somehow (I don't know if that is easily exposed at the moment), and if it's not yet initialized, add an observer for when it is which then checks canRestoreLastSession and unhides the button as needed.

(The underlying issue here is that we're loading the page before the window is painted, which I guess is bug 756313.)

Comment 10

[Dão Gottwald [:dao]]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #9)
> Comment on attachment 676804 [details] [diff] [review]
> patch
> 
> This will push session restore initialization into the startup path for some
> users, and undo most of the benefits of bug 715402 for them.

Really? The service shouldn't do much in case we're loading the homepage, right?

> A better
> solution that would avoid that problem is having BrowserOnAboutPageLoad
> check the session store service's initialization state somehow (I don't know
> if that is easily exposed at the moment), and if it's not yet initialized,
> add an observer for when it is which then checks canRestoreLastSession and
> unhides the button as needed.

This seems significantly more complex for questionable gain.

> (The underlying issue here is that we're loading the page before the window
> is painted, which I guess is bug 756313.)

Right, see comment 7.

Comment 11

[Bill Gianopoulos [:WG9s]]

So, summarizing the above.  Doesn't this only make it slower in the case where we lost the race?

Comment 12

[:Gavin Sharp [email: [e-mail]]]

(In reply to Dão Gottwald [:dao] from comment #10)
> Really? The service shouldn't do much in case we're loading the homepage,
> right?

Session store pretty much always does the same amount of startup work, regardless of whether we're actively restoring the session, AFAIK (to support after-the-fact session restoring from the menu or about:home). Given that this affects the behavior of canRestoreLastSession, I assume when this happens that we're triggering initService() from within init(), which does a non-trivial amount of work.

> This seems significantly more complex for questionable gain.

An observer isn't that complex. Let's not make guesses here, let's figure out exactly what the gain is.

Comment 13

[Alice0775 White]

I can reproduce in Windows7
Steps to reproduce:
  1. Start Firefox with minimize window mode
  2. After several seconds, Click Firefox taskbar button so that Firefox make normal size.

OR
Steps to reproduce:
  1. Start IE or any app and maximized it
  2. Start Firefox
  3. Click IE window immediately so that a window of IE becomes the front
  4. After several seconds, Click Firefox taskbar button so that Firefox to the front.

Actual results
  NO "Restore Previous Sessions" in about:home

Comment 14

[Alice0775 White]

bug 756313 fixes the problem of comment 13.

http://hg.mozilla.org/mozilla-central/rev/e35f252ca573
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121101140125

Comment 15

[Bill Gianopoulos [:WG9s]]

Now that the patch for bug 756313 is in today's nightly, I have been unable to reproduce this.