UM: Make the evaluation timeout configurable.
This allows the client to construct the UpdateManager with a custom
evaluation timeout for sync policy requests, which is in turn being
passed to each EvaluationContext instance used for sync policy
evaluation.
BUG=chromium:363790
TEST=Unit tests.
Change-Id: I5a6ec02a3ca2a2c611276eacbcda6aac8304e929
Reviewed-on: https://chromium-review.googlesource.com/204687
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
Tested-by: Gilad Arnold <garnold@chromium.org>
diff --git a/real_system_state.cc b/real_system_state.cc
index 4886b91..af227da 100644
--- a/real_system_state.cc
+++ b/real_system_state.cc
@@ -5,6 +5,7 @@
#include "update_engine/real_system_state.h"
#include <base/file_util.h>
+#include <base/time/time.h>
#include "update_engine/constants.h"
#include "update_engine/update_manager/state_factory.h"
@@ -52,7 +53,8 @@
return false;
}
update_manager_.reset(
- new chromeos_update_manager::UpdateManager(&clock_, um_state));
+ new chromeos_update_manager::UpdateManager(
+ &clock_, base::TimeDelta::FromSeconds(5), um_state));
if (!payload_state_.Initialize(this)) {
LOG(ERROR) << "Failed to initialize the payload state object.";
diff --git a/update_manager/chromeos_policy_unittest.cc b/update_manager/chromeos_policy_unittest.cc
index 140b6ba..6c6eeff 100644
--- a/update_manager/chromeos_policy_unittest.cc
+++ b/update_manager/chromeos_policy_unittest.cc
@@ -30,7 +30,7 @@
protected:
virtual void SetUp() {
SetUpDefaultClock();
- eval_ctx_ = new EvaluationContext(&fake_clock_);
+ eval_ctx_ = new EvaluationContext(&fake_clock_, TimeDelta::FromSeconds(5));
SetUpDefaultState();
SetUpDefaultDevicePolicy();
}
diff --git a/update_manager/evaluation_context.cc b/update_manager/evaluation_context.cc
index 4fc9a16..a24bf23 100644
--- a/update_manager/evaluation_context.cc
+++ b/update_manager/evaluation_context.cc
@@ -20,8 +20,10 @@
namespace chromeos_update_manager {
-EvaluationContext::EvaluationContext(ClockInterface* clock)
+EvaluationContext::EvaluationContext(ClockInterface* clock,
+ TimeDelta evaluation_timeout)
: clock_(clock),
+ evaluation_timeout_(evaluation_timeout),
weak_ptr_factory_(this) {
ResetEvaluation();
}
diff --git a/update_manager/evaluation_context.h b/update_manager/evaluation_context.h
index 97e5b97..30f9bd5 100644
--- a/update_manager/evaluation_context.h
+++ b/update_manager/evaluation_context.h
@@ -51,7 +51,8 @@
public base::RefCounted<EvaluationContext>,
private BaseVariable::ObserverInterface {
public:
- explicit EvaluationContext(chromeos_update_engine::ClockInterface* clock);
+ explicit EvaluationContext(chromeos_update_engine::ClockInterface* clock,
+ base::TimeDelta evaluation_timeout);
~EvaluationContext();
// Returns a pointer to the value returned by the passed variable |var|. The
@@ -138,8 +139,7 @@
// The timeout of an evaluation, used to compute the RemainingTime() of an
// evaluation.
- // TODO(deymo): Receive the timeout from the UpdateManager. crbug.com/363790
- base::TimeDelta evaluation_timeout_ = base::TimeDelta::FromSeconds(5);
+ const base::TimeDelta evaluation_timeout_;
// The timestamp in the ClockInterface::GetMonotonicTime() scale at which the
// current evaluation should finish. This is used to compute the
diff --git a/update_manager/evaluation_context_unittest.cc b/update_manager/evaluation_context_unittest.cc
index 33d91a4..9c51022 100644
--- a/update_manager/evaluation_context_unittest.cc
+++ b/update_manager/evaluation_context_unittest.cc
@@ -50,7 +50,7 @@
fake_clock_.SetMonotonicTime(Time::FromInternalValue(12345678L));
// Mar 2, 2006 1:23:45 UTC is 1141262625 since the Unix Epoch.
fake_clock_.SetWallclockTime(Time::FromTimeT(1141262625));
- eval_ctx_ = new EvaluationContext(&fake_clock_);
+ eval_ctx_ = new EvaluationContext(&fake_clock_, default_timeout_);
}
virtual void TearDown() {
@@ -62,8 +62,6 @@
EXPECT_TRUE(fake_poll_var_.observer_list_.empty());
}
- // TODO(deymo): Update the default timeout to the one passed on construction.
- // See crbug.com/363790
base::TimeDelta default_timeout_ = base::TimeDelta::FromSeconds(5);
FakeClock fake_clock_;
diff --git a/update_manager/fake_update_manager.h b/update_manager/fake_update_manager.h
index 0831b2e..f066df0 100644
--- a/update_manager/fake_update_manager.h
+++ b/update_manager/fake_update_manager.h
@@ -15,7 +15,7 @@
class FakeUpdateManager : public UpdateManager {
public:
explicit FakeUpdateManager(chromeos_update_engine::ClockInterface* clock)
- : UpdateManager(clock, new FakeState()) {
+ : UpdateManager(clock, base::TimeDelta::FromSeconds(5), new FakeState()) {
// The FakeUpdateManager uses a DefaultPolicy.
set_policy(new DefaultPolicy());
}
diff --git a/update_manager/update_manager-inl.h b/update_manager/update_manager-inl.h
index 362d4c5..44b0d25 100644
--- a/update_manager/update_manager-inl.h
+++ b/update_manager/update_manager-inl.h
@@ -84,7 +84,8 @@
std::string*, R*,
ExpectedArgs...) const,
R* result, ActualArgs... args) {
- scoped_refptr<EvaluationContext> ec(new EvaluationContext(clock_));
+ scoped_refptr<EvaluationContext> ec(
+ new EvaluationContext(clock_, evaluation_timeout_));
// A PolicyRequest allways consists on a single evaluation on a new
// EvaluationContext.
// IMPORTANT: To ensure that ActualArgs can be converted to ExpectedArgs, we
@@ -106,7 +107,8 @@
std::string*, R*,
ExpectedArgs...) const,
ActualArgs... args) {
- scoped_refptr<EvaluationContext> ec = new EvaluationContext(clock_);
+ scoped_refptr<EvaluationContext> ec =
+ new EvaluationContext(clock_, evaluation_timeout_);
// IMPORTANT: To ensure that ActualArgs can be converted to ExpectedArgs, we
// explicitly instantiate UpdateManager::OnPolicyReadyToEvaluate with the
// latter in lieu of the former.
diff --git a/update_manager/update_manager.cc b/update_manager/update_manager.cc
index d3281fd..d6ffcee 100644
--- a/update_manager/update_manager.cc
+++ b/update_manager/update_manager.cc
@@ -10,8 +10,8 @@
namespace chromeos_update_manager {
UpdateManager::UpdateManager(chromeos_update_engine::ClockInterface* clock,
- State* state)
- : state_(state), clock_(clock) {
+ base::TimeDelta evaluation_timeout, State* state)
+ : state_(state), clock_(clock), evaluation_timeout_(evaluation_timeout) {
// TODO(deymo): Make it possible to replace this policy with a different
// implementation with a build-time flag.
policy_.reset(new ChromeOSPolicy());
diff --git a/update_manager/update_manager.h b/update_manager/update_manager.h
index 8c96d7b..5d7a045 100644
--- a/update_manager/update_manager.h
+++ b/update_manager/update_manager.h
@@ -8,6 +8,7 @@
#include <base/callback.h>
#include <base/memory/ref_counted.h>
#include <base/memory/scoped_ptr.h>
+#include <base/time/time.h>
#include "update_engine/clock_interface.h"
#include "update_engine/update_manager/default_policy.h"
@@ -22,7 +23,7 @@
// Creates the UpdateManager instance, assuming ownership on the provided
// |state|.
UpdateManager(chromeos_update_engine::ClockInterface* clock,
- State* state);
+ base::TimeDelta evaluation_timeout, State* state);
virtual ~UpdateManager() {}
@@ -119,6 +120,9 @@
// Pointer to the mockable clock interface;
chromeos_update_engine::ClockInterface* clock_;
+ // Timeout for a policy evaluation.
+ const base::TimeDelta evaluation_timeout_;
+
DISALLOW_COPY_AND_ASSIGN(UpdateManager);
};
diff --git a/update_manager/update_manager_unittest.cc b/update_manager/update_manager_unittest.cc
index fa6f9dd..640fbb3 100644
--- a/update_manager/update_manager_unittest.cc
+++ b/update_manager/update_manager_unittest.cc
@@ -58,7 +58,8 @@
protected:
virtual void SetUp() {
fake_state_ = new FakeState();
- umut_.reset(new UpdateManager(&fake_clock_, fake_state_));
+ umut_.reset(new UpdateManager(&fake_clock_, TimeDelta::FromSeconds(5),
+ fake_state_));
}
FakeState* fake_state_; // Owned by the umut_.