Permission Action Reporting: Bundle report data.

Permission Action Reporting: Bundle report data.

Bundles the data for a PermissionReport into a struct and adds the
persiste decision.

BUG=613883
Committed: https://crrev.com/658733470ae3080b22dfed674be87e69b443dedc
Cr-Commit-Position: refs/heads/master@{#413023}

[kcarattini, 2016-08-18 04:49:39]

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

[kcarattini, 2016-08-18 04:49:47]

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

[commit-bot: I haz the power, 2016-08-18 04:49:55]

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-18 05:56:54]

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

[commit-bot: I haz the power, 2016-08-18 05:56:54]

Dry run: This issue passed the CQ dry run.

[raymes, 2016-08-18 07:09:57]

lgtm thanks for the cleanup :)

https://codereview.chromium.org/2254103003/diff/1/chrome/browser/safe_browsin...
File chrome/browser/safe_browsing/permission_reporter.h (right):

https://codereview.chromium.org/2254103003/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/permission_reporter.h:51: // The permission report
includes the origin of the site requesting the
nit: requesting the...

https://codereview.chromium.org/2254103003/diff/1/chrome/browser/safe_browsin...
File chrome/browser/safe_browsing/ui_manager.h (right):

https://codereview.chromium.org/2254103003/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/ui_manager.h:160: void
ReportPermissionAction(PermissionReportInfo report_info);
nit: should probably use a const reference here too

[kcarattini, 2016-08-18 07:23:27]

Thanks!

https://codereview.chromium.org/2254103003/diff/1/chrome/browser/safe_browsin...
File chrome/browser/safe_browsing/permission_reporter.h (right):

https://codereview.chromium.org/2254103003/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/permission_reporter.h:51: // The permission report
includes the origin of the site requesting the
On 2016/08/18 07:09:57, raymes wrote:
> nit: requesting the...

Done.

https://codereview.chromium.org/2254103003/diff/1/chrome/browser/safe_browsin...
File chrome/browser/safe_browsing/ui_manager.h (right):

https://codereview.chromium.org/2254103003/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/ui_manager.h:160: void
ReportPermissionAction(PermissionReportInfo report_info);
On 2016/08/18 07:09:57, raymes wrote:
> nit: should probably use a const reference here too

Done.

[Nathan Parker, 2016-08-19 00:14:34]

lgtm

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

https://codereview.chromium.org/2254103003/diff/20001/chrome/browser/permissi...
chrome/browser/permissions/permission_uma_util.h:45: PermissionReportInfo(
Having this constructor sort of defeats some of the vale, since it allows the
caller to pass things unnamed, and you could still mix up the last two ints.

If you think it's useful (it does reduce verbiage), I could be convinced.

[kcarattini, 2016-08-19 01:00:39]

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

https://codereview.chromium.org/2254103003/diff/20001/chrome/browser/permissi...
chrome/browser/permissions/permission_uma_util.h:45: PermissionReportInfo(
On 2016/08/19 00:14:34, Nathan Parker wrote:
> Having this constructor sort of defeats some of the vale, since it allows the
> caller to pass things unnamed, and you could still mix up the last two ints.
> 
> If you think it's useful (it does reduce verbiage), I could be convinced.

True, but at least it only happens in one place now. It also guarantees that all
of the fields are set. Setting each field individually seems somewhat excessive
to me.

[kcarattini, 2016-08-19 01:08:03]

The CQ bit was checked by kcarattini@chromium.org

[kcarattini, 2016-08-19 01:08:03]

The patchset sent to the CQ was uploaded after l-g-t-m from raymes@chromium.org
Link to the patchset: https://codereview.chromium.org/2254103003/#ps20001
(title: "Review Comments")

[commit-bot: I haz the power, 2016-08-19 01:08:39]

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-19 02:03:13]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-19 02:07:03]

Description was changed from

==========
Permission Action Reporting: Bundle report data.

Bundles the data for a PermissionReport into a struct and adds the
persiste decision.

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

to

==========
Permission Action Reporting: Bundle report data.

Bundles the data for a PermissionReport into a struct and adds the
persiste decision.

BUG=613883

Committed: https://crrev.com/658733470ae3080b22dfed674be87e69b443dedc
Cr-Commit-Position: refs/heads/master@{#413023}
==========

[commit-bot: I haz the power, 2016-08-19 02:07:04]

Patchset 2 (id:??) landed as
https://crrev.com/658733470ae3080b22dfed674be87e69b443dedc
Cr-Commit-Position: refs/heads/master@{#413023}