I was all set to admire the improvements to thumbnailing on about:home, but about a second after it displayed, I was whisked away into my session restored tabs. If we're restoring session, we shouldn't load/display about:home. Tested on Jan-18 aurora

Has there been any UX discussion around when we do session restore, as opposed to just showing "Tabs From Last Time" on about:home? I haven't been paying complete attention to these features, but it seems like if session restore is perceptibly slow, we'd be better off taking advantage the instant-ness of about:home.

From bug 712970, about the "Tabs from Last Time" in about:home: "- this is only when we're not restoring tabs automatically, because the user explicitly quit or we crashed on startup multiple times (the "well, this is embarrassing" case)"

Seeing this on restart after nightly update on 1-23 Aurora.

Certainly, we shouldn't show you the start page for a second or two only to whisk you to something else. This is very very distracting. The spec cited in comment 2 about using session restore unless the browser has crashed or was quit -- this assumes a fast session restore. If it's going to be very slow, then perhaps we should not do it, and use the tabs list instead.

Checks to see if sessionstore.bak exists. If it does, this means are doing a session restore, so don't immediately show about:home.

(In reply to Brian Nicholson (:bnicholson) from comment #5) > Created attachment 592001 [details] [diff] [review] > patch > > Checks to see if sessionstore.bak exists. If it does, this means are doing a > session restore, so don't immediately show about:home. Isn't the exists() call likely to cause startup regressions? As part of bug 721008, I'm working on a patch that allows us to figure out if we want to restore a session without necessarily having to touch the sessionstore.bak file by just adding some info to the bundle. Maybe wait to see how we can combine those things?

Comment on attachment 592001 [details] [diff] [review] patch Review of attachment 592001 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/GeckoApp.java @@ +1498,5 @@ > > + if (mLastUri == null || mLastUri.equals("") || mLastUri.equals("about:home")) { > + // show about:home if we aren't restoring previous session > + File profileDir = getProfileDir(); > + if (profileDir == null || !new File(profileDir, "sessionstore.bak").exists()) This will conflict a bit with SessionRestore.js's sessionstore.bak expiration code. IIUC, it checks the last modified date of sessionstore.js to decide if it will actually restore session. See: http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/SessionStore.js#103 So, if the sessionstore.bak is expired and exists, you'll avoid showing about:home unnecessarily. In this case, about:home will only be shown once Gecko starts and explicitly loads the about:home URL. Not something that will happen all the time but definitely something to keep in mind.

(In reply to Lucas Rocha (:lucasr) from comment #6) > Isn't the exists() call likely to cause startup regressions? As part of bug > 721008, I'm working on a patch that allows us to figure out if we want to > restore a session without necessarily having to touch the sessionstore.bak > file by just adding some info to the bundle. Maybe wait to see how we can > combine those things? How is that possible? There's two kinds of cases where we initiate the session restore: 1) We are killed by android OOM. This is already handled by the session restore boolean we save in the bundle and pass along to GeckoThread. 2) We crash. As far as I'm aware, there's no way to detect this on the Java side without either communicating with Gecko or checking for the existence of the sessionstore.bak/js file. How would you detect case #2 with a bundle?

(In reply to Lucas Rocha (:lucasr) from comment #7) > Comment on attachment 592001 [details] [diff] [review] > patch > > Review of attachment 592001 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: mobile/android/base/GeckoApp.java > @@ +1498,5 @@ > > > > + if (mLastUri == null || mLastUri.equals("") || mLastUri.equals("about:home")) { > > + // show about:home if we aren't restoring previous session > > + File profileDir = getProfileDir(); > > + if (profileDir == null || !new File(profileDir, "sessionstore.bak").exists()) > > This will conflict a bit with SessionRestore.js's sessionstore.bak > expiration code. IIUC, it checks the last modified date of sessionstore.js > to decide if it will actually restore session. See: > > http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/ > SessionStore.js#103 > > So, if the sessionstore.bak is expired and exists, you'll avoid showing > about:home unnecessarily. In this case, about:home will only be shown once > Gecko starts and explicitly loads the about:home URL. > > Not something that will happen all the time but definitely something to keep > in mind. Yep, I did see this case. It should be exceptionally rare though: a crash (not an OOM kill), followed by an hour or more of not restoring Fennec. The result is just a delayed (1-2 seconds) about:home screen. IMO, this isn't something to worry about optimizing - right?

(In reply to Brian Nicholson (:bnicholson) from comment #9) > > Not something that will happen all the time but definitely something to keep > > in mind. > > Yep, I did see this case. It should be exceptionally rare though: a crash > (not an OOM kill), followed by an hour or more of not restoring Fennec. The > result is just a delayed (1-2 seconds) about:home screen. IMO, this isn't > something to worry about optimizing - right? If it becomes common, we'll see a bug and need to fix it. We can read the sessionstore.bak file date, or check for exists, in a background thread while loading Java app, in onCreate or somewwhere, too. That would allow Java to know exactly what the Gecko session restore is when we start.

Actually, it'll be a bit more common than I suggested because of task killers. I don't know whether we should optimize for task killers or not since users really shouldn't be using them - we even have a Quit button.

Landed on inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/9c391570dbe1 As per Doug's recommendation, I included zerdatime log messages before/after the exists() check.

Brian, request aurora approval

Comment on attachment 592001 [details] [diff] [review] patch [Triage Comment] This missed fx11, which is now beta

fyi was going to land this: Justin@ORION /d/sources/mozilla-beta $ hg qpush applying abouthome_sessionrestore.patch patching file mobile/android/base/GeckoApp.java Hunk #1 FAILED at 1446 Hunk #2 FAILED at 1488 2 out of 2 hunks FAILED -- saving rejects to file mobile/android/base/GeckoApp.java.rej patch failed, unable to continue (try -v) patch failed, rejects left in working dir errors during apply, please fix and refresh abouthome_sessionrestore.patch

Comment on attachment 592001 [details] [diff] [review] patch Clearing approval for Aurora 12 and Beta 11 because we are not currently planning a Native Fennec release of these versions. If this changes in the future, we will likely do a mass uplift of all native fennec changes. For now, let's get these bugs off the channel triage radar. [Filter on the string "mbrubeck-bugspam" if you want to delete all of these emails at once.]

Verified fixed on: Firefox 13.0a1 (2012-02-28) 20120228031102 http://hg.mozilla.org/mozilla-central/rev/7ce4d9b55863 -- Device: Samsung Galaxy S2 OS: Android 2.3.4