995138 - Switching between normal and guest browsing doesn't keep the correct number of tabs open

Status: RESOLVED INCOMPLETE

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

Description

[Teodora Vermesan (:TeoVermesan)]

Device: LG Nexus 4 (Android 4.4.2)
Build: Firefox for Android 31.0a1 (2014-04-10)
Steps to reproduce:
1. Open 3 tabs with 3 websites
2. Go to Tools -> New Guest Session
3. Exit Guest Session
4, Close 2 tabs, to have only one tab left
5. Go back to Tools -> New Guest Session
6. Exit Guest Session

Expected results: Only one tab is in the tab tray
Actual: There are 3 tabs opened. (those opened at step 1)

Comment 1

[Brian Nicholson (:bnicholson)]

Testing this a bit, I see that I lose other changes as well if I do them quickly before starting a guest session, such as opening a URL. When the browser is closed, normally we go through onPause, which triggers a state save: http://mxr.mozilla.org/mozilla-central/source/mobile/android/components/SessionStore.js#152. Guest mode kills us in the foreground by doing System.exit(), so we won't go through this code path. We'll probably need to add extra logic for guest mode to make sure things get written out before we kill Gecko.

I imagine other components that rely on backgrounding are broken by guest mode too, such as prefs or the disk cache.

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

This reminds me of bug 947606, where we don't save state quickly when opening new tabs from another app and thus lose the opened tabs.

Comment 4

[Brian Nicholson (:bnicholson)]

Attachment: Send application-background event before quitting

SessionStore listens for application-background to do the flushing, so that's the event we should send.

The disk cache/prefs are also flushed when the app is backgrounded, but those are handled directly in AndroidGeckoEvent::APP_BACKGROUNDING, meaning they won't respond to the application-background event: http://mxr.mozilla.org/mozilla-central/source/widget/android/nsAppShell.cpp#332. We should move that code into an application-background listener so the disk cache and prefs don't get lost in these same situations. Filed bug 1050578.

Comment 5

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

Comment on attachment 8469686 [details] [diff] [review]
Send application-background event before quitting

>diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java

>     public void doRestart(String action, String args) {
>+        GeckoAppShell.sendEventToGecko(GeckoEvent.createBroadcastEvent("application-background", null));

This should become "diskcache-shutdown" (or whatever you end up with in bug 1050578.

>diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js

>       Services.obs.notifyObservers(null, "browser-lastwindow-close-granted", null);
>+      Services.obs.notifyObservers(null, "application-background", null);

Do we really need this one? Doesn't Gecko handle this as a normal shutdown and shutdown the diskcache normally? If not, then OK, add a notifyObservers for "diskcache-shutdown".

Comment 6

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

filter on [mass-p5]

Comment 7

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.