Permission Action Reporting: Do not report for Custom Passphrase users.

Permission Action Reporting: Do not report for Custom Passphrase users.

BUG=613883
Committed: https://crrev.com/787d1843f6e3b028b008b7a701432fe07225a8a7
Cr-Commit-Position: refs/heads/master@{#413038}

[kcarattini, 2016-08-18 06:08:56]

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

[kcarattini, 2016-08-18 06:08:57]

pavely@: Please review for correct usage of Sync calls.
raymes@: please review permissions/...
nparker@: please review safe_browsing/..

Thanks,
Kendra

[raymes, 2016-08-18 06:54:02]

lgtm

https://codereview.chromium.org/2220323002/diff/120001/chrome/browser/permiss...
File chrome/browser/permissions/permission_uma_util_unittest.cc (right):

https://codereview.chromium.org/2220323002/diff/120001/chrome/browser/permiss...
chrome/browser/permissions/permission_uma_util_unittest.cc:175:
syncer::ModelTypeSet preferred_types;
nit: maybe add a comment here

https://codereview.chromium.org/2220323002/diff/120001/chrome/browser/permiss...
chrome/browser/permissions/permission_uma_util_unittest.cc:222:
.WillRepeatedly(testing::Return(true));
Is WillRepeatedly needed here or will WillOnce do?

https://codereview.chromium.org/2220323002/diff/120001/chrome/browser/safe_br...
File chrome/browser/safe_browsing/permission_reporter_browsertest.cc (right):

https://codereview.chromium.org/2220323002/diff/120001/chrome/browser/safe_br...
chrome/browser/safe_browsing/permission_reporter_browsertest.cc:103: //
TODO(kcarattini): Address crbug/638316 to reenable this test.
It would be nice to have this working before the build makes it to beta if
possible.

[kcarattini, 2016-08-18 07:17:42]

Thanks!

https://codereview.chromium.org/2220323002/diff/120001/chrome/browser/permiss...
File chrome/browser/permissions/permission_uma_util_unittest.cc (right):

https://codereview.chromium.org/2220323002/diff/120001/chrome/browser/permiss...
chrome/browser/permissions/permission_uma_util_unittest.cc:175:
syncer::ModelTypeSet preferred_types;
On 2016/08/18 06:54:02, raymes wrote:
> nit: maybe add a comment here

Done.

https://codereview.chromium.org/2220323002/diff/120001/chrome/browser/permiss...
chrome/browser/permissions/permission_uma_util_unittest.cc:222:
.WillRepeatedly(testing::Return(true));
On 2016/08/18 06:54:02, raymes wrote:
> Is WillRepeatedly needed here or will WillOnce do?

Done.

https://codereview.chromium.org/2220323002/diff/120001/chrome/browser/safe_br...
File chrome/browser/safe_browsing/permission_reporter_browsertest.cc (right):

https://codereview.chromium.org/2220323002/diff/120001/chrome/browser/safe_br...
chrome/browser/safe_browsing/permission_reporter_browsertest.cc:103: //
TODO(kcarattini): Address crbug/638316 to reenable this test.
On 2016/08/18 06:54:02, raymes wrote:
> It would be nice to have this working before the build makes it to beta if
> possible.

Acknowledged.

[pavely, 2016-08-18 18:16:05]

lgtm

[Nathan Parker, 2016-08-19 00:08:49]

lgtm

https://codereview.chromium.org/2220323002/diff/140001/chrome/browser/permiss...
File chrome/browser/permissions/permission_uma_util_unittest.cc (right):

https://codereview.chromium.org/2220323002/diff/140001/chrome/browser/permiss...
chrome/browser/permissions/permission_uma_util_unittest.cc:159:
.WillOnce(testing::Return(false));
Does this work?  (I assume so).  Like, it overrides the expect_call you applied
in SetMockSyncService?  If so, cool.

[kcarattini, 2016-08-19 01:05:45]

Thanks all.

https://codereview.chromium.org/2220323002/diff/140001/chrome/browser/permiss...
File chrome/browser/permissions/permission_uma_util_unittest.cc (right):

https://codereview.chromium.org/2220323002/diff/140001/chrome/browser/permiss...
chrome/browser/permissions/permission_uma_util_unittest.cc:159:
.WillOnce(testing::Return(false));
On 2016/08/19 00:08:48, Nathan Parker wrote:
> Does this work?  (I assume so).  Like, it overrides the expect_call you
applied
> in SetMockSyncService?  If so, cool.

Yup.

[kcarattini, 2016-08-19 01:06:10]

The CQ bit was checked by kcarattini@chromium.org

[kcarattini, 2016-08-19 01:06:11]

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/2220323002/#ps140001
(title: "review comments")

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

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 03:04:05]

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

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

Failed to apply the patch.
On branch working_branch
Your branch is up-to-date with 'origin/refs/pending/heads/master'.
nothing to commit, working tree clean

[commit-bot: I haz the power, 2016-08-19 03:05:25]

Description was changed from

==========
Permission Action Reporting: Do not report for Custom Passphrase users.

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

to

==========
Permission Action Reporting: Do not report for Custom Passphrase users.

BUG=613883

Committed: https://crrev.com/787d1843f6e3b028b008b7a701432fe07225a8a7
Cr-Commit-Position: refs/heads/master@{#413038}
==========

[commit-bot: I haz the power, 2016-08-19 03:05:26]

Patchset 8 (id:??) landed as
https://crrev.com/787d1843f6e3b028b008b7a701432fe07225a8a7
Cr-Commit-Position: refs/heads/master@{#413038}