1335110 - Crash in java.lang.SecurityException: at android.app.ContextImpl.startServiceCommon(ContextImpl.java)

Status: RESOLVED FIXED

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

Description

[Andrzej Hunt :ahunt]

This bug was filed from the Socorro interface and is 
report bp-0e7e0890-770e-49b6-ac54-b3c482170130.
=============================================================

Number 12 top-crash on 51

The necessary permission appears to be requested on 51, so it's unclear why this should be happening:
https://hg.mozilla.org/releases/mozilla-release/file/3d46ed7cc6a8/mobile/android/base/GcmAndroidManifest_permissions.xml.in

Comment 1

[Andrzej Hunt :ahunt]

[Tracking Requested - why for this release]: 12th top crasher on release.

(Isn't visible on any other channels, presumably because gcm isn't enabled elsewhere?)

Comment 2

[Andrzej Hunt :ahunt]

I've only done a tiny bit of research - there's a possibility this is only happening on devices that don't have google play services installed.

Bug 1300230 touched push, and landed in 51, so could be related.

Comment 3

[Andrzej Hunt :ahunt]

CC grisha, since I think you're somewhat familiar with push?

Comment 4

[Kevin Brosnan [Ex-Mozilla]]

The Note 3 is 45% of the crashes. All of the Note 3 users are running https://forum.xda-developers.com/galaxy-note-3/development/android-7-0-cm14-trader418-unofficial-t3459973 so it seems to be a bug in the rom?

Comment 5

[vihsa]

similar crash on fennec 50.0.
crash ID: 803a788c-498b-4921-b46e-55e6b2161125
crash stats: https://crash-stats.mozilla.com/report/index/803a788c-498b-4921-b46e-55e6b2161125

Comment 6

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

This is a P1 bug without an assignee. 

P1 are bugs which are being worked on for the current release cycle/iteration/sprint. 

If the bug is not assigned by Monday, 28 August, the bug's priority will be reset to '--'.

Comment 7

[:Grisha Kruglov]

From the crash logs:

While obtaining a GCM token, we end up crashing because supposedly we don't have a com.google.android.c2dm.permission.RECEIVE permission.

Comment 8

[:Grisha Kruglov]

Attachment: Bug 1335110 - Catch security exceptions while getting a GCM token to fail gracefully

Exception handling upstream is already in place, and will ensure that we degrade gracefully.
Push won't work, but we won't crash either.

Review commit: https://reviewboard.mozilla.org/r/177800/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/177800/

Comment 9

[Edouard Oger [:eoger]]

Comment on attachment 8906045 [details]
Bug 1335110 - Catch security exceptions while getting a GCM token to fail gracefully

https://reviewboard.mozilla.org/r/177800/#review182858

:shipit:

Comment 10

[Mozilla Autoland]

hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
abort: push failed:
'repository changed while pushing - please try again'

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/33804f5a3451
Catch security exceptions while getting a GCM token to fail gracefully r=eoger

Comment 12

[:Grisha Kruglov]

Filed Bug 1398315 for weirdness in Comment 10.

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/33804f5a3451

Comment 14

[Ryan VanderMeulen [:RyanVM]]

This grafts cleanly to Beta. Please request approval when you get a chance.

Comment 15

[:Grisha Kruglov]

Attachment: fxa_push_security_exception.patch

Approval Request Comment
[Feature/Bug causing the regression]: Likely dates back to when GCM registration first landed in 47, e.g. Bug 1207714.

[User impact if declined]: Browser crashes while trying to sync, or after signing in.

Behaviour after this landed: instead of crashing, push functionality won't work. But, we'll retry again on next sync, and if user fixes permissions on their device, things will get back to normal. Bug 1398293 tracks adding UI for this error state.

[Is this code covered by automated tests?]: No

[Has the fix been verified in Nightly?]: It's on current Nightlies, and there are no crashes recorded with this signature after this landed (but nightly crash volume for this is rather low).

[Needs manual test from QE? If yes, steps to reproduce]: Not necessary.

[List of other uplifts needed for the feature/fix]: N/A

[Is the change risky?]: No.

[Why is the change risky/not risky?]: Instead of crashing, we throw an exception which is caught upstream. This operation is already considered as one that might fail, and so there's proper exception handling upstream in place already.

[String changes made/needed]: N/A

Comment 16

[Gerry Chang [:gchang]]

Comment on attachment 8906751 [details] [diff] [review]
fxa_push_security_exception.patch

Fix a crash. Beta56+.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ae9c15bff40e

Comment 18

[u549602]

Could not reproduce any crash in today's testing session for 56.0b13. Leaving this issue open until we have more crash-stats data available.