Nightly 25.0a1 2013-07-28 Asus EEE Transformer TF101 (Android 4.0.4) Steps to reproduce: 1) Set the "Don't keep activities" developer option to ON 2) Open Firefox and load any website 3) Minimize the app 4) Open a link from another app (Twitter, Gmail app) using Firefox Expected result: The website is loaded in a new tab Actual result: No new tabs are created and the website is never loaded

If we're already running, we skip over the call to loadStartupTab(), but we shouldn't do that if we're given a new URL to load.

Comment on attachment 784425 [details] [diff] [review] Always open a startup tab if given a URL Add a comment above the "if" block: // External URLs should always be loaded regardless of how we are restoring (or something like that)

Comment on attachment 784425 [details] [diff] [review] Always open a startup tab if given a URL Review of attachment 784425 [details] [diff] [review]: ----------------------------------------------------------------- Thinking about this more, this will still lead to problems. Consider the following STR: 1) Launch Fennec with an intent to open some URL 2) Cause Fennec to be killed in the background 3) Reopen Fennec from the recent apps list Fennec will be reopened in step 3 without a new intent, meaning it will still use the intent from step 1. And since the activity gets recreated, this code path gets hit again. Note that the proper fix here, whatever it will be, may overlap with bug 896170.

I think we can simplify our intent handled logic by simply checking for the existence of savedInstanceState. savedInstanceState will be non-null after the activity is killed (either with DKA or Fennec being background killed), and in those cases, onCreate() will always be called with the prior intent (and the new intent is given to onNewIntent(), which is called immediately after). I removed some old code from bug 732268 that checked if we were being launched from the recent apps list. We don't need this anymore since we're clearing the old intent in onCreate(). I also removed our ACTION_MAIN handling since that only creates an empty URI load event, which doesn't actually do anything. Mark suggested adding some tests for these, which I think is a good idea. Though we probably won't be able to enable Don't Keep Activities directly in robocop, we might be able to add special code in onDestroy() to prevent killing Gecko when the activity gets destroyed. That would allow us to create the activity again, essentially reproducing DKA's behavior. Also including Kats for review since he reviewed bug 896992.

(In reply to Brian Nicholson (:bnicholson) from comment #4) > Mark suggested adding some tests for these, which I think is a good idea. Filed bug 900799.

Comment on attachment 784739 [details] [diff] [review] Fix startup intent handling The approach seems like it should work and is cleaner. I assume you have tested the cases where you are removing code to make sure the bugs fixed by that code are still fixed.

Comment on attachment 784739 [details] [diff] [review] Fix startup intent handling Review of attachment 784739 [details] [diff] [review]: ----------------------------------------------------------------- This is definitely much nicer, if it works! I'm a little concerned that this might break existing codepaths that we rely on to restore tabs when going back into Fennec, but I guess if session restore handles all of those cases then it should be fine. It should at least be simpler to update the session restore code to restore things if this does break stuff since the logic is simpler now. r=me but this will need a good amount of testing. I'll be sure to run with DKA once this lands to test as well.

Also CC'ing liuche as FYI, since this basically removes all the code she added recently.

Verified as fixed on: Device: LG Nexus 4 (Android 4.2.2) Build: Nightly 26.0a1 (2013-08-15)

Comment on attachment 784739 [details] [diff] [review] Fix startup intent handling [Approval Request Comment] Bug caused by (feature/regressing bug #): unknown User impact if declined: tabs may not load if don't keep activities is enabled Testing completed (on m-c, etc.): m-c Risk to taking this patch (and alternatives if risky): low risk; baked on m-c for several weeks String or IDL/UUID changes made by this patch: none

Verified fixed on: Build: Firefox for Android 25.0b1 (2013-09-18) Device: LG Nexus 4 OS: Android 4.2.2