711193 - turn on "don't load tabs until selected" by default

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect)

Description

[Dietrich Ayala (:dietrich)]

Asa and Limi both support doing this.

Some thoughts:

* should enable this always for the update/add-on/crash restart scenario?
* do we have telemetry for how many people restore session by default?
* it seems obvious, but would be good to quantify how much it helps, given some example sessions

Comment 1

[Asa Dotzler [:asa]]

Yes, please.

Comment 2

[:Paolo Amadini]

Paul, with regard to the automated tests in "sessionstore/test", is there any one
of them for which it is important that the "don't load tabs until selected"
preference is set to false (as opposed to being effective with either value of
the preference)?

Comment 3

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to Paolo Amadini from comment #2)
> Paul, with regard to the automated tests in "sessionstore/test", is there
> any one
> of them for which it is important that the "don't load tabs until selected"
> preference is set to false (as opposed to being effective with either value
> of
> the preference)?

A bunch of them I think. My assumption is that they would timeout and/or fail so it should be obvious when you run them.

Off the top of my head, at least a bunch of tests in browser_586068-cascaded_restore.js. Also, I think anything using waitForBrowserState will need to be fixed too (since that waits for SSTabRestored from each tab).

Comment 4

[:Paolo Amadini]

(In reply to Paul O'Shannessy [:zpao] from comment #3)
> A bunch of them I think. My assumption is that they would timeout and/or
> fail so it should be obvious when you run them.

Fine, I was concerned mostly about tests that would not fail but would also not
test what they were designed for :-)

