Separate UMA histograms for user and device policy invalidation

chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc

Index: chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc
diff --git a/chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc b/chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc
index 75f7ec41acda48d0483228d4143d754b47b62e86..82ac9d358c8cd8d0418656e78985e4d394b3bd6f 100644
--- a/chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc
+++ b/chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc
@@ -29,10 +29,11 @@
 #include "components/policy/core/common/cloud/mock_cloud_policy_store.h"
 #include "components/policy/core/common/policy_types.h"
 #include "policy/policy_constants.h"
-#include "policy/proto/device_management_backend.pb.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace em = enterprise_management;
+
 namespace policy {
 
 class CloudPolicyInvalidatorTest : public testing::Test {
@@ -46,8 +47,6 @@ class CloudPolicyInvalidatorTest : public testing::Test {
 
   CloudPolicyInvalidatorTest();
 
-  virtual void SetUp() OVERRIDE;
-
   virtual void TearDown() OVERRIDE;
 
   // Starts the invalidator which will be tested.
@@ -147,10 +146,6 @@ class CloudPolicyInvalidatorTest : public testing::Test {
   // invalidation service.
   bool IsInvalidatorRegistered();
 
-  // Get the current count for the given metric.
-  base::HistogramBase::Count GetCount(MetricPolicyRefresh metric);
-  base::HistogramBase::Count GetInvalidationCount(PolicyInvalidationType type);
-
   // Advance the test clock.
   void AdvanceClock(base::TimeDelta delta);
 
@@ -164,6 +159,9 @@ class CloudPolicyInvalidatorTest : public testing::Test {
   // Get an invalidation version for the given time.
   int64 GetVersion(base::Time time);
 
+  // Get the policy type that the |invalidator_| is responsible for.
+  virtual em::DeviceRegisterRequest::Type GetPolicyType() const;
+
  private:
   // Checks that the policy was refreshed due to an invalidation with the given
   // base delay.
@@ -175,10 +173,6 @@ class CloudPolicyInvalidatorTest : public testing::Test {
   // Returns the object id of the given policy object.
   const invalidation::ObjectId& GetPolicyObjectId(PolicyObject object) const;
 
-  // Get histogram samples for the given histogram.
-  scoped_ptr<base::HistogramSamples> GetHistogramSamples(
-      const std::string& name) const;
-
   base::MessageLoop loop_;
 
   // Objects the invalidator depends on.
@@ -203,12 +197,6 @@ class CloudPolicyInvalidatorTest : public testing::Test {
 
   // The currently used policy value.
   const char* policy_value_cur_;
-
-  // Stores starting histogram counts for kMetricPolicyRefresh.
-  scoped_ptr<base::HistogramSamples> refresh_samples_;
-
-  // Stores starting histogram counts for kMetricPolicyInvalidations.
-  scoped_ptr<base::HistogramSamples> invalidations_samples_;
 };
 
 CloudPolicyInvalidatorTest::CloudPolicyInvalidatorTest()
@@ -228,12 +216,6 @@ CloudPolicyInvalidatorTest::CloudPolicyInvalidatorTest()
       base::Time::UnixEpoch() + base::TimeDelta::FromSeconds(987654321));
 }
 
-void CloudPolicyInvalidatorTest::SetUp() {
-  base::StatisticsRecorder::Initialize();
-  refresh_samples_ = GetHistogramSamples(kMetricPolicyRefresh);
-  invalidations_samples_ = GetHistogramSamples(kMetricPolicyInvalidations);
-}
-
 void CloudPolicyInvalidatorTest::TearDown() {
   if (invalidator_)
     invalidator_->Shutdown();
@@ -244,6 +226,7 @@ void CloudPolicyInvalidatorTest::StartInvalidator(
     bool initialize,
     bool start_refresh_scheduler) {
   invalidator_.reset(new CloudPolicyInvalidator(
+      GetPolicyType(),
       &core_,
       task_runner_,
       scoped_ptr<base::Clock>(clock_)));
@@ -287,8 +270,7 @@ void CloudPolicyInvalidatorTest::StorePolicy(
     int64 invalidation_version,
     bool policy_changed,
     const base::Time& time) {
-  enterprise_management::PolicyData* data =
-      new enterprise_management::PolicyData();
+  em::PolicyData* data = new em::PolicyData();
   if (object != POLICY_OBJECT_NONE) {
     data->set_invalidation_source(GetPolicyObjectId(object).source());
     data->set_invalidation_name(GetPolicyObjectId(object).name());
@@ -388,18 +370,6 @@ bool CloudPolicyInvalidatorTest::IsInvalidatorRegistered() {
       .GetRegisteredIds(invalidator_.get()).empty();
 }
 
-base::HistogramBase::Count CloudPolicyInvalidatorTest::GetCount(
-    MetricPolicyRefresh metric) {
-  return GetHistogramSamples(kMetricPolicyRefresh)->GetCount(metric) -
-      refresh_samples_->GetCount(metric);
-}
-
-base::HistogramBase::Count CloudPolicyInvalidatorTest::GetInvalidationCount(
-    PolicyInvalidationType type) {
-  return GetHistogramSamples(kMetricPolicyInvalidations)->GetCount(type) -
-      invalidations_samples_->GetCount(type);
-}
-
 void CloudPolicyInvalidatorTest::AdvanceClock(base::TimeDelta delta) {
   clock_->Advance(delta);
 }
@@ -416,6 +386,19 @@ int64 CloudPolicyInvalidatorTest::GetVersion(base::Time time) {
   return (time - base::Time::UnixEpoch()).InMicroseconds();
 }
 
+em::DeviceRegisterRequest::Type
+CloudPolicyInvalidatorTest::GetPolicyType() const {

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

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.

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

Done.

+#if defined(OS_CHROMEOS)
+    return em::DeviceRegisterRequest::USER;
+#elif defined(OS_ANDROID)
+    return em::DeviceRegisterRequest::ANDROID_BROWSER;
+#elif defined(OS_IOS)
+    return em::DeviceRegisterRequest::IOS_BROWSER;
+#else
+    return em::DeviceRegisterRequest::BROWSER;
+#endif
+}
+
 bool CloudPolicyInvalidatorTest::CheckPolicyRefreshed(base::TimeDelta delay) {
   base::TimeDelta max_delay = delay + base::TimeDelta::FromMilliseconds(
       CloudPolicyInvalidator::kMaxFetchDelayMin);
@@ -453,16 +436,6 @@ const invalidation::ObjectId& CloudPolicyInvalidatorTest::GetPolicyObjectId(
   return object == POLICY_OBJECT_A ? object_id_a_ : object_id_b_;
 }
 
-scoped_ptr<base::HistogramSamples>
-    CloudPolicyInvalidatorTest::GetHistogramSamples(
-        const std::string& name) const {
-  base::HistogramBase* histogram =
-      base::StatisticsRecorder::FindHistogram(name);
-  if (!histogram)
-    return scoped_ptr<base::HistogramSamples>(new base::SampleMap());
-  return histogram->SnapshotSamples();
-}
-
 TEST_F(CloudPolicyInvalidatorTest, Uninitialized) {
   // No invalidations should be processed if the invalidator is not initialized.
   StartInvalidator(false /* initialize */, true /* start_refresh_scheduler */);
@@ -795,7 +768,75 @@ TEST_F(CloudPolicyInvalidatorTest, Disconnect) {
   EXPECT_FALSE(InvalidationsEnabled());
 }
 
-TEST_F(CloudPolicyInvalidatorTest, RefreshMetricsUnregistered) {
+class CloudPolicyInvalidatorUserTypedTest
+    : public CloudPolicyInvalidatorTest,
+      public testing::WithParamInterface<em::DeviceRegisterRequest::Type>  {
+ protected:
+  // CloudPolicyInvalidatorTest:
+  virtual void SetUp() OVERRIDE;
+
+  // Get the current count for the given metric.
+  base::HistogramBase::Count GetCount(MetricPolicyRefresh metric);
+  base::HistogramBase::Count GetInvalidationCount(PolicyInvalidationType type);
+
+ private:
+  // CloudPolicyInvalidatorTest:
+  virtual em::DeviceRegisterRequest::Type GetPolicyType() const OVERRIDE;
+
+  // Get histogram samples for the given histogram.
+  scoped_ptr<base::HistogramSamples> GetHistogramSamples(
+      const std::string& name) const;
+
+  // Stores starting histogram counts for kMetricPolicyRefresh.
+  scoped_ptr<base::HistogramSamples> refresh_samples_;
+
+  // Stores starting histogram counts for kMetricPolicyInvalidations.
+  scoped_ptr<base::HistogramSamples> invalidations_samples_;
+};

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

Nit: DISALLOW_COPY_AND_ASSIGN

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

Done.

+
+void CloudPolicyInvalidatorUserTypedTest::SetUp() {
+  base::StatisticsRecorder::Initialize();
+  refresh_samples_ = GetHistogramSamples(
+      GetPolicyType() == em::DeviceRegisterRequest::DEVICE ?
+          kMetricDevicePolicyRefresh : kMetricUserPolicyRefresh);
+  invalidations_samples_ = GetHistogramSamples(
+      GetPolicyType() == em::DeviceRegisterRequest::DEVICE ?
+          kMetricDevicePolicyInvalidations : kMetricUserPolicyInvalidations);
+}
+
+base::HistogramBase::Count CloudPolicyInvalidatorUserTypedTest::GetCount(
+    MetricPolicyRefresh metric) {
+  return GetHistogramSamples(
+      GetPolicyType() == em::DeviceRegisterRequest::DEVICE ?
+          kMetricDevicePolicyRefresh : kMetricUserPolicyRefresh)->
+              GetCount(metric) - refresh_samples_->GetCount(metric);
+}
+
+base::HistogramBase::Count
+CloudPolicyInvalidatorUserTypedTest::GetInvalidationCount(
+    PolicyInvalidationType type) {
+  return GetHistogramSamples(
+      GetPolicyType() == em::DeviceRegisterRequest::DEVICE ?
+          kMetricDevicePolicyInvalidations : kMetricUserPolicyInvalidations)->
+          GetCount(type) - invalidations_samples_->GetCount(type);
+}
+
+em::DeviceRegisterRequest::Type
+CloudPolicyInvalidatorUserTypedTest::GetPolicyType() const {
+  return GetParam();
+}
+
+scoped_ptr<base::HistogramSamples>
+CloudPolicyInvalidatorUserTypedTest::GetHistogramSamples(
+    const std::string& name) const {
+  base::HistogramBase* histogram =
+      base::StatisticsRecorder::FindHistogram(name);
+  if (!histogram)
+    return scoped_ptr<base::HistogramSamples>(new base::SampleMap());
+  return histogram->SnapshotSamples();
+}
+
+TEST_P(CloudPolicyInvalidatorUserTypedTest, RefreshMetricsUnregistered) {
   // Store loads occurring before invalidation registration are not counted.
   StartInvalidator();
   StorePolicy(POLICY_OBJECT_NONE, 0, false /* policy_changed */);
@@ -807,7 +848,7 @@ TEST_F(CloudPolicyInvalidatorTest, RefreshMetricsUnregistered) {
   EXPECT_EQ(0, GetCount(METRIC_POLICY_REFRESH_INVALIDATED_UNCHANGED));
 }
 
-TEST_F(CloudPolicyInvalidatorTest, RefreshMetricsNoInvalidations) {
+TEST_P(CloudPolicyInvalidatorUserTypedTest, RefreshMetricsNoInvalidations) {
   // Store loads occurring while registered should be differentiated depending
   // on whether the invalidation service was enabled or not.
   StorePolicy(POLICY_OBJECT_A);
@@ -858,7 +899,7 @@ TEST_F(CloudPolicyInvalidatorTest, RefreshMetricsNoInvalidations) {
   EXPECT_EQ(0, GetCount(METRIC_POLICY_REFRESH_INVALIDATED_UNCHANGED));
 }
 
-TEST_F(CloudPolicyInvalidatorTest, RefreshMetricsInvalidation) {
+TEST_P(CloudPolicyInvalidatorUserTypedTest, RefreshMetricsInvalidation) {
   // Store loads after an invalidation are counted as invalidated, even if
   // the loads do not result in the invalidation being acknowledged.
   StartInvalidator();
@@ -887,7 +928,7 @@ TEST_F(CloudPolicyInvalidatorTest, RefreshMetricsInvalidation) {
   EXPECT_EQ(1, GetCount(METRIC_POLICY_REFRESH_INVALIDATED_UNCHANGED));
 }
 
-TEST_F(CloudPolicyInvalidatorTest, ExpiredInvalidations) {
+TEST_P(CloudPolicyInvalidatorUserTypedTest, ExpiredInvalidations) {
   StorePolicy(POLICY_OBJECT_A, 0, false, Now());
   StartInvalidator();
 
@@ -948,4 +989,18 @@ TEST_F(CloudPolicyInvalidatorTest, ExpiredInvalidations) {
   EXPECT_EQ(2, GetInvalidationCount(POLICY_INVALIDATION_TYPE_EXPIRED));
 }
 
+INSTANTIATE_TEST_CASE_P(
+    CloudPolicyInvalidatorUserTypedTestInstance,
+    CloudPolicyInvalidatorUserTypedTest,
+#if defined(OS_CHROMEOS)
+    testing::Values(em::DeviceRegisterRequest::USER,
+                    em::DeviceRegisterRequest::DEVICE));
+#elif defined(OS_ANDROID)
+    testing::Values(em::DeviceRegisterRequest::ANDROID_BROWSER));
+#elif defined(OS_IOS)
+    testing::Values(em::DeviceRegisterRequest::IOS_BROWSER));
+#else
+    testing::Values(em::DeviceRegisterRequest::BROWSER));
+#endif
+
 }  // namespace policy