Build: Firefox 23.0b8 07/23 Device: LG Nexus 4, Galaxy Nexus (4.2.2) Steps to reproduce: 1. Install app - Open it 2. Wait to get notification about data & stats 3. Swipe off Nightly to remove it from the foreground 4. Access data & stats from the notification shortcut 5. Press back (both from upper left corner or device's back) Expected Results: - You should be back in about:home Actual Results: - The page is very slow, tapping is almost not recognized etc Note: - See video here www.youtube.com/watch?v=KbpYke1vs1U - Not reproducible on other devices that we have in the office - You are not able to run the app until you clean data from Device's Settings. Might be Regression from Bug 873072

> - You are not able to run the app until you clean data from Device's > Settings. And kill the app. Opening it from recently used apps will open the settings and you will find yourself in the same loop.

> Might be Regression from Bug 873072 Confirmed. Patch from Bug 973072 c#8 produced this https://hg.mozilla.org/mozilla-central/rev/91cc9c75c3d1

we'll track a backout of bug 873072 for FF23 and hope for a forward fix in FF24 if possible, otherwise backout again.

I was only able to repro this on a Galaxy Nexus if I checked the developer option "Don't keep activities." Is that setting selected on the phones you're using, Ioana? If so, I think I can put together a quick fix for this. Since this is a notoriously problematic developer option that most users shouldn't even have turned on, I would really prefer to land and uplift a fix w/o backing out the original patch. Lukas, I'll add the backout patch to bug 873072 but I'll also try to get a patch for the developer options case tonight.

This adds one line of handling if GeckoApp has been killed from under Settings.

Ioana: Please try out the build at http://people.mozilla.org/~liuche/bug-896992/ Let me know if you still see the problem on your devices.

(In reply to Chenxia Liu [:liuche] from comment #8) > Ioana: Please try out the build at > http://people.mozilla.org/~liuche/bug-896992/ The build works fine on both LG Nexus 4 and Galaxy Nexus (also in both case - with & without Keep activity checked in Developer options. )

Comment on attachment 780811 [details] [diff] [review] Beta patch: handle restoring GeckoApp Review of attachment 780811 [details] [diff] [review]: ----------------------------------------------------------------- So if I'm understanding the code correctly, these checks will prevent the data notification settings pane from opening in some cases even when it should. I tried the following scenario with the APK you posted a couple of comments above: 1. Set "Don't keep activities" in the options 2. Start Fennec 3. Open the awesome bar (this will destroy GeckoApp) 4. Close the awesome bar (this will open GeckoApp with mIsRestoringActivity true) 5. Open the data notification from the android notification bar These steps will run through the "onNewIntent" codepath while mIsRestoringActivity is set to true, and so the data notification thing won't open when in fact it should. Here's another scenario: 1. Set "Don't keep activities" in the options 2. Start Fennec 3. Put Fennec in the background (GeckoApp will be killed) 4. Open the data notification from the android notification bar This will run through the initialize() code path with mIsRestoringActivity set to true, and so will not open the data notification pane. I'm not entirely sure what caused the original problem - is that when Android re-creates the GeckoApp activity, it re-delivers the intent to it? If so, maybe a better solution is to either null out the intent in GeckoApp or if that doesn't work, add a flag on the intent saying that it has been processed and check that instead of mIsRestoringActivity. ::: mobile/android/base/GeckoApp.java @@ +1973,5 @@ > String uri = getURIFromIntent(intent); > GeckoAppShell.sendEventToGecko(GeckoEvent.createURILoadEvent(uri)); > } else if (ACTION_ALERT_CALLBACK.equals(action)) { > processAlertCallback(intent); > + } if (ACTION_LAUNCH_SETTINGS.equals(action) && !mIsRestoringActivity) { Also: This is missing an "else" between the } and the "if"

This sets a flag to note when saving state that we have already launched settings and should clear that action. The apk is the beta2.apk at http://people.mozilla.com/~liuche/bug-896992/

Comment on attachment 781060 [details] [diff] [review] Patch: Beta patch: handle settings intent with flag v2 Review of attachment 781060 [details] [diff] [review]: ----------------------------------------------------------------- This looks ok to me. That being said I would advise against uplifting this to beta this late in the cycle unless you're sure you've tested every possible scenario. Patches that touch our startup codepath have a history of breaking things subtly because our startup is pretty brittle. Sometimes even adding innocuous lines of code causes a previously latent race condition to trigger.

Triage agrees with Kats. Let's only uplift this to Fx24.

This patch fixes the looping by adding flag to be tracked until GeckoApp is properly destroyed, but there seems to be some padding issue with the recreated GeckoApp, where the urlbar overlaps the content.

Lukas, should I remove tracking for 23 since we decided we're not going to uplift to beta at this late in the stage, and is there anything else I need to do since 23 is going to ship with this bug?

Test apk at: http://people.mozilla.org/~liuche/bug-896992/no-loop-nightly.apk Try build: https://tbpl.mozilla.org/?tree=Try&rev=796b5f4befbf

Leaving this tracked for FF24 in order to get a forward fix that doesn't bring back bug 873072

Comment on attachment 781377 [details] [diff] [review] Patch: fix looping with flag Review of attachment 781377 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. Some nits below but I don't feel strongly about them. ::: mobile/android/base/GeckoApp.java @@ +135,5 @@ > public static final String ACTION_BOOKMARK = "org.mozilla.gecko.BOOKMARK"; > public static final String ACTION_LOAD = "org.mozilla.gecko.LOAD"; > public static final String ACTION_LAUNCH_SETTINGS = "org.mozilla.gecko.SETTINGS"; > public static final String ACTION_INIT_PW = "org.mozilla.gecko.INIT_PW"; > + public static final String SAVED_STATE_ACTION_MAIN = "setActionMain"; nit: I would rename this SAVED_STATE_INTENT_HANDLED for consistency with the boolean variable. Change the value likewise. @@ +160,5 @@ > private static GeckoThread sGeckoThread; > private GeckoProfile mProfile; > public static int mOrientation; > protected boolean mIsRestoringActivity; > + private boolean mIntentHandled = false; No need to initialize this; boolean fields are initialized to false by default. @@ +637,5 @@ > > outState.putBoolean(SAVED_STATE_IN_BACKGROUND, isApplicationInBackground()); > outState.putString(SAVED_STATE_PRIVATE_SESSION, mPrivateBrowsingSession); > + > + // Bug 896992 - Replace intent action with ACTION_MAIN on restart. nit: trailing whitespace

[Approval Request Comment] Bug caused by (feature/regressing bug #): 873072 User impact if declined: Devices with low memory or "Don't keep activities" dev option turned on will be looped infinitely to the settings screen if they tap on the datareporting notification if Fennec has been killed Testing completed (on m-c, etc.): Manually tested, try build: https://tbpl.mozilla.org/?tree=Try&rev=9194ed28deaf Risk to taking this patch (and alternatives if risky): low risk, adding a single flag to startup code to be tracked between restarts String or IDL/UUID changes made by this patch: none

Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/dd1908156c2f

Landed in Aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/f06b3090f197

Target milestone tracks m-c landing. Tracking flags track branch uplifts.