Move IsOfficialBuild() and IsNormalBootMode() into HardwareInterface.
This makes the implementation of the two methods part of the
HardwareInterface, so that unit tests won't end up with meaningless
(and unpredictable) calls to the real functions.
BUG=None
TEST=unit tests
Change-Id: Ia23932634124987c1d6ff0683acb15cf4819bc5e
Reviewed-on: https://chromium-review.googlesource.com/175024
Reviewed-by: Chris Sosa <sosa@chromium.org>
Commit-Queue: Richard Barnette <jrbarnette@chromium.org>
Tested-by: Richard Barnette <jrbarnette@chromium.org>
diff --git a/dbus_service.cc b/dbus_service.cc
index 449c101..42bcfa3 100644
--- a/dbus_service.cc
+++ b/dbus_service.cc
@@ -12,11 +12,12 @@
#include "update_engine/connection_manager.h"
#include "update_engine/dbus_constants.h"
+#include "update_engine/hardware_interface.h"
#include "update_engine/marshal.glibmarshal.h"
#include "update_engine/omaha_request_params.h"
#include "update_engine/p2p_manager.h"
-#include "update_engine/update_attempter.h"
#include "update_engine/prefs.h"
+#include "update_engine/update_attempter.h"
#include "update_engine/utils.h"
using std::set;
@@ -96,7 +97,7 @@
// Only non-official (e.g., dev and test) builds can override the current
// version and update server URL over D-Bus. However, pointing to the
// hardcoded test update server URL is always allowed.
- if (!chromeos_update_engine::utils::IsOfficialBuild()) {
+ if (!self->system_state_->hardware()->IsOfficialBuild()) {
if (app_version) {
update_app_version = app_version;
}
diff --git a/fake_hardware.h b/fake_hardware.h
index 5428f28..4351e2c 100644
--- a/fake_hardware.h
+++ b/fake_hardware.h
@@ -14,12 +14,16 @@
public:
FakeHardware()
: boot_device_("/dev/sdz5"),
+ is_official_build_(true),
+ is_normal_boot_mode_(true),
hardware_class_("Fake HWID BLAH-1234"),
firmware_version_("Fake Firmware v1.0.1"),
ec_version_("Fake EC v1.0a") {}
// HardwareInterface methods.
virtual const std::string BootDevice() { return boot_device_; }
+ virtual bool IsOfficialBuild() { return is_official_build_; }
+ virtual bool IsNormalBootMode() { return is_normal_boot_mode_; }
virtual std::string GetHardwareClass() { return hardware_class_; }
virtual std::string GetFirmwareVersion() { return firmware_version_; }
virtual std::string GetECVersion() { return ec_version_; }
@@ -29,6 +33,14 @@
boot_device_ = boot_device;
}
+ void SetIsOfficialBuild(bool is_official_build) {
+ is_official_build_ = is_official_build;
+ }
+
+ void SetIsNormalBootMode(bool is_normal_boot_mode) {
+ is_normal_boot_mode_ = is_normal_boot_mode;
+ }
+
void SetHardwareClass(std::string hardware_class) {
hardware_class_ = hardware_class;
}
@@ -43,6 +55,8 @@
private:
std::string boot_device_;
+ bool is_official_build_;
+ bool is_normal_boot_mode_;
std::string hardware_class_;
std::string firmware_version_;
std::string ec_version_;
diff --git a/hardware.cc b/hardware.cc
index a44edcf..f04c340 100644
--- a/hardware.cc
+++ b/hardware.cc
@@ -4,6 +4,7 @@
#include "update_engine/hardware.h"
+#include <base/file_util.h>
#include <base/logging.h>
#include <base/string_util.h>
#include <rootdev/rootdev.h>
@@ -16,6 +17,9 @@
namespace chromeos_update_engine {
+static const char kDevImageMarker[] = "/root/.dev_mode";
+
+
const string Hardware::BootDevice() {
char boot_path[PATH_MAX];
// Resolve the boot device path fully, including dereferencing
@@ -33,6 +37,25 @@
return boot_path;
}
+bool Hardware::IsOfficialBuild() {
+ return !file_util::PathExists(FilePath(kDevImageMarker));
+}
+
+bool Hardware::IsNormalBootMode() {
+ // TODO(petkov): Convert to a library call once a crossystem library is
+ // available (crosbug.com/13291).
+ int exit_code = 0;
+ vector<string> cmd(1, "/usr/bin/crossystem");
+ cmd.push_back("devsw_boot?1");
+
+ // Assume dev mode if the dev switch is set to 1 and there was no error
+ // executing crossystem. Assume normal mode otherwise.
+ bool success = Subprocess::SynchronousExec(cmd, &exit_code, NULL);
+ bool dev_mode = success && exit_code == 0;
+ LOG_IF(INFO, dev_mode) << "Booted in dev mode.";
+ return !dev_mode;
+}
+
static string ReadValueFromCrosSystem(const string& key) {
int exit_code = 0;
vector<string> cmd(1, "/usr/bin/crossystem");
diff --git a/hardware.h b/hardware.h
index 7f895c9..5ab1fca 100644
--- a/hardware.h
+++ b/hardware.h
@@ -18,6 +18,8 @@
// HardwareInterface methods.
virtual const std::string BootDevice();
+ virtual bool IsOfficialBuild();
+ virtual bool IsNormalBootMode();
virtual std::string GetHardwareClass();
virtual std::string GetFirmwareVersion();
virtual std::string GetECVersion();
diff --git a/hardware_interface.h b/hardware_interface.h
index 55f19ea..8909058 100644
--- a/hardware_interface.h
+++ b/hardware_interface.h
@@ -22,8 +22,13 @@
// or something with equivalent funcionality to interpret those.
virtual const std::string BootDevice() = 0;
- // TODO(deymo): Move other hardware-dependent functions to this interface:
- // IsNormalBootMode and IsOfficialBuild.
+ // Returns true if this is an official Chrome OS build, false otherwise.
+ virtual bool IsOfficialBuild() = 0;
+
+ // Returns true if the boot mode is normal or if it's unable to
+ // determine the boot mode. Returns false if the boot mode is
+ // developer.
+ virtual bool IsNormalBootMode() = 0;
// Returns the HWID or an empty string on error.
virtual std::string GetHardwareClass() = 0;
diff --git a/libcurl_http_fetcher.cc b/libcurl_http_fetcher.cc
index c1d16d3..be49ef3 100644
--- a/libcurl_http_fetcher.cc
+++ b/libcurl_http_fetcher.cc
@@ -13,6 +13,7 @@
#include "update_engine/certificate_checker.h"
#include "update_engine/dbus_interface.h"
+#include "update_engine/hardware_interface.h"
#include "update_engine/utils.h"
using google::protobuf::NewCallback;
@@ -51,7 +52,8 @@
}
bool LibcurlHttpFetcher::IsOfficialBuild() const {
- return force_build_type_ ? forced_official_build_ : utils::IsOfficialBuild();
+ return force_build_type_ ? forced_official_build_
+ : system_state_->hardware()->IsOfficialBuild();
}
bool LibcurlHttpFetcher::GetProxyType(const std::string& proxy,
diff --git a/mock_system_state.h b/mock_system_state.h
index e18c403..193438b 100644
--- a/mock_system_state.h
+++ b/mock_system_state.h
@@ -32,7 +32,6 @@
virtual ~MockSystemState();
MOCK_METHOD0(IsOOBEComplete, bool());
- MOCK_METHOD0(IsOfficialBuild, bool());
MOCK_METHOD1(set_device_policy, void(const policy::DevicePolicy*));
MOCK_CONST_METHOD0(device_policy, const policy::DevicePolicy*());
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index 016b5a8..5ce59bb 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -21,6 +21,7 @@
#include "update_engine/action_pipe.h"
#include "update_engine/constants.h"
+#include "update_engine/hardware_interface.h"
#include "update_engine/omaha_hash_calculator.h"
#include "update_engine/omaha_request_params.h"
#include "update_engine/p2p_manager.h"
@@ -725,7 +726,7 @@
if (IsEvent()) {
CHECK(!HasOutputPipe()) << "No output pipe allowed for event requests.";
if (event_->result == OmahaEvent::kResultError && successful &&
- utils::IsOfficialBuild()) {
+ system_state_->hardware()->IsOfficialBuild()) {
LOG(INFO) << "Signalling Crash Reporter.";
utils::ScheduleCrashReporterUpload();
}
diff --git a/omaha_request_params.cc b/omaha_request_params.cc
index a8de28f..065d5bb 100644
--- a/omaha_request_params.cc
+++ b/omaha_request_params.cc
@@ -250,7 +250,8 @@
if (force_lock_down_) {
return forced_lock_down_;
}
- return utils::IsOfficialBuild() && utils::IsNormalBootMode();
+ return system_state_->hardware()->IsOfficialBuild() &&
+ system_state_->hardware()->IsNormalBootMode();
}
bool OmahaRequestParams::IsValidChannel(const std::string& channel) const {
diff --git a/omaha_response_handler_action.cc b/omaha_response_handler_action.cc
index 7921634..4625037 100644
--- a/omaha_response_handler_action.cc
+++ b/omaha_response_handler_action.cc
@@ -146,7 +146,7 @@
// opposed to waiving the checks when we're in dev mode, because we do want
// to enforce the hash checks when our end customers run in dev mode if they
// are using an official build, so that they are protected more.
- if (!utils::IsOfficialBuild()) {
+ if (!system_state_->hardware()->IsOfficialBuild()) {
LOG(INFO) << "Waiving payload hash checks for unofficial builds";
return false;
}
diff --git a/payload_state.cc b/payload_state.cc
index 9839a2b..a23b545 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -317,7 +317,7 @@
return false;
}
- if (!utils::IsOfficialBuild()) {
+ if (!system_state_->hardware()->IsOfficialBuild()) {
// Backoffs are needed only for official builds. We do not want any delays
// or update failures due to backoffs during testing or development.
LOG(INFO) << "No backoffs for test/dev images. "
@@ -1091,7 +1091,7 @@
void PayloadState::ComputeCandidateUrls() {
bool http_url_ok = true;
- if (system_state_->IsOfficialBuild()) {
+ if (system_state_->hardware()->IsOfficialBuild()) {
const policy::DevicePolicy* policy = system_state_->device_policy();
if (policy && policy->GetHttpDownloadsEnabled(&http_url_ok) && !http_url_ok)
LOG(INFO) << "Downloads via HTTP Url are not enabled by device policy";
diff --git a/payload_state_unittest.cc b/payload_state_unittest.cc
index 4efce28..eb231cb 100644
--- a/payload_state_unittest.cc
+++ b/payload_state_unittest.cc
@@ -1204,11 +1204,6 @@
MockSystemState mock_system_state;
PayloadState payload_state;
- // Pretend that this is an offical build so that the HTTP download policy
- // is honored.
- EXPECT_CALL(mock_system_state, IsOfficialBuild())
- .WillRepeatedly(Return(true));
-
policy::MockDevicePolicy disable_http_policy;
EXPECT_CALL(mock_system_state, device_policy())
.WillRepeatedly(Return(&disable_http_policy));
diff --git a/real_system_state.h b/real_system_state.h
index fa7b544..9bd50fe 100644
--- a/real_system_state.h
+++ b/real_system_state.h
@@ -27,7 +27,6 @@
virtual ~RealSystemState() {}
virtual bool IsOOBEComplete();
- virtual bool IsOfficialBuild();
virtual inline void set_device_policy(
const policy::DevicePolicy* device_policy) {
diff --git a/system_state.cc b/system_state.cc
index 5465196..4ca5ac4 100644
--- a/system_state.cc
+++ b/system_state.cc
@@ -72,8 +72,4 @@
return file_util::PathExists(FilePath(kOOBECompletedMarker));
}
-bool RealSystemState::IsOfficialBuild() {
- return utils::IsOfficialBuild();
-}
-
} // namespace chromeos_update_engine
diff --git a/system_state.h b/system_state.h
index f4bd19c..2f56ca6 100644
--- a/system_state.h
+++ b/system_state.h
@@ -42,10 +42,6 @@
// False otherwise.
virtual bool IsOOBEComplete() = 0;
- // Returns true if we're running an official (i.e, non-dev, non-test) build.
- // False otherwise.
- virtual bool IsOfficialBuild() = 0;
-
// Sets or gets the latest device policy.
virtual void set_device_policy(const policy::DevicePolicy* device_policy) = 0;
virtual const policy::DevicePolicy* device_policy() const = 0;
diff --git a/update_attempter.cc b/update_attempter.cc
index fb4d223..69a625d 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -1084,14 +1084,14 @@
uint32_t UpdateAttempter::GetErrorCodeFlags() {
uint32_t flags = 0;
- if (!utils::IsNormalBootMode())
+ if (!system_state_->hardware()->IsNormalBootMode())
flags |= kErrorCodeDevModeFlag;
if (response_handler_action_.get() &&
response_handler_action_->install_plan().is_resume)
flags |= kErrorCodeResumedFlag;
- if (!utils::IsOfficialBuild())
+ if (!system_state_->hardware()->IsOfficialBuild())
flags |= kErrorCodeTestImageFlag;
if (omaha_request_params_->update_url() != kProductionOmahaUrl)
diff --git a/update_check_scheduler.cc b/update_check_scheduler.cc
index 425ce3e..ecd844b 100644
--- a/update_check_scheduler.cc
+++ b/update_check_scheduler.cc
@@ -64,7 +64,7 @@
}
bool UpdateCheckScheduler::IsOfficialBuild() {
- return utils::IsOfficialBuild();
+ return system_state_->hardware()->IsOfficialBuild();
}
guint UpdateCheckScheduler::GTimeoutAddSeconds(guint interval,
diff --git a/utils.cc b/utils.cc
index d33a5cc..47f8d78 100644
--- a/utils.cc
+++ b/utils.cc
@@ -60,31 +60,10 @@
namespace utils {
-static const char kDevImageMarker[] = "/root/.dev_mode";
-
// Cgroup container is created in update-engine's upstart script located at
// /etc/init/update-engine.conf.
static const char kCGroupDir[] = "/sys/fs/cgroup/cpu/update-engine";
-bool IsOfficialBuild() {
- return !file_util::PathExists(FilePath(kDevImageMarker));
-}
-
-bool IsNormalBootMode() {
- // TODO(petkov): Convert to a library call once a crossystem library is
- // available (crosbug.com/13291).
- int exit_code = 0;
- vector<string> cmd(1, "/usr/bin/crossystem");
- cmd.push_back("devsw_boot?1");
-
- // Assume dev mode if the dev switch is set to 1 and there was no error
- // executing crossystem. Assume normal mode otherwise.
- bool success = Subprocess::SynchronousExec(cmd, &exit_code, NULL);
- bool dev_mode = success && exit_code == 0;
- LOG_IF(INFO, dev_mode) << "Booted in dev mode.";
- return !dev_mode;
-}
-
string ParseECVersion(string input_line) {
TrimWhitespaceASCII(input_line, TRIM_ALL, &input_line);
diff --git a/utils.h b/utils.h
index 2727831..ee99d86 100644
--- a/utils.h
+++ b/utils.h
@@ -29,13 +29,6 @@
namespace utils {
-// Returns true if this is an official Chrome OS build, false otherwise.
-bool IsOfficialBuild();
-
-// Returns true if the boot mode is normal or if it's unable to determine the
-// boot mode. Returns false if the boot mode is developer.
-bool IsNormalBootMode();
-
// Converts a struct timespec representing a number of seconds since
// the Unix epoch to a base::Time. Sub-microsecond time is rounded
// down.
diff --git a/utils_unittest.cc b/utils_unittest.cc
index 28ce197..1676444 100644
--- a/utils_unittest.cc
+++ b/utils_unittest.cc
@@ -27,16 +27,6 @@
class UtilsTest : public ::testing::Test { };
-TEST(UtilsTest, IsOfficialBuild) {
- // Pretty lame test...
- EXPECT_TRUE(utils::IsOfficialBuild());
-}
-
-TEST(UtilsTest, IsNormalBootMode) {
- // Pretty lame test...
- EXPECT_TRUE(utils::IsNormalBootMode());
-}
-
TEST(UtilsTest, CanParseECVersion) {
// Should be able to parse and valid key value line.
EXPECT_EQ("12345", utils::ParseECVersion("fw_version=12345"));