Closed Bug 996590 Opened 7 years ago Closed 6 years ago

Tabs are not restored if Firefox is closed with QuitNow addon

Categories

(Firefox for Android :: General, defect)

29 Branch
All
Android
defect
Not set
normal

Tracking

()

RESOLVED DUPLICATE of bug 995138
Tracking Status
firefox28 --- unaffected
firefox29 --- affected
firefox30 --- affected
firefox31 --- affected
firefox32 --- affected
firefox33 --- affected
fennec + ---

People

(Reporter: u421692, Assigned: bnicholson)

Details

(Keywords: regression)

Environment:
Build: Firefox 29 Beta 8, Nightly 31.0a1(2014-04-14)
Device: Alcatel One Touch 8008D(Android 4.1.2)/ASUS Transformer Pad TF300T (Android 4.1.1)/ZTE Grand X IN (Android 4.0.4) 

Steps to reproduce:
1. Open Menu->Settings->Customize->Tabs and select "Always restore"
2. Install QuitNow addon https://addons.mozilla.org/en-US/android/addon/quitnow/
3. Have some tabs opened
4. Open Menu->Quit
5. Open Firefox

Expected result:
Tabs are restored.

Actual result:
Tabs are not restored.

I see the following error in the logcat:
E/GeckoConsole(12505): [JavaScript Error: "privateData.windows[0] is undefined" {file: "jar:jar:file:///data/app/org.mozilla.firefox_beta-1.apk!/assets/omni.ja!/components/SessionStore.js" line: 420}]
This won't be specific to QuitNow ... the addon functions by issuing the same "Browser:Quit" message generated by the original MainMenu option ... while mostly disabled, we still provide it for users with pre-ICS or "tv" devices.

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java?rev=10e08afa522a#2168
Regression window:

Last good revision: dedf12c4e805 (2014-02-07)
Last good revision: cafe909f7e07 (2014-02-08)

Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dedf12c4e805&tochange=cafe909f7e07
tracking-fennec: --- → ?
Without looking at inbound builds, I don't see anything really standing out from that range.
Brian - Is this related to the Guest Browsing issue too?
Assignee: nobody → bnicholson
tracking-fennec: ? → +
(In reply to Mark Capella [:capella] from comment #2)
> This won't be specific to QuitNow ... the addon functions by issuing the
> same "Browser:Quit" message generated by the original MainMenu option ...
> while mostly disabled, we still provide it for users with pre-ICS or "tv"
> devices.
> 
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.
> java?rev=10e08afa522a#2168

The Quit option is hidden in latest Nightly, Firefox 32 is unaffected. Should I close this issue as WFM, since it's not a najor issue,or should we keep tracking for affected versions?
Flags: needinfo?(mark.finkle)
Flags: needinfo?(aaron.train)
Flags: needinfo?(aaron.train)
I am not too worried about issues that happen because of QuitNow specifically, but I was worried that this might be a deeper bug. It may be that QuitNow is just exposing a dormant bug, which should be fixed.

That's why I want Brian to take a look. The JS error could be part of the issue too.
Flags: needinfo?(mark.finkle) → needinfo?(bnicholson)
2014-02-06 - good
2014-02-07 - bad
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1e9f169c9715&tochange=dedf12c4e805

After bisecting I've found the regression:
The first bad revision is:
changeset:   167183:bf45888656ad
user:        Brian Nicholson <bnicholson@mozilla.com>
date:        Wed Feb 05 23:47:51 2014 -0800
summary:     Bug 965137 - Synchronously write session on application-background. r=mfinkle
(In reply to Mihai Pop from comment #6)
> The Quit option is hidden in latest Nightly, Firefox 32 is unaffected.
> Should I close this issue as WFM, since it's not a najor issue,or should we
> keep tracking for affected versions?

Not sure what you mean by Quit being hidden -- I just installed QuitNow on the latest Nightly and see the Quit button.
Flags: needinfo?(bnicholson)
(In reply to Mark Finkle (:mfinkle) from comment #5)
> Brian - Is this related to the Guest Browsing issue too?

Yeah, this is essentially a dupe of bug 995138 for the reasons described in https://bugzilla.mozilla.org/show_bug.cgi?id=995138#c1. Guest mode and Quit both use System.exit(), so we don't flush anything to disk.

Bug 965137 made things worse by also relying on flushing to disk in onPause, but the session isn't the only thing affected; prefs and the disk cache can also be lost if we don't quit cleanly. The fix for these issues is to make sure our System.exit() codepath(s) first send an application-background event to Gecko.

(In reply to Mihai Pop from comment #0)
> I see the following error in the logcat:
> E/GeckoConsole(12505): [JavaScript Error: "privateData.windows[0] is
> undefined" {file:
> "jar:jar:file:///data/app/org.mozilla.firefox_beta-1.apk!/assets/omni.ja!/
> components/SessionStore.js" line: 420}]

I only see this error when I hit Quit shortly after opening Fennec. I also see this in browser.js:
E/GeckoConsole( 7732): [JavaScript Error: "win is undefined" {file: "chrome://browser/content/browser.js" line: 3380}]

If we trigger Quit very early during startup, we might not get a handle on a browser window. I don't think this error is much concern.
(In reply to Brian Nicholson (:bnicholson) from comment #9)
> (In reply to Mihai Pop from comment #6)
> > The Quit option is hidden in latest Nightly, Firefox 32 is unaffected.
> > Should I close this issue as WFM, since it's not a najor issue,or should we
> > keep tracking for affected versions?
> 
> Not sure what you mean by Quit being hidden -- I just installed QuitNow on
> the latest Nightly and see the Quit button.

The Quit button is not displayed on ICS+ devices:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java?rev=10e08afa522a#2168

On pre-ICS devices the behaviour is the same, and there are two Quit buttons(the default one, and QuitNow add-on button), so I don't know if this add-on is useful anymore.
(In reply to Mihai Pop from comment #11)
> The Quit button is not displayed on ICS+ devices:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.
> java?rev=10e08afa522a#2168

Yes, we removed the Quit button for ICS+ way back in bug 800071 (Fx20). That's the reason this add-on is useful -- it adds the button back for people who want it in ICS+. QuitNow isn't meant to be used on pre-ICS since those devices already show the Quit button.
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 995138
You need to log in before you can comment on or make changes to this bug.