Separate UMA histograms for user and device policy invalidation

Separate UMA histograms for user and device policy invalidation

This CL separates the UMA histograms used for user and device policy
invalidation.

BUG=358699
TEST=Extended unit tests

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=289065

[bartfab (slow), 2014-08-11 13:08:16]

Hi Joao,

Could you take a look at this CL?

Hi Alexei,

Could you do an owner review for histograms.xml?

[Joao da Silva, 2014-08-11 13:21:27]

lgtm

I don't feel strongly about the suggestion inline, up to your preference.

https://codereview.chromium.org/465433002/diff/1/chrome/browser/policy/cloud/...
File chrome/browser/policy/cloud/cloud_policy_invalidator.h (right):

https://codereview.chromium.org/465433002/diff/1/chrome/browser/policy/cloud/...
chrome/browser/policy/cloud/cloud_policy_invalidator.h:68: bool
handling_user_policy,
WDYT of using enterprise_management::DeviceRegisterRequest::Type instead of a
bool?

The call site would be more obvious (new
CloudPolicyInvalidator(em::DeviceRegisterRequest::DEVICE), vs USER or BROWSER or
etc).

[bartfab (slow), 2014-08-11 14:01:59]

https://codereview.chromium.org/465433002/diff/1/chrome/browser/policy/cloud/...
File chrome/browser/policy/cloud/cloud_policy_invalidator.h (right):

https://codereview.chromium.org/465433002/diff/1/chrome/browser/policy/cloud/...
chrome/browser/policy/cloud/cloud_policy_invalidator.h:68: bool
handling_user_policy,
On 2014/08/11 13:21:27, Joao da Silva wrote:
> WDYT of using enterprise_management::DeviceRegisterRequest::Type instead of a
> bool?
> 
> The call site would be more obvious (new
> CloudPolicyInvalidator(em::DeviceRegisterRequest::DEVICE), vs USER or BROWSER
or
> etc).

Done.

[Joao da Silva, 2014-08-11 14:12:02]

still lgtm

https://codereview.chromium.org/465433002/diff/20001/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc (right):

https://codereview.chromium.org/465433002/diff/20001/chrome/browser/policy/cl...
chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc:273:
em::PolicyData* data = new enterprise_management::PolicyData();
new em::PolicyData

[bartfab (slow), 2014-08-11 14:21:31]

https://codereview.chromium.org/465433002/diff/20001/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc (right):

https://codereview.chromium.org/465433002/diff/20001/chrome/browser/policy/cl...
chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc:273:
em::PolicyData* data = new enterprise_management::PolicyData();
On 2014/08/11 14:12:01, Joao da Silva wrote:
> new em::PolicyData

Done.

[Alexei Svitkine (slow), 2014-08-11 16:55:26]

lgtm

https://codereview.chromium.org/465433002/diff/40001/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc (right):

https://codereview.chromium.org/465433002/diff/40001/chrome/browser/policy/cl...
chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc:390:
CloudPolicyInvalidatorTest::GetPolicyType() const {
Nit: Maybe make this function exist in the non-test code so that it could be
used by the ctor of user_cloud_policy_invalidator.cc, rather than duplicating
the same logic.

https://codereview.chromium.org/465433002/diff/40001/chrome/browser/policy/cl...
chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc:795: };
Nit: DISALLOW_COPY_AND_ASSIGN

[bartfab (slow), 2014-08-11 17:58:12]

https://codereview.chromium.org/465433002/diff/40001/chrome/browser/policy/cl...
File chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc (right):

https://codereview.chromium.org/465433002/diff/40001/chrome/browser/policy/cl...
chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc:390:
CloudPolicyInvalidatorTest::GetPolicyType() const {
On 2014/08/11 16:55:26, Alexei Svitkine wrote:
> Nit: Maybe make this function exist in the non-test code so that it could be
> used by the ctor of user_cloud_policy_invalidator.cc, rather than duplicating
> the same logic.

Done.

https://codereview.chromium.org/465433002/diff/40001/chrome/browser/policy/cl...
chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc:795: };
On 2014/08/11 16:55:26, Alexei Svitkine wrote:
> Nit: DISALLOW_COPY_AND_ASSIGN

Done.

[bartfab (slow), 2014-08-11 18:00:06]

The CQ bit was checked by bartfab@chromium.org

[commit-bot: I haz the power, 2014-08-11 19:17:23]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bartfab@chromium.org/465433002/80001

[commit-bot: I haz the power, 2014-08-11 21:05:17]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-11 21:36:40]

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

[commit-bot: I haz the power, 2014-08-11 21:36:42]

Try jobs failed on following builders:
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)

[bartfab (slow), 2014-08-12 09:58:48]

The CQ bit was checked by bartfab@chromium.org

[commit-bot: I haz the power, 2014-08-12 10:01:32]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bartfab@chromium.org/465433002/100001

[commit-bot: I haz the power, 2014-08-12 14:43:16]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-12 17:23:59]

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

[commit-bot: I haz the power, 2014-08-12 17:24:00]

Try jobs failed on following builders:
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[bartfab (slow), 2014-08-12 17:33:31]

The CQ bit was checked by bartfab@chromium.org

[commit-bot: I haz the power, 2014-08-12 17:35:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bartfab@chromium.org/465433002/100001

[commit-bot: I haz the power, 2014-08-12 21:34:14]

Change committed as 289065