Closed
Bug 889554
Opened 11 years ago
Closed 11 years ago
Restore session after Firefox has been updated
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox23 affected, firefox24 affected, firefox25 verified)
VERIFIED
FIXED
Firefox 25
People
(Reporter: bnicholson, Assigned: bnicholson)
Details
Attachments
(1 file, 1 obsolete file)
9.12 KB,
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
Currently, updating Firefox closes all tabs as if Firefox was force closed. We should always restore the session after an update to minimize interruptions.
Assignee | ||
Comment 1•11 years ago
|
||
This detects an upgrade by saving the version code to shared prefs and comparing versions at startup. I didn't think this would work at first because I didn't think Nightly builds always had new version codes, but it looks like they do. I changed RESTORE_OOM to RESTORE_NORMAL since this behavior is now used for more than just "OOM" (AKA, being killed in the background by Android).
Attachment #770387 -
Flags: review?(mark.finkle)
Assignee | ||
Updated•11 years ago
|
Status: NEW → ASSIGNED
Comment 2•11 years ago
|
||
Comment on attachment 770387 [details] [diff] [review] Save session between app upgrades >diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java >- restoreData.put("restoringOOM", mRestoreMode == RESTORE_OOM); >+ restoreData.put("restoringOOM", mRestoreMode == RESTORE_NORMAL); Not renaming "restoringOOM"? If it still makes sense, then I'm cool with keeping it as is. >- if (getProfile().shouldRestoreSession()) { >- return RESTORE_CRASH; >- } >+ restoreMode = RESTORE_CRASH; Why don't we need the getProfile check anymore? r+, but comment about the getProfile bit
Attachment #770387 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 3•11 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #2) > Comment on attachment 770387 [details] [diff] [review] > Save session between app upgrades > > >diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java > > >- restoreData.put("restoringOOM", mRestoreMode == RESTORE_OOM); > >+ restoreData.put("restoringOOM", mRestoreMode == RESTORE_NORMAL); > > Not renaming "restoringOOM"? If it still makes sense, then I'm cool with > keeping it as is. I guess I should change it since it's no longer only about OOM restores, which means I should probably also change similar variables in browser.js/SessionStore.js. > >- if (getProfile().shouldRestoreSession()) { > >- return RESTORE_CRASH; > >- } > >+ restoreMode = RESTORE_CRASH; > > Why don't we need the getProfile check anymore? Right...forgot to mention this. sessionstore.js will almost always exist, and for the unusual situations where it doesn't, restoreSessionTabs() will fail and we'll continue normally here: http://hg.mozilla.org/mozilla-central/file/b48e06621dc9/mobile/android/base/GeckoApp.java#l1404. So having this check here is unnecessary since it doesn't alter the behavior of anything, and it means we can get rid of the shouldRestoreSession() method in GeckoProfile (which I forgot to do -- I'll update the patch).
Assignee | ||
Comment 4•11 years ago
|
||
Attachment #770387 -
Attachment is obsolete: true
Attachment #770652 -
Flags: review?(mark.finkle)
Updated•11 years ago
|
Attachment #770652 -
Flags: review?(mark.finkle) → review+
Assignee | ||
Comment 5•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7211a160f688
Comment 6•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7211a160f688
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
Comment 7•11 years ago
|
||
Verified fixed on: Build: Firefox for Android 25.0a1(2013-07-06) Device: Samsung Galaxy Nexus OS: Android 4.1.1
Updated•11 years ago
|
Status: RESOLVED → VERIFIED
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•