Permission Action Reporting: Add num_prior_* fields.

Permission Action Reporting: Add num_prior_* fields.

Adds the number of prior dismissals and ignores to the permission report.

BUG=613883
Committed: https://crrev.com/67c8ff87e04bdc649f1be570995d21e7f5ac7475
Cr-Commit-Position: refs/heads/master@{#412700}

[kcarattini, 2016-08-17 02:18:18]

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

[kcarattini, 2016-08-17 02:18:37]

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

[commit-bot: I haz the power, 2016-08-17 02:18:45]

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, 2016-08-17 03:08:16]

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

[commit-bot: I haz the power, 2016-08-17 03:08:17]

Dry run: This issue passed the CQ dry run.

[dominickn, 2016-08-17 05:42:03]

lgtm

[raymes, 2016-08-17 06:08:00]

https://codereview.chromium.org/2250893002/diff/20001/chrome/browser/permissi...
File chrome/browser/permissions/permission_decision_auto_blocker.h (right):

https://codereview.chromium.org/2250893002/diff/20001/chrome/browser/permissi...
chrome/browser/permissions/permission_decision_auto_blocker.h:32: //
TODO(kcarattini): This and the below method should really be static.
I'd be ok making these 2 functions static. Should we just do that now?

[kcarattini, 2016-08-17 06:48:46]

PTAl, thanks!

https://codereview.chromium.org/2250893002/diff/20001/chrome/browser/permissi...
File chrome/browser/permissions/permission_decision_auto_blocker.h (right):

https://codereview.chromium.org/2250893002/diff/20001/chrome/browser/permissi...
chrome/browser/permissions/permission_decision_auto_blocker.h:32: //
TODO(kcarattini): This and the below method should really be static.
On 2016/08/17 06:08:00, raymes wrote:
> I'd be ok making these 2 functions static. Should we just do that now? 

Done.

[kcarattini, 2016-08-17 06:52:59]

kcarattini@chromium.org changed reviewers:
+ nparker@chromium.org

[kcarattini, 2016-08-17 06:52:59]

nparker@ for SB approval, thanks!

[kcarattini, 2016-08-17 06:54:03]

kcarattini@chromium.org changed reviewers:
+ msramek@chromium.org

[kcarattini, 2016-08-17 06:54:04]

msramek@: Please review browsing_data/...

[raymes, 2016-08-17 07:10:54]

permissions lgtm

https://codereview.chromium.org/2250893002/diff/60001/chrome/browser/permissi...
File chrome/browser/permissions/permission_decision_auto_blocker.cc (right):

https://codereview.chromium.org/2250893002/diff/60001/chrome/browser/permissi...
chrome/browser/permissions/permission_decision_auto_blocker.cc:103: int
PermissionDecisionAutoBlocker::GetActionCount(
nit: I guess we might as well move this into the anonymous namespace above (as a
standalone function) and take it out of the header

https://codereview.chromium.org/2250893002/diff/60001/chrome/browser/permissi...
File chrome/browser/permissions/permission_decision_auto_blocker.h (right):

https://codereview.chromium.org/2250893002/diff/60001/chrome/browser/permissi...
chrome/browser/permissions/permission_decision_auto_blocker.h:26: Profile*
profile);
optional: the profile tends to be the first argument to most functions but it's
not too important

[kcarattini, 2016-08-17 07:25:47]

https://codereview.chromium.org/2250893002/diff/60001/chrome/browser/permissi...
File chrome/browser/permissions/permission_decision_auto_blocker.cc (right):

https://codereview.chromium.org/2250893002/diff/60001/chrome/browser/permissi...
chrome/browser/permissions/permission_decision_auto_blocker.cc:103: int
PermissionDecisionAutoBlocker::GetActionCount(
On 2016/08/17 07:10:53, raymes wrote:
> nit: I guess we might as well move this into the anonymous namespace above (as
a
> standalone function) and take it out of the header

I believe Dom has this covered in a followup cl.

https://codereview.chromium.org/2250893002/diff/60001/chrome/browser/permissi...
File chrome/browser/permissions/permission_decision_auto_blocker.h (right):

https://codereview.chromium.org/2250893002/diff/60001/chrome/browser/permissi...
chrome/browser/permissions/permission_decision_auto_blocker.h:26: Profile*
profile);
On 2016/08/17 07:10:53, raymes wrote:
> optional: the profile tends to be the first argument to most functions but
it's
> not too important

Acknowledged. The rest of the metrics methods has profile last, though, so
leaving as-is for consistency there.

[msramek, 2016-08-17 08:19:49]

browsing_data/ LGTM

[Nathan Parker, 2016-08-17 20:06:01]

lgtm

https://codereview.chromium.org/2250893002/diff/60001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/permission_reporter.h (right):

https://codereview.chromium.org/2250893002/diff/60001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/permission_reporter.h:65: int num_prior_dismissals,
This is getting pretty long, and consecutive same-type params are dangerous in
that it's easy to swap them accidentally.  To consider: Pass in a single struct.
 Your call.

[kcarattini, 2016-08-17 22:37:26]

Thanks!

https://codereview.chromium.org/2250893002/diff/60001/chrome/browser/safe_bro...
File chrome/browser/safe_browsing/permission_reporter.h (right):

https://codereview.chromium.org/2250893002/diff/60001/chrome/browser/safe_bro...
chrome/browser/safe_browsing/permission_reporter.h:65: int num_prior_dismissals,
On 2016/08/17 20:06:01, Nathan Parker wrote:
> This is getting pretty long, and consecutive same-type params are dangerous in
> that it's easy to swap them accidentally.  To consider: Pass in a single
struct.
>  Your call.

Acknowledged. Added a TODO to address this in a followup cl.

[kcarattini, 2016-08-17 22:37:37]

The CQ bit was checked by kcarattini@chromium.org

[kcarattini, 2016-08-17 22:37:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominickn@chromium.org, nparker@chromium.org, raymes@chromium.org,
msramek@chromium.org
Link to the patchset: https://codereview.chromium.org/2250893002/#ps80001
(title: "todo")

[commit-bot: I haz the power, 2016-08-17 22:37:59]

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, 2016-08-18 00:07:40]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-18 00:13:52]

Description was changed from

==========
Permission Action Reporting: Add num_prior_* fields.

Adds the number of prior dismissals and ignores to the permission report.

BUG=613883
==========

to

==========
Permission Action Reporting: Add num_prior_* fields.

Adds the number of prior dismissals and ignores to the permission report.

BUG=613883

Committed: https://crrev.com/67c8ff87e04bdc649f1be570995d21e7f5ac7475
Cr-Commit-Position: refs/heads/master@{#412700}
==========

[commit-bot: I haz the power, 2016-08-18 00:13:53]

Patchset 5 (id:??) landed as
https://crrev.com/67c8ff87e04bdc649f1be570995d21e7f5ac7475
Cr-Commit-Position: refs/heads/master@{#412700}