919645 - crash in android.os.NetworkOnMainThreadException: at android.os.StrictMode$AndroidBlockGuardPolicy.onNetwork(StrictMode.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Kevin Brosnan [Ex-Mozilla]]

This bug was filed from the Socorro interface and is 
report bp-5a16ecda-791e-4945-9135-cfb6b2130922.
=============================================================

https://crash-stats.mozilla.com/report/list?product=FennecAndroid&signature=android.os.NetworkOnMainThreadException%3A+at+android.os.StrictMode%24AndroidBlockGuardPolicy.onNetwork%28StrictMode.java%29

This is simple to reproduce if you 
* enable 'Don't keep activities'
* browse to a site
* send Firefox to the background (home button or app switcher)
* repeat 2nd and 3rd bullets till you crash

Comment 1

[Richard Newman [:rnewman]]

There are reports for this going back to Fx16, according to crash-stats. (e.g., <https://crash-stats.mozilla.com/report/index/fd2ae56c-0772-466b-8b3c-5304d2130926>).

This code *should* be replaced by ckitching's work in Bug 914296, so there's a decent chance of this just going away after that lands, but that's not a patch I'd uplift.

Is this enough of a crasher that you want a fix for Aurora or Beta?

Comment 2

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

(In reply to Richard Newman [:rnewman] from comment #1)
> There are reports for this going back to Fx16, according to crash-stats.
> (e.g.,
> <https://crash-stats.mozilla.com/report/index/fd2ae56c-0772-466b-8b3c-
> 5304d2130926>).
> 
> This code *should* be replaced by ckitching's work in Bug 914296, so there's
> a decent chance of this just going away after that lands, but that's not a
> patch I'd uplift.
> 
> Is this enough of a crasher that you want a fix for Aurora or Beta?
There was a crash in 15 with the same signature, resolved in Fx17 (bug 774072). It seems we regressed in 26, so a patch should be uplifted to 26 to fix that regression.

Comment 3

[Richard Newman [:rnewman]]

Attachment: Proposed patch for Aurora. v1

I'll test this on Monday, but this looks like the right fix.

Comment 4

[Chris Kitching [:ckitching]]

(In reply to Richard Newman [:rnewman] from comment #3)
> Created attachment 811590 [details] [diff] [review]
> Proposed patch for Aurora. v1
> 
> I'll test this on Monday, but this looks like the right fix.

Ah. My regression - didn't realise this subtlety. Sorry.

Also - nitpicking: Your indentation is inconsistent.

Comment 5

[Richard Newman [:rnewman]]

(In reply to Chris Kitching [:ckitching] from comment #4)

> Also - nitpicking: Your indentation is inconsistent.

Yeah, my local Vim config is 2-space, and only some of our files have editor directives. Will fix locally. Thanks!

Comment 6

[Richard Newman [:rnewman]]

I can't repro this with current Aurora.

Could someone who *can* (Kevin?) try my APK?

http://people.mozilla.com/~rnewman/fennec-26.0a2.en-US.android-arm.apk

Comment 7

[Aaron Train [:aaronmt]]

Kevin's out for the next two weeks Richard. I'm not able to reproduce this at all on Nightly nor Aurora.

Comment 8

[Richard Newman [:rnewman]]

Thanks for checking, Aaron.

I propose landing this fix and watching crash stats. Anyone have any strong feelings otherwise?

Comment 9

[Richard Newman [:rnewman]]

Comment on attachment 811590 [details] [diff] [review]
Proposed patch for Aurora. v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  Not clear; recent favicon work is the likely culprit, but signature goes back a while.

User impact if declined: 
  Crashes when Android cleans up activities.

Testing completed (on m-c, etc.): 
  Only planned to land on Aurora. No obvious regressions during testing of builds with this fix, but the original issue is hard to repro, so not clear if this is enough. (It should be, but you know how bugs can be!)

Risk to taking this patch (and alternatives if risky): 
  Should be zero.

String or IDL/UUID changes made by this patch:
  None.

Comment 10

[Kyle Wooten]

Thanks!

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

(In reply to Richard Newman [:rnewman] from comment #9)

> Testing completed (on m-c, etc.): 
>   Only planned to land on Aurora. No obvious regressions during testing of
> builds with this fix, but the original issue is hard to repro, so not clear
> if this is enough. (It should be, but you know how bugs can be!)


If this can't be reproduced and we're just going to watch crash stats, why not land to Nightly?  We have similar number of users on both Nightly and Aurora - at the very least land to central first and see if anything changes in the next few days and then we could look at Aurora uplift too which would get some user overlap but I'm not keen to break our SOP for landing/uplifts on this without good reason.

Comment 12

[Richard Newman [:rnewman]]

(In reply to [e-mail] [:lsblakk] from comment #11)

> If this can't be reproduced and we're just going to watch crash stats, why
> not land to Nightly?  We have similar number of users on both Nightly and
> Aurora - at the very least land to central first and see if anything changes
> in the next few days and then we could look at Aurora uplift too which would
> get some user overlap but I'm not keen to break our SOP for landing/uplifts
> on this without good reason.

I had wanted to avoid further bitrotting of Bug 914296 before it lands (it's a difficult birth), but we can land this on m-c first if you prefer.

Comment 13

[Richard Newman [:rnewman]]

Landed in fx-team.

https://hg.mozilla.org/integration/fx-team/rev/54d196d5fdfb

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/54d196d5fdfb

Comment 15

[Richard Newman [:rnewman]]

Flagging myself to watch crash-stats for 27.

Comment 16

[Lukas Blakk [:lsblakk] use ?needinfo]

If I'm reading the crash stats correctly, this is still occurring on Nightly.

Comment 17

[Richard Newman [:rnewman]]

I see no Nightly builds (27.0a1) after 20131007 in 

https://crash-stats.mozilla.com/report/list?product=FennecAndroid&signature=android.os.NetworkOnMainThreadException%3A+at+android.os.StrictMode%24AndroidBlockGuardPolicy.onNetwork%28StrictMode.java%29

Comment 18

[Richard Newman [:rnewman]]

Specific search:

https://crash-stats.mozilla.com/report/list?signature=android.os.NetworkOnMainThreadException%3A+at+android.os.StrictMode%24AndroidBlockGuardPolicy.onNetwork%28StrictMode.java%29&product=FennecAndroid&query_type=contains&range_unit=weeks&process_type=any&version=FennecAndroid%3A27.0a1&hang_type=any&date=2013-10-11+22%3A00%3A00&range_value=1#reports

Comment 19

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 811590 [details] [diff] [review]
Proposed patch for Aurora. v1

Thank you for the link to crash stats, I was getting it wrong somehow.

Comment 20

[Richard Newman [:rnewman]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2a1d0c72e864