Only collect Permission Action Reporting data in official builds

Only collect Permission Action Reporting data in official builds

BUG=722730
Review-Url: https://codereview.chromium.org/2887613003
Cr-Commit-Position: refs/heads/master@{#474834}
Committed: https://chromium.googlesource.com/chromium/src/+/d3422dd5e7cac07e39e06cca707c76eeaf4aa927

[benwells, 2017-05-16 11:27:05]

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

[commit-bot: I haz the power, 2017-05-16 11:27:29]

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-05-16 11:56:46]

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

[commit-bot: I haz the power, 2017-05-16 11:56:46]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[benwells, 2017-05-24 04:21:04]

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

[commit-bot: I haz the power, 2017-05-24 04:21:54]

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-05-24 04:54:33]

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

[commit-bot: I haz the power, 2017-05-24 04:54:34]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[benwells, 2017-05-24 10:56:03]

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

[commit-bot: I haz the power, 2017-05-24 10:56:24]

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-05-24 11:30:20]

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

[commit-bot: I haz the power, 2017-05-24 11:30:21]

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

[benwells, 2017-05-24 20:58:38]

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

[commit-bot: I haz the power, 2017-05-24 20:59:22]

Dry run: CQ is trying da patch.

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

[benwells, 2017-05-24 22:32:42]

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

[commit-bot: I haz the power, 2017-05-24 22:47:50]

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

[commit-bot: I haz the power, 2017-05-24 22:47:51]

Dry run: This issue passed the CQ dry run.

[Nathan Parker, 2017-05-25 00:13:12]

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

[Nathan Parker, 2017-05-25 00:13:13]

+vakh for comment, in case the permission-reporting hash-checking code also
depends on official-chrome build.

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

https://codereview.chromium.org/2887613003/diff/60001/chrome/browser/permissi...
chrome/browser/permissions/permission_uma_util.cc:592: bool official_build =
gIsFakeOfficialBuildForTest;
Do these tests need to run on non-official test builds?  Would it be sufficient
to run them just on official-Chrome test builds? If so, then you could not build
all these files on non-official builds.

[benwells, 2017-05-25 01:17:41]

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

https://codereview.chromium.org/2887613003/diff/60001/chrome/browser/permissi...
chrome/browser/permissions/permission_uma_util.cc:592: bool official_build =
gIsFakeOfficialBuildForTest;
On 2017/05/25 00:13:13, Nathan Parker wrote:
> Do these tests need to run on non-official test builds?  Would it be
sufficient
> to run them just on official-Chrome test builds? If so, then you could not
build
> all these files on non-official builds.

I believe all the CQ and try bots are non-official / non-chrome-branded, but
I'll hack up a test to see if that is true.

There are official build bots but they are pretty late in the process, I'd like
any introduced bugs to be caught in the CQ / try stage.

[benwells, 2017-05-25 01:19:57]

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

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

Dry run: CQ is trying da patch.

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

[vakh (use Gerrit instead), 2017-05-25 01:59:17]

You could consider doing it like this also:
https://cs.chromium.org/chromium/src/components/safe_browsing_db/v4_local_dat...

https://codereview.chromium.org/2887613003/diff/100001/chrome/browser/permiss...
File chrome/browser/permissions/permission_uma_util.cc (right):

https://codereview.chromium.org/2887613003/diff/100001/chrome/browser/permiss...
chrome/browser/permissions/permission_uma_util.cc:583:
gIsFakeOfficialBuildForTest = true;
Perhaps add a CHECK(false) here for official builds?

https://codereview.chromium.org/2887613003/diff/100001/chrome/browser/permiss...
chrome/browser/permissions/permission_uma_util.cc:595: CHECK(false);
I must be reading this incorrectly but it looks like this will always crash the
official builds. If that's the case, why bother setting official_build?

[benwells, 2017-05-25 02:03:37]

https://codereview.chromium.org/2887613003/diff/100001/chrome/browser/permiss...
File chrome/browser/permissions/permission_uma_util.cc (right):

https://codereview.chromium.org/2887613003/diff/100001/chrome/browser/permiss...
chrome/browser/permissions/permission_uma_util.cc:583:
gIsFakeOfficialBuildForTest = true;
On 2017/05/25 01:59:17, vakh (Varun Khaneja) wrote:
> Perhaps add a CHECK(false) here for official builds?

The CHECK below is just for an experiment, not to land. I don't think I want any
CHECK(false)'s in the final version.

https://codereview.chromium.org/2887613003/diff/100001/chrome/browser/permiss...
chrome/browser/permissions/permission_uma_util.cc:595: CHECK(false);
On 2017/05/25 01:59:17, vakh (Varun Khaneja) wrote:
> I must be reading this incorrectly but it looks like this will always crash
the
> official builds. If that's the case, why bother setting official_build?

Sorry, this is just an experiment I'm doing to see if any of the CQ bots run in
this configuration. I would not land this!

[commit-bot: I haz the power, 2017-05-25 05:33:12]

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

[commit-bot: I haz the power, 2017-05-25 05:33:13]

Dry run: This issue passed the CQ dry run.

[benwells, 2017-05-25 06:46:00]

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

https://codereview.chromium.org/2887613003/diff/60001/chrome/browser/permissi...
chrome/browser/permissions/permission_uma_util.cc:592: bool official_build =
gIsFakeOfficialBuildForTest;
On 2017/05/25 01:17:41, benwells wrote:
> On 2017/05/25 00:13:13, Nathan Parker wrote:
> > Do these tests need to run on non-official test builds?  Would it be
> sufficient
> > to run them just on official-Chrome test builds? If so, then you could not
> build
> > all these files on non-official builds.
> 
> I believe all the CQ and try bots are non-official / non-chrome-branded, but
> I'll hack up a test to see if that is true.
> 
> There are official build bots but they are pretty late in the process, I'd
like
> any introduced bugs to be caught in the CQ / try stage.

OK - science done. Looks like no try bots run that configuration. So, I could
make this change super simple (i.e. just #if the whole thing out, including the
tests, unless it is an official build) but it won't end up getting tested except
on the internal official waterfall. I think I'd rather leave it like this and if
anyone breaks it we'll find  out in the CQ.

[vakh (use Gerrit instead), 2017-05-25 15:55:52]

lgtm

[Nathan Parker, 2017-05-25 20:51:18]

lgtm

[benwells, 2017-05-25 21:03:08]

The CQ bit was checked by benwells@chromium.org

[commit-bot: I haz the power, 2017-05-25 21:03:47]

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-05-25 22:57:08]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1495746188813860,
"parent_rev": "8914f114ee0aacaf04ed63fab14154ddcf7c1d3e", "commit_rev":
"d3422dd5e7cac07e39e06cca707c76eeaf4aa927"}

[commit-bot: I haz the power, 2017-05-25 22:57:19]

Description was changed from

==========
Only collect Permission Action Reporting data in official builds

BUG=722730
==========

to

==========
Only collect Permission Action Reporting data in official builds

BUG=722730

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

[commit-bot: I haz the power, 2017-05-25 22:57:20]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/d3422dd5e7cac07e39e06cca707c...