In the end, it turned out that 28 tests are affected by the change. I started a
tryserver build to see if tests in other areas of the tree are also affected
(<https://hg.mozilla.org/try/rev/86b89586eadf>).

Given the high number of affected tests, I'm not sure if I should follow the
simple approach of switching the preference to false as a default in "head.js"
(see https://hg.mozilla.org/try/rev/86b89586eadf) or go through each test and
modify it to set the preference (or force the background tab to load in some way).
Suggestions?

Comment 5

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to Paolo Amadini from comment #4)
> Given the high number of affected tests, I'm not sure if I should follow the
> simple approach of switching the preference to false as a default in
> "head.js"
> (see https://hg.mozilla.org/try/rev/86b89586eadf) or go through each test and
> modify it to set the preference (or force the background tab to load in some
> way).
> Suggestions?

Let's go with the head.js approach. Neither feels great (doing it in head.js is overly sweeping; doing it in individual tests is overly detailed). The tests were almost all written with the assumption that restoring would just happen so let's avoid breaking that unless explicitly testing the opposite.

My only nit from the patch on try is this...
> Ensure that tabs restored in the background are loaded immediately
"immediately" isn't quite right and I want to make sure this comment is completely correct for any future maintainers wondering wtf is happening. "without user interaction" or something would be better. They're still put into a queue (with few exceptions: selected tabs). Also mentioning "see firefox.js for details on how the pref works" would be helpful.

Comment 6

[:Paolo Amadini]

(In reply to Paul O'Shannessy [:zpao] from comment #5)
> Let's go with the head.js approach. Neither feels great (doing it in head.js
> is overly sweeping; doing it in individual tests is overly detailed). The
> tests were almost all written with the assumption that restoring would just
> happen so let's avoid breaking that unless explicitly testing the opposite.

Ok. If I remember correctly, browser-chrome tests could also run with different
default preferences on other trees (e.g. browser-prefs.js for SeaMonkey), and
this approach should take care of that too, providing a consistent environment.

I'm going to do the same for the tabview tests.

Comment 7

[:Paolo Amadini]

Attachment: The patch

This patch is now running on tryserver (<https://tbpl.mozilla.org/?tree=Try&rev=b13ad3d2eb10>). I've reworded the comment in the hope of making it clearer,
feel free to let me know if the language needs more fixes.

There is still one test that fails locally, but even after unapplying the patch:
browser_privatebrowsing_beforeunload_exit.js. Let's see if it fails during this
tryserver run too.

Comment 8

[:Paolo Amadini]

Attachment: Figured out privatebrowsing test failure

Understanding what was going on with
browser_privatebrowsing_beforeunload_enter.js and
browser_privatebrowsing_beforeunload_exit.js was somewhat tricky. In fact it
turns out that the latter depends on the former being executed before it.

The "enter" test apparently exits PBM correctly at the end, but leaves some
hidden state that is used when exiting PBM later in the "exit" test (this is
caused by toggling the preference to preserve the current session).

From the descriptions of the tests, it doesn't look like this is actually
being tested intentionally, so I reworked the tests to do their job
independently. Ehsan can probably determine if this is correct.

For the rest, my interpretation of the TBPL result is that this passes all the
tests on tryserver now (<https://tbpl.mozilla.org/?tree=Try&rev=d6d0330d9ee2>).

Comment 9

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

Why do we think this will be beneficial to the majority of our users? This has always been a power-user feature from my point of view (useful for large sessions, but not really for small ones), and it seems to me that for small sessions, it has the disadvantage of potentially confusing users about why their pages aren't loading immediately after a restart (and introducing unnecessary "jank" when they switch tabs).

Comment 10

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

(In reply to Dietrich Ayala (:dietrich) from comment #0)
> * do we have telemetry for how many people restore session by default?

This data, and data on the average session size (# of tabs) would be useful. Do we have it yet?

> * it seems obvious, but would be good to quantify how much it helps, given
> some example sessions

What do we expect this to help with, exactly? Startup time? Is the normal session store chunking not effective enough at mitigating that?

Comment 11

[Asa Dotzler [:asa]]

We have data that the number one reason people leave Firefox for Chrome is start-up perf and this has a small win for a large number of users and a big win for a small number of influential users.

Comment 12

[(dormant account)]

Gavin,
We have telemetry data that indicates session restore plays are key role in ridiculously long startups. We can land this feature and watch if our startup times decrease in a meaningful way. If you are right, they wont, but preliminary data shows that session restore triggers some rather expensive operations.

Comment 13

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

Ok, so it seems like you are expecting it to help startup time. Good to mention that in the bug!

Which startup measurements are we expecting it to help? Why do we expect it to impact those measurements? Do we have any anecdotal evidence that it does?

I'm fine with "let's give it a shot and see", but that kind of experimentation needs to be backed up by solid theories based on understanding of the code being affected - especially when there's non-trivial UX impact (users *will* be confused by this).

This pref doesn't affect the majority of the work done by sessionstore; it only affects whether the loads actually get triggered. Session store already splits the triggering of loads into chunks to avoid contention and maintain some responsiveness. Is that chunking not effective? If it isn't, then perhaps we could first try decreasing the chunk size (3 tabs) and/or introducing a bigger chunk interval.

Comment 14

[(dormant account)]

(In reply to Gavin Sharp (use [e-mail] for email) from comment #13)
> Ok, so it seems like you are expecting it to help startup time. Good to
> mention that in the bug!
> 
> Which startup measurements are we expecting it to help? Why do we expect it
> to impact those measurements? Do we have any anecdotal evidence that it does?

Sorry, I realized I wasn't being accurate here. It is likely that 715402 will fix the slow startup issue. That's what the telemetry is pointing at. This bug is yet another way of addressing that issue.

Once 715402 lands, the main goal here is to increase responsiveness after startup. We do not have a good way of measuring that yet.
> 
> I'm fine with "let's give it a shot and see", but that kind of
> experimentation needs to be backed up by solid theories based on
> understanding of the code being affected - especially when there's
> non-trivial UX impact (users *will* be confused by this).

You are right about setting up a controlled experiment. We should do add some responsiveness telemetry while session-restore tabs are being loaded before we act on this.

Is there an easy way to monitor onloads in all of the restored tabs? Once we have this, some sort of high frequency main thread ping(every 50ms) can track responsiveness into a histogram until the last tab is loaded.
 
> 
> This pref doesn't affect the majority of the work done by sessionstore; it
> only affects whether the loads actually get triggered. Session store already
> splits the triggering of loads into chunks to avoid contention and maintain
> some responsiveness. Is that chunking not effective? If it isn't, then
> perhaps we could first try decreasing the chunk size (3 tabs) and/or
> introducing a bigger chunk interval.

I don't suspect of session restore being the main overhead. Firefox is currently not capable of properly throttling background tabs. 
I'm worried that at the moment loading a single background tab can jank user experience in an active tab. So even if stagger one tab at a time things would still be suboptimal.

Comment 15

[Dão Gottwald [:dao]]

(In reply to Taras Glek (:taras) from comment #14)
> Firefox is currently not capable of properly throttling background tabs.

Bug 595574 seems to be the central tracking bug for this, by the way.

Comment 16

[(no longer active)]

Comment on attachment 586544 [details] [diff] [review]
Figured out privatebrowsing test failure

Review of attachment 586544 [details] [diff] [review]:
-----------------------------------------------------------------

So I never really liked the way we split the beforeunload test.  This was done in bug 565458 (http://hg.mozilla.org/mozilla-central/rev/421492990143).  What if you revert that change?  Would you still have problems with this change?  As the test has been disabled on Linux, I don't see a problem with doing that.

Comment 17

[(dormant account)]

(In reply to Dão Gottwald [:dao] from comment #15)
> (In reply to Taras Glek (:taras) from comment #14)
> > Firefox is currently not capable of properly throttling background tabs.
> 
> Bug 595574 seems to be the central tracking bug for this, by the way.

Reflows, js parsing, css parsing, etc can take a long time and are often not interrupt able. Even with that bug fixed we'll still be unresponsive during pageloads. So even if we load a single page at a time there would be jank. The only way to make use of something session-restore responsive would be something like bug 712478 to allow suspending of page-loading in background tabs while the user is doing stuff.

Comment 18

[:Paolo Amadini]

(In reply to Ehsan Akhgari [:ehsan] from comment #16)
> So I never really liked the way we split the beforeunload test.  This was
> done in bug 565458 (http://hg.mozilla.org/mozilla-central/rev/421492990143).
> What if you revert that change?  Would you still have problems with this
> change?  As the test has been disabled on Linux, I don't see a problem with
> doing that.

At first glance, I think reverting that change should fix the issues I had.
I'm not familiar with the mechanics of backing out old changes, should I just
produce a reverse patch in a separate bug?

Comment 19

[(no longer active)]

(In reply to Paolo Amadini from comment #18)
> (In reply to Ehsan Akhgari [:ehsan] from comment #16)
> > So I never really liked the way we split the beforeunload test.  This was
> > done in bug 565458 (http://hg.mozilla.org/mozilla-central/rev/421492990143).
> > What if you revert that change?  Would you still have problems with this
> > change?  As the test has been disabled on Linux, I don't see a problem with
> > doing that.
> 
> At first glance, I think reverting that change should fix the issues I had.
> I'm not familiar with the mechanics of backing out old changes, should I just
> produce a reverse patch in a separate bug?

Yes, please.  File it under Firefox::Private Browsing, and I will be happy to review it.  Note that there were two patches landed in bug 565458, and we want to back out the first one but keep the changes made by the second one.  The easiest way to do that I think is to use hg revert to revert their changes in reverse order, and then apply the changes made by the second patch manually.

Thanks!

Comment 20

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Comment on attachment 586544 [details] [diff] [review]
Figured out privatebrowsing test failure

Review of attachment 586544 [details] [diff] [review]:
-----------------------------------------------------------------

Let's get a try run with bug 718484 & (and updated version of) this. I just have one question below before I r+.

::: browser/base/content/test/browser_tabMatchesInAwesomebar.js
@@ +80,5 @@
> +    // Make sure that all restored tabs are loaded without waiting for the user
> +    // to bring them to the foreground. We ensure this by resetting the
> +    // related preference (see the "firefox.js" defaults file for details).
> +    ps.setBoolPref("browser.sessionstore.restore_on_demand", false);
> +

I'm guessing it's safe to assume this doesn't pass when true? I thought it would though since we do trickery to get currentURI working (explicitly for tab matching to work). In fact, we test that (browser_599909.js), though maybe the timing is off here.

::: browser/components/privatebrowsing/test/browser/browser_privatebrowsing_beforeunload_enter.js
@@ +114,5 @@
> +
> +          if (++loads != 3)
> +            return;
> +
> +          is(gBrowser.getBrowserForTab(gBrowser.tabContainer.firstChild).currentURI.spec, "about:blank",

Save yourself some breath & don't use getBrowserForTab. All that does is return TAB.linkedBrowser anyway. (Going from a browser to a tab isn't so easy though).

gBrowser.tabContainer.firstChild.linkedBrowser.currentURI.spec

Comment 21

[:Paolo Amadini]

Attachment: On top of bug 718484

(In reply to Paul O'Shannessy [:zpao] from comment #20)
> ::: browser/base/content/test/browser_tabMatchesInAwesomebar.js
> @@ +80,5 @@
> > +    // Make sure that all restored tabs are loaded without waiting for the user
> > +    // to bring them to the foreground. We ensure this by resetting the
> > +    // related preference (see the "firefox.js" defaults file for details).
> > +    ps.setBoolPref("browser.sessionstore.restore_on_demand", false);
> > +
> 
> I'm guessing it's safe to assume this doesn't pass when true? I thought it
> would though since we do trickery to get currentURI working (explicitly for
> tab matching to work). In fact, we test that (browser_599909.js), though
> maybe the timing is off here.

The issue here is just that some tests wait for "load" event on all tabs
before continuing. I haven't investigated whether it's actually important,
for the tests here, that the tabs are fully loaded.

Comment 22

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to Paolo Amadini from comment #21)
> The issue here is just that some tests wait for "load" event on all tabs
> before continuing. I haven't investigated whether it's actually important,
> for the tests here, that the tabs are fully loaded.

Now that I've actually looked at the rest of that test... while it is waiting for load, it doesn't use session restore so restore-on-demand doesn't affect it. You shouldn't need to change that test.

Comment 23

[:Paolo Amadini]

(In reply to Paul O'Shannessy [:zpao] from comment #22)
> (In reply to Paolo Amadini from comment #21)
> > The issue here is just that some tests wait for "load" event on all tabs
> > before continuing. I haven't investigated whether it's actually important,
> > for the tests here, that the tabs are fully loaded.
> 
> Now that I've actually looked at the rest of that test... while it is
> waiting for load, it doesn't use session restore so restore-on-demand
> doesn't affect it. You shouldn't need to change that test.

I don't follow you here: the test changes privateBrowsingEnabled, that triggers
the session restore behavior. Thus I need to set the preference for the test to
work properly. Or maybe we are saying the same thing, and you think I shouldn't
make additional changes to the test in addition to the ones I already did to
have it working?

Comment 24

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

Comment on attachment 589911 [details] [diff] [review]
On top of bug 718484

I don't think this is a good idea, per comment 9 - we should settle the matter of whether we want to do this before spending a lot of time ironing out implementation details.

Comment 25

[(dormant account)]

(In reply to Gavin Sharp (use [e-mail] for email) from comment #9)
> Why do we think this will be beneficial to the majority of our users? This
> has always been a power-user feature from my point of view (useful for large
> sessions, but not really for small ones), and it seems to me that for small
> sessions, it has the disadvantage of potentially confusing users about why
> their pages aren't loading immediately after a restart (and introducing
> unnecessary "jank" when they switch tabs).

It's a jank trade off: bulk jank while restoring a whole bunch of tabs vs jank for the visible tab only.

I think it's pretty clear that there is more continuous jank if you restore all of your tabs at once preventing people from using the browsers effectively.

Gavin, what sort of data do you expect to be able to say one is better than the other? UX study? Something else? I think "this is how things work right now" is not sufficient to block this.

Comment 26

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

(In reply to Taras Glek (:taras) from comment #25)
> I think it's pretty clear that there is more continuous jank if you restore
> all of your tabs at once preventing people from using the browsers
> effectively.

We don't "restore all of your tabs at once". We restore MAX_CONCURRENT_TAB_RESTORES at a time. Rather than go all-out and effectively disable session restore in a way that's going to be confusing to users, we could easily try tweaking this value based on experimental data, or try experimenting with adding additional yielding between tab restorations.

But before we do any of that, we should have evidence that sessionstore is actually an important factor in startup responsiveness, and know how to measure whether our proposed changes affect that. From what I can tell, we don't have either of those. I mentioned this to you on IRC, and you gave me the reasoning for some of your conclusions, but they were far from complete, and some of the assumptions behind them turned out to be incorrect.

Comment 27

[Dietrich Ayala (:dietrich)]

I personally use this setting to keep memory use low - which in turn drastically effects responsiveness thoughout app lifetime in my experience.

This could affect startup responsiveness in a non-trivial way though, since 1 tab is currently less than MAX_CONCURRENT_TAB_RESTORES tabs (3, iirc). Should we stick some telemetry in to measure that difference somehow?

Comment 28

[(dormant account)]

Problem is that whether we restore 1 tab or many, reflows still interrupt the event loop. So if you only do one tab at a time, you will consistently interrupt the user until the session is restored.

We can't measure this at the moment, because there is no observer notification to mark the end of session restore(we talked about adding one on irc).

Comment 29

[Dão Gottwald [:dao]]

(In reply to Asa Dotzler [:asa] from comment #11)
> this has a small win for a large number of users and a big
> win for a small number of influential users.

As a compromise and to help that small number of users with plenty of tabs, we could get bug 586067 fixed and restore only the X most recently selected tabs or only those tabs selected in the previous session.

Comment 30

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

(In reply to Taras Glek (:taras) from comment #28)
> Problem is that whether we restore 1 tab or many, reflows still interrupt
> the event loop. So if you only do one tab at a time, you will consistently
> interrupt the user until the session is restored.

The alternative is continuously interrupting the user when they later switch tabs, expecting to see a loaded page. That's not obviously a win to me.

> We can't measure this at the moment, because there is no observer
> notification to mark the end of session restore(we talked about adding one
> on irc).

Yep - filed bug 720855 for that.

Comment 31

[(dormant account)]

(In reply to Gavin Sharp (use [e-mail] for email) from comment #30)
> (In reply to Taras Glek (:taras) from comment #28)
> > Problem is that whether we restore 1 tab or many, reflows still interrupt
> > the event loop. So if you only do one tab at a time, you will consistently
> > interrupt the user until the session is restored.
> 
> The alternative is continuously interrupting the user when they later switch
> tabs, expecting to see a loaded page. That's not obviously a win to me.

Atleast in this case the webpage is obviously loading. In the other case the user is experiencing jank in the foreground tab long after the page finished loading. I'll leave it up to UX to decide as to which is better.

Thanks for filing bug 720855.

Comment 32

[(dormant account)]

A particularly bad use case is opening up the browser with a few tabs(10+) and going to firefox menu/bookmarks. On slower hardware this is basically unusable until session restore finishes.

Comment 33

[(dormant account)]

I just did a test. Started up the browser with 8tabs. With tabs on demand it is responsive in about 3seconds. >30seconds without.  I don't think we need telemetry to see that session restore adds a lot of overhead until the browser is responsive.

Comment 34

[(dormant account)]

(In reply to Taras Glek (:taras) from comment #33)
> I just did a test. Started up the browser with 8tabs. With tabs on demand it
> is responsive in about 3seconds. >30seconds without.  I don't think we need
> telemetry to see that session restore adds a lot of overhead until the
> browser is responsive.

https://plus.google.com/108936502671219351445/posts/UMzBAQr3xDS here is the test.

Comment 35

[Siddhartha Dugar [:sdrocking]]

We should have a user confirmation prompt, like we have for telemetry, for turning this on. Also see bug 720154.

Comment 36

[Asa Dotzler [:asa]]

(In reply to Siddhartha Dugar (sdrocking) from comment #35)
> We should have a user confirmation prompt, like we have for telemetry, for
> turning this on. Also see bug 720154.

No. We shouldn't.

Comment 37

[Siddhartha Dugar [:sdrocking]]

Cascaded loading like we had in Firefox 4 is a good option. I use the extension Load Tabs Progressively and it handles the page load quite smoothly.
In addition we can also use cascaded loading for background tabs like this addon. With this if I do "Open all in tabs" on a bookmarks folder, each page loads in reasonable time and the browser stays responsive.

Comment 38

[Justin Dolske [:Dolske]]

One extra piece of info, in favor of doing this (at some point): We've talked about wanting to have a way to "freeze" already-loaded tabs to reduce their memory footprint. EG, if a tab hasn't been accessed in X hours, save it's state in session store and unload it from memory. [Yes, there are lots of details to worry about to make this work right. No, this isn't the bug to discuss them.]

If we end up doing that, this bug becomes a much more natural behavior.

[And it suggests an incremental flavor -- at startup, only load tabs that had been selected in the last X hours (from startup? or last shutdown?)]

Comment 39

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to Justin Dolske [:Dolske] from comment #38)
> One extra piece of info, in favor of doing this (at some point): We've
> talked about wanting to have a way to "freeze" already-loaded tabs to reduce
> their memory footprint. EG, if a tab hasn't been accessed in X hours, save
> it's state in session store and unload it from memory. [Yes, there are lots
> of details to worry about to make this work right. No, this isn't the bug to
> discuss them.]

Bug 675539 is the right place though :)

Comment 40

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

Comment on attachment 589911 [details] [diff] [review]
On top of bug 718484

After further discussion with limi, I reluctantly agreed that we should give this a try. I don't think this is going to significantly affect startup performance, but responsiveness and memory use benefits are obvious, and the fact that we already have UI for this pref mitigates the potential negative impact somewhat. I still wish we had better data about the relative size of the audience that this will benefit (session size telemetry), though.

Comment 41

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Comment on attachment 589911 [details] [diff] [review]
On top of bug 718484

Ok, now that we've gotten the go ahead...

>diff --git a/browser/base/content/test/browser_tabMatchesInAwesomebar.js b/browser/base/content/test/browser_tabMatchesInAwesomebar.js

Paolo: you shouldn't need to change this test file. The test doesn't use session restore. Can you remove that and push to try one more time? r+ with the assumption it's green.

Comment 42

[:Paolo Amadini]

(In reply to Paul O'Shannessy [:zpao] from comment #41)
> >diff --git a/browser/base/content/test/browser_tabMatchesInAwesomebar.js b/browser/base/content/test/browser_tabMatchesInAwesomebar.js
> 
> Paolo: you shouldn't need to change this test file. The test doesn't use
> session restore.

It uses the private browsing service though, which in turn uses session restore.

> Can you remove that and push to try one more time?

Well, without the change the test failed locally, when I tested it some time ago.
Thus now I'd expect it to fail on try also, unless something has changed in the
meantime. I can do another local run after I do a global rebuild.

Back in comment 21 I suggested that instead of switching the preference we could
change the test to avoid waiting for tab loads, but we should touch the test in
any case.

Comment 43

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

(In reply to Paolo Amadini from comment #42)
> (In reply to Paul O'Shannessy [:zpao] from comment #41)
> > >diff --git a/browser/base/content/test/browser_tabMatchesInAwesomebar.js b/browser/base/content/test/browser_tabMatchesInAwesomebar.js
> > 
> > Paolo: you shouldn't need to change this test file. The test doesn't use
> > session restore.
> 
> It uses the private browsing service though, which in turn uses session
> restore.

Ah, I missed that :( My mistake. Carry on!

Comment 44

[(dormant account)]

Comment on attachment 589911 [details] [diff] [review]
On top of bug 718484

Is there work that remains before this patch can land?

Comment 45

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

Paolo, are you going to land this?

Comment 46

[:Paolo Amadini]

(In reply to Gavin Sharp (use [e-mail] for email) from comment #45)
> Paolo, are you going to land this?

Yes, the tryserver build already finished and I'll land this tomorrow.

Comment 47

[:Paolo Amadini]

https://hg.mozilla.org/integration/mozilla-inbound/rev/efa3c85f1f37

This includes the preference switch, and making the tests robust to the change.
In case we want to switch off the default preference again, it should be enough
to change firefox.js, as the tests should continue to work without modifications.

Comment 48

[Phil Ringnalda (:philor)]

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

Comment 49

[Siddhartha Dugar [:sdrocking]]

https://wiki.mozilla.org/Firefox/Roadmap#Q2 also mentions about enabling session restore by default. Is this planned for anytime soon? Is there a tracking bug?

Comment 50

[Lars Viklund]

This feature has significant downsides.

One is that there are several sites that communicate relevant information in page titles, like Google's Reader and Mail. The tab titles for those pages will be misleading as the page is actually not loaded and requires a manual focus every time you start the browser.

Another downside is that you get up to seconds of white background when you change to a tab, even though your browser has been up and running for a long while.

Sure, there may be potential benefits in startup time, but this feature has some severe usability issues.

I consider it a bug that tabs I had open never load on resume.

Comment 51

[Asa Dotzler [:asa]]

(In reply to Lars Viklund from comment #50)

> I consider it a bug that tabs I had open never load on resume.

You're in luck. We've provided you a preference switch.

Comment 52

[Gian-Carlo Pascutto [:gcp]]

Do we have follow-up bugs for trying to be smart about which tabs to restore in the background (after we've started up)?

Bug 586067 seems to be the one to collect this information.

If we don't do this, we'll have the unfortunate effect that this bug which is to make us "feel fast like Chrome" will really just make the browser more sluggish when switching tabs.

Comment 53

[Jim Jeffery not reading bug-mail 1/2/11]

This patch needs a telemetry probe to see how many folks 'revert' the change via the pref provided.  I suspect this change will only apply to those users that use massive amounts of tabs, and will be more an inconvenience and a perceived slow-down in performance when the user switches tabs and has to wait for the page the reload.

Comment 54

[Gian-Carlo Pascutto [:gcp]]

By the way, does this affect Live Bookmarks? If so, was that intentional?

Comment 55

[Dão Gottwald [:dao]]

(In reply to Gian-Carlo Pascutto (:gcp) from comment #54)
> By the way, does this affect Live Bookmarks? If so, was that intentional?

You're looking for bug 613588.

Comment 56

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

(In reply to Lars Viklund from comment #50)
> One is that there are several sites that communicate relevant information in
> page titles, like Google's Reader and Mail. The tab titles for those pages
> will be misleading as the page is actually not loaded and requires a manual
> focus every time you start the browser.

App tabs are always loaded, regardless of this pref's value, which should mitigate this somewhat (if those web apps are loaded in app tabs, they won't be affected). But I agree that this is a problem otherwise, yes.

> Another downside is that you get up to seconds of white background when you
> change to a tab, even though your browser has been up and running for a long
> while.

This was a known issue, and we decided that the trade-off is worth it. We might revisit that change in light of additional feedback from a larger testing audience.

Comment 57

[Lukas Blakk [:lsblakk] use ?needinfo]

[Triage Comment]
Let's track any bugs from any fallout, but no need to track this bug specifically.

Comment 58

[Mihaela Velimiroviciu (:mihaelav)]

This was verified during Tab on demand feature sign-of on Firefox 13 beta 1 builds across main platforms (Win 7 32 bit, Mac OS X 10.6.8, Ubuntu 11.10 32 bit).
Marking bug accordingly.

Comment 60

[Daniel Kirkdorffer]

(In reply to :Gavin Sharp (use [e-mail] for email) from comment #40)
> Comment on attachment 589911 [details] [diff] [review]
> On top of bug 718484
> 
> [...] I still wish we had better data about the relative size of
> the audience that this will benefit (session size telemetry), though.

FWIW, I typically always have about 40 tabs open.

My wife currently has over 650!

Believe me when I say that the "Don't load tabs until selected" option we've been enjoying relatively recently is a life-saver on reloads.  It also helps deal with memory usage bloat problems as I can pretty much kill my Firefox process and restart it to quickly reset things.

Comment 61

[boomman37]

Is there a way to disable this behavior in FF 13?

I experience constant crashes that are likely caused by Win8 buggy beta AMD drivers and having to click on every tab to revive its javascript is too much trouble.
Also, the way it reloads the page (slow and with hourglass) makes you think it downloads it from server, but no, it just gets page from local cache.

Comment 62

[Alex Shilov]

This option turn on by Default. this mean that you can easily uncheck it. 
in ff12: Open Settings > General - and there it is
in ff15: Open Settings > Tabs - its moved here.

Comment 63

[Mardeg]

(In reply to mr.troll from comment #62)
> This option turn on by Default. this mean that you can easily uncheck it. 
> in ff12: Open Settings > General - and there it is
> in ff15: Open Settings > Tabs - its moved here.

and in about:config it's always been the pref named:

browser.sessionstore.restore_on_demand

Comment 64

[User Dderss]

http://wiki.mozilla.org/Firefox/Roadmap cites this as "partially" completed, and the "partiality" of it is true.

If user have several tab groups, and over quantity of tabs is significant, FireFox takes too much space in memory, as well as loads slower than needed. "Don't load tabs until selected" will be fully implemented only when FireFox will *not* unfold whole SessionStore.JSON object in the memory when the browser starts.

SessionStore.JSONs should unfold in memory data on only specific tabs that you clicked on. This will speed up start of the browser, as well as dramatically cut memory usage. "Just started" state of FireFox already takes no less than half gigabyte in memory, with almost all of it completely unneeded. 

For speedier access to often-used tabs FF could save a list of most used tabs and unfold SessionStore.js data only on them.

Comment 65

[Nicolas Barbulesco]

(In reply to User Dderss from comment #64)

> "Just started" state of FireFox already takes
> no less than half gigabyte in memory, with almost all of it completely
> unneeded. 

Even more ! This morning, I had my Firefox (25) taking 1.3 GB just after restoring the session. And I had cut the wi-fi, so well that all my front tabs were "Address not found". So there was in fact zero tab loaded. So much memory for this is too much.

Comment 66

[Daniel Kirkdorffer]

Both of the past commenters make valid points.  On tab load should mean on tab load.  The amount of memory use when supposedly only one tab has been loaded is way too much.  The session store data need not be loaded all on startup.

Comment 67

[Gian-Carlo Pascutto [:gcp]]

There's no point whatsoever in having discussions about the problems with session restore in this bug, which was closed as verified+fixed 1.5 years ago. File new bugs (if they don't already exist, which'd surprise me) about improving the session restore mechanism instead.

Comment 68

[User Dderss]

Gian-Carlo, there is point discussing it here since Mozilla's official road map routes "click-to-load" feature exactly to this bug, and states that the status of this feature's implementation is partial.

If there is a new bug that would target covering the rest of the feature (not unfolding SessionStore object on tabs which are not loaded) -- then please add it to Mozilla wiki so everyone could see whether the company actually plans to fulfils the plan from the road map. For now, this is the only bug that relates to the issue as far as http://wiki.mozilla.org/Firefox/Roadmap concerned, and it correctly states the status as "partial" due to incredible blow up of memory usage in the state where no tabs yet loaded. I can not edit Mozilla's wiki, please help with that.

Comment 69

[Daniel Holbert [:dholbert]]

(In reply to User Dderss from comment #68)
> Gian-Carlo, there is point discussing it here since Mozilla's official road
> map routes "click-to-load" feature exactly to this bug

(Note that the "official roadmap" you link to is currently titled "Firefox 2012 Strategy & Roadmap", and it hasn't had any substantial changes in 1.5 years.  That likely explains why it links to this bug that was closed 1.5 years ago.)

Comment 70

[User Dderss]

Thanks for clarification, Daniel. Then where is new road map? Was this feature included there to be *actually* *fully* implemented versus "partially implemented" status as it is now? There has to be official assignment and the plan/road map for that. Mozilla could not just abandon and drop its own road map on this, right? (Or could it?)

Comment 71

[David Teller [:Yoric] - still alive but not very active]

The redesign of sessionstore.js is here: bug 810932.