Remove deprecated and out of date permissions metrics.

Remove deprecated and out of date permissions metrics.

This CL removes:

 - deprecated permissions RAPPOR metrics
 - secure/insecure origin metrics for geolocation, push, durable storage,
   and MidiSysEx (as they are restricted to secure origins)

BUG=605836, 638076
Review-Url: https://codereview.chromium.org/2711513005
Cr-Commit-Position: refs/heads/master@{#452741}
Committed: https://chromium.googlesource.com/chromium/src/+/f5a506a917f583f5aa6e32231f8a7df563dfe170

[dominickn, 2017-02-23 05:33:24]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 05:33:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dominickn, 2017-02-23 05:36:27]

dominickn@chromium.org changed reviewers:
+ holte@chromium.org, raymes@chromium.org

[dominickn, 2017-02-23 05:36:27]

PTAL, thanks!

[raymes, 2017-02-23 05:54:57]

Thanks for the cleanup Dom! Looks good just a couple questions/requests.

https://codereview.chromium.org/2711513005/diff/1/chrome/browser/permissions/...
File chrome/browser/permissions/permission_uma_util.cc (right):

https://codereview.chromium.org/2711513005/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_uma_util.cc:144: }
Do you still use this one? The comment in histograms.xml is:

Note this is probably not the metric you want - it does not correspond to
    the total number of times websites request a permission. Also, because
    specific permissions have code that can automatically block or grant
    permissions based on things like incognito, installed extensions etc., this
    does also not correspond to the number of times users are prompted to allow
    permissions.

    For a better metric to track how often users are prompted, use
    ContentSettings.PermissionsActions*.

I'm ok to keep it if you still use it though?

https://codereview.chromium.org/2711513005/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_uma_util.cc:171: }
Lines from here up to 146 can be removed.

https://codereview.chromium.org/2711513005/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_uma_util.cc:713:
static_cast<int>(PermissionAction::NUM));
Is there actually a benefit to changing these ones, even though we shouldn't be
getting insecure reports?

https://codereview.chromium.org/2711513005/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2711513005/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:118070: +  <suffix name="MidiSysEx"
label="Midi SysEx permsision actions"/>
I think midi sysex is only available on secure origins.

https://codereview.chromium.org/2711513005/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:118073: +  <suffix name="DurableStorage"
label="Durable Storage permission actions"/>
I think durable storage is only available on secure origins.

[dominickn, 2017-02-23 06:06:36]

Patchset #2 (id:20001) has been deleted

[dominickn, 2017-02-23 06:11:43]

Thanks!

https://codereview.chromium.org/2711513005/diff/1/chrome/browser/permissions/...
File chrome/browser/permissions/permission_uma_util.cc (right):

https://codereview.chromium.org/2711513005/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_uma_util.cc:144: }
On 2017/02/23 05:54:57, raymes wrote:
> Do you still use this one? The comment in histograms.xml is:
> 
> Note this is probably not the metric you want - it does not correspond to
>     the total number of times websites request a permission. Also, because
>     specific permissions have code that can automatically block or grant
>     permissions based on things like incognito, installed extensions etc.,
this
>     does also not correspond to the number of times users are prompted to
allow
>     permissions.
> 
>     For a better metric to track how often users are prompted, use
>     ContentSettings.PermissionsActions*.
> 
> I'm ok to keep it if you still use it though?

I don't use it. Happy to remove. But I'm not sure if anyone else uses it, maybe
we should send a PSA out?

https://codereview.chromium.org/2711513005/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_uma_util.cc:171: }
On 2017/02/23 05:54:57, raymes wrote:
> Lines from here up to 146 can be removed.

Done.

https://codereview.chromium.org/2711513005/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_uma_util.cc:713:
static_cast<int>(PermissionAction::NUM));
On 2017/02/23 05:54:57, raymes wrote:
> Is there actually a benefit to changing these ones, even though we shouldn't
be
> getting insecure reports? 

It's mainly following the lead of the mediastream permissions, which do this as
well (see below). Since we'll never unrestrict these permissions from insecure
origins, I don't see why we would care about the split. :)

https://codereview.chromium.org/2711513005/diff/1/tools/metrics/histograms/hi...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2711513005/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:118070: +  <suffix name="MidiSysEx"
label="Midi SysEx permsision actions"/>
On 2017/02/23 05:54:57, raymes wrote:
> I think midi sysex is only available on secure origins.

Done.

https://codereview.chromium.org/2711513005/diff/1/tools/metrics/histograms/hi...
tools/metrics/histograms/histograms.xml:118073: +  <suffix name="DurableStorage"
label="Durable Storage permission actions"/>
On 2017/02/23 05:54:57, raymes wrote:
> I think durable storage is only available on secure origins.

