731333 - Fennec uses old intent when restoring from OOM

Status: RESOLVED FIXED

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

Description

[Brian Nicholson (:bnicholson)]

STR:
1) Open Fennec and add some site to home screen (e.g., wikipedia.org)
2) Navigate to google.com
2) Push home and run oom-fennec
3) Click the bookmarked wikipedia link

Fennec will restore with wikipedia in the foreground and google in the background, as expected. However, the background tab (google) should be delayed until the user clicks it. Instead, both tabs are immediately loaded when Fennec is launched.

Comment 1

[Brian Nicholson (:bnicholson)]

The actual bug here is that we are reusing the intent we had originally before the OOM kill. It looks like this was only one of several symptoms caused by this bug (see also bug 716681 and 730301).

Comment 3

[Brian Nicholson (:bnicholson)]

Attachment: patch

After Fennec is killed from OOM, clicking an external link to reopen Fennec will call onNewIntent() before onResume() (since Fennec still has its original intent from before the OOM). This means that the following will happen:
- onNewIntent() is called with the new intent. It has the external URL in the intent data, and it fires an event to load the tab
- onResume() is called. It uses getIntent(), which is the intent that was used to first launch Fennec (before the OOM), rather than the intent given to onNewIntent(). This can cause a number of problems depending on the original intent. If the original intent had an external URL passed to it, this means it will get duplicated (bug 716681). Also, there will be a race condition for which tab is shown (bug 730301) since onNewIntent() opens the tab independently from the initialization.

To fix this, we want the intent given to us to be used for initialization, so we need to set the intent to the one given to us in onNewIntent().

Comment 4

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

Comment on attachment 601788 [details] [diff] [review]
patch

I wish we had a way to test these types of changes

Comment 5

[Brian Nicholson (:bnicholson)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/a46bfd248760

Comment 6

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/a46bfd248760

Comment 7

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

(In reply to Mark Finkle (:mfinkle) from comment #4)
> Comment on attachment 601788 [details] [diff] [review]
> patch
> 
> I wish we had a way to test these types of changes

JMaher, I have a little utility that allocates memory until fennec dies here (https://hg.mozilla.org/users/blassey_mozilla.com/oom-fennec/) Is that something we can do with an existing test suite? Perhaps robocop can bring itself to the foreground (such that it won't get killed by the OOM) then kick off this utility? From there we'd need to write some tests for fennec OOM restart behavior.

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

We run robocop inside the fennec process, so if fennec OOM's then we are out of luck.

Another thing is we would need to somehow install this tool on the devices and figure out a way to launch it when needed.

I think we can figure something out, but it won't be simple and probably wouldn't make sense to plug into our tegra framework