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)

All
Android
defect
Not set
normal

Tracking

(firefox23 affected, firefox24 affected, firefox25 verified)

VERIFIED FIXED
Firefox 25
Tracking Status
firefox23 --- affected
firefox24 --- affected
firefox25 --- verified

People

(Reporter: bnicholson, Assigned: bnicholson)

Details

Attachments

(1 file, 1 obsolete file)

Currently, updating Firefox closes all tabs as if Firefox was force closed. We should always restore the session after an update to minimize interruptions.
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)
Status: NEW → ASSIGNED
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+
(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).
Attachment #770387 - Attachment is obsolete: true
Attachment #770652 - Flags: review?(mark.finkle)
Attachment #770652 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/7211a160f688
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
Verified fixed on:
Build: Firefox for Android 25.0a1(2013-07-06)
Device: Samsung Galaxy Nexus
OS: Android 4.1.1
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: