update_engine: Switch to chrome-dbus for client requests in update_engine
update_engine daemon acts as DBus client to send DBus calls to shill,
power_manager and chrome, and to listen for signals from shill, chrome
and login_manager. This patch migrates these calls and signals to use
chrome-dbus framework instead of dbus-glib.
All references to dbus-glib code are removed.
BUG=chromium:419827
TEST=Updated unittest. Deployed on a link device and tested interactions with shill and chromium.
Change-Id: I31b389e0d1690cccb115ff3b6539c876ba81bd0e
Reviewed-on: https://chromium-review.googlesource.com/290990
Tested-by: Alex Deymo <deymo@chromium.org>
Reviewed-by: Alex Deymo <deymo@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>
Trybot-Ready: Alex Deymo <deymo@chromium.org>
diff --git a/update_manager/real_device_policy_provider.cc b/update_manager/real_device_policy_provider.cc
index 6f484f7..3586cc8 100644
--- a/update_manager/real_device_policy_provider.cc
+++ b/update_manager/real_device_policy_provider.cc
@@ -12,7 +12,6 @@
#include <chromeos/dbus/service_constants.h>
#include <policy/device_policy.h>
-#include "update_engine/glib_utils.h"
#include "update_engine/update_manager/generic_variables.h"
#include "update_engine/update_manager/real_shill_provider.h"
#include "update_engine/utils.h"
@@ -33,12 +32,6 @@
RealDevicePolicyProvider::~RealDevicePolicyProvider() {
MessageLoop::current()->CancelTask(scheduled_refresh_);
- // Detach signal handler, free manager proxy.
- dbus_->ProxyDisconnectSignal(manager_proxy_,
- login_manager::kPropertyChangeCompleteSignal,
- G_CALLBACK(HandlePropertyChangedCompletedStatic),
- this);
- dbus_->ProxyUnref(manager_proxy_);
}
bool RealDevicePolicyProvider::Init() {
@@ -49,38 +42,39 @@
// We also listen for signals from the session manager to force a device
// policy refresh.
- GError* error = nullptr;
- DBusGConnection* connection = dbus_->BusGet(DBUS_BUS_SYSTEM, &error);
- if (!connection) {
- LOG(ERROR) << "Failed to initialize DBus connection: "
- << chromeos_update_engine::utils::GetAndFreeGError(&error);
- return false;
- }
- manager_proxy_ = dbus_->ProxyNewForName(
- connection,
- login_manager::kSessionManagerServiceName,
- login_manager::kSessionManagerServicePath,
- login_manager::kSessionManagerInterface);
-
- // Subscribe to the session manager's PropertyChangeComplete signal.
- dbus_->ProxyAddSignal_1(manager_proxy_,
- login_manager::kPropertyChangeCompleteSignal,
- G_TYPE_STRING);
- dbus_->ProxyConnectSignal(manager_proxy_,
- login_manager::kPropertyChangeCompleteSignal,
- G_CALLBACK(HandlePropertyChangedCompletedStatic),
- this, nullptr);
+ session_manager_proxy_->RegisterPropertyChangeCompleteSignalHandler(
+ base::Bind(&RealDevicePolicyProvider::OnPropertyChangedCompletedSignal,
+ base::Unretained(this)),
+ base::Bind(&RealDevicePolicyProvider::OnSignalConnected,
+ base::Unretained(this)));
return true;
}
-// static
-void RealDevicePolicyProvider::HandlePropertyChangedCompletedStatic(
- DBusGProxy* proxy, const char* /* payload */, void* data) {
+void RealDevicePolicyProvider::OnPropertyChangedCompletedSignal(
+ const string& success) {
+ if (success != "success") {
+ LOG(WARNING) << "Received device policy updated signal with a failure.";
+ }
// We refresh the policy file even if the payload string is kSignalFailure.
- RealDevicePolicyProvider* policy_provider =
- reinterpret_cast<RealDevicePolicyProvider*>(data);
- LOG(INFO) << "Reloading device policy due to signal received.";
- policy_provider->RefreshDevicePolicy();
+ LOG(INFO) << "Reloading and re-scheduling device policy due to signal "
+ "received.";
+ MessageLoop::current()->CancelTask(scheduled_refresh_);
+ scheduled_refresh_ = MessageLoop::kTaskIdNull;
+ RefreshDevicePolicyAndReschedule();
+}
+
+void RealDevicePolicyProvider::OnSignalConnected(const string& interface_name,
+ const string& signal_name,
+ bool successful) {
+ if (!successful) {
+ LOG(WARNING) << "We couldn't connect to SessionManager signal for updates "
+ "on the device policy blob. We will reload the policy file "
+ "periodically.";
+ }
+ // We do a one-time refresh of the DevicePolicy just in case we missed a
+ // signal between the first refresh and the time the signal handler was
+ // actually connected.
+ RefreshDevicePolicy();
}
void RealDevicePolicyProvider::RefreshDevicePolicyAndReschedule() {
diff --git a/update_manager/real_device_policy_provider.h b/update_manager/real_device_policy_provider.h
index b052d59..41b67c2 100644
--- a/update_manager/real_device_policy_provider.h
+++ b/update_manager/real_device_policy_provider.h
@@ -12,7 +12,7 @@
#include <gtest/gtest_prod.h> // for FRIEND_TEST
#include <policy/libpolicy.h>
-#include "update_engine/dbus_wrapper_interface.h"
+#include "update_engine/dbus_proxies.h"
#include "update_engine/update_manager/device_policy_provider.h"
#include "update_engine/update_manager/generic_variables.h"
@@ -21,11 +21,11 @@
// DevicePolicyProvider concrete implementation.
class RealDevicePolicyProvider : public DevicePolicyProvider {
public:
- RealDevicePolicyProvider(
- chromeos_update_engine::DBusWrapperInterface* const dbus,
- policy::PolicyProvider* policy_provider)
+ RealDevicePolicyProvider(org::chromium::SessionManagerInterfaceProxyInterface*
+ session_manager_proxy,
+ policy::PolicyProvider* policy_provider)
: policy_provider_(policy_provider),
- dbus_(dbus) {}
+ session_manager_proxy_(session_manager_proxy) {}
~RealDevicePolicyProvider();
// Initializes the provider and returns whether it succeeded.
@@ -79,9 +79,13 @@
// A static handler for the PropertyChangedCompleted signal from the session
// manager used as a callback.
- static void HandlePropertyChangedCompletedStatic(DBusGProxy* proxy,
- const char* payload,
- void* data);
+ void OnPropertyChangedCompletedSignal(const std::string& success);
+
+ // Called when the signal in UpdateEngineLibcrosProxyResolvedInterface is
+ // connected.
+ void OnSignalConnected(const std::string& interface_name,
+ const std::string& signal_name,
+ bool successful);
// Schedules a call to periodically refresh the device policy.
void RefreshDevicePolicyAndReschedule();
@@ -116,12 +120,12 @@
policy::PolicyProvider* policy_provider_;
// Used to schedule refreshes of the device policy.
- chromeos::MessageLoop::TaskId scheduled_refresh_ =
- chromeos::MessageLoop::kTaskIdNull;
+ chromeos::MessageLoop::TaskId scheduled_refresh_{
+ chromeos::MessageLoop::kTaskIdNull};
- // The DBus interface (mockable) and a session manager proxy.
- chromeos_update_engine::DBusWrapperInterface* const dbus_;
- DBusGProxy* manager_proxy_ = nullptr;
+ // The DBus (mockable) session manager proxy, owned by the caller.
+ org::chromium::SessionManagerInterfaceProxyInterface* session_manager_proxy_{
+ nullptr};
// Variable exposing whether the policy is loaded.
AsyncCopyVariable<bool> var_device_policy_is_loaded_{
diff --git a/update_manager/real_device_policy_provider_unittest.cc b/update_manager/real_device_policy_provider_unittest.cc
index 75d7c48..e909b9c 100644
--- a/update_manager/real_device_policy_provider_unittest.cc
+++ b/update_manager/real_device_policy_provider_unittest.cc
@@ -10,17 +10,19 @@
#include <chromeos/message_loops/fake_message_loop.h>
#include <chromeos/message_loops/message_loop.h>
#include <chromeos/message_loops/message_loop_utils.h>
+#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <policy/mock_device_policy.h>
#include <policy/mock_libpolicy.h>
-#include "update_engine/mock_dbus_wrapper.h"
+#include "update_engine/dbus_mocks.h"
+#include "update_engine/dbus_test_utils.h"
#include "update_engine/test_utils.h"
#include "update_engine/update_manager/umtest_utils.h"
using base::TimeDelta;
using chromeos::MessageLoop;
-using chromeos::MessageLoopRunMaxIterations;
+using chromeos_update_engine::dbus_test_utils::MockSignalHandler;
using std::set;
using std::string;
using std::unique_ptr;
@@ -28,50 +30,31 @@
using testing::Mock;
using testing::Return;
using testing::ReturnRef;
-using testing::SaveArg;
-using testing::SetArgumentPointee;
-using testing::StrEq;
+using testing::SetArgPointee;
using testing::_;
-namespace {
-
-// Fake dbus-glib objects. These should be different values, to ease diagnosis
-// of errors.
-DBusGConnection* const kFakeConnection = reinterpret_cast<DBusGConnection*>(1);
-DBusGProxy* const kFakeManagerProxy = reinterpret_cast<DBusGProxy*>(2);
-
-} // namespace
-
namespace chromeos_update_manager {
class UmRealDevicePolicyProviderTest : public ::testing::Test {
protected:
void SetUp() override {
loop_.SetAsCurrent();
- provider_.reset(new RealDevicePolicyProvider(&mock_dbus_,
+ provider_.reset(new RealDevicePolicyProvider(&session_manager_proxy_mock_,
&mock_policy_provider_));
// By default, we have a device policy loaded. Tests can call
// SetUpNonExistentDevicePolicy() to override this.
SetUpExistentDevicePolicy();
- SetUpDBusSignalExpectations();
+ // Setup the session manager_proxy such that it will accept the signal
+ // handler and store it in the |property_change_complete_| once registered.
+ MOCK_SIGNAL_HANDLER_EXPECT_SIGNAL_HANDLER(property_change_complete_,
+ session_manager_proxy_mock_,
+ PropertyChangeComplete);
}
void TearDown() override {
- // Check for leaked callbacks on the main loop.
- EXPECT_EQ(0, MessageLoopRunMaxIterations(MessageLoop::current(), 100));
-
- // We need to set these expectation before the object is destroyed but
- // after it finished running the test so the values of signal_callback_ and
- // signal_callback_data_ are correct.
- EXPECT_CALL(mock_dbus_, ProxyDisconnectSignal(
- kFakeManagerProxy,
- StrEq(login_manager::kPropertyChangeCompleteSignal),
- signal_callback_,
- signal_callback_data_));
- EXPECT_CALL(mock_dbus_, ProxyUnref(kFakeManagerProxy));
-
provider_.reset();
+ // Check for leaked callbacks on the main loop.
EXPECT_FALSE(loop_.PendingTasks());
}
@@ -93,58 +76,40 @@
.WillByDefault(ReturnRef(mock_device_policy_));
}
- void SetUpDBusSignalExpectations() {
- // Setup the DBus connection with default actions that should be performed
- // once.
- EXPECT_CALL(mock_dbus_, BusGet(_, _)).WillOnce(
- Return(kFakeConnection));
- EXPECT_CALL(mock_dbus_, ProxyNewForName(
- kFakeConnection, StrEq(login_manager::kSessionManagerServiceName),
- StrEq(login_manager::kSessionManagerServicePath),
- StrEq(login_manager::kSessionManagerInterface)))
- .WillOnce(Return(kFakeManagerProxy));
-
- // Expect the signal to be added, registered and released.
- EXPECT_CALL(mock_dbus_, ProxyAddSignal_1(
- kFakeManagerProxy,
- StrEq(login_manager::kPropertyChangeCompleteSignal),
- G_TYPE_STRING));
- EXPECT_CALL(mock_dbus_, ProxyConnectSignal(
- kFakeManagerProxy,
- StrEq(login_manager::kPropertyChangeCompleteSignal),
- _ /* callback */, _ /* data */, _ /* free function */))
- .WillOnce(DoAll(SaveArg<2>(&signal_callback_),
- SaveArg<3>(&signal_callback_data_)));
- }
-
chromeos::FakeMessageLoop loop_{nullptr};
- chromeos_update_engine::MockDBusWrapper mock_dbus_;
+ org::chromium::SessionManagerInterfaceProxyMock session_manager_proxy_mock_;
testing::NiceMock<policy::MockDevicePolicy> mock_device_policy_;
testing::NiceMock<policy::MockPolicyProvider> mock_policy_provider_;
unique_ptr<RealDevicePolicyProvider> provider_;
// The registered signal handler for the signal.
- GCallback signal_callback_ = nullptr;
- void* signal_callback_data_ = nullptr;
+ MockSignalHandler<void(const string&)> property_change_complete_;
};
TEST_F(UmRealDevicePolicyProviderTest, RefreshScheduledTest) {
- // Check that the RefreshPolicy gets scheduled by checking the EventId.
+ // Check that the RefreshPolicy gets scheduled by checking the TaskId.
EXPECT_TRUE(provider_->Init());
EXPECT_NE(MessageLoop::kTaskIdNull, provider_->scheduled_refresh_);
+ loop_.RunOnce(false);
}
TEST_F(UmRealDevicePolicyProviderTest, FirstReload) {
- // Checks that the policy is reloaded and the DevicePolicy is consulted.
+ // Checks that the policy is reloaded and the DevicePolicy is consulted twice:
+ // once on Init() and once again when the signal is connected.
EXPECT_CALL(mock_policy_provider_, Reload());
EXPECT_TRUE(provider_->Init());
+ Mock::VerifyAndClearExpectations(&mock_policy_provider_);
+
+ EXPECT_CALL(mock_policy_provider_, Reload());
+ loop_.RunOnce(false);
}
TEST_F(UmRealDevicePolicyProviderTest, NonExistentDevicePolicyReloaded) {
// Checks that the policy is reloaded by RefreshDevicePolicy().
SetUpNonExistentDevicePolicy();
- EXPECT_CALL(mock_policy_provider_, Reload()).Times(2);
+ EXPECT_CALL(mock_policy_provider_, Reload()).Times(3);
EXPECT_TRUE(provider_->Init());
+ loop_.RunOnce(false);
// Force the policy refresh.
provider_->RefreshDevicePolicy();
}
@@ -154,22 +119,19 @@
SetUpNonExistentDevicePolicy();
EXPECT_CALL(mock_policy_provider_, Reload()).Times(2);
EXPECT_TRUE(provider_->Init());
+ loop_.RunOnce(false);
+ Mock::VerifyAndClearExpectations(&mock_policy_provider_);
- ASSERT_NE(nullptr, signal_callback_);
- // Convert the GCallback to a function pointer and call it. GCallback is just
- // a void function pointer to ensure that the type of the passed callback is a
- // pointer. We need to cast it back to the right function type before calling
- // it.
- typedef void (*StaticSignalHandler)(DBusGProxy*, const char*, void*);
- StaticSignalHandler signal_handler = reinterpret_cast<StaticSignalHandler>(
- signal_callback_);
- (*signal_handler)(kFakeManagerProxy, "success", signal_callback_data_);
+ EXPECT_CALL(mock_policy_provider_, Reload());
+ ASSERT_TRUE(property_change_complete_.IsHandlerRegistered());
+ property_change_complete_.signal_callback().Run("success");
}
TEST_F(UmRealDevicePolicyProviderTest, NonExistentDevicePolicyEmptyVariables) {
SetUpNonExistentDevicePolicy();
EXPECT_CALL(mock_policy_provider_, GetDevicePolicy()).Times(0);
EXPECT_TRUE(provider_->Init());
+ loop_.RunOnce(false);
UmTestUtils::ExpectVariableHasValue(false,
provider_->var_device_policy_is_loaded());
@@ -189,14 +151,14 @@
TEST_F(UmRealDevicePolicyProviderTest, ValuesUpdated) {
SetUpNonExistentDevicePolicy();
EXPECT_TRUE(provider_->Init());
+ loop_.RunOnce(false);
Mock::VerifyAndClearExpectations(&mock_policy_provider_);
// Reload the policy with a good one and set some values as present. The
// remaining values are false.
SetUpExistentDevicePolicy();
EXPECT_CALL(mock_device_policy_, GetReleaseChannel(_))
- .WillOnce(DoAll(SetArgumentPointee<0>(string("mychannel")),
- Return(true)));
+ .WillOnce(DoAll(SetArgPointee<0>(string("mychannel")), Return(true)));
EXPECT_CALL(mock_device_policy_, GetAllowedConnectionTypesForUpdate(_))
.WillOnce(Return(false));
@@ -215,8 +177,10 @@
TEST_F(UmRealDevicePolicyProviderTest, ScatterFactorConverted) {
SetUpExistentDevicePolicy();
EXPECT_CALL(mock_device_policy_, GetScatterFactorInSeconds(_))
- .WillOnce(DoAll(SetArgumentPointee<0>(1234), Return(true)));
+ .Times(2)
+ .WillRepeatedly(DoAll(SetArgPointee<0>(1234), Return(true)));
EXPECT_TRUE(provider_->Init());
+ loop_.RunOnce(false);
UmTestUtils::ExpectVariableHasValue(TimeDelta::FromSeconds(1234),
provider_->var_scatter_factor());
@@ -225,8 +189,10 @@
TEST_F(UmRealDevicePolicyProviderTest, NegativeScatterFactorIgnored) {
SetUpExistentDevicePolicy();
EXPECT_CALL(mock_device_policy_, GetScatterFactorInSeconds(_))
- .WillOnce(DoAll(SetArgumentPointee<0>(-1), Return(true)));
+ .Times(2)
+ .WillRepeatedly(DoAll(SetArgPointee<0>(-1), Return(true)));
EXPECT_TRUE(provider_->Init());
+ loop_.RunOnce(false);
UmTestUtils::ExpectVariableNotSet(provider_->var_scatter_factor());
}
@@ -234,10 +200,12 @@
TEST_F(UmRealDevicePolicyProviderTest, AllowedTypesConverted) {
SetUpExistentDevicePolicy();
EXPECT_CALL(mock_device_policy_, GetAllowedConnectionTypesForUpdate(_))
- .WillOnce(DoAll(SetArgumentPointee<0>(
- set<string>{"bluetooth", "wifi", "not-a-type"}),
- Return(true)));
+ .Times(2)
+ .WillRepeatedly(DoAll(
+ SetArgPointee<0>(set<string>{"bluetooth", "wifi", "not-a-type"}),
+ Return(true)));
EXPECT_TRUE(provider_->Init());
+ loop_.RunOnce(false);
UmTestUtils::ExpectVariableHasValue(
set<ConnectionType>{ConnectionType::kWifi, ConnectionType::kBluetooth},
diff --git a/update_manager/real_shill_provider.cc b/update_manager/real_shill_provider.cc
index 609fd01..dcf2eb2 100644
--- a/update_manager/real_shill_provider.cc
+++ b/update_manager/real_shill_provider.cc
@@ -10,182 +10,158 @@
#include <base/strings/stringprintf.h>
#include <chromeos/dbus/service_constants.h>
-#include "update_engine/glib_utils.h"
-
+using org::chromium::flimflam::ManagerProxyInterface;
+using org::chromium::flimflam::ServiceProxyInterface;
using std::string;
-namespace {
-
-// Looks up a |key| in a GLib |hash_table| and returns the unboxed string from
-// the corresponding GValue, if found.
-const char* GetStrProperty(GHashTable* hash_table, const char* key) {
- auto gval = reinterpret_cast<GValue*>(g_hash_table_lookup(hash_table, key));
- return (gval ? g_value_get_string(gval) : nullptr);
-}
-
-}; // namespace
-
-
namespace chromeos_update_manager {
-RealShillProvider::~RealShillProvider() {
- // Detach signal handler, free manager proxy.
- dbus_->ProxyDisconnectSignal(manager_proxy_, shill::kMonitorPropertyChanged,
- G_CALLBACK(HandlePropertyChangedStatic),
- this);
- dbus_->ProxyUnref(manager_proxy_);
-}
-
-ConnectionType RealShillProvider::ParseConnectionType(const char* type_str) {
- if (!strcmp(type_str, shill::kTypeEthernet))
+ConnectionType RealShillProvider::ParseConnectionType(const string& type_str) {
+ if (type_str == shill::kTypeEthernet) {
return ConnectionType::kEthernet;
- if (!strcmp(type_str, shill::kTypeWifi))
+ } else if (type_str == shill::kTypeWifi) {
return ConnectionType::kWifi;
- if (!strcmp(type_str, shill::kTypeWimax))
+ } else if (type_str == shill::kTypeWimax) {
return ConnectionType::kWimax;
- if (!strcmp(type_str, shill::kTypeBluetooth))
+ } else if (type_str == shill::kTypeBluetooth) {
return ConnectionType::kBluetooth;
- if (!strcmp(type_str, shill::kTypeCellular))
+ } else if (type_str == shill::kTypeCellular) {
return ConnectionType::kCellular;
-
+ }
return ConnectionType::kUnknown;
}
ConnectionTethering RealShillProvider::ParseConnectionTethering(
- const char* tethering_str) {
- if (!strcmp(tethering_str, shill::kTetheringNotDetectedState))
+ const string& tethering_str) {
+ if (tethering_str == shill::kTetheringNotDetectedState) {
return ConnectionTethering::kNotDetected;
- if (!strcmp(tethering_str, shill::kTetheringSuspectedState))
+ } else if (tethering_str == shill::kTetheringSuspectedState) {
return ConnectionTethering::kSuspected;
- if (!strcmp(tethering_str, shill::kTetheringConfirmedState))
+ } else if (tethering_str == shill::kTetheringConfirmedState) {
return ConnectionTethering::kConfirmed;
-
+ }
return ConnectionTethering::kUnknown;
}
bool RealShillProvider::Init() {
- // Obtain a DBus connection.
- GError* error = nullptr;
- connection_ = dbus_->BusGet(DBUS_BUS_SYSTEM, &error);
- if (!connection_) {
- LOG(ERROR) << "Failed to initialize DBus connection: "
- << chromeos_update_engine::utils::GetAndFreeGError(&error);
+ ManagerProxyInterface* manager_proxy = shill_proxy_->GetManagerProxy();
+ if (!manager_proxy)
return false;
- }
-
- // Allocate a shill manager proxy.
- manager_proxy_ = GetProxy(shill::kFlimflamServicePath,
- shill::kFlimflamManagerInterface);
// Subscribe to the manager's PropertyChanged signal.
- dbus_->ProxyAddSignal_2(manager_proxy_, shill::kMonitorPropertyChanged,
- G_TYPE_STRING, G_TYPE_VALUE);
- dbus_->ProxyConnectSignal(manager_proxy_, shill::kMonitorPropertyChanged,
- G_CALLBACK(HandlePropertyChangedStatic),
- this, nullptr);
+ manager_proxy->RegisterPropertyChangedSignalHandler(
+ base::Bind(&RealShillProvider::OnManagerPropertyChanged,
+ base::Unretained(this)),
+ base::Bind(&RealShillProvider::OnSignalConnected,
+ base::Unretained(this)));
// Attempt to read initial connection status. Even if this fails because shill
// is not responding (e.g. it is down) we'll be notified via "PropertyChanged"
// signal as soon as it comes up, so this is not a critical step.
- GHashTable* hash_table = nullptr;
- if (GetProperties(manager_proxy_, &hash_table)) {
- GValue* value = reinterpret_cast<GValue*>(
- g_hash_table_lookup(hash_table, shill::kDefaultServiceProperty));
- ProcessDefaultService(value);
- g_hash_table_unref(hash_table);
+ chromeos::VariantDictionary properties;
+ chromeos::ErrorPtr error;
+ if (!manager_proxy->GetProperties(&properties, &error))
+ return true;
+
+ const auto& prop_default_service =
+ properties.find(shill::kDefaultServiceProperty);
+ if (prop_default_service != properties.end()) {
+ OnManagerPropertyChanged(prop_default_service->first,
+ prop_default_service->second);
}
return true;
}
-DBusGProxy* RealShillProvider::GetProxy(const char* path,
- const char* interface) {
- return dbus_->ProxyNewForName(connection_, shill::kFlimflamServiceName,
- path, interface);
+void RealShillProvider::OnManagerPropertyChanged(const string& name,
+ const chromeos::Any& value) {
+ if (name == shill::kDefaultServiceProperty)
+ ProcessDefaultService(value.TryGet<dbus::ObjectPath>().value());
}
-bool RealShillProvider::GetProperties(DBusGProxy* proxy,
- GHashTable** result_p) {
- GError* error = nullptr;
- if (!dbus_->ProxyCall_0_1(proxy, shill::kGetPropertiesFunction, &error,
- result_p)) {
- LOG(ERROR) << "Calling shill via DBus proxy failed: "
- << chromeos_update_engine::utils::GetAndFreeGError(&error);
- return false;
+void RealShillProvider::OnSignalConnected(const string& interface_name,
+ const string& signal_name,
+ bool successful) {
+ if (!successful) {
+ LOG(ERROR) << "Couldn't connect to the signal " << interface_name << "."
+ << signal_name;
}
- return true;
}
-bool RealShillProvider::ProcessDefaultService(GValue* value) {
- // Decode the string from the boxed value.
- const char* default_service_path_str = nullptr;
- if (!(value && (default_service_path_str = g_value_get_string(value))))
- return false;
-
- // Anything changed?
- if (default_service_path_ == default_service_path_str)
+bool RealShillProvider::ProcessDefaultService(
+ const string& default_service_path) {
+ // We assume that if the service path didn't change, then the connection
+ // type and the tethering status of it also didn't change.
+ if (default_service_path_ == default_service_path)
return true;
// Update the connection status.
- default_service_path_ = default_service_path_str;
+ default_service_path_ = default_service_path;
bool is_connected = (default_service_path_ != "/");
var_is_connected_.SetValue(is_connected);
var_conn_last_changed_.SetValue(clock_->GetWallclockTime());
- // Update the connection attributes.
- if (is_connected) {
- DBusGProxy* service_proxy = GetProxy(default_service_path_.c_str(),
- shill::kFlimflamServiceInterface);
- GHashTable* hash_table = nullptr;
- if (GetProperties(service_proxy, &hash_table)) {
- // Get the connection type.
- const char* type_str = GetStrProperty(hash_table, shill::kTypeProperty);
- if (type_str && !strcmp(type_str, shill::kTypeVPN)) {
- type_str = GetStrProperty(hash_table,
- shill::kPhysicalTechnologyProperty);
- }
- if (type_str) {
- var_conn_type_.SetValue(ParseConnectionType(type_str));
- } else {
- var_conn_type_.UnsetValue();
- LOG(ERROR) << "Could not find connection type ("
- << default_service_path_ << ")";
- }
-
- // Get the connection tethering mode.
- const char* tethering_str = GetStrProperty(hash_table,
- shill::kTetheringProperty);
- if (tethering_str) {
- var_conn_tethering_.SetValue(ParseConnectionTethering(tethering_str));
- } else {
- var_conn_tethering_.UnsetValue();
- LOG(ERROR) << "Could not find connection tethering mode ("
- << default_service_path_ << ")";
- }
-
- g_hash_table_unref(hash_table);
- }
- dbus_->ProxyUnref(service_proxy);
- } else {
+ if (!is_connected) {
var_conn_type_.UnsetValue();
var_conn_tethering_.UnsetValue();
+ return true;
+ }
+
+ // We create and dispose the ServiceProxyInterface on every request.
+ std::unique_ptr<ServiceProxyInterface> service =
+ shill_proxy_->GetServiceForPath(default_service_path_);
+
+ // Get the connection properties synchronously.
+ chromeos::VariantDictionary properties;
+ chromeos::ErrorPtr error;
+ if (!service->GetProperties(&properties, &error)) {
+ var_conn_type_.UnsetValue();
+ var_conn_tethering_.UnsetValue();
+ return false;
+ }
+
+ // Get the connection tethering mode.
+ const auto& prop_tethering = properties.find(shill::kTetheringProperty);
+ if (prop_tethering == properties.end()) {
+ // Remove the value if not present on the service. This most likely means an
+ // error in shill and the policy will handle it, but we will print a log
+ // message as well for accessing an unused variable.
+ var_conn_tethering_.UnsetValue();
+ LOG(ERROR) << "Could not find connection type (" << default_service_path_
+ << ")";
+ } else {
+ // If the property doesn't contain a string value, the empty string will
+ // become kUnknown.
+ var_conn_tethering_.SetValue(
+ ParseConnectionTethering(prop_tethering->second.TryGet<string>()));
+ }
+
+ // Get the connection type.
+ const auto& prop_type = properties.find(shill::kTypeProperty);
+ if (prop_type == properties.end()) {
+ var_conn_type_.UnsetValue();
+ LOG(ERROR) << "Could not find connection tethering mode ("
+ << default_service_path_ << ")";
+ } else {
+ string type_str = prop_type->second.TryGet<string>();
+ if (type_str == shill::kTypeVPN) {
+ const auto& prop_physical =
+ properties.find(shill::kPhysicalTechnologyProperty);
+ if (prop_physical == properties.end()) {
+ LOG(ERROR) << "No PhysicalTechnology property found for a VPN"
+ << " connection (service: " << default_service_path
+ << "). Using default kUnknown value.";
+ var_conn_type_.SetValue(ConnectionType::kUnknown);
+ } else {
+ var_conn_type_.SetValue(
+ ParseConnectionType(prop_physical->second.TryGet<string>()));
+ }
+ } else {
+ var_conn_type_.SetValue(ParseConnectionType(type_str));
+ }
}
return true;
}
-void RealShillProvider::HandlePropertyChanged(DBusGProxy* proxy,
- const char* name, GValue* value) {
- if (!strcmp(name, shill::kDefaultServiceProperty))
- ProcessDefaultService(value);
-}
-
-void RealShillProvider::HandlePropertyChangedStatic(DBusGProxy* proxy,
- const char* name,
- GValue* value,
- void* data) {
- auto obj = reinterpret_cast<RealShillProvider*>(data);
- obj->HandlePropertyChanged(proxy, name, value);
-}
-
} // namespace chromeos_update_manager
diff --git a/update_manager/real_shill_provider.h b/update_manager/real_shill_provider.h
index 5b50bf4..5864319 100644
--- a/update_manager/real_shill_provider.h
+++ b/update_manager/real_shill_provider.h
@@ -14,22 +14,21 @@
#include <base/time/time.h>
#include "update_engine/clock_interface.h"
-#include "update_engine/dbus_wrapper_interface.h"
+#include "update_engine/dbus_proxies.h"
+#include "update_engine/shill_proxy_interface.h"
#include "update_engine/update_manager/generic_variables.h"
#include "update_engine/update_manager/shill_provider.h"
-using chromeos_update_engine::ClockInterface;
-using chromeos_update_engine::DBusWrapperInterface;
-
namespace chromeos_update_manager {
// ShillProvider concrete implementation.
class RealShillProvider : public ShillProvider {
public:
- RealShillProvider(DBusWrapperInterface* dbus, ClockInterface* clock)
- : dbus_(dbus), clock_(clock) {}
+ RealShillProvider(chromeos_update_engine::ShillProxyInterface* shill_proxy,
+ chromeos_update_engine::ClockInterface* clock)
+ : shill_proxy_(shill_proxy), clock_(clock) {}
- ~RealShillProvider() override;
+ ~RealShillProvider() override = default;
// Initializes the provider and returns whether it succeeded.
bool Init();
@@ -51,37 +50,33 @@
}
// Helper methods for converting shill strings into symbolic values.
- static ConnectionType ParseConnectionType(const char* type_str);
+ static ConnectionType ParseConnectionType(const std::string& type_str);
static ConnectionTethering ParseConnectionTethering(
- const char* tethering_str);
+ const std::string& tethering_str);
private:
- // Return a DBus proxy for a given |path| and |interface| within shill.
- DBusGProxy* GetProxy(const char* path, const char* interface);
+ // A handler for ManagerProxy.PropertyChanged signal.
+ void OnManagerPropertyChanged(const std::string& name,
+ const chromeos::Any& value);
- // Issues a GetProperties call through a given |proxy|, storing the result to
- // |*result_p|. Returns true on success.
- bool GetProperties(DBusGProxy* proxy, GHashTable** result_p);
+ // Called when the signal in ManagerProxy.PropertyChanged is connected.
+ void OnSignalConnected(const std::string& interface_name,
+ const std::string& signal_name,
+ bool successful);
- // Process a default connection value, update last change time as needed.
- bool ProcessDefaultService(GValue* value);
-
- // A handler for manager PropertyChanged signal, and a static version.
- void HandlePropertyChanged(DBusGProxy* proxy, const char* name,
- GValue* value);
- static void HandlePropertyChangedStatic(DBusGProxy* proxy, const char* name,
- GValue* value, void* data);
+ // Get the connection and populate the type and tethering status of the given
+ // default connection.
+ bool ProcessDefaultService(const std::string& default_service_path);
// The current default service path, if connected.
std::string default_service_path_;
- // The DBus interface (mockable), connection, and a shill manager proxy.
- DBusWrapperInterface* const dbus_;
- DBusGConnection* connection_ = nullptr;
- DBusGProxy* manager_proxy_ = nullptr;
+ // The mockable interface to access the shill DBus proxies, owned by the
+ // caller.
+ chromeos_update_engine::ShillProxyInterface* shill_proxy_;
// A clock abstraction (mockable).
- ClockInterface* const clock_;
+ chromeos_update_engine::ClockInterface* const clock_;
// The provider's variables.
AsyncCopyVariable<bool> var_is_connected_{"is_connected"};
diff --git a/update_manager/real_shill_provider_unittest.cc b/update_manager/real_shill_provider_unittest.cc
index 78bdd93..5b81e91 100644
--- a/update_manager/real_shill_provider_unittest.cc
+++ b/update_manager/real_shill_provider_unittest.cc
@@ -1,7 +1,6 @@
// Copyright (c) 2014 The Chromium OS 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 "update_engine/update_manager/real_shill_provider.h"
#include <memory>
@@ -9,46 +8,32 @@
#include <base/time/time.h>
#include <chromeos/dbus/service_constants.h>
-#include <glib.h>
+#include <chromeos/make_unique_ptr.h>
+#include <chromeos/message_loops/fake_message_loop.h>
+#include <gmock/gmock.h>
#include <gtest/gtest.h>
+#include "update_engine/dbus_mocks.h"
+#include "update_engine/dbus_proxies.h"
+#include "update_engine/dbus_test_utils.h"
#include "update_engine/fake_clock.h"
-#include "update_engine/mock_dbus_wrapper.h"
+#include "update_engine/fake_shill_proxy.h"
#include "update_engine/test_utils.h"
#include "update_engine/update_manager/umtest_utils.h"
using base::Time;
using base::TimeDelta;
using chromeos_update_engine::FakeClock;
-using chromeos_update_engine::MockDBusWrapper;
-using chromeos_update_engine::test_utils::GValueFree;
-using chromeos_update_engine::test_utils::GValueNewString;
-using std::pair;
+using org::chromium::flimflam::ManagerProxyMock;
+using org::chromium::flimflam::ServiceProxyMock;
using std::unique_ptr;
-using testing::A;
-using testing::Eq;
using testing::Mock;
using testing::Return;
-using testing::SaveArg;
using testing::SetArgPointee;
-using testing::StrEq;
-using testing::StrictMock;
using testing::_;
namespace {
-// Fake dbus-glib objects. These should be different values, to ease diagnosis
-// of errors.
-DBusGConnection* const kFakeConnection = reinterpret_cast<DBusGConnection*>(1);
-DBusGProxy* const kFakeManagerProxy = reinterpret_cast<DBusGProxy*>(2);
-DBusGProxy* const kFakeEthernetServiceProxy = reinterpret_cast<DBusGProxy*>(3);
-DBusGProxy* const kFakeWifiServiceProxy = reinterpret_cast<DBusGProxy*>(4);
-DBusGProxy* const kFakeWimaxServiceProxy = reinterpret_cast<DBusGProxy*>(5);
-DBusGProxy* const kFakeBluetoothServiceProxy = reinterpret_cast<DBusGProxy*>(6);
-DBusGProxy* const kFakeCellularServiceProxy = reinterpret_cast<DBusGProxy*>(7);
-DBusGProxy* const kFakeVpnServiceProxy = reinterpret_cast<DBusGProxy*>(8);
-DBusGProxy* const kFakeUnknownServiceProxy = reinterpret_cast<DBusGProxy*>(9);
-
// Fake service paths.
const char* const kFakeEthernetServicePath = "/fake-ethernet-service";
const char* const kFakeWifiServicePath = "/fake-wifi-service";
@@ -64,89 +49,23 @@
class UmRealShillProviderTest : public ::testing::Test {
protected:
+ // Initialize the RealShillProvider under test.
void SetUp() override {
- // By default, initialize the provider so that it gets an initial connection
- // status from shill. This simulates the common case where shill is
- // available and responding during RealShillProvider initialization.
- Init(true);
+ fake_clock_.SetWallclockTime(InitTime());
+ loop_.SetAsCurrent();
+ provider_.reset(new RealShillProvider(&fake_shill_proxy_, &fake_clock_));
+
+ ManagerProxyMock* manager_proxy_mock = fake_shill_proxy_.GetManagerProxy();
+
+ // The PropertyChanged signal should be subscribed to.
+ MOCK_SIGNAL_HANDLER_EXPECT_SIGNAL_HANDLER(
+ manager_property_changed_, *manager_proxy_mock, PropertyChanged);
}
void TearDown() override {
- Shutdown();
- }
-
- // Initialize the RealShillProvider under test. If |do_init_conn_status| is
- // true, configure mocks to respond to the initial connection status check
- // with shill. Otherwise, the initial check will fail.
- void Init(bool do_init_conn_status) {
- // Properly shutdown a previously initialized provider.
- if (provider_.get())
- Shutdown();
-
- provider_.reset(new RealShillProvider(&mock_dbus_, &fake_clock_));
- ASSERT_NE(nullptr, provider_.get());
- fake_clock_.SetWallclockTime(InitTime());
-
- // A DBus connection should only be obtained once.
- EXPECT_CALL(mock_dbus_, BusGet(_, _)).WillOnce(
- Return(kFakeConnection));
-
- // A manager proxy should only be obtained once.
- EXPECT_CALL(mock_dbus_, ProxyNewForName(
- kFakeConnection, StrEq(shill::kFlimflamServiceName),
- StrEq(shill::kFlimflamServicePath),
- StrEq(shill::kFlimflamManagerInterface)))
- .WillOnce(Return(kFakeManagerProxy));
-
- // The PropertyChanged signal should be subscribed to.
- EXPECT_CALL(mock_dbus_, ProxyAddSignal_2(
- kFakeManagerProxy, StrEq(shill::kMonitorPropertyChanged),
- G_TYPE_STRING, G_TYPE_VALUE))
- .WillOnce(Return());
- EXPECT_CALL(mock_dbus_, ProxyConnectSignal(
- kFakeManagerProxy, StrEq(shill::kMonitorPropertyChanged),
- _, _, _))
- .WillOnce(
- DoAll(SaveArg<2>(reinterpret_cast<void (**)()>(&signal_handler_)),
- SaveArg<3>(&signal_data_),
- Return()));
-
- // Mock a response to an initial connection check (optional).
- GHashTable* manager_properties = nullptr;
- if (do_init_conn_status) {
- pair<const char*, const char*> manager_pairs[] = {
- {shill::kDefaultServiceProperty, "/"},
- };
- manager_properties = SetupGetPropertiesOkay(
- kFakeManagerProxy, arraysize(manager_pairs), manager_pairs);
- } else {
- SetupGetPropertiesFail(kFakeManagerProxy);
- }
-
- // Check that provider initializes correctly.
- ASSERT_TRUE(provider_->Init());
-
- // All mocked calls should have been exercised by now.
- Mock::VerifyAndClear(&mock_dbus_);
-
- // Release properties hash table (if provided).
- if (manager_properties)
- g_hash_table_unref(manager_properties);
- }
-
- // Deletes the RealShillProvider under test.
- void Shutdown() {
- // Make sure that DBus resources get freed.
- EXPECT_CALL(mock_dbus_, ProxyDisconnectSignal(
- kFakeManagerProxy, StrEq(shill::kMonitorPropertyChanged),
- Eq(reinterpret_cast<void (*)()>(signal_handler_)),
- Eq(signal_data_)))
- .WillOnce(Return());
- EXPECT_CALL(mock_dbus_, ProxyUnref(kFakeManagerProxy)).WillOnce(Return());
provider_.reset();
-
- // All mocked calls should have been exercised by now.
- Mock::VerifyAndClear(&mock_dbus_);
+ // Check for leaked callbacks on the main loop.
+ EXPECT_FALSE(loop_.PendingTasks());
}
// These methods generate fixed timestamps for use in faking the current time.
@@ -167,53 +86,41 @@
return InitTime() + TimeDelta::FromSeconds(10);
}
- // Sets up a successful mock "GetProperties" call on |proxy|, writing a hash
- // table containing |num_entries| entries formed by key/value pairs from
- // |key_val_pairs| and returning true. Keys and values are plain C strings
- // (const char*). The proxy call is expected to be made exactly once. Returns
- // a pointer to a newly allocated hash table, which should be unreffed with
- // g_hash_table_unref() when done.
- GHashTable* SetupGetPropertiesOkay(
- DBusGProxy* proxy, size_t num_entries,
- pair<const char*, const char*>* key_val_pairs) {
- // Allocate and populate the hash table.
- GHashTable* properties = g_hash_table_new_full(g_str_hash, g_str_equal,
- free, GValueFree);
- for (size_t i = 0; i < num_entries; i++) {
- g_hash_table_insert(properties, strdup(key_val_pairs[i].first),
- GValueNewString(key_val_pairs[i].second));
- }
+ // Sets the default_service object path in the response from the
+ // ManagerProxyMock instance.
+ void SetManagerReply(const char* default_service, bool reply_succeeds);
- // Set mock expectations.
- EXPECT_CALL(mock_dbus_,
- ProxyCall_0_1(proxy, StrEq(shill::kGetPropertiesFunction),
- _, A<GHashTable**>()))
- .WillOnce(DoAll(SetArgPointee<3>(g_hash_table_ref(properties)),
- Return(true)));
+ // Sets the |service_type|, |physical_technology| and |service_tethering|
+ // properties in the mocked service |service_path|. If any of the three
+ // const char* is a nullptr, the corresponding property will not be included
+ // in the response.
+ // Returns the mock object pointer, owned by the |fake_shill_proxy_|.
+ ServiceProxyMock* SetServiceReply(const std::string& service_path,
+ const char* service_type,
+ const char* physical_technology,
+ const char* service_tethering);
- return properties;
- }
-
- // Sets up a failing mock "GetProperties" call on |proxy|, returning false.
- // The proxy call is expected to be made exactly once.
- void SetupGetPropertiesFail(DBusGProxy* proxy) {
- EXPECT_CALL(mock_dbus_,
- ProxyCall_0_1(proxy, StrEq(shill::kGetPropertiesFunction),
- _, A<GHashTable**>()))
- .WillOnce(Return(false));
+ void InitWithDefaultService(const char* default_service) {
+ SetManagerReply(default_service, true);
+ // Check that provider initializes correctly.
+ EXPECT_TRUE(provider_->Init());
+ // RunOnce to notify the signal handler was connected properly.
+ EXPECT_TRUE(loop_.RunOnce(false));
}
// Sends a signal informing the provider about a default connection
- // |service_path|. Returns the fake connection change time.
- Time SendDefaultServiceSignal(const char* service_path) {
- auto default_service_gval = GValueNewString(service_path);
+ // |service_path|. Sets the fake connection change time in
+ // |conn_change_time_p| if provided.
+ void SendDefaultServiceSignal(const std::string& service_path,
+ Time* conn_change_time_p) {
const Time conn_change_time = ConnChangedTime();
fake_clock_.SetWallclockTime(conn_change_time);
- signal_handler_(kFakeManagerProxy, shill::kDefaultServiceProperty,
- default_service_gval, signal_data_);
+ ASSERT_TRUE(manager_property_changed_.IsHandlerRegistered());
+ manager_property_changed_.signal_callback().Run(
+ shill::kDefaultServiceProperty, dbus::ObjectPath(service_path));
fake_clock_.SetWallclockTime(conn_change_time + TimeDelta::FromSeconds(5));
- GValueFree(default_service_gval);
- return conn_change_time;
+ if (conn_change_time_p)
+ *conn_change_time_p = conn_change_time;
}
// Sets up expectations for detection of a connection |service_path| with type
@@ -221,31 +128,18 @@
// new connection status and change time are properly detected by the
// provider. Writes the fake connection change time to |conn_change_time_p|,
// if provided.
- void SetupConnectionAndAttrs(const char* service_path,
- DBusGProxy* service_proxy,
- const char* shill_type_str,
- const char* shill_tethering_str,
+ void SetupConnectionAndAttrs(const std::string& service_path,
+ const char* shill_type,
+ const char* shill_tethering,
Time* conn_change_time_p) {
- // Mock logic for querying the default service attributes.
- EXPECT_CALL(mock_dbus_,
- ProxyNewForName(
- kFakeConnection, StrEq(shill::kFlimflamServiceName),
- StrEq(service_path),
- StrEq(shill::kFlimflamServiceInterface)))
- .WillOnce(Return(service_proxy));
- EXPECT_CALL(mock_dbus_, ProxyUnref(service_proxy)).WillOnce(Return());
- pair<const char*, const char*> service_pairs[] = {
- {shill::kTypeProperty, shill_type_str},
- {shill::kTetheringProperty, shill_tethering_str},
- };
- auto service_properties = SetupGetPropertiesOkay(
- service_proxy, arraysize(service_pairs), service_pairs);
+ SetServiceReply(service_path, shill_type, nullptr, shill_tethering);
+ // Note: We don't setup this |service_path| as the default service path but
+ // we instead send a signal notifying the change since the code won't call
+ // GetProperties on the Manager object at this point.
// Send a signal about a new default service.
- auto conn_change_time = SendDefaultServiceSignal(service_path);
-
- // Release the service properties hash tables.
- g_hash_table_unref(service_properties);
+ Time conn_change_time;
+ SendDefaultServiceSignal(service_path, &conn_change_time);
// Query the connection status, ensure last change time reported correctly.
UmTestUtils::ExpectVariableHasValue(true, provider_->var_is_connected());
@@ -260,12 +154,12 @@
// Sets up a connection and tests that its type is being properly detected by
// the provider.
void SetupConnectionAndTestType(const char* service_path,
- DBusGProxy* service_proxy,
- const char* shill_type_str,
+ const char* shill_type,
ConnectionType expected_conn_type) {
// Set up and test the connection, record the change time.
Time conn_change_time;
- SetupConnectionAndAttrs(service_path, service_proxy, shill_type_str,
+ SetupConnectionAndAttrs(service_path,
+ shill_type,
shill::kTetheringNotDetectedState,
&conn_change_time);
@@ -279,13 +173,13 @@
// Sets up a connection and tests that its tethering mode is being properly
// detected by the provider.
void SetupConnectionAndTestTethering(
- const char* service_path, DBusGProxy* service_proxy,
- const char* shill_tethering_str,
+ const char* service_path,
+ const char* shill_tethering,
ConnectionTethering expected_conn_tethering) {
// Set up and test the connection, record the change time.
Time conn_change_time;
- SetupConnectionAndAttrs(service_path, service_proxy, shill::kTypeEthernet,
- shill_tethering_str, &conn_change_time);
+ SetupConnectionAndAttrs(
+ service_path, shill::kTypeEthernet, shill_tethering, &conn_change_time);
// Query the connection tethering, ensure last change time did not change.
UmTestUtils::ExpectVariableHasValue(expected_conn_tethering,
@@ -294,16 +188,74 @@
provider_->var_conn_last_changed());
}
- StrictMock<MockDBusWrapper> mock_dbus_;
+ chromeos::FakeMessageLoop loop_{nullptr};
FakeClock fake_clock_;
+ chromeos_update_engine::FakeShillProxy fake_shill_proxy_;
+
+ // The registered signal handler for the signal Manager.PropertyChanged.
+ chromeos_update_engine::dbus_test_utils::MockSignalHandler<
+ void(const std::string&, const chromeos::Any&)> manager_property_changed_;
+
unique_ptr<RealShillProvider> provider_;
- void (*signal_handler_)(DBusGProxy*, const char*, GValue*, void*);
- void* signal_data_;
};
+void UmRealShillProviderTest::SetManagerReply(const char* default_service,
+ bool reply_succeeds) {
+ ManagerProxyMock* manager_proxy_mock = fake_shill_proxy_.GetManagerProxy();
+ if (!reply_succeeds) {
+ EXPECT_CALL(*manager_proxy_mock, GetProperties(_, _, _))
+ .WillOnce(Return(false));
+ return;
+ }
+
+ // Create a dictionary of properties and optionally include the default
+ // service.
+ chromeos::VariantDictionary reply_dict;
+ reply_dict["SomeOtherProperty"] = 0xC0FFEE;
+
+ if (default_service) {
+ reply_dict[shill::kDefaultServiceProperty] =
+ dbus::ObjectPath(default_service);
+ }
+ EXPECT_CALL(*manager_proxy_mock, GetProperties(_, _, _))
+ .WillOnce(DoAll(SetArgPointee<0>(reply_dict), Return(true)));
+}
+
+ServiceProxyMock* UmRealShillProviderTest::SetServiceReply(
+ const std::string& service_path,
+ const char* service_type,
+ const char* physical_technology,
+ const char* service_tethering) {
+ chromeos::VariantDictionary reply_dict;
+ reply_dict["SomeOtherProperty"] = 0xC0FFEE;
+
+ if (service_type)
+ reply_dict[shill::kTypeProperty] = std::string(service_type);
+
+ if (physical_technology) {
+ reply_dict[shill::kPhysicalTechnologyProperty] =
+ std::string(physical_technology);
+ }
+
+ if (service_tethering)
+ reply_dict[shill::kTetheringProperty] = std::string(service_tethering);
+
+ ServiceProxyMock* service_proxy_mock = new ServiceProxyMock();
+
+ // Plumb return value into mock object.
+ EXPECT_CALL(*service_proxy_mock, GetProperties(_, _, _))
+ .WillOnce(DoAll(SetArgPointee<0>(reply_dict), Return(true)));
+
+ fake_shill_proxy_.SetServiceForPath(
+ service_path, chromeos::make_unique_ptr(service_proxy_mock));
+ return service_proxy_mock;
+}
+
+
// Query the connection status, type and time last changed, as they were set
// during initialization (no signals).
TEST_F(UmRealShillProviderTest, ReadBaseValues) {
+ InitWithDefaultService("/");
// Query the provider variables.
UmTestUtils::ExpectVariableHasValue(false, provider_->var_is_connected());
UmTestUtils::ExpectVariableNotSet(provider_->var_conn_type());
@@ -313,86 +265,77 @@
// Test that Ethernet connection is identified correctly.
TEST_F(UmRealShillProviderTest, ReadConnTypeEthernet) {
+ InitWithDefaultService("/");
SetupConnectionAndTestType(kFakeEthernetServicePath,
- kFakeEthernetServiceProxy,
shill::kTypeEthernet,
ConnectionType::kEthernet);
}
// Test that Wifi connection is identified correctly.
TEST_F(UmRealShillProviderTest, ReadConnTypeWifi) {
+ InitWithDefaultService("/");
SetupConnectionAndTestType(kFakeWifiServicePath,
- kFakeWifiServiceProxy,
shill::kTypeWifi,
ConnectionType::kWifi);
}
// Test that Wimax connection is identified correctly.
TEST_F(UmRealShillProviderTest, ReadConnTypeWimax) {
+ InitWithDefaultService("/");
SetupConnectionAndTestType(kFakeWimaxServicePath,
- kFakeWimaxServiceProxy,
shill::kTypeWimax,
ConnectionType::kWimax);
}
// Test that Bluetooth connection is identified correctly.
TEST_F(UmRealShillProviderTest, ReadConnTypeBluetooth) {
+ InitWithDefaultService("/");
SetupConnectionAndTestType(kFakeBluetoothServicePath,
- kFakeBluetoothServiceProxy,
shill::kTypeBluetooth,
ConnectionType::kBluetooth);
}
// Test that Cellular connection is identified correctly.
TEST_F(UmRealShillProviderTest, ReadConnTypeCellular) {
+ InitWithDefaultService("/");
SetupConnectionAndTestType(kFakeCellularServicePath,
- kFakeCellularServiceProxy,
shill::kTypeCellular,
ConnectionType::kCellular);
}
// Test that an unknown connection is identified as such.
TEST_F(UmRealShillProviderTest, ReadConnTypeUnknown) {
+ InitWithDefaultService("/");
SetupConnectionAndTestType(kFakeUnknownServicePath,
- kFakeUnknownServiceProxy,
"FooConnectionType",
ConnectionType::kUnknown);
}
// Tests that VPN connection is identified correctly.
TEST_F(UmRealShillProviderTest, ReadConnTypeVpn) {
+ InitWithDefaultService("/");
// Mock logic for returning a default service path and its type.
- EXPECT_CALL(mock_dbus_, ProxyNewForName(
- kFakeConnection, StrEq(shill::kFlimflamServiceName),
- StrEq(kFakeVpnServicePath), StrEq(shill::kFlimflamServiceInterface)))
- .WillOnce(Return(kFakeVpnServiceProxy));
- EXPECT_CALL(mock_dbus_, ProxyUnref(kFakeVpnServiceProxy)).WillOnce(Return());
- pair<const char*, const char*> service_pairs[] = {
- {shill::kTypeProperty, shill::kTypeVPN},
- {shill::kPhysicalTechnologyProperty, shill::kTypeWifi},
- };
- auto service_properties = SetupGetPropertiesOkay(kFakeVpnServiceProxy,
- arraysize(service_pairs),
- service_pairs);
+ SetServiceReply(kFakeVpnServicePath,
+ shill::kTypeVPN,
+ shill::kTypeWifi,
+ shill::kTetheringNotDetectedState);
// Send a signal about a new default service.
- Time conn_change_time = SendDefaultServiceSignal(kFakeVpnServicePath);
+ Time conn_change_time;
+ SendDefaultServiceSignal(kFakeVpnServicePath, &conn_change_time);
// Query the connection type, ensure last change time reported correctly.
UmTestUtils::ExpectVariableHasValue(ConnectionType::kWifi,
provider_->var_conn_type());
UmTestUtils::ExpectVariableHasValue(conn_change_time,
provider_->var_conn_last_changed());
-
- // Release properties hash tables.
- g_hash_table_unref(service_properties);
}
// Ensure that the connection type is properly cached in the provider through
// subsequent variable readings.
TEST_F(UmRealShillProviderTest, ConnTypeCacheUsed) {
+ InitWithDefaultService("/");
SetupConnectionAndTestType(kFakeEthernetServicePath,
- kFakeEthernetServiceProxy,
shill::kTypeEthernet,
ConnectionType::kEthernet);
@@ -403,12 +346,12 @@
// Ensure that the cached connection type remains valid even when a default
// connection signal occurs but the connection is not changed.
TEST_F(UmRealShillProviderTest, ConnTypeCacheRemainsValid) {
+ InitWithDefaultService("/");
SetupConnectionAndTestType(kFakeEthernetServicePath,
- kFakeEthernetServiceProxy,
shill::kTypeEthernet,
ConnectionType::kEthernet);
- SendDefaultServiceSignal(kFakeEthernetServicePath);
+ SendDefaultServiceSignal(kFakeEthernetServicePath, nullptr);
UmTestUtils::ExpectVariableHasValue(ConnectionType::kEthernet,
provider_->var_conn_type());
@@ -417,53 +360,52 @@
// Ensure that the cached connection type is invalidated and re-read when the
// default connection changes.
TEST_F(UmRealShillProviderTest, ConnTypeCacheInvalidated) {
+ InitWithDefaultService("/");
SetupConnectionAndTestType(kFakeEthernetServicePath,
- kFakeEthernetServiceProxy,
shill::kTypeEthernet,
ConnectionType::kEthernet);
SetupConnectionAndTestType(kFakeWifiServicePath,
- kFakeWifiServiceProxy,
shill::kTypeWifi,
ConnectionType::kWifi);
}
// Test that a non-tethering mode is identified correctly.
TEST_F(UmRealShillProviderTest, ReadConnTetheringNotDetected) {
+ InitWithDefaultService("/");
SetupConnectionAndTestTethering(kFakeWifiServicePath,
- kFakeWifiServiceProxy,
shill::kTetheringNotDetectedState,
ConnectionTethering::kNotDetected);
}
// Test that a suspected tethering mode is identified correctly.
TEST_F(UmRealShillProviderTest, ReadConnTetheringSuspected) {
+ InitWithDefaultService("/");
SetupConnectionAndTestTethering(kFakeWifiServicePath,
- kFakeWifiServiceProxy,
shill::kTetheringSuspectedState,
ConnectionTethering::kSuspected);
}
// Test that a confirmed tethering mode is identified correctly.
TEST_F(UmRealShillProviderTest, ReadConnTetheringConfirmed) {
+ InitWithDefaultService("/");
SetupConnectionAndTestTethering(kFakeWifiServicePath,
- kFakeWifiServiceProxy,
shill::kTetheringConfirmedState,
ConnectionTethering::kConfirmed);
}
// Test that an unknown tethering mode is identified as such.
TEST_F(UmRealShillProviderTest, ReadConnTetheringUnknown) {
+ InitWithDefaultService("/");
SetupConnectionAndTestTethering(kFakeWifiServicePath,
- kFakeWifiServiceProxy,
"FooConnTethering",
ConnectionTethering::kUnknown);
}
// Ensure that the connection tethering mode is properly cached in the provider.
TEST_F(UmRealShillProviderTest, ConnTetheringCacheUsed) {
+ InitWithDefaultService("/");
SetupConnectionAndTestTethering(kFakeEthernetServicePath,
- kFakeEthernetServiceProxy,
shill::kTetheringNotDetectedState,
ConnectionTethering::kNotDetected);
@@ -474,12 +416,12 @@
// Ensure that the cached connection tethering mode remains valid even when a
// default connection signal occurs but the connection is not changed.
TEST_F(UmRealShillProviderTest, ConnTetheringCacheRemainsValid) {
+ InitWithDefaultService("/");
SetupConnectionAndTestTethering(kFakeEthernetServicePath,
- kFakeEthernetServiceProxy,
shill::kTetheringNotDetectedState,
ConnectionTethering::kNotDetected);
- SendDefaultServiceSignal(kFakeEthernetServicePath);
+ SendDefaultServiceSignal(kFakeEthernetServicePath, nullptr);
UmTestUtils::ExpectVariableHasValue(ConnectionTethering::kNotDetected,
provider_->var_conn_tethering());
@@ -488,13 +430,12 @@
// Ensure that the cached connection tethering mode is invalidated and re-read
// when the default connection changes.
TEST_F(UmRealShillProviderTest, ConnTetheringCacheInvalidated) {
+ InitWithDefaultService("/");
SetupConnectionAndTestTethering(kFakeEthernetServicePath,
- kFakeEthernetServiceProxy,
shill::kTetheringNotDetectedState,
ConnectionTethering::kNotDetected);
SetupConnectionAndTestTethering(kFakeWifiServicePath,
- kFakeWifiServiceProxy,
shill::kTetheringConfirmedState,
ConnectionTethering::kConfirmed);
}
@@ -504,14 +445,19 @@
// changed, making sure that it is the time when the first signal was sent (and
// not the second).
TEST_F(UmRealShillProviderTest, ReadLastChangedTimeTwoSignals) {
+ InitWithDefaultService("/");
// Send a default service signal twice, advancing the clock in between.
Time conn_change_time;
- SetupConnectionAndAttrs(kFakeEthernetServicePath, kFakeEthernetServiceProxy,
+ SetupConnectionAndAttrs(kFakeEthernetServicePath,
shill::kTypeEthernet,
- shill::kTetheringNotDetectedState, &conn_change_time);
- SendDefaultServiceSignal(kFakeEthernetServicePath);
+ shill::kTetheringNotDetectedState,
+ &conn_change_time);
+ // This will set the service path to the same value, so it should not call
+ // GetProperties() again.
+ SendDefaultServiceSignal(kFakeEthernetServicePath, nullptr);
- // Query the connection status, ensure last change time reported correctly.
+ // Query the connection status, ensure last change time reported as the first
+ // time the signal was sent.
UmTestUtils::ExpectVariableHasValue(true, provider_->var_is_connected());
UmTestUtils::ExpectVariableHasValue(conn_change_time,
provider_->var_conn_last_changed());
@@ -521,8 +467,10 @@
// responding, that variables can be obtained, and that they all return a null
// value (indicating that the underlying values were not set).
TEST_F(UmRealShillProviderTest, NoInitConnStatusReadBaseValues) {
- // Re-initialize the provider, no initial connection status response.
- Init(false);
+ // Initialize the provider, no initial connection status response.
+ SetManagerReply(nullptr, false);
+ EXPECT_TRUE(provider_->Init());
+ EXPECT_TRUE(loop_.RunOnce(false));
UmTestUtils::ExpectVariableNotSet(provider_->var_is_connected());
UmTestUtils::ExpectVariableNotSet(provider_->var_conn_type());
UmTestUtils::ExpectVariableNotSet(provider_->var_conn_last_changed());
@@ -531,12 +479,16 @@
// Test that, once a signal is received, the connection status and other info
// can be read correctly.
TEST_F(UmRealShillProviderTest, NoInitConnStatusReadConnTypeEthernet) {
- // Re-initialize the provider, no initial connection status response.
- Init(false);
- SetupConnectionAndTestType(kFakeEthernetServicePath,
- kFakeEthernetServiceProxy,
- shill::kTypeEthernet,
- ConnectionType::kEthernet);
+ // Initialize the provider with no initial connection status response.
+ SetManagerReply(nullptr, false);
+ EXPECT_TRUE(provider_->Init());
+ EXPECT_TRUE(loop_.RunOnce(false));
+
+ SetupConnectionAndAttrs(kFakeEthernetServicePath,
+ shill::kTypeEthernet,
+ shill::kTetheringNotDetectedState,
+ nullptr);
+ UmTestUtils::ExpectVariableHasValue(true, provider_->var_is_connected());
}
} // namespace chromeos_update_manager
diff --git a/update_manager/state_factory.cc b/update_manager/state_factory.cc
index 54c0205..acefff4 100644
--- a/update_manager/state_factory.cc
+++ b/update_manager/state_factory.cc
@@ -22,17 +22,19 @@
namespace chromeos_update_manager {
-State* DefaultStateFactory(policy::PolicyProvider* policy_provider,
- chromeos_update_engine::DBusWrapperInterface* dbus,
- chromeos_update_engine::SystemState* system_state) {
+State* DefaultStateFactory(
+ policy::PolicyProvider* policy_provider,
+ chromeos_update_engine::ShillProxy* shill_proxy,
+ org::chromium::SessionManagerInterfaceProxyInterface* session_manager_proxy,
+ chromeos_update_engine::SystemState* system_state) {
chromeos_update_engine::ClockInterface* const clock = system_state->clock();
unique_ptr<RealConfigProvider> config_provider(
new RealConfigProvider(system_state->hardware()));
unique_ptr<RealDevicePolicyProvider> device_policy_provider(
- new RealDevicePolicyProvider(dbus, policy_provider));
+ new RealDevicePolicyProvider(session_manager_proxy, policy_provider));
unique_ptr<RealRandomProvider> random_provider(new RealRandomProvider());
unique_ptr<RealShillProvider> shill_provider(
- new RealShillProvider(dbus, clock));
+ new RealShillProvider(shill_proxy, clock));
unique_ptr<RealSystemProvider> system_provider(
new RealSystemProvider(system_state->hardware()));
unique_ptr<RealTimeProvider> time_provider(new RealTimeProvider(clock));
diff --git a/update_manager/state_factory.h b/update_manager/state_factory.h
index fa1164c..fd37ea7 100644
--- a/update_manager/state_factory.h
+++ b/update_manager/state_factory.h
@@ -5,7 +5,8 @@
#ifndef UPDATE_ENGINE_UPDATE_MANAGER_STATE_FACTORY_H_
#define UPDATE_ENGINE_UPDATE_MANAGER_STATE_FACTORY_H_
-#include "update_engine/dbus_wrapper_interface.h"
+#include "update_engine/dbus_proxies.h"
+#include "update_engine/shill_proxy.h"
#include "update_engine/system_state.h"
#include "update_engine/update_manager/state.h"
@@ -18,7 +19,8 @@
// to initialize.
State* DefaultStateFactory(
policy::PolicyProvider* policy_provider,
- chromeos_update_engine::DBusWrapperInterface* dbus,
+ chromeos_update_engine::ShillProxy* shill_proxy,
+ org::chromium::SessionManagerInterfaceProxyInterface* session_manager_proxy,
chromeos_update_engine::SystemState* system_state);
} // namespace chromeos_update_manager