diff options
author | bartfab@chromium.org <bartfab@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-08-12 21:34:05 +0000 |
---|---|---|
committer | bartfab@chromium.org <bartfab@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-08-12 21:35:32 +0000 |
commit | d809066a8027ae95705cb70eb07b8fcc983c8759 (patch) | |
tree | f5dd0e45f6f5e32328b70506832450e72d36bc5c /chrome/browser/policy/cloud | |
parent | 58d9b9dc45fe73c575e98af0fa19017e0ddce2d6 (diff) | |
download | chromium_src-d809066a8027ae95705cb70eb07b8fcc983c8759.zip chromium_src-d809066a8027ae95705cb70eb07b8fcc983c8759.tar.gz chromium_src-d809066a8027ae95705cb70eb07b8fcc983c8759.tar.bz2 |
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
Diffstat (limited to 'chrome/browser/policy/cloud')
5 files changed, 171 insertions, 64 deletions
diff --git a/chrome/browser/policy/cloud/cloud_policy_invalidator.cc b/chrome/browser/policy/cloud/cloud_policy_invalidator.cc index 6ba5047..ee83bbe 100644 --- a/chrome/browser/policy/cloud/cloud_policy_invalidator.cc +++ b/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; diff --git a/chrome/browser/policy/cloud/cloud_policy_invalidator.h b/chrome/browser/policy/cloud/cloud_policy_invalidator.h index ef51880..6653a5d 100644 --- a/chrome/browser/policy/cloud/cloud_policy_invalidator.h +++ b/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_; diff --git a/chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc b/chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc index 75f7ec4..3e37658 100644 --- a/chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc +++ b/chrome/browser/policy/cloud/cloud_policy_invalidator_unittest.cc @@ -20,6 +20,7 @@ #include "base/values.h" #include "chrome/browser/invalidation/fake_invalidation_service.h" #include "chrome/browser/policy/cloud/cloud_policy_invalidator.h" +#include "chrome/browser/policy/cloud/user_cloud_policy_invalidator.h" #include "components/invalidation/invalidation_util.h" #include "components/policy/core/common/cloud/cloud_policy_constants.h" #include "components/policy/core/common/cloud/cloud_policy_core.h" @@ -29,10 +30,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 +48,6 @@ class CloudPolicyInvalidatorTest : public testing::Test { CloudPolicyInvalidatorTest(); - virtual void SetUp() OVERRIDE; - virtual void TearDown() OVERRIDE; // Starts the invalidator which will be tested. @@ -147,10 +147,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 +160,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 +174,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 +198,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 +217,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 +227,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 +271,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 +371,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 +387,11 @@ int64 CloudPolicyInvalidatorTest::GetVersion(base::Time time) { return (time - base::Time::UnixEpoch()).InMicroseconds(); } +em::DeviceRegisterRequest::Type +CloudPolicyInvalidatorTest::GetPolicyType() const { + return UserCloudPolicyInvalidator::GetPolicyType(); +} + bool CloudPolicyInvalidatorTest::CheckPolicyRefreshed(base::TimeDelta delay) { base::TimeDelta max_delay = delay + base::TimeDelta::FromMilliseconds( CloudPolicyInvalidator::kMaxFetchDelayMin); @@ -453,16 +429,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 +761,86 @@ TEST_F(CloudPolicyInvalidatorTest, Disconnect) { EXPECT_FALSE(InvalidationsEnabled()); } -TEST_F(CloudPolicyInvalidatorTest, RefreshMetricsUnregistered) { +class CloudPolicyInvalidatorUserTypedTest + : public CloudPolicyInvalidatorTest, + public testing::WithParamInterface<em::DeviceRegisterRequest::Type> { + protected: + CloudPolicyInvalidatorUserTypedTest(); + virtual ~CloudPolicyInvalidatorUserTypedTest(); + + // 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_; + + DISALLOW_COPY_AND_ASSIGN(CloudPolicyInvalidatorUserTypedTest); +}; + +CloudPolicyInvalidatorUserTypedTest::CloudPolicyInvalidatorUserTypedTest() { +} + +CloudPolicyInvalidatorUserTypedTest::~CloudPolicyInvalidatorUserTypedTest() { +} + +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 +852,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 +903,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 +932,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 +993,27 @@ TEST_F(CloudPolicyInvalidatorTest, ExpiredInvalidations) { EXPECT_EQ(2, GetInvalidationCount(POLICY_INVALIDATION_TYPE_EXPIRED)); } +#if defined(OS_CHROMEOS) +INSTANTIATE_TEST_CASE_P( + CloudPolicyInvalidatorUserTypedTestInstance, + CloudPolicyInvalidatorUserTypedTest, + testing::Values(em::DeviceRegisterRequest::USER, + em::DeviceRegisterRequest::DEVICE)); +#elif defined(OS_ANDROID) +INSTANTIATE_TEST_CASE_P( + CloudPolicyInvalidatorUserTypedTestInstance, + CloudPolicyInvalidatorUserTypedTest, + testing::Values(em::DeviceRegisterRequest::ANDROID_BROWSER)); +#elif defined(OS_IOS) +INSTANTIATE_TEST_CASE_P( + CloudPolicyInvalidatorUserTypedTestInstance, + CloudPolicyInvalidatorUserTypedTest, + testing::Values(em::DeviceRegisterRequest::IOS_BROWSER)); +#else +INSTANTIATE_TEST_CASE_P( + CloudPolicyInvalidatorUserTypedTestInstance, + CloudPolicyInvalidatorUserTypedTest, + testing::Values(em::DeviceRegisterRequest::BROWSER)); +#endif + } // namespace policy diff --git a/chrome/browser/policy/cloud/user_cloud_policy_invalidator.cc b/chrome/browser/policy/cloud/user_cloud_policy_invalidator.cc index 504abafc..7770616 100644 --- a/chrome/browser/policy/cloud/user_cloud_policy_invalidator.cc +++ b/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(); } diff --git a/chrome/browser/policy/cloud/user_cloud_policy_invalidator.h b/chrome/browser/policy/cloud/user_cloud_policy_invalidator.h index a5e7247..f9bea6c 100644 --- a/chrome/browser/policy/cloud/user_cloud_policy_invalidator.h +++ b/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; |