1244316 - Upload Java telemetry in restricted profiles according to admin settings

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Finkle, any thoughts here?

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

This has already been discussed in bug 1125286 and bug 1232761. You can just obey the preferences for telemetry. They are set by the admin profile and not editable by the restricted profile.

Comment 2

[:Margaret Leibovic]

(In reply to Sebastian Kaspari (:sebastian) from comment #1)
> This has already been discussed in bug 1125286 and bug 1232761. You can just
> obey the preferences for telemetry. They are set by the admin profile and
> not editable by the restricted profile.

What Sebastian said.

Comment 3

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Sebastian Kaspari (:sebastian) from comment #1)
> This has already been discussed in bug 1125286 and bug 1232761. You can just
> obey the preferences for telemetry. They are set by the admin profile and
> not editable by the restricted profile.

If the FHR preference is not set, I default to `true`, or enabled – does the preference get explicitly set by the restricted profile code?

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Michael Comella (:mcomella) from comment #3)
> If the FHR preference is not set, I default to `true`, or enabled – does the
> preference get explicitly set by the restricted profile code?

We always set the preference - but in browser.js. Could this lead to us uploading telemetry before browser.js is executed?
https://dxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#545-546

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Assign self to see if this is something we want to investigate.

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

We do attempt upload before Gecko is started (which is a requirement to upload ASAP) and presumably before browser.js is read.

Sebastian, do we have any options to determine if we're in a restricted profile when we're in Java that don't rely on Gecko?

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Michael Comella (:mcomella) from comment #6)
> Sebastian, do we have any options to determine if we're in a restricted
> profile when we're in Java that don't rely on Gecko?

Yeah, you can use Restrictions.isRestrictedProfile():
https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/restrictions/Restrictions.java#68(In reply to 


Michael Comella (:mcomella) from comment #6)
> We do attempt upload before Gecko is started (which is a requirement to
> upload ASAP) and presumably before browser.js is read.

I hope this doesn't mean that we are sacrificing startup time for this (We are doing a lot of asynchronous stuff on startup).

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Sebastian Kaspari (:sebastian) from comment #7)
> > We do attempt upload before Gecko is started (which is a requirement to
> > upload ASAP) and presumably before browser.js is read.
> 
> I hope this doesn't mean that we are sacrificing startup time for this (We
> are doing a lot of asynchronous stuff on startup).

We begin upload in onStart and have to make a number of calls which read & write to disk (from the background thread, granted) so start-up will be affected in some way, hopefully marginally. bug 1277091 would fix this issue and I made a note in bug 1277091 comment 2 that if that bug is closed, we should consider delaying upload until later. Unfortunately, I don't think we'll have time to do this for a while though.

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Sebastian Kaspari (:sebastian) from comment #7)
> Yeah, you can use Restrictions.isRestrictedProfile():
> https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/
> mozilla/gecko/restrictions/Restrictions.java#68

As best I can tell, this doesn't give us information about whether the primary profile allowed upload and will just allow us to disable upload if the account is restricted (by the system's definition of restricted).

Barbara, with the current implementation, in a restricted profile, we can either:
  1) Upload unconditionally for cold startup (when Gecko isn't loaded), then obey preferences set by the admin account
  2) Don't upload at all
  3) With some complexity (that I feel we shouldn't do right now), if we're in a restricted profile, we could wait until Gecko is started and then check the preference before uploading

What do you think we should do?

fwiw, we can get around this by uploading later, always (bug 1277091), but we wouldn't get around to implementing that for a while.

Comment 10

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Bug 1244316 - Don't upload in restricted profiles when admin disables FHR.

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

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

re comment 9, we're currently doing #1 & I wrote a patch (comment 10) for #2 so it shouldn't be much work but in changing things, we're more likely to break them.

Comment 12

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Michael Comella (:mcomella) from comment #9)
> (In reply to Sebastian Kaspari (:sebastian) from comment #7)
> > Yeah, you can use Restrictions.isRestrictedProfile():
> > https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/
> > mozilla/gecko/restrictions/Restrictions.java#68
> 
> As best I can tell, this doesn't give us information about whether the
> primary profile allowed upload and will just allow us to disable upload if
> the account is restricted (by the system's definition of restricted).

You can do the same browser.js does [1] in Java by writing:

> if (Restrictions.isRestrictedProfile()) {
>     boolean isTelemetryEnabled = Restrictions.isAllowed(context, Restrictable.TELEMETRY);
>     boolean isHealthReportEnabled = Restrictions.isAllowed(context, Restrictable.HEALTH_REPORT);
>     ..
> }

[1] https://dxr.mozilla.org/mozilla-central/rev/1828937da9493b2cd54862b9c520b2ba5c7db92b/mobile/android/chrome/content/browser.js#474

Comment 13

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

(In reply to Michael Comella (:mcomella) from comment #11)
> re comment 9, we're currently doing #1 & I wrote a patch (comment 10) for #2
> so it shouldn't be much work but in changing things, we're more likely to
> break them.

Oh, why did you decide to go ahead with #1?
Will we encounter any major startup delays?

Comment 14

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Barbara Bermes [:barbara] from comment #13)
> Oh, why did you decide to go ahead with #1?

It was simplest.

> Will we encounter any major startup delays?

I assume there will be a minor affect because we touch the disk.

---

That being said, it looks like Sebastian resolved comment 9 by demonstrating that we can access all of this data in Java (which I did not think was possible before).

Comment 15

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8760421 [details]
Bug 1244316 - Don't upload in restricted profiles when admin disables FHR.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57986/diff/1-2/

Comment 16

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8760421 [details]
Bug 1244316 - Don't upload in restricted profiles when admin disables FHR.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57986/diff/2-3/

Comment 17

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Edge case: the admin profile can disable health report upload for themselves
but (I assume) the new profile won't automatically match these settings. While
I'm not sure if this is even possible, it's outside the scope of this bug.

Comment 18

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8760421 [details]
Bug 1244316 - Don't upload in restricted profiles when admin disables FHR.

https://reviewboard.mozilla.org/r/57986/#review55292

Comment 19

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

https://hg.mozilla.org/integration/fx-team/rev/fcd8a5d33cb63d088911f02c601145f2829106ec
Bug 1244316 - Don't upload in restricted profiles when admin disables FHR. r=sebastian

Comment 20

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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