Done.

[raymes, 2017-02-23 06:30:55]

lgtm

https://codereview.chromium.org/2711513005/diff/1/chrome/browser/permissions/...
File chrome/browser/permissions/permission_uma_util.cc (right):

https://codereview.chromium.org/2711513005/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_uma_util.cc:144: }
On 2017/02/23 06:11:43, dominickn wrote:
> On 2017/02/23 05:54:57, raymes wrote:
> > Do you still use this one? The comment in histograms.xml is:
> > 
> > Note this is probably not the metric you want - it does not correspond to
> >     the total number of times websites request a permission. Also, because
> >     specific permissions have code that can automatically block or grant
> >     permissions based on things like incognito, installed extensions etc.,
> this
> >     does also not correspond to the number of times users are prompted to
> allow
> >     permissions.
> > 
> >     For a better metric to track how often users are prompted, use
> >     ContentSettings.PermissionsActions*.
> > 
> > I'm ok to keep it if you still use it though?
> 
> I don't use it. Happy to remove. But I'm not sure if anyone else uses it,
maybe
> we should send a PSA out?

Sending out a PSA sounds good. We don't have to block this CL on it.

https://codereview.chromium.org/2711513005/diff/1/chrome/browser/permissions/...
chrome/browser/permissions/permission_uma_util.cc:713:
static_cast<int>(PermissionAction::NUM));
On 2017/02/23 06:11:42, dominickn wrote:
> On 2017/02/23 05:54:57, raymes wrote:
> > Is there actually a benefit to changing these ones, even though we shouldn't
> be
> > getting insecure reports? 
> 
> It's mainly following the lead of the mediastream permissions, which do this
as
> well (see below). Since we'll never unrestrict these permissions from insecure
> origins, I don't see why we would care about the split. :)

Ok, sounds good to me

[Steven Holte, 2017-02-23 19:39:41]

lgtm

[dominickn, 2017-02-23 21:40:06]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 21:40:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dominickn, 2017-02-23 21:43:15]

Description was changed from

==========
Remove deprecated and out of date permissions metrics.

This CL removes:

 - deprecated permissions RAPPOR metrics
 - secure/insecure origin metrics for geolocation and push (as they are
   restricted to secure origins only)

BUG=605836,638076
==========

to

==========
Remove deprecated and out of date permissions metrics.

This CL removes:

 - deprecated permissions RAPPOR metrics
 - secure/insecure origin metrics for geolocation, push, durable storage,
   and MidiSysEx (as they are restricted to secure origins)

BUG=605836,638076
==========

[commit-bot: I haz the power, 2017-02-23 21:52:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 21:52:35]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dominickn, 2017-02-23 22:52:26]

The CQ bit was checked by dominickn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-23 22:53:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-23 23:05:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-23 23:05:25]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[dominickn, 2017-02-24 03:19:35]

The CQ bit was checked by dominickn@chromium.org

[dominickn, 2017-02-24 03:19:36]

The patchset sent to the CQ was uploaded after l-g-t-m from raymes@chromium.org,
holte@chromium.org
Link to the patchset: https://codereview.chromium.org/2711513005/#ps80001
(title: "Re-rebase")

[commit-bot: I haz the power, 2017-02-24 03:20:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-24 03:36:55]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1487906375779870,
"parent_rev": "06203bafdd105bf358fd8f216fd77ae0053f4ec7", "commit_rev":
"f5a506a917f583f5aa6e32231f8a7df563dfe170"}

[commit-bot: I haz the power, 2017-02-24 03:37:32]

Description was changed from

==========
Remove deprecated and out of date permissions metrics.

This CL removes:

 - deprecated permissions RAPPOR metrics
 - secure/insecure origin metrics for geolocation, push, durable storage,
   and MidiSysEx (as they are restricted to secure origins)

BUG=605836,638076
==========

to

==========
Remove deprecated and out of date permissions metrics.

This CL removes:

 - deprecated permissions RAPPOR metrics
 - secure/insecure origin metrics for geolocation, push, durable storage,
   and MidiSysEx (as they are restricted to secure origins)

BUG=605836,638076

Review-Url: https://codereview.chromium.org/2711513005
Cr-Commit-Position: refs/heads/master@{#452741}
Committed:
https://chromium.googlesource.com/chromium/src/+/f5a506a917f583f5aa6e32231f8a...
==========

[commit-bot: I haz the power, 2017-02-24 03:37:33]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/f5a506a917f583f5aa6e32231f8a...