Skip to content

GitLab

T
tangled
Commit d5675611 authored by stepco@chromium.org's avatar stepco@chromium.org
Browse files
Options

Remove cloud policy refresh rate limit. 

BUG=391902

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282565 0039d316-1c4b-4281-b951-d872f2087c98
parent 70e12b0e
Hide whitespace changes
Inline Side-by-side
Showing with 5 additions and 277 deletions
components/components_tests.gyp
View file @ d5675611
......@@ -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',
......
components/policy/core/common/BUILD.gn
View file @ d5675611
......@@ -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",
......
components/policy/core/common/cloud/cloud_policy_refresh_scheduler.cc
View file @ d5675611
......@@ -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(
......
components/policy/core/common/cloud/cloud_policy_refresh_scheduler.h
View file @ d5675611
......@@ -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_;
......
components/policy/core/common/cloud/cloud_policy_refresh_scheduler_unittest.cc
View file @ d5675611
......@@ -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_);
}
......
components/policy/core/common/cloud/rate_limiter.cc deleted 100644 → 0
View file @ 70e12b0e
// 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
components/policy/core/common/cloud/rate_limiter.h deleted 100644 → 0
View file @ 70e12b0e
// 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_
components/policy/core/common/cloud/rate_limiter_unittest.cc deleted 100644 → 0
View file @ 70e12b0e
// 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
components/policy/policy_common.gypi
View file @ d5675611
......@@ -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',
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment