Add rappor metrics for infobar/bubble operations for Geolocation and Notifications

Add rappor metrics for infobar/bubble operations for Geolocation and Notifications

BUG=466091
Committed: https://crrev.com/e5ea1b74e2643c92685411dfe992f292e5a61d31
Cr-Commit-Position: refs/heads/master@{#322493}

[Miguel Garcia, 2015-03-18 16:55:39]

miguelg@chromium.org changed reviewers:
+ holte@chromium.org, markusheintz@chromium.org

[Miguel Garcia, 2015-03-18 16:56:07]

Steven for the rappor metrics
Markus for content_setting owners

[Steven Holte, 2015-03-18 20:39:21]

Also, please give Chrome privacy a heads up about your new Rappor metrics.

https://codereview.chromium.org/1019543002/diff/1/chrome/browser/content_sett...
File chrome/browser/content_settings/permission_context_uma_util.cc (right):

https://codereview.chromium.org/1019543002/diff/1/chrome/browser/content_sett...
chrome/browser/content_settings/permission_context_uma_util.cc:82:
PERMISSION_ACTION_RAPPOR(
The rappor::Sample method doesn't initialize a static variable like the
histogram macros do, so it is probably better to just compute the metric name
here, rather than having the macro.

[markusheintz_, 2015-03-20 14:14:56]

LGTM

[Miguel Garcia, 2015-03-25 02:29:37]

Apologies for the long cycle. Fixed the one comment from Steven now.

https://codereview.chromium.org/1019543002/diff/1/chrome/browser/content_sett...
File chrome/browser/content_settings/permission_context_uma_util.cc (right):

https://codereview.chromium.org/1019543002/diff/1/chrome/browser/content_sett...
chrome/browser/content_settings/permission_context_uma_util.cc:82:
PERMISSION_ACTION_RAPPOR(
On 2015/03/18 20:39:20, Steven Holte wrote:
> The rappor::Sample method doesn't initialize a static variable like the
> histogram macros do, so it is probably better to just compute the metric name
> here, rather than having the macro.

Oh that's great news! Changed it to generate it dynamically.

[Miguel Garcia, 2015-03-26 17:09:52]

On 2015/03/25 02:29:37, Miguel Garcia wrote:
> Apologies for the long cycle. Fixed the one comment from Steven now.
> 
>
https://codereview.chromium.org/1019543002/diff/1/chrome/browser/content_sett...
> File chrome/browser/content_settings/permission_context_uma_util.cc (right):
> 
>
https://codereview.chromium.org/1019543002/diff/1/chrome/browser/content_sett...
> chrome/browser/content_settings/permission_context_uma_util.cc:82:
> PERMISSION_ACTION_RAPPOR(
> On 2015/03/18 20:39:20, Steven Holte wrote:
> > The rappor::Sample method doesn't initialize a static variable like the
> > histogram macros do, so it is probably better to just compute the metric
name
> > here, rather than having the macro.
> 
> Oh that's great news! Changed it to generate it dynamically.

Friendly ping on this change

[Steven Holte, 2015-03-26 21:02:23]

On 2015/03/26 17:09:52, Miguel Garcia wrote:
> On 2015/03/25 02:29:37, Miguel Garcia wrote:
> > Apologies for the long cycle. Fixed the one comment from Steven now.
> > 
> >
>
https://codereview.chromium.org/1019543002/diff/1/chrome/browser/content_sett...
> > File chrome/browser/content_settings/permission_context_uma_util.cc (right):
> > 
> >
>
https://codereview.chromium.org/1019543002/diff/1/chrome/browser/content_sett...
> > chrome/browser/content_settings/permission_context_uma_util.cc:82:
> > PERMISSION_ACTION_RAPPOR(
> > On 2015/03/18 20:39:20, Steven Holte wrote:
> > > The rappor::Sample method doesn't initialize a static variable like the
> > > histogram macros do, so it is probably better to just compute the metric
> name
> > > here, rather than having the macro.
> > 
> > Oh that's great news! Changed it to generate it dynamically.
> 
> Friendly ping on this change

From the bug it looks like you were thinking to reduce this set to only
Request/Granted metrics for these permissions?  Or am I reading that wrong?

[Miguel Garcia, 2015-03-26 21:05:34]

I updated the bug, Dismiss/Ignore are also important :)

On 2015/03/26 21:02:23, Steven Holte wrote:
> On 2015/03/26 17:09:52, Miguel Garcia wrote:
> > On 2015/03/25 02:29:37, Miguel Garcia wrote:
> > > Apologies for the long cycle. Fixed the one comment from Steven now.
> > > 
> > >
> >
>
https://codereview.chromium.org/1019543002/diff/1/chrome/browser/content_sett...
> > > File chrome/browser/content_settings/permission_context_uma_util.cc
(right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1019543002/diff/1/chrome/browser/content_sett...
> > > chrome/browser/content_settings/permission_context_uma_util.cc:82:
> > > PERMISSION_ACTION_RAPPOR(
> > > On 2015/03/18 20:39:20, Steven Holte wrote:
> > > > The rappor::Sample method doesn't initialize a static variable like the
> > > > histogram macros do, so it is probably better to just compute the metric
> > name
> > > > here, rather than having the macro.
> > > 
> > > Oh that's great news! Changed it to generate it dynamically.
> > 
> > Friendly ping on this change
> 
> From the bug it looks like you were thinking to reduce this set to only
> Request/Granted metrics for these permissions?  Or am I reading that wrong?

[Steven Holte, 2015-03-26 21:12:50]

lgtm

[Miguel Garcia, 2015-03-26 21:39:12]

On 2015/03/26 21:12:50, Steven Holte wrote:
> lgtm

thanks!

[Miguel Garcia, 2015-03-26 21:39:17]

The CQ bit was checked by miguelg@chromium.org

[Miguel Garcia, 2015-03-26 21:39:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
markusheintz@chromium.org
Link to the patchset: https://chromiumcodereview.appspot.com/1019543002/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2015-03-26 21:39:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1019543002/40001

[commit-bot: I haz the power, 2015-03-26 21:42:22]

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

[commit-bot: I haz the power, 2015-03-26 21:42:23]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[Miguel Garcia, 2015-03-26 22:35:07]

The CQ bit was checked by miguelg@chromium.org

[Miguel Garcia, 2015-03-26 22:35:08]

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

[commit-bot: I haz the power, 2015-03-26 22:35:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1019543002/60001

[commit-bot: I haz the power, 2015-03-26 23:30:14]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-03-26 23:30:55]

Patchset 4 (id:??) landed as
https://crrev.com/e5ea1b74e2643c92685411dfe992f292e5a61d31
Cr-Commit-Position: refs/heads/master@{#322493}