721577 - Race condition in about:home for tabs from last time

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P3)

Description

[Brian Nicholson (:bnicholson)]

About:home reads the previous session JSON from sessionstore.js, but sessionstore.js may contain the current session if about:home reads it too late. This isn't much of a problem right now since we always show about:home immediately (meaning sessionstore.js is read before anything is written to it), but this will change when bug 719195 is fixed.

Comment 1

[Brian Nicholson (:bnicholson)]

Attachment: patch

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

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

This looks OK, but I wanted to pitch another thought too: Could we check to see if sessionstore.bak exists and use it. If it doesn't, we could use sessionstore.js ?

The only reason I might like that better is we would be using real file existence as a check, not gecko being ready flag - a flag which does not exactly match up on the session store state.

Thoughts?

Comment 3

[Brian Nicholson (:bnicholson)]

(In reply to Mark Finkle (:mfinkle) from comment #2)
> Comment on attachment 592000 [details] [diff] [review]
> patch
> 
> This looks OK, but I wanted to pitch another thought too: Could we check to
> see if sessionstore.bak exists and use it. If it doesn't, we could use
> sessionstore.js ?
> 
> The only reason I might like that better is we would be using real file
> existence as a check, not gecko being ready flag - a flag which does not
> exactly match up on the session store state.
> 
> Thoughts?

I originally wanted to do this too, but unfortunately, I don't think that will work. It's possible for sessionstore.bak to have stale data from 2 sessions ago. Consider this sequence:

- have tab google.com open; google.com JSON written to sessionstore.js
- quit
- reopen fennec; google.com JSON moved to sessionstore.bak at session store init
- visit tab cnn.com; cnn.com JSON written to sessionstore.js
- browser crashes
- sessionstore.bak contains google.com JSON rather than cnn.com JSON

Comment 4

[Brian Nicholson (:bnicholson)]

Oops - comment collision accidentally killed your flags.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

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

OK, lets see where this gets us

Comment 6

[Brian Nicholson (:bnicholson)]

Landed on inbound:
http://hg.mozilla.org/integration/mozilla-inbound/rev/04e40f6991f6

Comment 7

[Joe Drew (not getting mail)]

https://hg.mozilla.org/mozilla-central/rev/04e40f6991f6

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

Brian, please request approval for aurora

Comment 9

[Alex Keybl [:akeybl]]

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

[Triage Comment]
Mobile only - approved for Beta 11.

Comment 10

[Brad Lassey [:blassey] (use needinfo?)]

https://hg.mozilla.org/releases/mozilla-beta/rev/cac5504d4637