Make HardwareChromeOS own debugd.
It only exists in Chrome OS, Brillo doesn't have it.
Test: ./update_engine_unittests
Bug: 28800946
Change-Id: I49d2024dbad5e0bf78bbc479f97dabb569b32c56
diff --git a/common/fake_hardware.h b/common/fake_hardware.h
index 2da12ad..f7c0286 100644
--- a/common/fake_hardware.h
+++ b/common/fake_hardware.h
@@ -41,6 +41,10 @@
bool IsNormalBootMode() const override { return is_normal_boot_mode_; }
+ bool AreDevFeaturesEnabled() const override {
+ return are_dev_features_enabled_;
+ }
+
bool IsOOBEEnabled() const override { return is_oobe_enabled_; }
bool IsOOBEComplete(base::Time* out_time_of_oobe) const override {
@@ -86,6 +90,10 @@
is_normal_boot_mode_ = is_normal_boot_mode;
}
+ void SetAreDevFeaturesEnabled(bool are_dev_features_enabled) {
+ are_dev_features_enabled_ = are_dev_features_enabled;
+ }
+
// Sets the SetIsOOBEEnabled to |is_oobe_enabled|.
void SetIsOOBEEnabled(bool is_oobe_enabled) {
is_oobe_enabled_ = is_oobe_enabled;
@@ -120,6 +128,7 @@
private:
bool is_official_build_{true};
bool is_normal_boot_mode_{true};
+ bool are_dev_features_enabled_{false};
bool is_oobe_enabled_{true};
bool is_oobe_complete_{true};
base::Time oobe_timestamp_{base::Time::FromTimeT(1169280000)}; // Jan 20, 2007
diff --git a/common/hardware_interface.h b/common/hardware_interface.h
index f5f900e..316ad3d 100644
--- a/common/hardware_interface.h
+++ b/common/hardware_interface.h
@@ -44,6 +44,9 @@
// features.
virtual bool IsNormalBootMode() const = 0;
+ // Returns whether the developer features are enabled.
+ virtual bool AreDevFeaturesEnabled() const = 0;
+
// Returns whether the device has an OOBE flow that the user must go through
// before getting non-critical updates. Use IsOOBEComplete() to determine if
// that flow is complete.
diff --git a/common/mock_hardware.h b/common/mock_hardware.h
index 826b3b3..1c4253a 100644
--- a/common/mock_hardware.h
+++ b/common/mock_hardware.h
@@ -36,6 +36,9 @@
ON_CALL(*this, IsNormalBootMode())
.WillByDefault(testing::Invoke(&fake_,
&FakeHardware::IsNormalBootMode));
+ ON_CALL(*this, AreDevFeaturesEnabled())
+ .WillByDefault(testing::Invoke(&fake_,
+ &FakeHardware::AreDevFeaturesEnabled));
ON_CALL(*this, IsOOBEEnabled())
.WillByDefault(testing::Invoke(&fake_,
&FakeHardware::IsOOBEEnabled));
diff --git a/hardware_android.cc b/hardware_android.cc
index 8de02c7..653ccf9 100644
--- a/hardware_android.cc
+++ b/hardware_android.cc
@@ -122,6 +122,10 @@
return property_get_bool("ro.debuggable", 0) != 1;
}
+bool HardwareAndroid::AreDevFeaturesEnabled() const {
+ return !IsNormalBootMode();
+}
+
bool HardwareAndroid::IsOOBEEnabled() const {
// No OOBE flow blocking updates for Android-based boards.
return false;
diff --git a/hardware_android.h b/hardware_android.h
index 88e00fa..78af871 100644
--- a/hardware_android.h
+++ b/hardware_android.h
@@ -36,6 +36,7 @@
// HardwareInterface methods.
bool IsOfficialBuild() const override;
bool IsNormalBootMode() const override;
+ bool AreDevFeaturesEnabled() const override;
bool IsOOBEEnabled() const override;
bool IsOOBEComplete(base::Time* out_time_of_oobe) const override;
std::string GetHardwareClass() const override;
diff --git a/hardware_chromeos.cc b/hardware_chromeos.cc
index 591cb3c..4b0b82f 100644
--- a/hardware_chromeos.cc
+++ b/hardware_chromeos.cc
@@ -23,6 +23,7 @@
#include <base/strings/string_util.h>
#include <brillo/key_value_store.h>
#include <brillo/make_unique_ptr.h>
+#include <debugd/dbus-constants.h>
#include <vboot/crossystem.h>
extern "C" {
@@ -35,6 +36,7 @@
#include "update_engine/common/platform_constants.h"
#include "update_engine/common/subprocess.h"
#include "update_engine/common/utils.h"
+#include "update_engine/dbus_connection.h"
using std::string;
using std::vector;
@@ -84,6 +86,8 @@
void HardwareChromeOS::Init() {
LoadConfig("" /* root_prefix */, IsNormalBootMode());
+ debugd_proxy_.reset(
+ new org::chromium::debugdProxy(DBusConnection::Get()->GetDBus()));
}
bool HardwareChromeOS::IsOfficialBuild() const {
@@ -95,6 +99,24 @@
return !dev_mode;
}
+bool HardwareChromeOS::AreDevFeaturesEnabled() const {
+ // Even though the debugd tools are also gated on devmode, checking here can
+ // save us a D-Bus call so it's worth doing explicitly.
+ if (IsNormalBootMode())
+ return false;
+
+ int32_t dev_features = debugd::DEV_FEATURES_DISABLED;
+ brillo::ErrorPtr error;
+ // Some boards may not include debugd so it's expected that this may fail,
+ // in which case we treat it as disabled.
+ if (debugd_proxy_ && debugd_proxy_->QueryDevFeatures(&dev_features, &error) &&
+ !(dev_features & debugd::DEV_FEATURES_DISABLED)) {
+ LOG(INFO) << "Debugd dev tools enabled.";
+ return true;
+ }
+ return false;
+}
+
bool HardwareChromeOS::IsOOBEEnabled() const {
return is_oobe_enabled_;
}
diff --git a/hardware_chromeos.h b/hardware_chromeos.h
index bc0e891..03ad750 100644
--- a/hardware_chromeos.h
+++ b/hardware_chromeos.h
@@ -22,6 +22,7 @@
#include <base/macros.h>
#include <base/time/time.h>
+#include <debugd/dbus-proxies.h>
#include "update_engine/common/hardware_interface.h"
@@ -39,6 +40,7 @@
// HardwareInterface methods.
bool IsOfficialBuild() const override;
bool IsNormalBootMode() const override;
+ bool AreDevFeaturesEnabled() const override;
bool IsOOBEEnabled() const override;
bool IsOOBEComplete(base::Time* out_time_of_oobe) const override;
std::string GetHardwareClass() const override;
@@ -60,6 +62,8 @@
bool is_oobe_enabled_;
+ std::unique_ptr<org::chromium::debugdProxyInterface> debugd_proxy_;
+
DISALLOW_COPY_AND_ASSIGN(HardwareChromeOS);
};
diff --git a/update_attempter.cc b/update_attempter.cc
index 49f00a2..a2125a6 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -32,9 +32,9 @@
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
#include <brillo/bind_lambda.h>
+#include <brillo/errors/error_codes.h>
#include <brillo/make_unique_ptr.h>
#include <brillo/message_loops/message_loop.h>
-#include <debugd/dbus-constants.h>
#include <policy/device_policy.h>
#include <policy/libpolicy.h>
#include <update_engine/dbus-constants.h>
@@ -50,9 +50,6 @@
#include "update_engine/common/prefs_interface.h"
#include "update_engine/common/subprocess.h"
#include "update_engine/common/utils.h"
-#if USE_DBUS
-#include "update_engine/dbus_connection.h"
-#endif // USE_DBUS
#include "update_engine/metrics.h"
#include "update_engine/omaha_request_action.h"
#include "update_engine/omaha_request_params.h"
@@ -164,13 +161,6 @@
#if USE_LIBCROS
chrome_proxy_resolver_.Init();
#endif // USE_LIBCROS
-
-#if USE_DBUS
- // unittest can set this to a mock before calling Init().
- if (!debugd_proxy_)
- debugd_proxy_.reset(
- new org::chromium::debugdProxy(DBusConnection::Get()->GetDBus()));
-#endif // USE_DBUS
}
void UpdateAttempter::ScheduleUpdates() {
@@ -1090,11 +1080,7 @@
channel, false /* powerwash_allowed */, &error_message)) {
brillo::Error::AddTo(error,
FROM_HERE,
-#if USE_DBUS
brillo::errors::dbus::kDomain,
-#else
- "dbus",
-#endif // USE_DBUS
"set_target_error",
error_message);
return false;
@@ -1592,30 +1578,13 @@
return true;
}
- // Even though the debugd tools are also gated on devmode, checking here can
- // save us a D-Bus call so it's worth doing explicitly.
- if (system_state_->hardware()->IsNormalBootMode()) {
- LOG(INFO) << "Not in devmode; disallowing custom update sources.";
- return false;
- }
-
-#if USE_DBUS
- // Official images in devmode are allowed a custom update source iff the
- // debugd dev tools are enabled.
- if (!debugd_proxy_)
- return false;
- int32_t dev_features = debugd::DEV_FEATURES_DISABLED;
- brillo::ErrorPtr error;
- bool success = debugd_proxy_->QueryDevFeatures(&dev_features, &error);
-
- // Some boards may not include debugd so it's expected that this may fail,
- // in which case we default to disallowing custom update sources.
- if (success && !(dev_features & debugd::DEV_FEATURES_DISABLED)) {
- LOG(INFO) << "Debugd dev tools enabled; allowing any update source.";
+ if (system_state_->hardware()->AreDevFeaturesEnabled()) {
+ LOG(INFO) << "Developer features enabled; allowing custom update sources.";
return true;
}
-#endif // USE_DBUS
- LOG(INFO) << "Debugd dev tools disabled; disallowing custom update sources.";
+
+ LOG(INFO)
+ << "Developer features disabled; disallowing custom update sources.";
return false;
}
diff --git a/update_attempter.h b/update_attempter.h
index 78b35f0..0b83d5b 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -29,9 +29,6 @@
#include <base/time/time.h>
#include <gtest/gtest_prod.h> // for FRIEND_TEST
-#if USE_DBUS
-#include "debugd/dbus-proxies.h"
-#endif // USE_DBUS
#if USE_LIBCROS
#include "update_engine/chrome_browser_proxy_resolver.h"
#endif // USE_LIBCROS
@@ -510,10 +507,6 @@
std::string forced_app_version_;
std::string forced_omaha_url_;
-#if USE_DBUS
- std::unique_ptr<org::chromium::debugdProxyInterface> debugd_proxy_;
-#endif // USE_DBUS
-
DISALLOW_COPY_AND_ASSIGN(UpdateAttempter);
};
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 6ef1c15..dfb0465 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -27,11 +27,6 @@
#include <brillo/message_loops/base_message_loop.h>
#include <brillo/message_loops/message_loop.h>
#include <brillo/message_loops/message_loop_utils.h>
-#if USE_DBUS
-#include <debugd/dbus-constants.h>
-#include <debugd/dbus-proxies.h>
-#include <debugd/dbus-proxy-mocks.h>
-#endif // USE_DBUS
#include <gtest/gtest.h>
#include <policy/libpolicy.h>
#include <policy/mock_device_policy.h>
@@ -118,9 +113,6 @@
protected:
UpdateAttempterTest()
:
-#if USE_DBUS
- debugd_proxy_mock_(new org::chromium::debugdProxyMock()),
-#endif // USE_DBUS
#if USE_LIBCROS
service_interface_mock_(new LibCrosServiceInterfaceProxyMock()),
ue_proxy_resolved_interface_mock_(
@@ -138,10 +130,7 @@
certificate_checker_.Init();
-// Finish initializing the attempter.
-#if USE_DBUS
- attempter_.debugd_proxy_.reset(debugd_proxy_mock_);
-#endif // USE_DBUS
+ // Finish initializing the attempter.
attempter_.Init();
}
@@ -203,9 +192,6 @@
brillo::BaseMessageLoop loop_{&base_loop_};
FakeSystemState fake_system_state_;
-#if USE_DBUS
- org::chromium::debugdProxyMock* debugd_proxy_mock_;
-#endif // USE_DBUS
#if USE_LIBCROS
LibCrosServiceInterfaceProxyMock* service_interface_mock_;
UpdateEngineLibcrosProxyResolvedInterfaceProxyMock*
@@ -963,58 +949,28 @@
EXPECT_TRUE(attempter_.IsAnyUpdateSourceAllowed());
}
-#if USE_DBUS
TEST_F(UpdateAttempterTest, AnyUpdateSourceAllowedOfficialDevmode) {
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
- fake_system_state_.fake_hardware()->SetIsNormalBootMode(false);
- EXPECT_CALL(*debugd_proxy_mock_, QueryDevFeatures(_, _, _))
- .WillRepeatedly(DoAll(SetArgumentPointee<0>(0), Return(true)));
+ fake_system_state_.fake_hardware()->SetAreDevFeaturesEnabled(true);
EXPECT_TRUE(attempter_.IsAnyUpdateSourceAllowed());
}
-#endif // USE_DBUS
TEST_F(UpdateAttempterTest, AnyUpdateSourceDisallowedOfficialNormal) {
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
- fake_system_state_.fake_hardware()->SetIsNormalBootMode(true);
- // debugd should not be queried in this case.
-#if USE_DBUS
- EXPECT_CALL(*debugd_proxy_mock_, QueryDevFeatures(_, _, _)).Times(0);
-#endif // USE_DBUS
- EXPECT_FALSE(attempter_.IsAnyUpdateSourceAllowed());
-}
-
-TEST_F(UpdateAttempterTest, AnyUpdateSourceDisallowedDebugdDisabled) {
- fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
- fake_system_state_.fake_hardware()->SetIsNormalBootMode(false);
-#if USE_DBUS
- using debugd::DEV_FEATURES_DISABLED;
- EXPECT_CALL(*debugd_proxy_mock_, QueryDevFeatures(_, _, _))
- .WillRepeatedly(
- DoAll(SetArgumentPointee<0>(DEV_FEATURES_DISABLED), Return(true)));
-#endif // USE_DBUS
- EXPECT_FALSE(attempter_.IsAnyUpdateSourceAllowed());
-}
-
-TEST_F(UpdateAttempterTest, AnyUpdateSourceDisallowedDebugdFailure) {
- fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
- fake_system_state_.fake_hardware()->SetIsNormalBootMode(false);
-#if USE_DBUS
- EXPECT_CALL(*debugd_proxy_mock_, QueryDevFeatures(_, _, _))
- .WillRepeatedly(Return(false));
-#endif // USE_DBUS
+ fake_system_state_.fake_hardware()->SetAreDevFeaturesEnabled(false);
EXPECT_FALSE(attempter_.IsAnyUpdateSourceAllowed());
}
TEST_F(UpdateAttempterTest, CheckForUpdateAUTest) {
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
- fake_system_state_.fake_hardware()->SetIsNormalBootMode(true);
+ fake_system_state_.fake_hardware()->SetAreDevFeaturesEnabled(false);
attempter_.CheckForUpdate("", "autest", true);
EXPECT_EQ(constants::kOmahaDefaultAUTestURL, attempter_.forced_omaha_url());
}
TEST_F(UpdateAttempterTest, CheckForUpdateScheduledAUTest) {
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
- fake_system_state_.fake_hardware()->SetIsNormalBootMode(true);
+ fake_system_state_.fake_hardware()->SetAreDevFeaturesEnabled(false);
attempter_.CheckForUpdate("", "autest-scheduled", true);
EXPECT_EQ(constants::kOmahaDefaultAUTestURL, attempter_.forced_omaha_url());
}
