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

Review URL: https://codereview.chromium.org/465433002

Cr-Commit-Position: refs/heads/master@{#289065}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289065 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/chromeos/policy/device_cloud_policy_invalidator.cc

@@ -35,6 +35,7 @@
 #include "content/public/browser/notification_service.h"
 #include "google_apis/gaia/identity_provider.h"
 #include "net/url_request/url_request_context_getter.h"
+#include "policy/proto/device_management_backend.pb.h"
 
 class Profile;
 
@@ -251,6 +252,7 @@ void DeviceCloudPolicyInvalidator::CreateInvalidator(
     invalidation::InvalidationService* invalidation_service) {
   invalidation_service_ = invalidation_service;
   invalidator_.reset(new CloudPolicyInvalidator(
+      enterprise_management::DeviceRegisterRequest::DEVICE,
       g_browser_process->platform_part()->browser_policy_connector_chromeos()->
           GetDeviceCloudPolicyManager()->core(),
       base::MessageLoopProxy::current(),

chrome/browser/policy/cloud/cloud_policy_invalidator.cc

@@ -31,10 +31,12 @@ const int CloudPolicyInvalidator::kUnknownVersionIgnorePeriod = 30;
 const int CloudPolicyInvalidator::kMaxInvalidationTimeDelta = 300;
 
 CloudPolicyInvalidator::CloudPolicyInvalidator(
+    enterprise_management::DeviceRegisterRequest::Type type,
     CloudPolicyCore* core,
     const scoped_refptr<base::SequencedTaskRunner>& task_runner,
     scoped_ptr<base::Clock> clock)
     : state_(UNINITIALIZED),
+      type_(type),
       core_(core),
       task_runner_(task_runner),
       clock_(clock.Pass()),
@@ -141,11 +143,16 @@ void CloudPolicyInvalidator::OnStoreLoaded(CloudPolicyStore* store) {
   bool policy_changed = IsPolicyChanged(store->policy());
 
   if (is_registered_) {
-    // Update the kMetricPolicyRefresh histogram.
-    UMA_HISTOGRAM_ENUMERATION(
-        kMetricPolicyRefresh,
-        GetPolicyRefreshMetric(policy_changed),
-        METRIC_POLICY_REFRESH_SIZE);
+    // Update the kMetricDevicePolicyRefresh/kMetricUserPolicyRefresh histogram.
+    if (type_ == enterprise_management::DeviceRegisterRequest::DEVICE) {
+      UMA_HISTOGRAM_ENUMERATION(kMetricDevicePolicyRefresh,
+                                GetPolicyRefreshMetric(policy_changed),
+                                METRIC_POLICY_REFRESH_SIZE);
+    } else {
+      UMA_HISTOGRAM_ENUMERATION(kMetricUserPolicyRefresh,
+                                GetPolicyRefreshMetric(policy_changed),
+                                METRIC_POLICY_REFRESH_SIZE);
+    }
 
     // If the policy was invalid and the version stored matches the latest
     // invalidation version, acknowledge the latest invalidation.
@@ -189,10 +196,18 @@ void CloudPolicyInvalidator::HandleInvalidation(
 
   // Ignore the invalidation if it is expired.
   bool is_expired = IsInvalidationExpired(version);
-  UMA_HISTOGRAM_ENUMERATION(
-      kMetricPolicyInvalidations,
-      GetInvalidationMetric(payload.empty(), is_expired),
-      POLICY_INVALIDATION_TYPE_SIZE);
+
+  if (type_ == enterprise_management::DeviceRegisterRequest::DEVICE) {
+    UMA_HISTOGRAM_ENUMERATION(
+        kMetricDevicePolicyInvalidations,
+        GetInvalidationMetric(payload.empty(), is_expired),
+        POLICY_INVALIDATION_TYPE_SIZE);
+  } else {
+    UMA_HISTOGRAM_ENUMERATION(
+        kMetricUserPolicyInvalidations,
+        GetInvalidationMetric(payload.empty(), is_expired),
+        POLICY_INVALIDATION_TYPE_SIZE);
+  }
   if (is_expired) {
     invalidation.Acknowledge();
     return;

chrome/browser/policy/cloud/cloud_policy_invalidator.h

@@ -9,6 +9,7 @@
 
 #include "base/basictypes.h"
 #include "base/callback.h"
+#include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
@@ -18,6 +19,7 @@
 #include "components/policy/core/common/cloud/cloud_policy_core.h"
 #include "components/policy/core/common/cloud/cloud_policy_store.h"
 #include "google/cacheinvalidation/include/types.h"
+#include "policy/proto/device_management_backend.pb.h"
 
 namespace base {
 class Clock;
@@ -56,12 +58,14 @@ class CloudPolicyInvalidator : public syncer::InvalidationHandler,
   // invalidation timestamps when determining if an invalidation is expired.
   static const int kMaxInvalidationTimeDelta;
 
+  // |type| indicates the policy type that this invalidator is responsible for.
   // |core| is the cloud policy core which connects the various policy objects.
   // It must remain valid until Shutdown is called.
   // |task_runner| is used for scheduling delayed tasks. It must post tasks to
   // the main policy thread.
   // |clock| is used to get the current time.
   CloudPolicyInvalidator(
+      enterprise_management::DeviceRegisterRequest::Type type,
       CloudPolicyCore* core,
       const scoped_refptr<base::SequencedTaskRunner>& task_runner,
       scoped_ptr<base::Clock> clock);
@@ -156,6 +160,9 @@ class CloudPolicyInvalidator : public syncer::InvalidationHandler,
   };
   State state_;
 
+  // The policy type this invalidator is responsible for.
+  const enterprise_management::DeviceRegisterRequest::Type type_;
+
   // The cloud policy core.
   CloudPolicyCore* core_;
 

chrome/browser/policy/cloud/user_cloud_policy_invalidator.cc

@@ -19,10 +19,10 @@ namespace policy {
 UserCloudPolicyInvalidator::UserCloudPolicyInvalidator(
     Profile* profile,
     CloudPolicyManager* policy_manager)
-    : CloudPolicyInvalidator(
-          policy_manager->core(),
-          base::MessageLoopProxy::current(),
-          scoped_ptr<base::Clock>(new base::DefaultClock())),
+    : CloudPolicyInvalidator(GetPolicyType(),
+                             policy_manager->core(),
+                             base::MessageLoopProxy::current(),
+                             scoped_ptr<base::Clock>(new base::DefaultClock())),
       profile_(profile) {
   DCHECK(profile);
 
@@ -38,6 +38,20 @@ UserCloudPolicyInvalidator::UserCloudPolicyInvalidator(
                  content::Source<Profile>(profile));
 }
 
+// static
+enterprise_management::DeviceRegisterRequest::Type
+UserCloudPolicyInvalidator::GetPolicyType() {
+#if defined(OS_CHROMEOS)
+  return enterprise_management::DeviceRegisterRequest::USER;
+#elif defined(OS_ANDROID)
+  return enterprise_management::DeviceRegisterRequest::ANDROID_BROWSER;
+#elif defined(OS_IOS)
+  return enterprise_management::DeviceRegisterRequest::IOS_BROWSER;
+#else
+  return enterprise_management::DeviceRegisterRequest::BROWSER;
+#endif
+}
+
 void UserCloudPolicyInvalidator::Shutdown() {
   CloudPolicyInvalidator::Shutdown();
 }

chrome/browser/policy/cloud/user_cloud_policy_invalidator.h

@@ -9,6 +9,7 @@
 #include "components/keyed_service/core/keyed_service.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
+#include "policy/proto/device_management_backend.pb.h"
 
 class Profile;
 
@@ -31,6 +32,8 @@ class UserCloudPolicyInvalidator : public CloudPolicyInvalidator,
       Profile* profile,
       CloudPolicyManager* policy_manager);
 
+  static enterprise_management::DeviceRegisterRequest::Type GetPolicyType();
+
   // KeyedService:
   virtual void Shutdown() OVERRIDE;
 

components/policy/core/common/cloud/enterprise_metrics.cc

@@ -10,7 +10,10 @@ const char kMetricToken[] = "Enterprise.DMToken";
 const char kMetricPolicy[] = "Enterprise.Policy";
 const char kMetricEnrollment[] = "Enterprise.Enrollment";
 const char kMetricEnrollmentRecovery[] = "Enterprise.EnrollmentRecovery";
-const char kMetricPolicyRefresh[] = "Enterprise.PolicyRefresh";
-const char kMetricPolicyInvalidations[] = "Enterprise.PolicyInvalidations";
+const char kMetricUserPolicyRefresh[] = "Enterprise.PolicyRefresh";
+const char kMetricUserPolicyInvalidations[] = "Enterprise.PolicyInvalidations";
+const char kMetricDevicePolicyRefresh[] = "Enterprise.DevicePolicyRefresh";
+const char kMetricDevicePolicyInvalidations[] =
+    "Enterprise.DevicePolicyInvalidations";
 
 }  // namespace policy

components/policy/core/common/cloud/enterprise_metrics.h

@@ -239,8 +239,10 @@ POLICY_EXPORT extern const char kMetricToken[];
 POLICY_EXPORT extern const char kMetricPolicy[];
 POLICY_EXPORT extern const char kMetricEnrollment[];
 POLICY_EXPORT extern const char kMetricEnrollmentRecovery[];
-POLICY_EXPORT extern const char kMetricPolicyRefresh[];
-POLICY_EXPORT extern const char kMetricPolicyInvalidations[];
+POLICY_EXPORT extern const char kMetricUserPolicyRefresh[];
+POLICY_EXPORT extern const char kMetricUserPolicyInvalidations[];
+POLICY_EXPORT extern const char kMetricDevicePolicyRefresh[];
+POLICY_EXPORT extern const char kMetricDevicePolicyInvalidations[];
 
 }  // namespace policy
 

tools/metrics/histograms/histograms.xml

@@ -5781,6 +5781,27 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   <summary>URL fetcher status for auto-enrollment requests.</summary>
 </histogram>
 
+<histogram name="Enterprise.DevicePolicyInvalidations"
+    enum="EnterprisePolicyInvalidations">
+  <owner>bartfab@chromium.org</owner>
+  <summary>
+    Events for counting device policy invalidations received with and without
+    payloads. Invalidations indicate that a policy has been updated and should
+    be refreshed. Payloads provide context about the policy update, but may be
+    absent if dropped by the invalidation service.
+  </summary>
+</histogram>
+
+<histogram name="Enterprise.DevicePolicyRefresh" enum="EnterprisePolicyRefresh">
+  <owner>bartfab@chromium.org</owner>
+  <summary>
+    Events measuring effectiveness of refreshing device policy when
+    invalidations are received from a service. For each refresh, indicates
+    whether the policy changed, and whether the policy was invalidated at the
+    time of the refresh.
+  </summary>
+</histogram>
+
 <histogram name="Enterprise.DMToken" enum="EnterpriseDMTokenType">
   <owner>joaodasilva@chromium.org</owner>
   <summary>
@@ -5880,9 +5901,9 @@ Therefore, the affected-histogram name has to have at least one dot in it.
     enum="EnterprisePolicyInvalidations">
   <owner>joaodasilva@chromium.org</owner>
   <summary>
-    Events for counting policy invalidations received with and without payloads.
-    Invalidations indicate that a policy has been updated and should be
-    refreshed. Payloads provide context about the policy update, but may be
+    Events for counting user policy invalidations received with and without
+    payloads. Invalidations indicate that a policy has been updated and should
+    be refreshed. Payloads provide context about the policy update, but may be
     absent if dropped by the invalidation service.
   </summary>
 </histogram>
@@ -5907,8 +5928,8 @@ Therefore, the affected-histogram name has to have at least one dot in it.
 <histogram name="Enterprise.PolicyRefresh" enum="EnterprisePolicyRefresh">
   <owner>joaodasilva@chromium.org</owner>
   <summary>
-    Events measuring effectiveness of refreshing policy when invalidations are
-    received from a service. For each refresh, indicates whether the policy
+    Events measuring effectiveness of refreshing user policy when invalidations
+    are received from a service. For each refresh, indicates whether the policy
     changed, and whether the policy was invalidated at the time of the refresh.
   </summary>
 </histogram>