diff options
9 files changed, 5 insertions, 277 deletions
diff --git a/components/components_tests.gyp b/components/components_tests.gyp index 7a2342f..61d2ffb 100644 --- a/components/components_tests.gyp +++ b/components/components_tests.gyp @@ -621,7 +621,6 @@ 'policy/core/common/cloud/external_policy_data_updater_unittest.cc', 'policy/core/common/cloud/policy_header_io_helper_unittest.cc', 'policy/core/common/cloud/policy_header_service_unittest.cc', - 'policy/core/common/cloud/rate_limiter_unittest.cc', 'policy/core/common/cloud/resource_cache_unittest.cc', 'policy/core/common/cloud/user_cloud_policy_manager_unittest.cc', 'policy/core/common/cloud/user_cloud_policy_store_unittest.cc', diff --git a/components/policy/core/common/BUILD.gn b/components/policy/core/common/BUILD.gn index 991927b..da7cb0d 100644 --- a/components/policy/core/common/BUILD.gn +++ b/components/policy/core/common/BUILD.gn @@ -47,8 +47,6 @@ source_set("common") { "cloud/policy_header_io_helper.h", "cloud/policy_header_service.cc", "cloud/policy_header_service.h", - "cloud/rate_limiter.cc", - "cloud/rate_limiter.h", "cloud/resource_cache.cc", "cloud/resource_cache.h", "cloud/system_policy_request_context.cc", diff --git a/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.cc b/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.cc index ea4d53d..77f86c2 100644 --- a/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.cc +++ b/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.cc @@ -11,19 +11,10 @@ #include "base/memory/scoped_ptr.h" #include "base/metrics/histogram.h" #include "base/sequenced_task_runner.h" -#include "base/time/default_tick_clock.h" -#include "base/time/tick_clock.h" #include "components/policy/core/common/cloud/cloud_policy_constants.h" namespace policy { -namespace { - -// The maximum rate at which to refresh policies. -const size_t kMaxRefreshesPerHour = 5; - -} // namespace - #if defined(OS_ANDROID) || defined(OS_IOS) const int64 CloudPolicyRefreshScheduler::kDefaultRefreshDelayMs = @@ -73,12 +64,6 @@ CloudPolicyRefreshScheduler::CloudPolicyRefreshScheduler( task_runner_(task_runner), error_retry_delay_ms_(kInitialErrorRetryDelayMs), refresh_delay_ms_(kDefaultRefreshDelayMs), - rate_limiter_(kMaxRefreshesPerHour, - base::TimeDelta::FromHours(1), - base::Bind(&CloudPolicyRefreshScheduler::RefreshNow, - base::Unretained(this)), - task_runner_, - scoped_ptr<base::TickClock>(new base::DefaultTickClock())), invalidations_available_(false), creation_time_(base::Time::NowFromSystemTime()) { client_->AddObserver(this); @@ -102,7 +87,7 @@ void CloudPolicyRefreshScheduler::SetRefreshDelay(int64 refresh_delay) { } void CloudPolicyRefreshScheduler::RefreshSoon() { - rate_limiter_.PostRequest(); + RefreshNow(); } void CloudPolicyRefreshScheduler::SetInvalidationServiceAvailability( diff --git a/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.h b/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.h index a0e4090..efdb37b 100644 --- a/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.h +++ b/components/policy/core/common/cloud/cloud_policy_refresh_scheduler.h @@ -11,7 +11,6 @@ #include "base/time/time.h" #include "components/policy/core/common/cloud/cloud_policy_client.h" #include "components/policy/core/common/cloud/cloud_policy_store.h" -#include "components/policy/core/common/cloud/rate_limiter.h" #include "components/policy/policy_export.h" #include "net/base/network_change_notifier.h" @@ -52,8 +51,7 @@ class POLICY_EXPORT CloudPolicyRefreshScheduler // Sets the refresh delay to |refresh_delay| (subject to min/max clamping). void SetRefreshDelay(int64 refresh_delay); - // Requests a policy refresh to be performed soon. This may apply throttling, - // and the request may not be immediately sent. + // Requests a policy refresh to be performed soon. void RefreshSoon(); // The refresh scheduler starts by assuming that invalidations are not @@ -121,9 +119,6 @@ class POLICY_EXPORT CloudPolicyRefreshScheduler // The refresh delay. int64 refresh_delay_ms_; - // Used to limit the rate at which refreshes are scheduled. - RateLimiter rate_limiter_; - // Whether the invalidations service is available and receiving notifications // of policy updates. bool invalidations_available_; diff --git a/components/policy/core/common/cloud/cloud_policy_refresh_scheduler_unittest.cc b/components/policy/core/common/cloud/cloud_policy_refresh_scheduler_unittest.cc index 63149fc..d0f8671 100644 --- a/components/policy/core/common/cloud/cloud_policy_refresh_scheduler_unittest.cc +++ b/components/policy/core/common/cloud/cloud_policy_refresh_scheduler_unittest.cc @@ -193,19 +193,11 @@ TEST_F(CloudPolicyRefreshSchedulerTest, Unregistered) { EXPECT_TRUE(task_runner_->GetPendingTasks().empty()); } -TEST_F(CloudPolicyRefreshSchedulerTest, RefreshSoonRateLimit) { +TEST_F(CloudPolicyRefreshSchedulerTest, RefreshSoon) { scoped_ptr<CloudPolicyRefreshScheduler> scheduler(CreateRefreshScheduler()); - // Max out the request rate. - for (int i = 0; i < 5; ++i) { - EXPECT_CALL(client_, FetchPolicy()).Times(1); - scheduler->RefreshSoon(); - task_runner_->RunUntilIdle(); - Mock::VerifyAndClearExpectations(&client_); - } - // The next refresh is throttled. - EXPECT_CALL(client_, FetchPolicy()).Times(0); + EXPECT_CALL(client_, FetchPolicy()).Times(1); scheduler->RefreshSoon(); - task_runner_->RunPendingTasks(); + task_runner_->RunUntilIdle(); Mock::VerifyAndClearExpectations(&client_); } diff --git a/components/policy/core/common/cloud/rate_limiter.cc b/components/policy/core/common/cloud/rate_limiter.cc deleted file mode 100644 index 010d7d2..0000000 --- a/components/policy/core/common/cloud/rate_limiter.cc +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright (c) 2013 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "components/policy/core/common/cloud/rate_limiter.h" - -#include "base/bind.h" -#include "base/bind_helpers.h" -#include "base/location.h" -#include "base/logging.h" -#include "base/sequenced_task_runner.h" -#include "base/time/tick_clock.h" - -namespace policy { - -RateLimiter::RateLimiter(size_t max_requests, - const base::TimeDelta& duration, - const base::Closure& callback, - scoped_refptr<base::SequencedTaskRunner> task_runner, - scoped_ptr<base::TickClock> clock) - : max_requests_(max_requests), - duration_(duration), - callback_(callback), - task_runner_(task_runner), - clock_(clock.Pass()) { - DCHECK_GT(max_requests_, 0u); -} - -RateLimiter::~RateLimiter() {} - -void RateLimiter::PostRequest() { - DCHECK(CalledOnValidThread()); - - const base::TimeTicks now = clock_->NowTicks(); - const base::TimeTicks period_start = now - duration_; - while (!invocation_times_.empty() && - invocation_times_.front() <= period_start) { - invocation_times_.pop(); - } - - delayed_callback_.Cancel(); - - if (invocation_times_.size() < max_requests_) { - invocation_times_.push(now); - callback_.Run(); - } else { - // From the while() loop above we have front() > period_start, - // so time_until_next_callback > 0. - const base::TimeDelta time_until_next_callback = - invocation_times_.front() - period_start; - delayed_callback_.Reset( - base::Bind(&RateLimiter::PostRequest, base::Unretained(this))); - task_runner_->PostDelayedTask( - FROM_HERE, delayed_callback_.callback(), time_until_next_callback); - } -} - -} // namespace policy diff --git a/components/policy/core/common/cloud/rate_limiter.h b/components/policy/core/common/cloud/rate_limiter.h deleted file mode 100644 index 5949257..0000000 --- a/components/policy/core/common/cloud/rate_limiter.h +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright (c) 2013 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef COMPONENTS_POLICY_CORE_COMMON_CLOUD_RATE_LIMITER_H_ -#define COMPONENTS_POLICY_CORE_COMMON_CLOUD_RATE_LIMITER_H_ - -#include <queue> - -#include "base/basictypes.h" -#include "base/callback.h" -#include "base/cancelable_callback.h" -#include "base/memory/ref_counted.h" -#include "base/memory/scoped_ptr.h" -#include "base/threading/non_thread_safe.h" -#include "base/time/time.h" -#include "components/policy/policy_export.h" - -namespace base { -class SequencedTaskRunner; -class TickClock; -} - -namespace policy { - -// A simple class to limit the rate at which a callback is invoked. -class POLICY_EXPORT RateLimiter : public base::NonThreadSafe { - public: - // Will limit invocations of |callback| to |max_requests| per |duration|. - // |task_runner| is used to post delayed tasks, and |clock| is used to - // measure elapsed time. - RateLimiter(size_t max_requests, - const base::TimeDelta& duration, - const base::Closure& callback, - scoped_refptr<base::SequencedTaskRunner> task_runner, - scoped_ptr<base::TickClock> clock); - ~RateLimiter(); - - // Posts a request to invoke |callback_|. It is invoked immediately if the - // rate in the preceding |duration_| period is within the limit, otherwise - // the callback will be invoked later, ensuring the allowed rate is not - // exceeded. - void PostRequest(); - - private: - const size_t max_requests_; - const base::TimeDelta duration_; - base::Closure callback_; - scoped_refptr<base::SequencedTaskRunner> task_runner_; - scoped_ptr<base::TickClock> clock_; - - std::queue<base::TimeTicks> invocation_times_; - base::CancelableClosure delayed_callback_; - - DISALLOW_COPY_AND_ASSIGN(RateLimiter); -}; - -} // namespace policy - -#endif // COMPONENTS_POLICY_CORE_COMMON_CLOUD_RATE_LIMITER_H_ diff --git a/components/policy/core/common/cloud/rate_limiter_unittest.cc b/components/policy/core/common/cloud/rate_limiter_unittest.cc deleted file mode 100644 index 029ae81..0000000 --- a/components/policy/core/common/cloud/rate_limiter_unittest.cc +++ /dev/null @@ -1,121 +0,0 @@ -// Copyright (c) 2013 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "components/policy/core/common/cloud/rate_limiter.h" - -#include "base/bind.h" -#include "base/bind_helpers.h" -#include "base/test/simple_test_tick_clock.h" -#include "base/test/test_simple_task_runner.h" -#include "base/time/tick_clock.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace policy { - -class RateLimiterTest : public testing::Test { - public: - RateLimiterTest() - : task_runner_(new base::TestSimpleTaskRunner()), - clock_(new base::SimpleTestTickClock()), - callbacks_(0), - max_requests_(5), - duration_(base::TimeDelta::FromHours(1)), - small_delta_(base::TimeDelta::FromMinutes(1)), - limiter_(max_requests_, - duration_, - base::Bind(&RateLimiterTest::Callback, base::Unretained(this)), - task_runner_, - scoped_ptr<base::TickClock>(clock_).Pass()) {} - virtual ~RateLimiterTest() {} - - protected: - void Callback() { - callbacks_++; - } - - scoped_refptr<base::TestSimpleTaskRunner> task_runner_; - base::SimpleTestTickClock* clock_; - size_t callbacks_; - const size_t max_requests_; - const base::TimeDelta duration_; - const base::TimeDelta small_delta_; - RateLimiter limiter_; -}; - -TEST_F(RateLimiterTest, LimitRequests) { - size_t count = 0; - for (size_t i = 0; i < max_requests_; ++i) { - EXPECT_EQ(count, callbacks_); - limiter_.PostRequest(); - ++count; - EXPECT_EQ(count, callbacks_); - EXPECT_TRUE(task_runner_->GetPendingTasks().empty()); - clock_->Advance(small_delta_); - } - - for (size_t i = 0; i < 10; ++i) { - limiter_.PostRequest(); - EXPECT_EQ(max_requests_, callbacks_); - clock_->Advance(small_delta_); - EXPECT_FALSE(task_runner_->GetPendingTasks().empty()); - } - - // Now advance the clock beyond the duration. The callback is invoked once. - callbacks_ = 0; - clock_->Advance(duration_); - task_runner_->RunPendingTasks(); - EXPECT_EQ(1u, callbacks_); - EXPECT_TRUE(task_runner_->GetPendingTasks().empty()); -} - -TEST_F(RateLimiterTest, Steady) { - const base::TimeDelta delta = duration_ / 2; - size_t count = 0; - for (int i = 0; i < 100; ++i) { - EXPECT_EQ(count, callbacks_); - limiter_.PostRequest(); - ++count; - EXPECT_EQ(count, callbacks_); - EXPECT_TRUE(task_runner_->GetPendingTasks().empty()); - clock_->Advance(delta); - } -} - -TEST_F(RateLimiterTest, RetryAfterDelay) { - size_t count = 0; - base::TimeDelta total_delta; - // Fill the queue. - for (size_t i = 0; i < max_requests_; ++i) { - EXPECT_EQ(count, callbacks_); - limiter_.PostRequest(); - ++count; - EXPECT_EQ(count, callbacks_); - EXPECT_TRUE(task_runner_->GetPendingTasks().empty()); - clock_->Advance(small_delta_); - total_delta += small_delta_; - } - - // Now post a request that will be delayed. - EXPECT_EQ(max_requests_, callbacks_); - limiter_.PostRequest(); - EXPECT_EQ(max_requests_, callbacks_); - EXPECT_FALSE(task_runner_->GetPendingTasks().empty()); - - while (total_delta < duration_) { - task_runner_->RunPendingTasks(); - // The queue is still full, so another task is immediately posted. - EXPECT_FALSE(task_runner_->GetPendingTasks().empty()); - clock_->Advance(small_delta_); - total_delta += small_delta_; - } - - // Now advance time beyond the initial duration. It will immediately execute - // the callback. - EXPECT_EQ(max_requests_, callbacks_); - task_runner_->RunPendingTasks(); - EXPECT_TRUE(task_runner_->GetPendingTasks().empty()); - EXPECT_EQ(max_requests_ + 1, callbacks_); -} - -} // namespace policy diff --git a/components/policy/policy_common.gypi b/components/policy/policy_common.gypi index 6afa3d8..721e1ca 100644 --- a/components/policy/policy_common.gypi +++ b/components/policy/policy_common.gypi @@ -65,8 +65,6 @@ 'core/common/cloud/policy_header_io_helper.h', 'core/common/cloud/policy_header_service.cc', 'core/common/cloud/policy_header_service.h', - 'core/common/cloud/rate_limiter.cc', - 'core/common/cloud/rate_limiter.h', 'core/common/cloud/resource_cache.cc', 'core/common/cloud/resource_cache.h', 'core/common/cloud/system_policy_request_context.cc', |