1221678 - Add new GCM permission to Android manifest

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Nick Alexander :nalexander [he/him]]

Using GCM requires a new Android permission:

<uses-permission android:name="com.google.android.c2dm.permission.RECEIVE" />

This impacts Fennec (and possibly b2gdroid).  We need to hash out a schedule for this permission to ride the trains.

Comment 1

[Nick Alexander :nalexander [he/him]]

margaret: I haven't been involved with a permission bump like this before.  The patch is trivial.  Can you explain what goes into this decision?

Comment 2

[:Margaret Leibovic]

(In reply to Nick Alexander :nalexander from comment #1)
> margaret: I haven't been involved with a permission bump like this before. 
> The patch is trivial.  Can you explain what goes into this decision?

We usually do a cost-benefit analysis to see if the new feature we're getting would be worth the potential update drop-off from users needing to manually approve a new permission. We try to group together new permissions as much as possible, but I don't think we have any other things on deck right now.

We were hoping that we'd have Android M incremental permissions in place by the time we needed to do another permission bump, but here we are... that also wouldn't help users on older versions of Android.

We'll want to message to users why this new permission is needed. If push notifications are important, and it seems like they are, we'll probably have to bite the bullet and just do this.

Barbara, what do you think?

Comment 3

[Barbara Bermes [:barbara] - NI please!]

Just for my understanding, adding GCM to Fennec is just the first phase of push notification on Fennec? 

I'm basically asking if by the time we ask in the release for a permission bump if we are able to tell the user a good story why and how they will benefit from it. If we only implement the underlying technology first without giving the user true value for that bump (web notifications), I'm a bit hesitant.

Thoughts?

Comment 4

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

(In reply to Nick Alexander :nalexander from comment #1)
> margaret: I haven't been involved with a permission bump like this before. 
> The patch is trivial.  Can you explain what goes into this decision?

In addition to what Margaret said, we also need to figure out how this would ride the trains. We would probably not allow the permission bump to ride the trains until the features were ready as well. Until then, it would stay behind a flag.

Comment 5

[Nick Alexander :nalexander [he/him]]

(In reply to Barbara Bermes [:barbara] from comment #3)
> Just for my understanding, adding GCM to Fennec is just the first phase of
> push notification on Fennec? 
> 
> I'm basically asking if by the time we ask in the release for a permission
> bump if we are able to tell the user a good story why and how they will
> benefit from it. If we only implement the underlying technology first
> without giving the user true value for that bump (web notifications), I'm a
> bit hesitant.

Yeah, we'd only really bump permissions for a real push consumer, Web Push or Firefox Accounts/Sync/Send Tab.  Behind flags (for Release and Beta, for sure) until we were ready to roll the feature out to our userbase.

Comment 6

[Nick Alexander :nalexander [he/him]]

Attachment: MozReview Request: Bug 1221678 - Add new Android permission to Fennec Nightly (when GCM is enabled). r?margaret

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

Comment 7

[Nick Alexander :nalexander [he/him]]

margaret: I flagged you for review, but could you also suggest if there are additional permissions bumps in the pipeline, so we can coalesce and bump everything all at once?  I don't want to manage this cross-feature co-ordination.

Thanks!

Comment 8

[:Margaret Leibovic]

(In reply to Nick Alexander :nalexander from comment #7)
> margaret: I flagged you for review, but could you also suggest if there are
> additional permissions bumps in the pipeline, so we can coalesce and bump
> everything all at once?  I don't want to manage this cross-feature
> co-ordination.

I don't know of anything in the pipeline right now. Redirecting to Barbara to manage this.

Is this still an issue with runtime permissions? I realize that will only help Android M users, but at least it's something.

I don't think we're going to want to ship this new permission without a new feature to go along with it... is it possible to back this out from 47 if we don't ship push in that release?

Comment 9

[:Margaret Leibovic]

Comment on attachment 8725804 [details]
MozReview Request: Bug 1221678 - Add new Android permission to Fennec Nightly (when GCM is enabled). r?margaret

https://reviewboard.mozilla.org/r/37649/#review34231

::: mobile/android/base/FennecManifest_permissions.xml.in:14
(Diff revision 1)
> +#ifdef NIGHTLY_BUILD

Looking at this patch, I'm answering my own question. Let's keep this behind a flag until we're ready to ship it.

Comment 10

[Nick Alexander :nalexander [he/him]]

Also, according to my local testing and http://android.stackexchange.com/a/61794, this will appear as "NEW - receive data from internet".  Very helpful, Google.

Comment 11

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Nick Alexander :nalexander from comment #7)
> margaret: I flagged you for review, but could you also suggest if there are
> additional permissions bumps in the pipeline, so we can coalesce and bump
> everything all at once?  I don't want to manage this cross-feature
> co-ordination.

In bug 1241810 ("Content notifications") I want to add the RECEIVE_BOOT_COMPLETED permission to restore alarms (AlarmManager) after reboot. Even though we already have a broadcast receiver listening to BOOT_COMPLETED (Stumbler) we do not have the permission yet.

Comment 12

[:Margaret Leibovic]

(In reply to Sebastian Kaspari (:sebastian) from comment #11)
> (In reply to Nick Alexander :nalexander from comment #7)
> > margaret: I flagged you for review, but could you also suggest if there are
> > additional permissions bumps in the pipeline, so we can coalesce and bump
> > everything all at once?  I don't want to manage this cross-feature
> > co-ordination.
> 
> In bug 1241810 ("Content notifications") I want to add the
> RECEIVE_BOOT_COMPLETED permission to restore alarms (AlarmManager) after
> reboot. Even though we already have a broadcast receiver listening to
> BOOT_COMPLETED (Stumbler) we do not have the permission yet.

Let's also flag this for Barbara. We have (or should have) a tag in Aha for tracking permissions bumps, and this lets us group them together in the roadmap.

Comment 13

[Pulsebot]

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

Comment 14

[Carsten Book [:Tomcat]]

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

Comment 15

[Barbara Bermes [:barbara] - NI please!]

(In reply to :Margaret Leibovic from comment #8)
> (In reply to Nick Alexander :nalexander from comment #7)
> > margaret: I flagged you for review, but could you also suggest if there are
> > additional permissions bumps in the pipeline, so we can coalesce and bump
> > everything all at once?  I don't want to manage this cross-feature
> > co-ordination.
> 
> I don't know of anything in the pipeline right now. Redirecting to Barbara
> to manage this.
> 
> Is this still an issue with runtime permissions? I realize that will only
> help Android M users, but at least it's something.
> 
> I don't think we're going to want to ship this new permission without a new
> feature to go along with it... is it possible to back this out from 47 if we
> don't ship push in that release?

I don't think there is any permission bump in the pipeline for 46/47. 

So I agree, if there is nothing visibly beneficial to the user to ask for a permission bump, we should hold off with this until we can tie this with a user facing benefit/feature.

How does this work fit into https://mozilla.aha.io/features/FENN-199? As this card is ready for 46. Should I create a new card to track the GCM work specifically as I don't see anything related in 47 in Aha?

Comment 16

[Nick Alexander :nalexander [he/him]]

(In reply to Barbara Bermes [:barbara] from comment #15)
> (In reply to :Margaret Leibovic from comment #8)
> > (In reply to Nick Alexander :nalexander from comment #7)
> > > margaret: I flagged you for review, but could you also suggest if there are
> > > additional permissions bumps in the pipeline, so we can coalesce and bump
> > > everything all at once?  I don't want to manage this cross-feature
> > > co-ordination.
> > 
> > I don't know of anything in the pipeline right now. Redirecting to Barbara
> > to manage this.
> > 
> > Is this still an issue with runtime permissions? I realize that will only
> > help Android M users, but at least it's something.
> > 
> > I don't think we're going to want to ship this new permission without a new
> > feature to go along with it... is it possible to back this out from 47 if we
> > don't ship push in that release?
> 
> I don't think there is any permission bump in the pipeline for 46/47. 
> 
> So I agree, if there is nothing visibly beneficial to the user to ask for a
> permission bump, we should hold off with this until we can tie this with a
> user facing benefit/feature.
> 
> How does this work fit into https://mozilla.aha.io/features/FENN-199? As
> this card is ready for 46. Should I create a new card to track the GCM work
> specifically as I don't see anything related in 47 in Aha?

This isn't technically related to https://mozilla.aha.io/features/FENN-199.  That tracks Web Notifications, which are the display part.  This work tracks Web Push, which is the messaging part and isn't /necessarily/ user visible.  It is user visible in that sites will prompt to push updates to the user, and the behaviour can include displayed notifications.

This is Nightly only for now.