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/chrome_browser_proxy_resolver.cc b/chrome_browser_proxy_resolver.cc
index 116c67a..932fb9c 100644
--- a/chrome_browser_proxy_resolver.cc
+++ b/chrome_browser_proxy_resolver.cc
@@ -12,11 +12,7 @@
#include <base/bind.h>
#include <base/strings/string_tokenizer.h>
#include <base/strings/string_util.h>
-#include <dbus/dbus-glib-lowlevel.h>
-#include <dbus/dbus-glib.h>
-#include "update_engine/dbus_constants.h"
-#include "update_engine/glib_utils.h"
#include "update_engine/utils.h"
namespace chromeos_update_engine {
@@ -29,22 +25,10 @@
using std::pair;
using std::string;
-#define LIB_CROS_PROXY_RESOLVE_NAME "ProxyResolved"
-#define LIB_CROS_PROXY_RESOLVE_SIGNAL_INTERFACE \
- "org.chromium.UpdateEngineLibcrosProxyResolvedInterface"
const char kLibCrosServiceName[] = "org.chromium.LibCrosService";
-const char kLibCrosServicePath[] = "/org/chromium/LibCrosService";
-const char kLibCrosServiceInterface[] = "org.chromium.LibCrosServiceInterface";
-const char kLibCrosServiceResolveNetworkProxyMethodName[] =
- "ResolveNetworkProxy";
-const char kLibCrosProxyResolveName[] = LIB_CROS_PROXY_RESOLVE_NAME;
+const char kLibCrosProxyResolveName[] = "ProxyResolved";
const char kLibCrosProxyResolveSignalInterface[] =
- LIB_CROS_PROXY_RESOLVE_SIGNAL_INTERFACE;
-const char kLibCrosProxyResolveSignalFilter[] = "type='signal', "
- "interface='" LIB_CROS_PROXY_RESOLVE_SIGNAL_INTERFACE "', "
- "member='" LIB_CROS_PROXY_RESOLVE_NAME "'";
-#undef LIB_CROS_PROXY_RESOLVE_SIGNAL_INTERFACE
-#undef LIB_CROS_PROXY_RESOLVE_NAME
+ "org.chromium.UpdateEngineLibcrosProxyResolvedInterface";
namespace {
@@ -53,59 +37,21 @@
} // namespace
ChromeBrowserProxyResolver::ChromeBrowserProxyResolver(
- DBusWrapperInterface* dbus)
- : dbus_(dbus), timeout_(kTimeout) {}
+ LibCrosProxy* libcros_proxy)
+ : libcros_proxy_(libcros_proxy), timeout_(kTimeout) {}
bool ChromeBrowserProxyResolver::Init() {
- if (proxy_)
- return true; // Already initialized.
-
- // Set up signal handler. Code lifted from libcros.
- GError* g_error = nullptr;
- DBusGConnection* bus = dbus_->BusGet(DBUS_BUS_SYSTEM, &g_error);
- TEST_AND_RETURN_FALSE(bus);
- DBusConnection* connection = dbus_->ConnectionGetConnection(bus);
- TEST_AND_RETURN_FALSE(connection);
-
- DBusError dbus_error;
- dbus_error_init(&dbus_error);
- dbus_->DBusBusAddMatch(connection, kLibCrosProxyResolveSignalFilter,
- &dbus_error);
- TEST_AND_RETURN_FALSE(!dbus_error_is_set(&dbus_error));
- TEST_AND_RETURN_FALSE(dbus_->DBusConnectionAddFilter(
- connection,
- &ChromeBrowserProxyResolver::StaticFilterMessage,
- this,
- nullptr));
-
- proxy_ = dbus_->ProxyNewForName(bus, kLibCrosServiceName, kLibCrosServicePath,
- kLibCrosServiceInterface);
- if (!proxy_) {
- dbus_->DBusConnectionRemoveFilter(
- connection,
- &ChromeBrowserProxyResolver::StaticFilterMessage,
- this);
- }
- TEST_AND_RETURN_FALSE(proxy_); // For the error log
+ libcros_proxy_->ue_proxy_resolved_interface()
+ ->RegisterProxyResolvedSignalHandler(
+ base::Bind(&ChromeBrowserProxyResolver::OnProxyResolvedSignal,
+ base::Unretained(this)),
+ base::Bind(&ChromeBrowserProxyResolver::OnSignalConnected,
+ base::Unretained(this)));
return true;
}
ChromeBrowserProxyResolver::~ChromeBrowserProxyResolver() {
- // Remove DBus connection filters and Kill proxy object.
- if (proxy_) {
- GError* gerror = nullptr;
- DBusGConnection* gbus = dbus_->BusGet(DBUS_BUS_SYSTEM, &gerror);
- if (gbus) {
- DBusConnection* connection = dbus_->ConnectionGetConnection(gbus);
- dbus_->DBusConnectionRemoveFilter(
- connection,
- &ChromeBrowserProxyResolver::StaticFilterMessage,
- this);
- }
- dbus_->ProxyUnref(proxy_);
- }
-
- // Kill outstanding timers
+ // Kill outstanding timers.
for (auto& timer : timers_) {
MessageLoop::current()->CancelTask(timer.second);
timer.second = MessageLoop::kTaskIdNull;
@@ -115,26 +61,14 @@
bool ChromeBrowserProxyResolver::GetProxiesForUrl(const string& url,
ProxiesResolvedFn callback,
void* data) {
- GError* error = nullptr;
- guint timeout = timeout_;
- if (proxy_) {
- if (!dbus_->ProxyCall_3_0(proxy_,
- kLibCrosServiceResolveNetworkProxyMethodName,
- &error,
- url.c_str(),
- kLibCrosProxyResolveSignalInterface,
- kLibCrosProxyResolveName)) {
- if (error) {
- LOG(WARNING) << "dbus_g_proxy_call failed, continuing with no proxy: "
- << utils::GetAndFreeGError(&error);
- } else {
- LOG(WARNING) << "dbus_g_proxy_call failed with no error string, "
- "continuing with no proxy.";
- }
- timeout = 0;
- }
- } else {
- LOG(WARNING) << "dbus proxy object missing, continuing with no proxy.";
+ int timeout = timeout_;
+ chromeos::ErrorPtr error;
+ if (!libcros_proxy_->service_interface_proxy()->ResolveNetworkProxy(
+ url.c_str(),
+ kLibCrosProxyResolveSignalInterface,
+ kLibCrosProxyResolveName,
+ &error)) {
+ LOG(WARNING) << "Can't resolve the proxy. Continuing with no proxy.";
timeout = 0;
}
@@ -149,36 +83,6 @@
return true;
}
-DBusHandlerResult ChromeBrowserProxyResolver::FilterMessage(
- DBusConnection* connection,
- DBusMessage* message) {
- // Code lifted from libcros.
- if (!dbus_->DBusMessageIsSignal(message,
- kLibCrosProxyResolveSignalInterface,
- kLibCrosProxyResolveName)) {
- return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
- }
- // Get args
- char* source_url = nullptr;
- char* proxy_list = nullptr;
- char* error = nullptr;
- DBusError arg_error;
- dbus_error_init(&arg_error);
- if (!dbus_->DBusMessageGetArgs_3(message, &arg_error,
- &source_url,
- &proxy_list,
- &error)) {
- LOG(ERROR) << "Error reading dbus signal.";
- return DBUS_HANDLER_RESULT_HANDLED;
- }
- if (!source_url || !proxy_list) {
- LOG(ERROR) << "Error getting url, proxy list from dbus signal.";
- return DBUS_HANDLER_RESULT_HANDLED;
- }
- HandleReply(source_url, proxy_list);
- return DBUS_HANDLER_RESULT_HANDLED;
-}
-
bool ChromeBrowserProxyResolver::DeleteUrlState(
const string& source_url,
bool delete_timer,
@@ -202,11 +106,25 @@
return true;
}
-void ChromeBrowserProxyResolver::HandleReply(const string& source_url,
- const string& proxy_list) {
+void ChromeBrowserProxyResolver::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;
+ }
+}
+
+void ChromeBrowserProxyResolver::OnProxyResolvedSignal(
+ const string& source_url,
+ const string& proxy_info,
+ const string& error_message) {
pair<ProxiesResolvedFn, void*> callback;
TEST_AND_RETURN(DeleteUrlState(source_url, true, &callback));
- (*callback.first)(ParseProxyString(proxy_list), callback.second);
+ if (!error_message.empty()) {
+ LOG(WARNING) << "ProxyResolved error: " << error_message;
+ }
+ (*callback.first)(ParseProxyString(proxy_info), callback.second);
}
void ChromeBrowserProxyResolver::HandleTimeout(string source_url) {
diff --git a/chrome_browser_proxy_resolver.h b/chrome_browser_proxy_resolver.h
index ab0a92c..aef87ee 100644
--- a/chrome_browser_proxy_resolver.h
+++ b/chrome_browser_proxy_resolver.h
@@ -10,44 +10,31 @@
#include <string>
#include <utility>
-#include <dbus/dbus-glib.h>
-#include <dbus/dbus-glib-lowlevel.h>
#include <gtest/gtest_prod.h> // for FRIEND_TEST
#include <chromeos/message_loops/message_loop.h>
-#include "update_engine/dbus_wrapper_interface.h"
+#include "update_engine/dbus_proxies.h"
+#include "update_engine/libcros_proxy.h"
#include "update_engine/proxy_resolver.h"
namespace chromeos_update_engine {
extern const char kLibCrosServiceName[];
-extern const char kLibCrosServicePath[];
-extern const char kLibCrosServiceInterface[];
-extern const char kLibCrosServiceResolveNetworkProxyMethodName[];
extern const char kLibCrosProxyResolveName[];
extern const char kLibCrosProxyResolveSignalInterface[];
-extern const char kLibCrosProxyResolveSignalFilter[];
class ChromeBrowserProxyResolver : public ProxyResolver {
public:
- explicit ChromeBrowserProxyResolver(DBusWrapperInterface* dbus);
+ explicit ChromeBrowserProxyResolver(LibCrosProxy* libcros_proxy);
~ChromeBrowserProxyResolver() override;
+
+ // Initialize the ProxyResolver using the provided DBus proxies.
bool Init();
bool GetProxiesForUrl(const std::string& url,
ProxiesResolvedFn callback,
void* data) override;
- void set_timeout(int seconds) { timeout_ = seconds; }
-
- // Public for test
- static DBusHandlerResult StaticFilterMessage(
- DBusConnection* connection,
- DBusMessage* message,
- void* data) {
- return reinterpret_cast<ChromeBrowserProxyResolver*>(data)->FilterMessage(
- connection, message);
- }
private:
FRIEND_TEST(ChromeBrowserProxyResolverTest, ParseTest);
@@ -56,12 +43,17 @@
CallbacksMap;
typedef std::multimap<std::string, chromeos::MessageLoop::TaskId> TimeoutsMap;
+ // Called when the signal in UpdateEngineLibcrosProxyResolvedInterface is
+ // connected.
+ void OnSignalConnected(const std::string& interface_name,
+ const std::string& signal_name,
+ bool successful);
+
// Handle a reply from Chrome:
- void HandleReply(const std::string& source_url,
- const std::string& proxy_list);
- DBusHandlerResult FilterMessage(
- DBusConnection* connection,
- DBusMessage* message);
+ void OnProxyResolvedSignal(const std::string& source_url,
+ const std::string& proxy_info,
+ const std::string& error_message);
+
// Handle no reply:
void HandleTimeout(std::string source_url);
@@ -79,9 +71,11 @@
// Shutdown the dbus proxy object.
void Shutdown();
- DBusWrapperInterface* dbus_;
- DBusGProxy* proxy_{nullptr};
- DBusGProxy* peer_proxy_{nullptr};
+ // DBus proxies to request a HTTP proxy resolution. The request is done in the
+ // service_interface_proxy() interface and the response is received as a
+ // signal in the ue_proxy_resolved_interface().
+ LibCrosProxy* libcros_proxy_;
+
int timeout_;
TimeoutsMap timers_;
CallbacksMap callbacks_;
diff --git a/chrome_browser_proxy_resolver_unittest.cc b/chrome_browser_proxy_resolver_unittest.cc
index 3866358..62ee2e0 100644
--- a/chrome_browser_proxy_resolver_unittest.cc
+++ b/chrome_browser_proxy_resolver_unittest.cc
@@ -6,81 +6,171 @@
#include <deque>
#include <string>
+#include <vector>
#include <gtest/gtest.h>
#include <base/bind.h>
+#include <chromeos/make_unique_ptr.h>
#include <chromeos/message_loops/fake_message_loop.h>
-#include "update_engine/mock_dbus_wrapper.h"
+#include "update_engine/dbus_mocks.h"
+#include "update_engine/dbus_test_utils.h"
using ::testing::Return;
-using ::testing::SetArgPointee;
using ::testing::StrEq;
using ::testing::_;
using chromeos::MessageLoop;
+using org::chromium::LibCrosServiceInterfaceProxyMock;
+using org::chromium::UpdateEngineLibcrosProxyResolvedInterfaceProxyMock;
using std::deque;
using std::string;
+using std::vector;
namespace chromeos_update_engine {
class ChromeBrowserProxyResolverTest : public ::testing::Test {
protected:
+ ChromeBrowserProxyResolverTest()
+ : service_interface_mock_(new LibCrosServiceInterfaceProxyMock()),
+ ue_proxy_resolved_interface_mock_(
+ new UpdateEngineLibcrosProxyResolvedInterfaceProxyMock()),
+ libcros_proxy_(
+ chromeos::make_unique_ptr(service_interface_mock_),
+ chromeos::make_unique_ptr(ue_proxy_resolved_interface_mock_)) {}
+
void SetUp() override {
loop_.SetAsCurrent();
+ // The ProxyResolved signal should be subscribed to.
+ MOCK_SIGNAL_HANDLER_EXPECT_SIGNAL_HANDLER(
+ ue_proxy_resolved_signal_,
+ *ue_proxy_resolved_interface_mock_,
+ ProxyResolved);
+
+ EXPECT_TRUE(resolver_.Init());
+ // Run the loop once to dispatch the successfully registered signal handler.
+ EXPECT_TRUE(loop_.RunOnce(false));
}
void TearDown() override {
EXPECT_FALSE(loop_.PendingTasks());
}
+ // Send the signal to the callback passed during registration of the
+ // ProxyResolved.
+ void SendReplySignal(const string& source_url,
+ const string& proxy_info,
+ const string& error_message);
+
+ void RunTest(bool chrome_replies, bool chrome_alive);
+
private:
chromeos::FakeMessageLoop loop_{nullptr};
+
+ // Local pointers to the mocks. The instances are owned by the
+ // |libcros_proxy_|.
+ LibCrosServiceInterfaceProxyMock* service_interface_mock_;
+ UpdateEngineLibcrosProxyResolvedInterfaceProxyMock*
+ ue_proxy_resolved_interface_mock_;
+
+ // The registered signal handler for the signal
+ // UpdateEngineLibcrosProxyResolvedInterface.ProxyResolved.
+ chromeos_update_engine::dbus_test_utils::MockSignalHandler<
+ void(const string&, const string&, const string&)>
+ ue_proxy_resolved_signal_;
+
+ LibCrosProxy libcros_proxy_;
+ ChromeBrowserProxyResolver resolver_{&libcros_proxy_};
};
+
+void ChromeBrowserProxyResolverTest::SendReplySignal(
+ const string& source_url,
+ const string& proxy_info,
+ const string& error_message) {
+ ASSERT_TRUE(ue_proxy_resolved_signal_.IsHandlerRegistered());
+ ue_proxy_resolved_signal_.signal_callback().Run(
+ source_url, proxy_info, error_message);
+}
+
+namespace {
+void CheckResponseResolved(const deque<string>& proxies,
+ void* /* pirv_data */) {
+ EXPECT_EQ(2, proxies.size());
+ EXPECT_EQ("socks5://192.168.52.83:5555", proxies[0]);
+ EXPECT_EQ(kNoProxy, proxies[1]);
+ MessageLoop::current()->BreakLoop();
+}
+
+void CheckResponseNoReply(const deque<string>& proxies, void* /* pirv_data */) {
+ EXPECT_EQ(1, proxies.size());
+ EXPECT_EQ(kNoProxy, proxies[0]);
+ MessageLoop::current()->BreakLoop();
+}
+} // namespace
+
+// chrome_replies should be set to whether or not we fake a reply from
+// chrome. If there's no reply, the resolver should time out.
+// If chrome_alive is false, assume that sending to chrome fails.
+void ChromeBrowserProxyResolverTest::RunTest(bool chrome_replies,
+ bool chrome_alive) {
+ char kUrl[] = "http://example.com/blah";
+ char kProxyConfig[] = "SOCKS5 192.168.52.83:5555;DIRECT";
+
+ EXPECT_CALL(*service_interface_mock_,
+ ResolveNetworkProxy(StrEq(kUrl),
+ StrEq(kLibCrosProxyResolveSignalInterface),
+ StrEq(kLibCrosProxyResolveName),
+ _,
+ _))
+ .WillOnce(Return(chrome_alive));
+
+ ProxiesResolvedFn get_proxies_response = &CheckResponseNoReply;
+ if (chrome_replies) {
+ get_proxies_response = &CheckResponseResolved;
+ MessageLoop::current()->PostDelayedTask(
+ FROM_HERE,
+ base::Bind(&ChromeBrowserProxyResolverTest::SendReplySignal,
+ base::Unretained(this),
+ kUrl,
+ kProxyConfig,
+ ""),
+ base::TimeDelta::FromSeconds(1));
+ }
+
+ EXPECT_TRUE(resolver_.GetProxiesForUrl(kUrl, get_proxies_response, nullptr));
+ MessageLoop::current()->Run();
+}
+
+
TEST_F(ChromeBrowserProxyResolverTest, ParseTest) {
// Test ideas from
// http://src.chromium.org/svn/trunk/src/net/proxy/proxy_list_unittest.cc
- const char* inputs[] = {
- "PROXY foopy:10",
- " DIRECT", // leading space.
- "PROXY foopy1 ; proxy foopy2;\t DIRECT",
- "proxy foopy1 ; SOCKS foopy2",
- "DIRECT ; proxy foopy1 ; DIRECT ; SOCKS5 foopy2;DIRECT ",
- "DIRECT ; proxy foopy1:80; DIRECT ; DIRECT",
- "PROXY-foopy:10",
- "PROXY",
- "PROXY foopy1 ; JUNK ; JUNK ; SOCKS5 foopy2 ; ;",
- "HTTP foopy1; SOCKS5 foopy2"
- };
- deque<string> outputs[arraysize(inputs)];
- outputs[0].push_back("http://foopy:10");
- outputs[0].push_back(kNoProxy);
- outputs[1].push_back(kNoProxy);
- outputs[2].push_back("http://foopy1");
- outputs[2].push_back("http://foopy2");
- outputs[2].push_back(kNoProxy);
- outputs[3].push_back("http://foopy1");
- outputs[3].push_back("socks4://foopy2");
- outputs[3].push_back(kNoProxy);
- outputs[4].push_back(kNoProxy);
- outputs[4].push_back("http://foopy1");
- outputs[4].push_back(kNoProxy);
- outputs[4].push_back("socks5://foopy2");
- outputs[4].push_back(kNoProxy);
- outputs[5].push_back(kNoProxy);
- outputs[5].push_back("http://foopy1:80");
- outputs[5].push_back(kNoProxy);
- outputs[5].push_back(kNoProxy);
- outputs[6].push_back(kNoProxy);
- outputs[7].push_back(kNoProxy);
- outputs[8].push_back("http://foopy1");
- outputs[8].push_back("socks5://foopy2");
- outputs[8].push_back(kNoProxy);
- outputs[9].push_back("socks5://foopy2");
- outputs[9].push_back(kNoProxy);
+ vector<string> inputs = {
+ "PROXY foopy:10",
+ " DIRECT", // leading space.
+ "PROXY foopy1 ; proxy foopy2;\t DIRECT",
+ "proxy foopy1 ; SOCKS foopy2",
+ "DIRECT ; proxy foopy1 ; DIRECT ; SOCKS5 foopy2;DIRECT ",
+ "DIRECT ; proxy foopy1:80; DIRECT ; DIRECT",
+ "PROXY-foopy:10",
+ "PROXY",
+ "PROXY foopy1 ; JUNK ; JUNK ; SOCKS5 foopy2 ; ;",
+ "HTTP foopy1; SOCKS5 foopy2"};
+ vector<deque<string>> outputs = {
+ {"http://foopy:10", kNoProxy},
+ {kNoProxy},
+ {"http://foopy1", "http://foopy2", kNoProxy},
+ {"http://foopy1", "socks4://foopy2", kNoProxy},
+ {kNoProxy, "http://foopy1", kNoProxy, "socks5://foopy2", kNoProxy},
+ {kNoProxy, "http://foopy1:80", kNoProxy, kNoProxy},
+ {kNoProxy},
+ {kNoProxy},
+ {"http://foopy1", "socks5://foopy2", kNoProxy},
+ {"socks5://foopy2", kNoProxy}};
+ ASSERT_EQ(inputs.size(), outputs.size());
- for (size_t i = 0; i < arraysize(inputs); i++) {
+ for (size_t i = 0; i < inputs.size(); i++) {
deque<string> results =
ChromeBrowserProxyResolver::ParseProxyString(inputs[i]);
deque<string>& expected = outputs[i];
@@ -93,110 +183,6 @@
}
}
-namespace {
-void DBusWrapperTestResolved(const deque<string>& proxies,
- void* /* pirv_data */) {
- EXPECT_EQ(2, proxies.size());
- EXPECT_EQ("socks5://192.168.52.83:5555", proxies[0]);
- EXPECT_EQ(kNoProxy, proxies[1]);
- MessageLoop::current()->BreakLoop();
-}
-void DBusWrapperTestResolvedNoReply(const deque<string>& proxies,
- void* /* pirv_data */) {
- EXPECT_EQ(1, proxies.size());
- EXPECT_EQ(kNoProxy, proxies[0]);
- MessageLoop::current()->BreakLoop();
-}
-
-void SendReply(DBusConnection* connection,
- DBusMessage* message,
- ChromeBrowserProxyResolver* resolver) {
- LOG(INFO) << "Calling SendReply";
- ChromeBrowserProxyResolver::StaticFilterMessage(connection,
- message,
- resolver);
-}
-
-// chrome_replies should be set to whether or not we fake a reply from
-// chrome. If there's no reply, the resolver should time out.
-// If chrome_alive is false, assume that sending to chrome fails.
-void RunTest(bool chrome_replies, bool chrome_alive) {
- intptr_t number = 1;
- DBusGConnection* kMockSystemGBus =
- reinterpret_cast<DBusGConnection*>(number++);
- DBusConnection* kMockSystemBus =
- reinterpret_cast<DBusConnection*>(number++);
- DBusGProxy* kMockDbusProxy =
- reinterpret_cast<DBusGProxy*>(number++);
- DBusMessage* kMockDbusMessage =
- reinterpret_cast<DBusMessage*>(number++);
-
- char kUrl[] = "http://example.com/blah";
- char kProxyConfig[] = "SOCKS5 192.168.52.83:5555;DIRECT";
-
- testing::StrictMock<MockDBusWrapper> dbus_iface;
-
- EXPECT_CALL(dbus_iface, BusGet(_, _))
- .Times(2)
- .WillRepeatedly(Return(kMockSystemGBus));
- EXPECT_CALL(dbus_iface,
- ConnectionGetConnection(kMockSystemGBus))
- .Times(2)
- .WillRepeatedly(Return(kMockSystemBus));
- EXPECT_CALL(dbus_iface, DBusBusAddMatch(kMockSystemBus, _, _));
- EXPECT_CALL(dbus_iface,
- DBusConnectionAddFilter(kMockSystemBus, _, _, _))
- .WillOnce(Return(1));
- EXPECT_CALL(dbus_iface,
- ProxyNewForName(kMockSystemGBus,
- StrEq(kLibCrosServiceName),
- StrEq(kLibCrosServicePath),
- StrEq(kLibCrosServiceInterface)))
- .WillOnce(Return(kMockDbusProxy));
- EXPECT_CALL(dbus_iface, ProxyUnref(kMockDbusProxy));
-
- EXPECT_CALL(dbus_iface, ProxyCall_3_0(
- kMockDbusProxy,
- StrEq(kLibCrosServiceResolveNetworkProxyMethodName),
- _,
- StrEq(kUrl),
- StrEq(kLibCrosProxyResolveSignalInterface),
- StrEq(kLibCrosProxyResolveName)))
- .WillOnce(Return(chrome_alive ? TRUE : FALSE));
-
- EXPECT_CALL(dbus_iface,
- DBusConnectionRemoveFilter(kMockSystemBus, _, _));
-
- if (chrome_replies) {
- EXPECT_CALL(dbus_iface,
- DBusMessageIsSignal(kMockDbusMessage,
- kLibCrosProxyResolveSignalInterface,
- kLibCrosProxyResolveName))
- .WillOnce(Return(1));
- EXPECT_CALL(dbus_iface,
- DBusMessageGetArgs_3(kMockDbusMessage, _, _, _, _))
- .WillOnce(DoAll(SetArgPointee<2>(static_cast<char*>(kUrl)),
- SetArgPointee<3>(static_cast<char*>(kProxyConfig)),
- Return(TRUE)));
- }
-
- ChromeBrowserProxyResolver resolver(&dbus_iface);
- EXPECT_EQ(true, resolver.Init());
- resolver.set_timeout(1);
- ProxiesResolvedFn get_proxies_response = &DBusWrapperTestResolvedNoReply;
-
- if (chrome_replies) {
- get_proxies_response = &DBusWrapperTestResolved;
- MessageLoop::current()->PostTask(
- FROM_HERE,
- base::Bind(&SendReply, kMockSystemBus, kMockDbusMessage, &resolver));
- }
-
- EXPECT_TRUE(resolver.GetProxiesForUrl(kUrl, get_proxies_response, nullptr));
- MessageLoop::current()->Run();
-}
-} // namespace
-
TEST_F(ChromeBrowserProxyResolverTest, SuccessTest) {
RunTest(true, true);
}
diff --git a/connection_manager.cc b/connection_manager.cc
index 45b6467..d0d3392 100644
--- a/connection_manager.cc
+++ b/connection_manager.cc
@@ -10,7 +10,6 @@
#include <base/stl_util.h>
#include <base/strings/string_util.h>
#include <chromeos/dbus/service_constants.h>
-#include <dbus/dbus-glib.h>
#include <glib.h>
#include <policy/device_policy.h>
@@ -18,6 +17,8 @@
#include "update_engine/system_state.h"
#include "update_engine/utils.h"
+using org::chromium::flimflam::ManagerProxyInterface;
+using org::chromium::flimflam::ServiceProxyInterface;
using std::set;
using std::string;
@@ -25,163 +26,38 @@
namespace {
-// Gets the DbusGProxy for FlimFlam. Must be free'd with ProxyUnref()
-bool GetFlimFlamProxy(DBusWrapperInterface* dbus_iface,
- const char* path,
- const char* interface,
- DBusGProxy** out_proxy) {
- DBusGConnection* bus;
- DBusGProxy* proxy;
- GError* error = nullptr;
-
- bus = dbus_iface->BusGet(DBUS_BUS_SYSTEM, &error);
- if (!bus) {
- LOG(ERROR) << "Failed to get system bus";
- return false;
- }
- proxy = dbus_iface->ProxyNewForName(bus, shill::kFlimflamServiceName, path,
- interface);
- *out_proxy = proxy;
- return true;
-}
-
-// On success, caller owns the GHashTable at out_hash_table.
-// Returns true on success.
-bool GetProperties(DBusWrapperInterface* dbus_iface,
- const char* path,
- const char* interface,
- GHashTable** out_hash_table) {
- DBusGProxy* proxy;
- GError* error = nullptr;
-
- TEST_AND_RETURN_FALSE(GetFlimFlamProxy(dbus_iface,
- path,
- interface,
- &proxy));
-
- gboolean rc = dbus_iface->ProxyCall_0_1(proxy,
- "GetProperties",
- &error,
- out_hash_table);
- dbus_iface->ProxyUnref(proxy);
- if (rc == FALSE) {
- LOG(ERROR) << "dbus_g_proxy_call failed";
- return false;
- }
-
- return true;
-}
-
-// Returns (via out_path) the default network path, or empty string if
-// there's no network up.
-// Returns true on success.
-bool GetDefaultServicePath(DBusWrapperInterface* dbus_iface, string* out_path) {
- GHashTable* hash_table = nullptr;
-
- TEST_AND_RETURN_FALSE(GetProperties(dbus_iface,
- shill::kFlimflamServicePath,
- shill::kFlimflamManagerInterface,
- &hash_table));
-
- GValue* value = reinterpret_cast<GValue*>(g_hash_table_lookup(hash_table,
- "Services"));
- GPtrArray* array = nullptr;
- bool success = false;
- if (G_VALUE_HOLDS(value, DBUS_TYPE_G_OBJECT_PATH_ARRAY) &&
- (array = reinterpret_cast<GPtrArray*>(g_value_get_boxed(value))) &&
- (array->len > 0)) {
- *out_path = static_cast<const char*>(g_ptr_array_index(array, 0));
- success = true;
- }
-
- g_hash_table_unref(hash_table);
- return success;
-}
-
-NetworkConnectionType ParseConnectionType(const char* type_str) {
- if (!strcmp(type_str, shill::kTypeEthernet)) {
+NetworkConnectionType ParseConnectionType(const string& type_str) {
+ if (type_str == shill::kTypeEthernet) {
return NetworkConnectionType::kEthernet;
- } else if (!strcmp(type_str, shill::kTypeWifi)) {
+ } else if (type_str == shill::kTypeWifi) {
return NetworkConnectionType::kWifi;
- } else if (!strcmp(type_str, shill::kTypeWimax)) {
+ } else if (type_str == shill::kTypeWimax) {
return NetworkConnectionType::kWimax;
- } else if (!strcmp(type_str, shill::kTypeBluetooth)) {
+ } else if (type_str == shill::kTypeBluetooth) {
return NetworkConnectionType::kBluetooth;
- } else if (!strcmp(type_str, shill::kTypeCellular)) {
+ } else if (type_str == shill::kTypeCellular) {
return NetworkConnectionType::kCellular;
}
return NetworkConnectionType::kUnknown;
}
-NetworkTethering ParseTethering(const char* tethering_str) {
- if (!strcmp(tethering_str, shill::kTetheringNotDetectedState)) {
+NetworkTethering ParseTethering(const string& tethering_str) {
+ if (tethering_str == shill::kTetheringNotDetectedState) {
return NetworkTethering::kNotDetected;
- } else if (!strcmp(tethering_str, shill::kTetheringSuspectedState)) {
+ } else if (tethering_str == shill::kTetheringSuspectedState) {
return NetworkTethering::kSuspected;
- } else if (!strcmp(tethering_str, shill::kTetheringConfirmedState)) {
+ } else if (tethering_str == shill::kTetheringConfirmedState) {
return NetworkTethering::kConfirmed;
}
LOG(WARNING) << "Unknown Tethering value: " << tethering_str;
return NetworkTethering::kUnknown;
}
-bool GetServicePathProperties(DBusWrapperInterface* dbus_iface,
- const string& path,
- NetworkConnectionType* out_type,
- NetworkTethering* out_tethering) {
- GHashTable* hash_table = nullptr;
-
- TEST_AND_RETURN_FALSE(GetProperties(dbus_iface,
- path.c_str(),
- shill::kFlimflamServiceInterface,
- &hash_table));
-
- // Populate the out_tethering.
- GValue* value =
- reinterpret_cast<GValue*>(g_hash_table_lookup(hash_table,
- shill::kTetheringProperty));
- const char* tethering_str = nullptr;
-
- if (value != nullptr)
- tethering_str = g_value_get_string(value);
- if (tethering_str != nullptr) {
- *out_tethering = ParseTethering(tethering_str);
- } else {
- // Set to Unknown if not present.
- *out_tethering = NetworkTethering::kUnknown;
- }
-
- // Populate the out_type property.
- value = reinterpret_cast<GValue*>(g_hash_table_lookup(hash_table,
- shill::kTypeProperty));
- const char* type_str = nullptr;
- bool success = false;
- if (value != nullptr && (type_str = g_value_get_string(value)) != nullptr) {
- success = true;
- if (!strcmp(type_str, shill::kTypeVPN)) {
- value = reinterpret_cast<GValue*>(
- g_hash_table_lookup(hash_table, shill::kPhysicalTechnologyProperty));
- if (value != nullptr &&
- (type_str = g_value_get_string(value)) != nullptr) {
- *out_type = ParseConnectionType(type_str);
- } else {
- LOG(ERROR) << "No PhysicalTechnology property found for a VPN"
- << " connection (service: " << path << "). Returning default"
- << " NetworkConnectionType::kUnknown value.";
- *out_type = NetworkConnectionType::kUnknown;
- }
- } else {
- *out_type = ParseConnectionType(type_str);
- }
- }
- g_hash_table_unref(hash_table);
- return success;
-}
-
} // namespace
-ConnectionManager::ConnectionManager(SystemState *system_state)
- : system_state_(system_state) {}
+ConnectionManager::ConnectionManager(ShillProxyInterface* shill_proxy,
+ SystemState* system_state)
+ : shill_proxy_(shill_proxy), system_state_(system_state) {}
bool ConnectionManager::IsUpdateAllowedOver(NetworkConnectionType type,
NetworkTethering tethering) const {
@@ -273,33 +149,81 @@
return "Unknown";
}
-// static
-const char* ConnectionManager::StringForTethering(NetworkTethering tethering) {
- switch (tethering) {
- case NetworkTethering::kNotDetected:
- return shill::kTetheringNotDetectedState;
- case NetworkTethering::kSuspected:
- return shill::kTetheringSuspectedState;
- case NetworkTethering::kConfirmed:
- return shill::kTetheringConfirmedState;
- case NetworkTethering::kUnknown:
- return "Unknown";
- }
- // The program shouldn't reach this point, but the compiler isn't smart
- // enough to infer that.
- return "Unknown";
+bool ConnectionManager::GetConnectionProperties(
+ NetworkConnectionType* out_type,
+ NetworkTethering* out_tethering) {
+ string default_service_path;
+ TEST_AND_RETURN_FALSE(GetDefaultServicePath(&default_service_path));
+ if (default_service_path.empty())
+ return false;
+ TEST_AND_RETURN_FALSE(
+ GetServicePathProperties(default_service_path, out_type, out_tethering));
+ return true;
}
-bool ConnectionManager::GetConnectionProperties(
- DBusWrapperInterface* dbus_iface,
+bool ConnectionManager::GetDefaultServicePath(string* out_path) {
+ chromeos::VariantDictionary properties;
+ chromeos::ErrorPtr error;
+ ManagerProxyInterface* manager_proxy = shill_proxy_->GetManagerProxy();
+ if (!manager_proxy)
+ return false;
+ TEST_AND_RETURN_FALSE(manager_proxy->GetProperties(&properties, &error));
+
+ const auto& prop_default_service =
+ properties.find(shill::kDefaultServiceProperty);
+ if (prop_default_service == properties.end())
+ return false;
+
+ *out_path = prop_default_service->second.TryGet<dbus::ObjectPath>().value();
+ return !out_path->empty();
+}
+
+bool ConnectionManager::GetServicePathProperties(
+ const string& path,
NetworkConnectionType* out_type,
- NetworkTethering* out_tethering) const {
- string default_service_path;
- TEST_AND_RETURN_FALSE(GetDefaultServicePath(dbus_iface,
- &default_service_path));
- TEST_AND_RETURN_FALSE(GetServicePathProperties(dbus_iface,
- default_service_path,
- out_type, out_tethering));
+ NetworkTethering* out_tethering) {
+ // We create and dispose the ServiceProxyInterface on every request.
+ std::unique_ptr<ServiceProxyInterface> service =
+ shill_proxy_->GetServiceForPath(path);
+
+ chromeos::VariantDictionary properties;
+ chromeos::ErrorPtr error;
+ TEST_AND_RETURN_FALSE(service->GetProperties(&properties, &error));
+
+ // Populate the out_tethering.
+ const auto& prop_tethering = properties.find(shill::kTetheringProperty);
+ if (prop_tethering == properties.end()) {
+ // Set to Unknown if not present.
+ *out_tethering = NetworkTethering::kUnknown;
+ } else {
+ // If the property doesn't contain a string value, the empty string will
+ // become kUnknown.
+ *out_tethering = ParseTethering(prop_tethering->second.TryGet<string>());
+ }
+
+ // Populate the out_type property.
+ const auto& prop_type = properties.find(shill::kTypeProperty);
+ if (prop_type == properties.end()) {
+ // Set to Unknown if not present.
+ *out_type = NetworkConnectionType::kUnknown;
+ return false;
+ }
+
+ 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: " << path << "). Returning default"
+ << " kUnknown value.";
+ *out_type = NetworkConnectionType::kUnknown;
+ } else {
+ *out_type = ParseConnectionType(prop_physical->second.TryGet<string>());
+ }
+ } else {
+ *out_type = ParseConnectionType(type_str);
+ }
return true;
}
diff --git a/connection_manager.h b/connection_manager.h
index 551c4f3..870b088 100644
--- a/connection_manager.h
+++ b/connection_manager.h
@@ -5,10 +5,13 @@
#ifndef UPDATE_ENGINE_CONNECTION_MANAGER_H_
#define UPDATE_ENGINE_CONNECTION_MANAGER_H_
+#include <string>
+
#include <base/macros.h>
#include "update_engine/connection_manager_interface.h"
-#include "update_engine/dbus_wrapper_interface.h"
+#include "update_engine/dbus_proxies.h"
+#include "update_engine/shill_proxy_interface.h"
namespace chromeos_update_engine {
@@ -16,30 +19,38 @@
// This class implements the concrete class that talks with the connection
// manager (shill) over DBus.
+// TODO(deymo): Remove this class and use ShillProvider from the UpdateManager.
class ConnectionManager : public ConnectionManagerInterface {
public:
// Returns the string representation corresponding to the given
// connection type.
static const char* StringForConnectionType(NetworkConnectionType type);
- // Returns the string representation corresponding to the given tethering
- // state.
- static const char* StringForTethering(NetworkTethering tethering);
-
// Constructs a new ConnectionManager object initialized with the
// given system state.
- explicit ConnectionManager(SystemState* system_state);
+ ConnectionManager(ShillProxyInterface* shill_proxy,
+ SystemState* system_state);
~ConnectionManager() override = default;
- // ConnectionManagerInterface overrides
- bool GetConnectionProperties(DBusWrapperInterface* dbus_iface,
- NetworkConnectionType* out_type,
- NetworkTethering* out_tethering) const override;
+ // ConnectionManagerInterface overrides.
+ bool GetConnectionProperties(NetworkConnectionType* out_type,
+ NetworkTethering* out_tethering) override;
bool IsUpdateAllowedOver(NetworkConnectionType type,
NetworkTethering tethering) const override;
private:
- // The global context for update_engine
+ // Returns (via out_path) the default network path, or empty string if
+ // there's no network up. Returns true on success.
+ bool GetDefaultServicePath(std::string* out_path);
+
+ bool GetServicePathProperties(const std::string& path,
+ NetworkConnectionType* out_type,
+ NetworkTethering* out_tethering);
+
+ // The mockable interface to access the shill DBus proxies.
+ ShillProxyInterface* shill_proxy_;
+
+ // The global context for update_engine.
SystemState* system_state_;
DISALLOW_COPY_AND_ASSIGN(ConnectionManager);
diff --git a/connection_manager_interface.h b/connection_manager_interface.h
index aa04946..642f60f 100644
--- a/connection_manager_interface.h
+++ b/connection_manager_interface.h
@@ -7,8 +7,6 @@
#include <base/macros.h>
-#include "update_engine/dbus_wrapper_interface.h"
-
namespace chromeos_update_engine {
enum class NetworkConnectionType {
@@ -37,10 +35,8 @@
// Populates |out_type| with the type of the network connection
// that we are currently connected and |out_tethering| with the estimate of
// whether that network is being tethered.
- virtual bool GetConnectionProperties(
- DBusWrapperInterface* dbus_iface,
- NetworkConnectionType* out_type,
- NetworkTethering* out_tethering) const = 0;
+ virtual bool GetConnectionProperties(NetworkConnectionType* out_type,
+ NetworkTethering* out_tethering) = 0;
// Returns true if we're allowed to update the system when we're
// connected to the internet through the given network connection type and the
diff --git a/connection_manager_unittest.cc b/connection_manager_unittest.cc
index 0c96272..1867108 100644
--- a/connection_manager_unittest.cc
+++ b/connection_manager_unittest.cc
@@ -8,45 +8,51 @@
#include <string>
#include <base/logging.h>
+#include <chromeos/any.h>
#include <chromeos/dbus/service_constants.h>
+#include <chromeos/make_unique_ptr.h>
+#include <chromeos/message_loops/fake_message_loop.h>
+#include <chromeos/variant_dictionary.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
+#include "update_engine/fake_shill_proxy.h"
#include "update_engine/fake_system_state.h"
-#include "update_engine/mock_dbus_wrapper.h"
#include "update_engine/test_utils.h"
+using org::chromium::flimflam::ManagerProxyMock;
+using org::chromium::flimflam::ServiceProxyMock;
using std::set;
using std::string;
-using testing::A;
-using testing::AnyNumber;
using testing::Return;
-using testing::SetArgumentPointee;
-using testing::StrEq;
+using testing::SetArgPointee;
using testing::_;
namespace chromeos_update_engine {
class ConnectionManagerTest : public ::testing::Test {
public:
- ConnectionManagerTest()
- : kMockFlimFlamManagerProxy_(nullptr),
- kMockFlimFlamServiceProxy_(nullptr),
- kServicePath_(nullptr),
- cmut_(&fake_system_state_) {
+ void SetUp() override {
+ loop_.SetAsCurrent();
fake_system_state_.set_connection_manager(&cmut_);
}
- protected:
- void SetupMocks(const char* service_path);
- void SetManagerReply(const char* reply_value, const GType& reply_type);
+ void TearDown() override { EXPECT_FALSE(loop_.PendingTasks()); }
- // Sets the |service_type| Type and the |physical_technology|
- // PhysicalTechnology properties in the mocked service. If a null
- // |physical_technology| is passed, the property is not set (not present).
- void SetServiceReply(const char* service_type,
+ protected:
+ // Sets the default_service object path in the response from the
+ // ManagerProxyMock instance.
+ void SetManagerReply(const char* default_service, bool reply_succeeds);
+
+ // 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.
+ void SetServiceReply(const string& service_path,
+ const char* service_type,
const char* physical_technology,
const char* service_tethering);
+
void TestWithServiceType(
const char* service_type,
const char* physical_technology,
@@ -55,156 +61,95 @@
const char* service_tethering,
NetworkTethering expected_tethering);
- static const char* kGetPropertiesMethod;
- DBusGProxy* kMockFlimFlamManagerProxy_;
- DBusGProxy* kMockFlimFlamServiceProxy_;
- DBusGConnection* kMockSystemBus_;
- const char* kServicePath_;
- testing::StrictMock<MockDBusWrapper> dbus_iface_;
- ConnectionManager cmut_; // ConnectionManager under test.
+ chromeos::FakeMessageLoop loop_{nullptr};
FakeSystemState fake_system_state_;
+ FakeShillProxy fake_shill_proxy_;
+
+ // ConnectionManager under test.
+ ConnectionManager cmut_{&fake_shill_proxy_, &fake_system_state_};
};
-// static
-const char* ConnectionManagerTest::kGetPropertiesMethod = "GetProperties";
+void ConnectionManagerTest::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;
+ }
-void ConnectionManagerTest::SetupMocks(const char* service_path) {
- int number = 1;
- kMockSystemBus_ = reinterpret_cast<DBusGConnection*>(number++);
- kMockFlimFlamManagerProxy_ = reinterpret_cast<DBusGProxy*>(number++);
- kMockFlimFlamServiceProxy_ = reinterpret_cast<DBusGProxy*>(number++);
- ASSERT_NE(kMockSystemBus_, static_cast<DBusGConnection*>(nullptr));
+ // Create a dictionary of properties and optionally include the default
+ // service.
+ chromeos::VariantDictionary reply_dict;
+ reply_dict["SomeOtherProperty"] = 0xC0FFEE;
- kServicePath_ = service_path;
-
- ON_CALL(dbus_iface_, BusGet(DBUS_BUS_SYSTEM, _))
- .WillByDefault(Return(kMockSystemBus_));
- EXPECT_CALL(dbus_iface_, BusGet(DBUS_BUS_SYSTEM, _))
- .Times(AnyNumber());
+ 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)));
}
-void ConnectionManagerTest::SetManagerReply(const char *reply_value,
- const GType& reply_type) {
- ASSERT_TRUE(dbus_g_type_is_collection(reply_type));
-
- // Create the GPtrArray array holding the |reply_value| pointer. The
- // |reply_value| string is duplicated because it should be mutable on the
- // interface and is because dbus-glib collections will g_free() each element
- // of the GPtrArray automatically when the |array_as_value| GValue is unset.
- // The g_strdup() is not being leaked.
- GPtrArray* array = g_ptr_array_new();
- ASSERT_NE(nullptr, array);
- g_ptr_array_add(array, g_strdup(reply_value));
-
- GValue* array_as_value = g_new0(GValue, 1);
- EXPECT_EQ(array_as_value, g_value_init(array_as_value, reply_type));
- g_value_take_boxed(array_as_value, array);
-
- // Initialize return value for D-Bus call to Manager object, which is a
- // hash table of static strings (char*) in GValue* containing a single array.
- GHashTable* manager_hash_table = g_hash_table_new_full(
- g_str_hash, g_str_equal,
- nullptr, // no key_destroy_func because keys are static.
- test_utils::GValueFree); // value_destroy_func
- g_hash_table_insert(manager_hash_table,
- const_cast<char*>("Services"),
- array_as_value);
-
- // Plumb return value into mock object.
- EXPECT_CALL(dbus_iface_, ProxyCall_0_1(kMockFlimFlamManagerProxy_,
- StrEq(kGetPropertiesMethod),
- _, A<GHashTable**>()))
- .WillOnce(DoAll(SetArgumentPointee<3>(manager_hash_table), Return(TRUE)));
-
- // Set other expectations.
- EXPECT_CALL(dbus_iface_,
- ProxyNewForName(kMockSystemBus_,
- StrEq(shill::kFlimflamServiceName),
- StrEq(shill::kFlimflamServicePath),
- StrEq(shill::kFlimflamManagerInterface)))
- .WillOnce(Return(kMockFlimFlamManagerProxy_));
- EXPECT_CALL(dbus_iface_, ProxyUnref(kMockFlimFlamManagerProxy_));
- EXPECT_CALL(dbus_iface_, BusGet(DBUS_BUS_SYSTEM, _))
- .RetiresOnSaturation();
-}
-
-void ConnectionManagerTest::SetServiceReply(const char* service_type,
+void ConnectionManagerTest::SetServiceReply(const string& service_path,
+ const char* service_type,
const char* physical_technology,
const char* service_tethering) {
- // Initialize return value for D-Bus call to Service object, which is a
- // hash table of static strings (char*) in GValue*.
- GHashTable* service_hash_table = g_hash_table_new_full(
- g_str_hash, g_str_equal,
- nullptr, // no key_destroy_func because keys are static.
- test_utils::GValueFree); // value_destroy_func
- GValue* service_type_value = test_utils::GValueNewString(service_type);
- g_hash_table_insert(service_hash_table,
- const_cast<char*>("Type"),
- service_type_value);
+ chromeos::VariantDictionary reply_dict;
+ reply_dict["SomeOtherProperty"] = 0xC0FFEE;
+
+ if (service_type)
+ reply_dict[shill::kTypeProperty] = string(service_type);
if (physical_technology) {
- GValue* physical_technology_value =
- test_utils::GValueNewString(physical_technology);
- g_hash_table_insert(service_hash_table,
- const_cast<char*>("PhysicalTechnology"),
- physical_technology_value);
+ reply_dict[shill::kPhysicalTechnologyProperty] =
+ string(physical_technology);
}
- if (service_tethering) {
- GValue* service_tethering_value =
- test_utils::GValueNewString(service_tethering);
- g_hash_table_insert(service_hash_table,
- const_cast<char*>("Tethering"),
- service_tethering_value);
- }
+ if (service_tethering)
+ reply_dict[shill::kTetheringProperty] = string(service_tethering);
+
+ std::unique_ptr<ServiceProxyMock> service_proxy_mock(new ServiceProxyMock());
// Plumb return value into mock object.
- EXPECT_CALL(dbus_iface_, ProxyCall_0_1(kMockFlimFlamServiceProxy_,
- StrEq(kGetPropertiesMethod),
- _, A<GHashTable**>()))
- .WillOnce(DoAll(SetArgumentPointee<3>(service_hash_table), Return(TRUE)));
+ EXPECT_CALL(*service_proxy_mock.get(), GetProperties(_, _, _))
+ .WillOnce(DoAll(SetArgPointee<0>(reply_dict), Return(true)));
- // Set other expectations.
- EXPECT_CALL(dbus_iface_,
- ProxyNewForName(kMockSystemBus_,
- StrEq(shill::kFlimflamServiceName),
- StrEq(kServicePath_),
- StrEq(shill::kFlimflamServiceInterface)))
- .WillOnce(Return(kMockFlimFlamServiceProxy_));
- EXPECT_CALL(dbus_iface_, ProxyUnref(kMockFlimFlamServiceProxy_));
- EXPECT_CALL(dbus_iface_, BusGet(DBUS_BUS_SYSTEM, _))
- .RetiresOnSaturation();
+ fake_shill_proxy_.SetServiceForPath(service_path,
+ std::move(service_proxy_mock));
}
void ConnectionManagerTest::TestWithServiceType(
const char* service_type,
const char* physical_technology,
NetworkConnectionType expected_type) {
-
- SetupMocks("/service/guest-network");
- SetManagerReply(kServicePath_, DBUS_TYPE_G_OBJECT_PATH_ARRAY);
- SetServiceReply(service_type, physical_technology,
+ SetManagerReply("/service/guest-network", true);
+ SetServiceReply("/service/guest-network",
+ service_type,
+ physical_technology,
shill::kTetheringNotDetectedState);
NetworkConnectionType type;
NetworkTethering tethering;
- EXPECT_TRUE(cmut_.GetConnectionProperties(&dbus_iface_, &type, &tethering));
+ EXPECT_TRUE(cmut_.GetConnectionProperties(&type, &tethering));
EXPECT_EQ(expected_type, type);
- testing::Mock::VerifyAndClearExpectations(&dbus_iface_);
+ testing::Mock::VerifyAndClearExpectations(
+ fake_shill_proxy_.GetManagerProxy());
}
void ConnectionManagerTest::TestWithServiceTethering(
const char* service_tethering,
NetworkTethering expected_tethering) {
-
- SetupMocks("/service/guest-network");
- SetManagerReply(kServicePath_, DBUS_TYPE_G_OBJECT_PATH_ARRAY);
- SetServiceReply(shill::kTypeWifi, nullptr, service_tethering);
+ SetManagerReply("/service/guest-network", true);
+ SetServiceReply(
+ "/service/guest-network", shill::kTypeWifi, nullptr, service_tethering);
NetworkConnectionType type;
NetworkTethering tethering;
- EXPECT_TRUE(cmut_.GetConnectionProperties(&dbus_iface_, &type, &tethering));
+ EXPECT_TRUE(cmut_.GetConnectionProperties(&type, &tethering));
EXPECT_EQ(expected_tethering, tethering);
+ testing::Mock::VerifyAndClearExpectations(
+ fake_shill_proxy_.GetManagerProxy());
}
TEST_F(ConnectionManagerTest, SimpleTest) {
@@ -279,7 +224,7 @@
EXPECT_CALL(allow_3g_policy, GetAllowedConnectionTypesForUpdate(_))
.Times(1)
- .WillOnce(DoAll(SetArgumentPointee<0>(allowed_set), Return(true)));
+ .WillOnce(DoAll(SetArgPointee<0>(allowed_set), Return(true)));
EXPECT_TRUE(cmut_.IsUpdateAllowedOver(NetworkConnectionType::kCellular,
NetworkTethering::kUnknown));
@@ -301,7 +246,7 @@
EXPECT_CALL(allow_3g_policy, GetAllowedConnectionTypesForUpdate(_))
.Times(3)
- .WillRepeatedly(DoAll(SetArgumentPointee<0>(allowed_set), Return(true)));
+ .WillRepeatedly(DoAll(SetArgPointee<0>(allowed_set), Return(true)));
EXPECT_TRUE(cmut_.IsUpdateAllowedOver(NetworkConnectionType::kEthernet,
NetworkTethering::kUnknown));
@@ -355,7 +300,7 @@
EXPECT_CALL(block_3g_policy, GetAllowedConnectionTypesForUpdate(_))
.Times(1)
- .WillOnce(DoAll(SetArgumentPointee<0>(allowed_set), Return(true)));
+ .WillOnce(DoAll(SetArgPointee<0>(allowed_set), Return(true)));
EXPECT_FALSE(cmut_.IsUpdateAllowedOver(NetworkConnectionType::kCellular,
NetworkTethering::kUnknown));
@@ -375,7 +320,7 @@
// the string set above.
EXPECT_CALL(allow_3g_policy, GetAllowedConnectionTypesForUpdate(_))
.Times(1)
- .WillOnce(DoAll(SetArgumentPointee<0>(allowed_set), Return(false)));
+ .WillOnce(DoAll(SetArgPointee<0>(allowed_set), Return(false)));
EXPECT_FALSE(cmut_.IsUpdateAllowedOver(NetworkConnectionType::kCellular,
NetworkTethering::kUnknown));
@@ -405,7 +350,7 @@
.WillOnce(Return(true));
EXPECT_CALL(*prefs, GetBoolean(kPrefsUpdateOverCellularPermission, _))
.Times(1)
- .WillOnce(DoAll(SetArgumentPointee<1>(true), Return(true)));
+ .WillOnce(DoAll(SetArgPointee<1>(true), Return(true)));
EXPECT_TRUE(cmut_.IsUpdateAllowedOver(NetworkConnectionType::kCellular,
NetworkTethering::kUnknown));
@@ -415,7 +360,7 @@
.WillOnce(Return(true));
EXPECT_CALL(*prefs, GetBoolean(kPrefsUpdateOverCellularPermission, _))
.Times(1)
- .WillOnce(DoAll(SetArgumentPointee<1>(false), Return(true)));
+ .WillOnce(DoAll(SetArgPointee<1>(false), Return(true)));
EXPECT_FALSE(cmut_.IsUpdateAllowedOver(NetworkConnectionType::kCellular,
NetworkTethering::kUnknown));
}
@@ -440,12 +385,11 @@
}
TEST_F(ConnectionManagerTest, MalformedServiceList) {
- SetupMocks("/service/guest-network");
- SetManagerReply(kServicePath_, DBUS_TYPE_G_STRING_ARRAY);
+ SetManagerReply("/service/guest-network", false);
NetworkConnectionType type;
NetworkTethering tethering;
- EXPECT_FALSE(cmut_.GetConnectionProperties(&dbus_iface_, &type, &tethering));
+ EXPECT_FALSE(cmut_.GetConnectionProperties(&type, &tethering));
}
} // namespace chromeos_update_engine
diff --git a/dbus_bindings/org.chromium.LibCrosService.xml b/dbus_bindings/org.chromium.LibCrosService.xml
new file mode 100644
index 0000000..2ea8313
--- /dev/null
+++ b/dbus_bindings/org.chromium.LibCrosService.xml
@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+
+<node name="/org/chromium/LibCrosService"
+ xmlns:tp="http://telepathy.freedesktop.org/wiki/DbusSpec#extensions-v0">
+ <interface name="org.chromium.LibCrosServiceInterface">
+ <method name="ResolveNetworkProxy">
+ <arg name="source_url" type="s" direction="in" />
+ <arg name="signal_interface" type="s" direction="in" />
+ <arg name="signal_name" type="s" direction="in" />
+ <annotation name="org.chromium.DBus.Method.Kind" value="simple" />
+ </method>
+ </interface>
+ <interface name="org.chromium.UpdateEngineLibcrosProxyResolvedInterface">
+ <signal name="ProxyResolved">
+ <arg name="source_url" type="s" direction="out" />
+ <arg name="proxy_info" type="s" direction="out" />
+ <arg name="error_message" type="s" direction="out" />
+ </signal>
+ </interface>
+</node>
diff --git a/dbus_test_utils.h b/dbus_test_utils.h
new file mode 100644
index 0000000..99e6d5e
--- /dev/null
+++ b/dbus_test_utils.h
@@ -0,0 +1,77 @@
+// Copyright 2015 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.
+
+#ifndef UPDATE_ENGINE_DBUS_TEST_UTILS_H_
+#define UPDATE_ENGINE_DBUS_TEST_UTILS_H_
+
+#include <set>
+#include <string>
+
+#include <base/bind.h>
+#include <chromeos/message_loops/message_loop.h>
+#include <gmock/gmock.h>
+
+namespace chromeos_update_engine {
+namespace dbus_test_utils {
+
+#define MOCK_SIGNAL_HANDLER_EXPECT_SIGNAL_HANDLER( \
+ mock_signal_handler, mock_proxy, signal) \
+ do { \
+ EXPECT_CALL((mock_proxy), \
+ Register##signal##SignalHandler(::testing::_, ::testing::_)) \
+ .WillOnce(::chromeos_update_engine::dbus_test_utils::GrabCallbacks( \
+ &(mock_signal_handler))); \
+ } while (false)
+
+template <typename T>
+class MockSignalHandler {
+ public:
+ MockSignalHandler() = default;
+ ~MockSignalHandler() {
+ if (callback_connected_task_ != chromeos::MessageLoop::kTaskIdNull)
+ chromeos::MessageLoop::current()->CancelTask(callback_connected_task_);
+ }
+
+ // Returns whether the signal handler is registered.
+ bool IsHandlerRegistered() const { return signal_callback_ != nullptr; }
+
+ const base::Callback<T>& signal_callback() { return *signal_callback_.get(); }
+
+ void GrabCallbacks(
+ const base::Callback<T>& signal_callback,
+ dbus::ObjectProxy::OnConnectedCallback on_connected_callback) {
+ signal_callback_.reset(new base::Callback<T>(signal_callback));
+ on_connected_callback_.reset(
+ new dbus::ObjectProxy::OnConnectedCallback(on_connected_callback));
+ // Notify from the main loop that the callback was connected.
+ callback_connected_task_ = chromeos::MessageLoop::current()->PostTask(
+ FROM_HERE,
+ base::Bind(&MockSignalHandler<T>::OnCallbackConnected,
+ base::Unretained(this)));
+ }
+
+ private:
+ void OnCallbackConnected() {
+ callback_connected_task_ = chromeos::MessageLoop::kTaskIdNull;
+ on_connected_callback_->Run("", "", true);
+ }
+
+ chromeos::MessageLoop::TaskId callback_connected_task_{
+ chromeos::MessageLoop::kTaskIdNull};
+
+ std::unique_ptr<base::Callback<T>> signal_callback_;
+ std::unique_ptr<dbus::ObjectProxy::OnConnectedCallback>
+ on_connected_callback_;
+};
+
+// Defines the action that will call MockSignalHandler<T>::GrabCallbacks for the
+// right type.
+ACTION_P(GrabCallbacks, mock_signal_handler) {
+ mock_signal_handler->GrabCallbacks(arg0, arg1);
+}
+
+} // namespace dbus_test_utils
+} // namespace chromeos_update_engine
+
+#endif // UPDATE_ENGINE_DBUS_TEST_UTILS_H_
diff --git a/dbus_wrapper_interface.h b/dbus_wrapper_interface.h
deleted file mode 100644
index 57211c5..0000000
--- a/dbus_wrapper_interface.h
+++ /dev/null
@@ -1,125 +0,0 @@
-// Copyright (c) 2010 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.
-
-#ifndef UPDATE_ENGINE_DBUS_WRAPPER_INTERFACE_H_
-#define UPDATE_ENGINE_DBUS_WRAPPER_INTERFACE_H_
-
-// A mockable interface for DBus.
-
-#include <dbus/dbus.h>
-#include <dbus/dbus-glib.h>
-
-#ifndef DBUS_TYPE_G_OBJECT_PATH_ARRAY
-#define DBUS_TYPE_G_OBJECT_PATH_ARRAY \
- (dbus_g_type_get_collection("GPtrArray", DBUS_TYPE_G_OBJECT_PATH))
-#endif
-
-#ifndef DBUS_TYPE_G_STRING_ARRAY
-#define DBUS_TYPE_G_STRING_ARRAY \
- (dbus_g_type_get_collection("GPtrArray", G_TYPE_STRING))
-#endif
-
-namespace chromeos_update_engine {
-
-class DBusWrapperInterface {
- public:
- virtual ~DBusWrapperInterface() = default;
-
- // Wraps dbus_g_proxy_new_for_name().
- virtual DBusGProxy* ProxyNewForName(DBusGConnection* connection,
- const char* name,
- const char* path,
- const char* interface) = 0;
-
- // Wraps g_object_unref().
- virtual void ProxyUnref(DBusGProxy* proxy) = 0;
-
- // Wraps dbus_g_bus_get().
- virtual DBusGConnection* BusGet(DBusBusType type, GError** error) = 0;
-
- // Wraps dbus_g_proxy_call(). Since this is a variadic function without a
- // va_list equivalent, we have to list specific wrappers depending on the
- // number of input and output arguments, based on the required usage. Note,
- // however, that we do rely on automatic signature overriding to facilitate
- // different types of input/output arguments.
- virtual gboolean ProxyCall_0_1(DBusGProxy* proxy,
- const char* method,
- GError** error,
- GHashTable** out1) = 0;
-
- virtual gboolean ProxyCall_0_1(DBusGProxy* proxy,
- const char* method,
- GError** error,
- gint* out1) = 0;
-
- virtual gboolean ProxyCall_1_0(DBusGProxy* proxy,
- const char* method,
- GError** error,
- gint in1) = 0;
-
- virtual gboolean ProxyCall_3_0(DBusGProxy* proxy,
- const char* method,
- GError** error,
- const char* in1,
- const char* in2,
- const char* in3) = 0;
-
- // Wrappers for dbus_g_proxy_add_signal() (variadic).
- virtual void ProxyAddSignal_1(DBusGProxy* proxy,
- const char* signal_name,
- GType type1) = 0;
-
- virtual void ProxyAddSignal_2(DBusGProxy* proxy,
- const char* signal_name,
- GType type1,
- GType type2) = 0;
-
- // Wrapper for dbus_g_proxy_{connect,disconnect}_signal().
- virtual void ProxyConnectSignal(DBusGProxy* proxy,
- const char* signal_name,
- GCallback handler,
- void* data,
- GClosureNotify free_data_func) = 0;
- virtual void ProxyDisconnectSignal(DBusGProxy* proxy,
- const char* signal_name,
- GCallback handler,
- void* data) = 0;
-
- // Wraps dbus_g_connection_get_connection().
- virtual DBusConnection* ConnectionGetConnection(DBusGConnection* gbus) = 0;
-
- // Wraps dbus_bus_add_match().
- virtual void DBusBusAddMatch(DBusConnection* connection,
- const char* rule,
- DBusError* error) = 0;
-
- // Wraps dbus_connection_add_filter().
- virtual dbus_bool_t DBusConnectionAddFilter(
- DBusConnection* connection,
- DBusHandleMessageFunction function,
- void* user_data,
- DBusFreeFunction free_data_function) = 0;
-
- // Wraps dbus_connection_remove_filter().
- virtual void DBusConnectionRemoveFilter(DBusConnection* connection,
- DBusHandleMessageFunction function,
- void* user_data) = 0;
-
- // Wraps dbus_message_is_signal().
- virtual dbus_bool_t DBusMessageIsSignal(DBusMessage* message,
- const char* interface,
- const char* signal_name) = 0;
-
- // Wraps dbus_message_get_args(). Deploys the same approach for handling
- // variadic arguments as ProxyCall above.
- virtual dbus_bool_t DBusMessageGetArgs_3(DBusMessage* message,
- DBusError* error,
- char** out1,
- char** out2,
- char** out3) = 0;
-};
-
-} // namespace chromeos_update_engine
-
-#endif // UPDATE_ENGINE_DBUS_WRAPPER_INTERFACE_H_
diff --git a/fake_shill_proxy.cc b/fake_shill_proxy.cc
new file mode 100644
index 0000000..e38784f
--- /dev/null
+++ b/fake_shill_proxy.cc
@@ -0,0 +1,37 @@
+// Copyright 2015 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/fake_shill_proxy.h"
+
+#include "update_engine/dbus_proxies.h"
+
+using org::chromium::flimflam::ManagerProxyMock;
+using org::chromium::flimflam::ServiceProxyInterface;
+
+namespace chromeos_update_engine {
+
+FakeShillProxy::FakeShillProxy()
+ : manager_proxy_mock_(new ManagerProxyMock()) {}
+
+ManagerProxyMock* FakeShillProxy::GetManagerProxy() {
+ return manager_proxy_mock_.get();
+}
+
+std::unique_ptr<ServiceProxyInterface> FakeShillProxy::GetServiceForPath(
+ const std::string& path) {
+ auto it = service_proxy_mocks_.find(path);
+ CHECK(it != service_proxy_mocks_.end()) << "No ServiceProxyMock set for "
+ << path;
+ std::unique_ptr<ServiceProxyInterface> result = std::move(it->second);
+ service_proxy_mocks_.erase(it);
+ return result;
+}
+
+void FakeShillProxy::SetServiceForPath(
+ const std::string& path,
+ std::unique_ptr<ServiceProxyInterface> service_proxy) {
+ service_proxy_mocks_[path] = std::move(service_proxy);
+}
+
+} // namespace chromeos_update_engine
diff --git a/fake_shill_proxy.h b/fake_shill_proxy.h
new file mode 100644
index 0000000..a60e996
--- /dev/null
+++ b/fake_shill_proxy.h
@@ -0,0 +1,54 @@
+// Copyright 2015 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.
+
+#ifndef UPDATE_ENGINE_FAKE_SHILL_PROXY_H_
+#define UPDATE_ENGINE_FAKE_SHILL_PROXY_H_
+
+#include <map>
+#include <memory>
+#include <string>
+
+#include <base/macros.h>
+
+#include "update_engine/dbus_mocks.h"
+#include "update_engine/dbus_proxies.h"
+#include "update_engine/shill_proxy_interface.h"
+
+namespace chromeos_update_engine {
+
+// This class implements the connection to shill using real DBus calls.
+class FakeShillProxy : public ShillProxyInterface {
+ public:
+ FakeShillProxy();
+ ~FakeShillProxy() override = default;
+
+ // ShillProxyInterface overrides.
+
+ // GetManagerProxy returns the subclass ManagerProxyMock so tests can easily
+ // use it. Mocks for the return value of GetServiceForPath() can be provided
+ // with SetServiceForPath().
+ org::chromium::flimflam::ManagerProxyMock* GetManagerProxy() override;
+ std::unique_ptr<org::chromium::flimflam::ServiceProxyInterface>
+ GetServiceForPath(const std::string& path) override;
+
+ // Sets the service_proxy that will be returned by GetServiceForPath().
+ void SetServiceForPath(
+ const std::string& path,
+ std::unique_ptr<org::chromium::flimflam::ServiceProxyInterface>
+ service_proxy);
+
+ private:
+ std::unique_ptr<org::chromium::flimflam::ManagerProxyMock>
+ manager_proxy_mock_;
+
+ std::map<std::string,
+ std::unique_ptr<org::chromium::flimflam::ServiceProxyInterface>>
+ service_proxy_mocks_;
+
+ DISALLOW_COPY_AND_ASSIGN(FakeShillProxy);
+};
+
+} // namespace chromeos_update_engine
+
+#endif // UPDATE_ENGINE_FAKE_SHILL_PROXY_H_
diff --git a/fake_system_state.cc b/fake_system_state.cc
index c10dca1..6f1bd02 100644
--- a/fake_system_state.cc
+++ b/fake_system_state.cc
@@ -9,22 +9,22 @@
// Mock the SystemStateInterface so that we could lie that
// OOBE is completed even when there's no such marker file, etc.
FakeSystemState::FakeSystemState()
- : mock_update_attempter_(this, &dbus_),
- mock_request_params_(this),
- fake_update_manager_(&fake_clock_),
- clock_(&fake_clock_),
- connection_manager_(&mock_connection_manager_),
- hardware_(&fake_hardware_),
- metrics_lib_(&mock_metrics_lib_),
- prefs_(&mock_prefs_),
- powerwash_safe_prefs_(&mock_powerwash_safe_prefs_),
- payload_state_(&mock_payload_state_),
- update_attempter_(&mock_update_attempter_),
- request_params_(&mock_request_params_),
- p2p_manager_(&mock_p2p_manager_),
- update_manager_(&fake_update_manager_),
- device_policy_(nullptr),
- fake_system_rebooted_(false) {
+ : mock_update_attempter_(this, nullptr, nullptr),
+ mock_request_params_(this),
+ fake_update_manager_(&fake_clock_),
+ clock_(&fake_clock_),
+ connection_manager_(&mock_connection_manager_),
+ hardware_(&fake_hardware_),
+ metrics_lib_(&mock_metrics_lib_),
+ prefs_(&mock_prefs_),
+ powerwash_safe_prefs_(&mock_powerwash_safe_prefs_),
+ payload_state_(&mock_payload_state_),
+ update_attempter_(&mock_update_attempter_),
+ request_params_(&mock_request_params_),
+ p2p_manager_(&mock_p2p_manager_),
+ update_manager_(&fake_update_manager_),
+ device_policy_(nullptr),
+ fake_system_rebooted_(false) {
mock_payload_state_.Initialize(this);
mock_update_attempter_.Init();
}
diff --git a/fake_system_state.h b/fake_system_state.h
index 1f3f58f..47c27e4 100644
--- a/fake_system_state.h
+++ b/fake_system_state.h
@@ -10,10 +10,11 @@
#include <policy/mock_device_policy.h>
#include "metrics/metrics_library_mock.h"
+#include "update_engine/dbus_mocks.h"
+#include "update_engine/dbus_proxies.h"
#include "update_engine/fake_clock.h"
#include "update_engine/fake_hardware.h"
#include "update_engine/mock_connection_manager.h"
-#include "update_engine/mock_dbus_wrapper.h"
#include "update_engine/mock_omaha_request_params.h"
#include "update_engine/mock_p2p_manager.h"
#include "update_engine/mock_payload_state.h"
@@ -79,6 +80,11 @@
return update_manager_;
}
+ inline org::chromium::PowerManagerProxyInterface* power_manager_proxy()
+ override {
+ return power_manager_proxy_;
+ }
+
inline bool system_rebooted() override { return fake_system_rebooted_; }
// Setters for the various members, can be used for overriding the default
@@ -212,6 +218,7 @@
testing::NiceMock<MockOmahaRequestParams> mock_request_params_;
testing::NiceMock<MockP2PManager> mock_p2p_manager_;
chromeos_update_manager::FakeUpdateManager fake_update_manager_;
+ org::chromium::PowerManagerProxyMock mock_power_manager_;
// Pointers to objects that client code can override. They are initialized to
// the default implementations above.
@@ -226,12 +233,13 @@
OmahaRequestParams* request_params_;
P2PManager* p2p_manager_;
chromeos_update_manager::UpdateManager* update_manager_;
+ org::chromium::PowerManagerProxyInterface* power_manager_proxy_{
+ &mock_power_manager_};
// Other object pointers (not preinitialized).
const policy::DevicePolicy* device_policy_;
// Other data members.
- MockDBusWrapper dbus_;
bool fake_system_rebooted_;
};
diff --git a/glib_utils.cc b/glib_utils.cc
deleted file mode 100644
index 625d8d1..0000000
--- a/glib_utils.cc
+++ /dev/null
@@ -1,37 +0,0 @@
-// Copyright 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/glib_utils.h"
-
-#include <base/strings/stringprintf.h>
-
-using std::string;
-
-namespace chromeos_update_engine {
-namespace utils {
-
-string GetAndFreeGError(GError** error) {
- if (!*error) {
- return "Unknown GLib error.";
- }
- string message =
- base::StringPrintf("GError(%d): %s",
- (*error)->code,
- (*error)->message ? (*error)->message : "(unknown)");
- g_error_free(*error);
- *error = nullptr;
- return message;
-}
-
-gchar** StringVectorToGStrv(const std::vector<string> &vec_str) {
- GPtrArray *p = g_ptr_array_new();
- for (const string& str : vec_str) {
- g_ptr_array_add(p, g_strdup(str.c_str()));
- }
- g_ptr_array_add(p, nullptr);
- return reinterpret_cast<gchar**>(g_ptr_array_free(p, FALSE));
-}
-
-} // namespace utils
-} // namespace chromeos_update_engine
diff --git a/glib_utils.h b/glib_utils.h
deleted file mode 100644
index 97020cb..0000000
--- a/glib_utils.h
+++ /dev/null
@@ -1,46 +0,0 @@
-// Copyright 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.
-
-#ifndef UPDATE_ENGINE_GLIB_UTILS_H_
-#define UPDATE_ENGINE_GLIB_UTILS_H_
-
-#include <string>
-#include <vector>
-
-#include <glib.h>
-
-namespace chromeos_update_engine {
-namespace utils {
-
-// Returns the error message, if any, from a GError pointer. Frees the GError
-// object and resets error to null.
-std::string GetAndFreeGError(GError** error);
-
-// Converts a vector of strings to a NUL-terminated array of
-// strings. The resulting array should be freed with g_strfreev()
-// when are you done with it.
-gchar** StringVectorToGStrv(const std::vector<std::string>& vec_str);
-
-// A base::FreeDeleter that frees memory using g_free(). Useful when
-// integrating with GLib since it can be used with std::unique_ptr to
-// automatically free memory when going out of scope.
-struct GLibFreeDeleter {
- inline void operator()(void* ptr) const {
- g_free(static_cast<gpointer>(ptr));
- }
-};
-
-// A base::FreeDeleter that frees memory using g_strfreev(). Useful
-// when integrating with GLib since it can be used with std::unique_ptr to
-// automatically free memory when going out of scope.
-struct GLibStrvFreeDeleter {
- inline void operator()(void* ptr) const {
- g_strfreev(static_cast<gchar**>(ptr));
- }
-};
-
-} // namespace utils
-} // namespace chromeos_update_engine
-
-#endif // UPDATE_ENGINE_GLIB_UTILS_H_
diff --git a/libcros_proxy.cc b/libcros_proxy.cc
new file mode 100644
index 0000000..9f76ea9
--- /dev/null
+++ b/libcros_proxy.cc
@@ -0,0 +1,47 @@
+// Copyright 2015 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/libcros_proxy.h"
+
+#include "update_engine/dbus_proxies.h"
+
+using org::chromium::LibCrosServiceInterfaceProxy;
+using org::chromium::LibCrosServiceInterfaceProxyInterface;
+using org::chromium::UpdateEngineLibcrosProxyResolvedInterfaceProxy;
+using org::chromium::UpdateEngineLibcrosProxyResolvedInterfaceProxyInterface;
+
+namespace {
+const char kLibCrosServiceName[] = "org.chromium.LibCrosService";
+} // namespace
+
+namespace chromeos_update_engine {
+
+LibCrosProxy::LibCrosProxy(
+ std::unique_ptr<LibCrosServiceInterfaceProxyInterface>
+ service_interface_proxy,
+ std::unique_ptr<UpdateEngineLibcrosProxyResolvedInterfaceProxyInterface>
+ ue_proxy_resolved_interface)
+ : service_interface_proxy_(std::move(service_interface_proxy)),
+ ue_proxy_resolved_interface_(std::move(ue_proxy_resolved_interface)) {
+}
+
+LibCrosProxy::LibCrosProxy(const scoped_refptr<dbus::Bus>& bus)
+ : service_interface_proxy_(
+ new LibCrosServiceInterfaceProxy(bus, kLibCrosServiceName)),
+ ue_proxy_resolved_interface_(
+ new UpdateEngineLibcrosProxyResolvedInterfaceProxy(
+ bus,
+ kLibCrosServiceName)) {
+}
+
+LibCrosServiceInterfaceProxyInterface* LibCrosProxy::service_interface_proxy() {
+ return service_interface_proxy_.get();
+}
+
+UpdateEngineLibcrosProxyResolvedInterfaceProxyInterface*
+LibCrosProxy::ue_proxy_resolved_interface() {
+ return ue_proxy_resolved_interface_.get();
+}
+
+} // namespace chromeos_update_engine
diff --git a/libcros_proxy.h b/libcros_proxy.h
new file mode 100644
index 0000000..4c0b8bc
--- /dev/null
+++ b/libcros_proxy.h
@@ -0,0 +1,50 @@
+// Copyright 2015 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.
+
+#ifndef UPDATE_ENGINE_LIBCROS_PROXY_H_
+#define UPDATE_ENGINE_LIBCROS_PROXY_H_
+
+#include <memory>
+
+#include <base/macros.h>
+#include <dbus/bus.h>
+
+#include "update_engine/dbus_proxies.h"
+
+namespace chromeos_update_engine {
+
+// This class handles the DBus connection with chrome to resolve proxies. This
+// is a thin class to just hold the generated proxies (real or mocked ones).
+class LibCrosProxy final {
+ public:
+ explicit LibCrosProxy(const scoped_refptr<dbus::Bus>& bus);
+ LibCrosProxy(
+ std::unique_ptr<org::chromium::LibCrosServiceInterfaceProxyInterface>
+ service_interface_proxy,
+ std::unique_ptr<
+ org::chromium::
+ UpdateEngineLibcrosProxyResolvedInterfaceProxyInterface>
+ ue_proxy_resolved_interface);
+
+ ~LibCrosProxy() = default;
+
+ // Getters for the two proxies.
+ org::chromium::LibCrosServiceInterfaceProxyInterface*
+ service_interface_proxy();
+ org::chromium::UpdateEngineLibcrosProxyResolvedInterfaceProxyInterface*
+ ue_proxy_resolved_interface();
+
+ private:
+ std::unique_ptr<org::chromium::LibCrosServiceInterfaceProxyInterface>
+ service_interface_proxy_;
+ std::unique_ptr<
+ org::chromium::UpdateEngineLibcrosProxyResolvedInterfaceProxyInterface>
+ ue_proxy_resolved_interface_;
+
+ DISALLOW_COPY_AND_ASSIGN(LibCrosProxy);
+};
+
+} // namespace chromeos_update_engine
+
+#endif // UPDATE_ENGINE_LIBCROS_PROXY_H_
diff --git a/main.cc b/main.cc
index 4fe0c84..0e5f4e5 100644
--- a/main.cc
+++ b/main.cc
@@ -20,7 +20,6 @@
#include <sys/types.h>
#include "update_engine/daemon.h"
-#include "update_engine/glib_utils.h"
#include "update_engine/terminator.h"
#include "update_engine/utils.h"
@@ -95,8 +94,6 @@
DEFINE_bool(foreground, false,
"Don't daemon()ize; run in foreground.");
- ::g_type_init();
- dbus_threads_init_default();
chromeos_update_engine::Terminator::Init();
chromeos::FlagHelper::Init(argc, argv, "Chromium OS Update Engine");
chromeos_update_engine::SetupLogging(FLAGS_logtostderr);
diff --git a/mock_connection_manager.h b/mock_connection_manager.h
index 609b51d..e885e2f 100644
--- a/mock_connection_manager.h
+++ b/mock_connection_manager.h
@@ -18,10 +18,9 @@
public:
MockConnectionManager() = default;
- MOCK_CONST_METHOD3(GetConnectionProperties,
- bool(DBusWrapperInterface* dbus_iface,
- NetworkConnectionType* out_type,
- NetworkTethering* out_tethering));
+ MOCK_METHOD2(GetConnectionProperties,
+ bool(NetworkConnectionType* out_type,
+ NetworkTethering* out_tethering));
MOCK_CONST_METHOD2(IsUpdateAllowedOver, bool(NetworkConnectionType type,
NetworkTethering tethering));
diff --git a/mock_dbus_wrapper.h b/mock_dbus_wrapper.h
deleted file mode 100644
index 5d630a6..0000000
--- a/mock_dbus_wrapper.h
+++ /dev/null
@@ -1,94 +0,0 @@
-// Copyright (c) 2010 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.
-
-#ifndef UPDATE_ENGINE_MOCK_DBUS_WRAPPER_H_
-#define UPDATE_ENGINE_MOCK_DBUS_WRAPPER_H_
-
-#include <gmock/gmock.h>
-
-#include "update_engine/dbus_wrapper_interface.h"
-
-namespace chromeos_update_engine {
-
-class MockDBusWrapper : public DBusWrapperInterface {
- public:
- MOCK_METHOD4(ProxyNewForName, DBusGProxy*(DBusGConnection *connection,
- const char *name,
- const char *path,
- const char *interface));
-
- MOCK_METHOD1(ProxyUnref, void(DBusGProxy* proxy));
-
- MOCK_METHOD2(BusGet, DBusGConnection*(DBusBusType type, GError **error));
-
- MOCK_METHOD4(ProxyCall_0_1, gboolean(DBusGProxy *proxy,
- const char *method,
- GError **error,
- GHashTable** out1));
- MOCK_METHOD4(ProxyCall_0_1, gboolean(DBusGProxy *proxy,
- const char *method,
- GError **error,
- gint* out1));
- MOCK_METHOD4(ProxyCall_1_0, gboolean(DBusGProxy *proxy,
- const char *method,
- GError **error,
- gint in1));
- MOCK_METHOD6(ProxyCall_3_0, gboolean(DBusGProxy* proxy,
- const char* method,
- GError** error,
- const char* in1,
- const char* in2,
- const char* in3));
-
- MOCK_METHOD3(ProxyAddSignal_1, void(DBusGProxy* proxy,
- const char* signal_name,
- GType type1));
-
- MOCK_METHOD4(ProxyAddSignal_2, void(DBusGProxy* proxy,
- const char* signal_name,
- GType type1,
- GType type2));
-
- MOCK_METHOD5(ProxyConnectSignal, void(DBusGProxy* proxy,
- const char* signal_name,
- GCallback handler,
- void* data,
- GClosureNotify free_data_func));
-
- MOCK_METHOD4(ProxyDisconnectSignal, void(DBusGProxy* proxy,
- const char* signal_name,
- GCallback handler,
- void* data));
-
- MOCK_METHOD1(ConnectionGetConnection, DBusConnection*(DBusGConnection* gbus));
-
- MOCK_METHOD3(DBusBusAddMatch, void(DBusConnection* connection,
- const char* rule,
- DBusError* error));
-
- MOCK_METHOD4(DBusConnectionAddFilter, dbus_bool_t(
- DBusConnection* connection,
- DBusHandleMessageFunction function,
- void* user_data,
- DBusFreeFunction free_data_function));
-
- MOCK_METHOD3(DBusConnectionRemoveFilter, void(
- DBusConnection* connection,
- DBusHandleMessageFunction function,
- void* user_data));
-
- MOCK_METHOD3(DBusMessageIsSignal, dbus_bool_t(DBusMessage* message,
- const char* interface,
- const char* signal_name));
-
- MOCK_METHOD5(DBusMessageGetArgs_3, dbus_bool_t(DBusMessage* message,
- DBusError* error,
- char** out1,
- char** out2,
- char** out3));
-};
-
-} // namespace chromeos_update_engine
-
-#endif // UPDATE_ENGINE_MOCK_DBUS_WRAPPER_H_
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index 2f361e5..3d7cccc 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -29,7 +29,6 @@
#include "update_engine/p2p_manager.h"
#include "update_engine/payload_state_interface.h"
#include "update_engine/prefs_interface.h"
-#include "update_engine/real_dbus_wrapper.h"
#include "update_engine/utils.h"
using base::Time;
@@ -1448,11 +1447,9 @@
bool OmahaRequestAction::IsUpdateAllowedOverCurrentConnection() const {
NetworkConnectionType type;
NetworkTethering tethering;
- RealDBusWrapper dbus_iface;
ConnectionManagerInterface* connection_manager =
system_state_->connection_manager();
- if (!connection_manager->GetConnectionProperties(&dbus_iface,
- &type, &tethering)) {
+ if (!connection_manager->GetConnectionProperties(&type, &tethering)) {
LOG(INFO) << "We could not determine our connection type. "
<< "Defaulting to allow updates.";
return true;
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index 5b8fba9..aae0506 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -448,10 +448,11 @@
MockConnectionManager mock_cm;
fake_system_state_.set_connection_manager(&mock_cm);
- EXPECT_CALL(mock_cm, GetConnectionProperties(_, _, _)).WillRepeatedly(
- DoAll(SetArgumentPointee<1>(NetworkConnectionType::kEthernet),
- SetArgumentPointee<2>(NetworkTethering::kUnknown),
- Return(true)));
+ EXPECT_CALL(mock_cm, GetConnectionProperties(_, _))
+ .WillRepeatedly(
+ DoAll(SetArgumentPointee<0>(NetworkConnectionType::kEthernet),
+ SetArgumentPointee<1>(NetworkTethering::kUnknown),
+ Return(true)));
EXPECT_CALL(mock_cm, IsUpdateAllowedOver(NetworkConnectionType::kEthernet, _))
.WillRepeatedly(Return(false));
diff --git a/p2p_manager.cc b/p2p_manager.cc
index b1b4438..daebe91 100644
--- a/p2p_manager.cc
+++ b/p2p_manager.cc
@@ -34,7 +34,6 @@
#include <base/strings/string_util.h>
#include <base/strings/stringprintf.h>
-#include "update_engine/glib_utils.h"
#include "update_engine/subprocess.h"
#include "update_engine/update_manager/policy.h"
#include "update_engine/update_manager/update_manager.h"
diff --git a/payload_state.cc b/payload_state.cc
index a15d68b..aa4c73c 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -18,7 +18,6 @@
#include "update_engine/install_plan.h"
#include "update_engine/omaha_request_params.h"
#include "update_engine/prefs.h"
-#include "update_engine/real_dbus_wrapper.h"
#include "update_engine/system_state.h"
#include "update_engine/utils.h"
@@ -174,11 +173,9 @@
metrics::ConnectionType type;
NetworkConnectionType network_connection_type;
NetworkTethering tethering;
- RealDBusWrapper dbus_iface;
ConnectionManagerInterface* connection_manager =
system_state_->connection_manager();
- if (!connection_manager->GetConnectionProperties(&dbus_iface,
- &network_connection_type,
+ if (!connection_manager->GetConnectionProperties(&network_connection_type,
&tethering)) {
LOG(ERROR) << "Failed to determine connection type.";
type = metrics::ConnectionType::kUnknown;
diff --git a/real_dbus_wrapper.h b/real_dbus_wrapper.h
deleted file mode 100644
index 0c6847b..0000000
--- a/real_dbus_wrapper.h
+++ /dev/null
@@ -1,158 +0,0 @@
-// Copyright (c) 2010 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.
-
-#ifndef UPDATE_ENGINE_REAL_DBUS_WRAPPER_H_
-#define UPDATE_ENGINE_REAL_DBUS_WRAPPER_H_
-
-// A mockable interface for DBus.
-
-#include <base/macros.h>
-#include <dbus/dbus-glib.h>
-#include <dbus/dbus-glib-lowlevel.h>
-
-#include "update_engine/dbus_wrapper_interface.h"
-
-namespace chromeos_update_engine {
-
-class RealDBusWrapper : public DBusWrapperInterface {
- DBusGProxy* ProxyNewForName(DBusGConnection* connection,
- const char* name,
- const char* path,
- const char* interface) override {
- return dbus_g_proxy_new_for_name(connection,
- name,
- path,
- interface);
- }
-
- void ProxyUnref(DBusGProxy* proxy) override {
- g_object_unref(proxy);
- }
-
- DBusGConnection* BusGet(DBusBusType type, GError** error) override {
- return dbus_g_bus_get(type, error);
- }
-
- gboolean ProxyCall_0_1(DBusGProxy* proxy,
- const char* method,
- GError** error,
- GHashTable** out1) override {
- return dbus_g_proxy_call(proxy, method, error, G_TYPE_INVALID,
- dbus_g_type_get_map("GHashTable",
- G_TYPE_STRING,
- G_TYPE_VALUE),
- out1, G_TYPE_INVALID);
- }
-
- gboolean ProxyCall_0_1(DBusGProxy* proxy,
- const char* method,
- GError** error,
- gint* out1) override {
- return dbus_g_proxy_call(proxy, method, error, G_TYPE_INVALID,
- G_TYPE_INT, out1, G_TYPE_INVALID);
- }
-
- gboolean ProxyCall_1_0(DBusGProxy* proxy,
- const char* method,
- GError** error,
- gint in1) override {
- return dbus_g_proxy_call(proxy, method, error,
- G_TYPE_INT, in1,
- G_TYPE_INVALID, G_TYPE_INVALID);
- }
-
- gboolean ProxyCall_3_0(DBusGProxy* proxy,
- const char* method,
- GError** error,
- const char* in1,
- const char* in2,
- const char* in3) override {
- return dbus_g_proxy_call(
- proxy, method, error,
- G_TYPE_STRING, in1, G_TYPE_STRING, in2, G_TYPE_STRING, in3,
- G_TYPE_INVALID, G_TYPE_INVALID);
- }
-
- void ProxyAddSignal_1(DBusGProxy* proxy,
- const char* signal_name,
- GType type1) override {
- dbus_g_object_register_marshaller(
- g_cclosure_marshal_generic, G_TYPE_NONE, type1, G_TYPE_INVALID);
- dbus_g_proxy_add_signal(proxy, signal_name, type1, G_TYPE_INVALID);
- }
-
- void ProxyAddSignal_2(DBusGProxy* proxy,
- const char* signal_name,
- GType type1,
- GType type2) override {
- dbus_g_object_register_marshaller(
- g_cclosure_marshal_generic, G_TYPE_NONE, type1, type2, G_TYPE_INVALID);
- dbus_g_proxy_add_signal(proxy, signal_name, type1, type2, G_TYPE_INVALID);
- }
-
- void ProxyConnectSignal(DBusGProxy* proxy,
- const char* signal_name,
- GCallback handler,
- void* data,
- GClosureNotify free_data_func) override {
- dbus_g_proxy_connect_signal(proxy, signal_name, handler, data,
- free_data_func);
- }
-
- void ProxyDisconnectSignal(DBusGProxy* proxy,
- const char* signal_name,
- GCallback handler,
- void* data) override {
- dbus_g_proxy_disconnect_signal(proxy, signal_name, handler, data);
- }
-
- DBusConnection* ConnectionGetConnection(DBusGConnection* gbus) override {
- return dbus_g_connection_get_connection(gbus);
- }
-
- void DBusBusAddMatch(DBusConnection* connection,
- const char* rule,
- DBusError* error) override {
- dbus_bus_add_match(connection, rule, error);
- }
-
- dbus_bool_t DBusConnectionAddFilter(
- DBusConnection* connection,
- DBusHandleMessageFunction function,
- void* user_data,
- DBusFreeFunction free_data_function) override {
- return dbus_connection_add_filter(connection,
- function,
- user_data,
- free_data_function);
- }
-
- void DBusConnectionRemoveFilter(DBusConnection* connection,
- DBusHandleMessageFunction function,
- void* user_data) override {
- dbus_connection_remove_filter(connection, function, user_data);
- }
-
- dbus_bool_t DBusMessageIsSignal(DBusMessage* message,
- const char* interface,
- const char* signal_name) override {
- return dbus_message_is_signal(message, interface, signal_name);
- }
-
- dbus_bool_t DBusMessageGetArgs_3(DBusMessage* message,
- DBusError* error,
- char** out1,
- char** out2,
- char** out3) override {
- return dbus_message_get_args(message, error,
- DBUS_TYPE_STRING, out1,
- DBUS_TYPE_STRING, out2,
- DBUS_TYPE_STRING, out3,
- G_TYPE_INVALID);
- }
-};
-
-} // namespace chromeos_update_engine
-
-#endif // UPDATE_ENGINE_REAL_DBUS_WRAPPER_H_
diff --git a/real_system_state.cc b/real_system_state.cc
index f16f2f8..861a702 100644
--- a/real_system_state.cc
+++ b/real_system_state.cc
@@ -6,6 +6,7 @@
#include <base/files/file_util.h>
#include <base/time/time.h>
+#include <chromeos/dbus/service_constants.h>
#include "update_engine/constants.h"
#include "update_engine/update_manager/state_factory.h"
@@ -13,16 +14,22 @@
namespace chromeos_update_engine {
-RealSystemState::RealSystemState()
- : device_policy_(nullptr),
- connection_manager_(this),
- update_attempter_(this, &dbus_),
- request_params_(this),
- system_rebooted_(false) {}
+RealSystemState::RealSystemState(const scoped_refptr<dbus::Bus>& bus)
+ : debugd_proxy_(bus, debugd::kDebugdServiceName),
+ power_manager_proxy_(bus, power_manager::kPowerManagerServiceName),
+ session_manager_proxy_(bus, login_manager::kSessionManagerServiceName),
+ shill_proxy_(bus),
+ libcros_proxy_(bus) {
+}
bool RealSystemState::Initialize() {
metrics_lib_.Init();
+ if (!shill_proxy_.Init()) {
+ LOG(ERROR) << "Failed to initialize shill proxy.";
+ return false;
+ }
+
if (!prefs_.Init(base::FilePath(kPrefsDirectory))) {
LOG(ERROR) << "Failed to initialize preferences.";
return false;
@@ -44,7 +51,7 @@
// Initialize the Update Manager using the default state factory.
chromeos_update_manager::State* um_state =
chromeos_update_manager::DefaultStateFactory(
- &policy_provider_, &dbus_, this);
+ &policy_provider_, &shill_proxy_, &session_manager_proxy_, this);
if (!um_state) {
LOG(ERROR) << "Failed to initialize the Update Manager.";
return false;
diff --git a/real_system_state.h b/real_system_state.h
index 4e97b95..be7778e 100644
--- a/real_system_state.h
+++ b/real_system_state.h
@@ -14,11 +14,12 @@
#include "update_engine/clock.h"
#include "update_engine/connection_manager.h"
+#include "update_engine/dbus_proxies.h"
#include "update_engine/hardware.h"
#include "update_engine/p2p_manager.h"
#include "update_engine/payload_state.h"
#include "update_engine/prefs.h"
-#include "update_engine/real_dbus_wrapper.h"
+#include "update_engine/shill_proxy.h"
#include "update_engine/update_attempter.h"
#include "update_engine/update_manager/update_manager.h"
@@ -30,7 +31,7 @@
public:
// Constructs all system objects that do not require separate initialization;
// see Initialize() below for the remaining ones.
- RealSystemState();
+ explicit RealSystemState(const scoped_refptr<dbus::Bus>& bus);
// Initializes and sets systems objects that require an initialization
// separately from construction. Returns |true| on success.
@@ -81,18 +82,30 @@
return update_manager_.get();
}
+ inline org::chromium::PowerManagerProxyInterface* power_manager_proxy()
+ override {
+ return &power_manager_proxy_;
+ }
+
inline bool system_rebooted() override { return system_rebooted_; }
private:
+ // Real DBus proxies using the DBus connection.
+ org::chromium::debugdProxy debugd_proxy_;
+ org::chromium::PowerManagerProxy power_manager_proxy_;
+ org::chromium::SessionManagerInterfaceProxy session_manager_proxy_;
+ ShillProxy shill_proxy_;
+ LibCrosProxy libcros_proxy_;
+
// Interface for the clock.
Clock clock_;
// The latest device policy object from the policy provider.
- const policy::DevicePolicy* device_policy_;
+ const policy::DevicePolicy* device_policy_{nullptr};
- // The connection manager object that makes download
- // decisions depending on the current type of connection.
- ConnectionManager connection_manager_;
+ // The connection manager object that makes download decisions depending on
+ // the current type of connection.
+ ConnectionManager connection_manager_{&shill_proxy_, this};
// Interface for the hardware functions.
Hardware hardware_;
@@ -106,18 +119,15 @@
// Interface for persisted store that persists across powerwashes.
Prefs powerwash_safe_prefs_;
- // All state pertaining to payload state such as
- // response, URL, backoff states.
+ // All state pertaining to payload state such as response, URL, backoff
+ // states.
PayloadState payload_state_;
- // The dbus object used to initialize the update attempter.
- RealDBusWrapper dbus_;
-
// Pointer to the update attempter object.
- UpdateAttempter update_attempter_;
+ UpdateAttempter update_attempter_{this, &libcros_proxy_, &debugd_proxy_};
// Common parameters for all Omaha requests.
- OmahaRequestParams request_params_;
+ OmahaRequestParams request_params_{this};
std::unique_ptr<P2PManager> p2p_manager_;
@@ -128,7 +138,7 @@
// If true, this is the first instance of the update engine since the system
// rebooted. Important for tracking whether you are running instance of the
// update engine on first boot or due to a crash/restart.
- bool system_rebooted_;
+ bool system_rebooted_{false};
};
} // namespace chromeos_update_engine
diff --git a/shill_proxy.cc b/shill_proxy.cc
new file mode 100644
index 0000000..f72ee8b
--- /dev/null
+++ b/shill_proxy.cc
@@ -0,0 +1,39 @@
+// Copyright 2015 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/shill_proxy.h"
+
+#include <chromeos/dbus/service_constants.h>
+
+#include "update_engine/dbus_proxies.h"
+
+using org::chromium::flimflam::ManagerProxy;
+using org::chromium::flimflam::ManagerProxyInterface;
+using org::chromium::flimflam::ServiceProxy;
+using org::chromium::flimflam::ServiceProxyInterface;
+
+namespace chromeos_update_engine {
+
+ShillProxy::ShillProxy(const scoped_refptr<dbus::Bus>& bus) : bus_(bus) {}
+
+bool ShillProxy::Init() {
+ manager_proxy_.reset(
+ new ManagerProxy(bus_,
+ shill::kFlimflamServiceName,
+ dbus::ObjectPath(shill::kFlimflamServicePath)));
+ return true;
+}
+
+ManagerProxyInterface* ShillProxy::GetManagerProxy() {
+ return manager_proxy_.get();
+}
+
+std::unique_ptr<ServiceProxyInterface> ShillProxy::GetServiceForPath(
+ const std::string& path) {
+ DCHECK(bus_.get());
+ return std::unique_ptr<ServiceProxyInterface>(new ServiceProxy(
+ bus_, shill::kFlimflamServiceName, dbus::ObjectPath(path)));
+}
+
+} // namespace chromeos_update_engine
diff --git a/shill_proxy.h b/shill_proxy.h
new file mode 100644
index 0000000..3d1b376
--- /dev/null
+++ b/shill_proxy.h
@@ -0,0 +1,45 @@
+// Copyright 2015 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.
+
+#ifndef UPDATE_ENGINE_SHILL_PROXY_H_
+#define UPDATE_ENGINE_SHILL_PROXY_H_
+
+#include <memory>
+#include <string>
+
+#include <base/macros.h>
+#include <dbus/bus.h>
+
+#include "update_engine/dbus_proxies.h"
+#include "update_engine/shill_proxy_interface.h"
+
+namespace chromeos_update_engine {
+
+// This class implements the connection to shill using real DBus calls.
+class ShillProxy : public ShillProxyInterface {
+ public:
+ explicit ShillProxy(const scoped_refptr<dbus::Bus>& bus);
+ ~ShillProxy() override = default;
+
+ // Initializes the ShillProxy instance creating the manager proxy from the
+ // |bus_|.
+ bool Init();
+
+ // ShillProxyInterface overrides.
+ org::chromium::flimflam::ManagerProxyInterface* GetManagerProxy() override;
+ std::unique_ptr<org::chromium::flimflam::ServiceProxyInterface>
+ GetServiceForPath(const std::string& path) override;
+
+ private:
+ // A reference to the main bus for creating new ServiceProxy instances.
+ scoped_refptr<dbus::Bus> bus_;
+ std::unique_ptr<org::chromium::flimflam::ManagerProxyInterface>
+ manager_proxy_;
+
+ DISALLOW_COPY_AND_ASSIGN(ShillProxy);
+};
+
+} // namespace chromeos_update_engine
+
+#endif // UPDATE_ENGINE_SHILL_PROXY_H_
diff --git a/shill_proxy_interface.h b/shill_proxy_interface.h
new file mode 100644
index 0000000..a4d3aa3
--- /dev/null
+++ b/shill_proxy_interface.h
@@ -0,0 +1,44 @@
+// Copyright 2015 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.
+
+#ifndef UPDATE_ENGINE_SHILL_PROXY_INTERFACE_H_
+#define UPDATE_ENGINE_SHILL_PROXY_INTERFACE_H_
+
+#include <memory>
+#include <string>
+
+#include <base/macros.h>
+
+#include "update_engine/dbus_proxies.h"
+
+namespace chromeos_update_engine {
+
+// This class handles the DBus connection with shill daemon. The DBus interface
+// with shill requires to monitor or request the current service by interacting
+// with the org::chromium::flimflam::ManagerProxy and then request or monitor
+// properties on the selected org::chromium::flimflam::ServiceProxy. This class
+// provides a mockable way to access that.
+class ShillProxyInterface {
+ public:
+ virtual ~ShillProxyInterface() = default;
+
+ // Return the ManagerProxy instance of the shill daemon. The instance is owned
+ // by this ShillProxyInterface instance.
+ virtual org::chromium::flimflam::ManagerProxyInterface* GetManagerProxy() = 0;
+
+ // Return a ServiceProxy for the given path. The ownership of the returned
+ // instance is transferred to the caller.
+ virtual std::unique_ptr<org::chromium::flimflam::ServiceProxyInterface>
+ GetServiceForPath(const std::string& path) = 0;
+
+ protected:
+ ShillProxyInterface() = default;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(ShillProxyInterface);
+};
+
+} // namespace chromeos_update_engine
+
+#endif // UPDATE_ENGINE_SHILL_PROXY_INTERFACE_H_
diff --git a/system_state.h b/system_state.h
index 65d4e48..e32aa0e 100644
--- a/system_state.h
+++ b/system_state.h
@@ -5,6 +5,8 @@
#ifndef UPDATE_ENGINE_SYSTEM_STATE_H_
#define UPDATE_ENGINE_SYSTEM_STATE_H_
+#include "update_engine/dbus_proxies.h"
+
class MetricsLibraryInterface;
namespace chromeos_update_manager {
@@ -26,7 +28,6 @@
// the required classes.
class ClockInterface;
class ConnectionManagerInterface;
-class GpioHandler;
class HardwareInterface;
class OmahaRequestParams;
class P2PManager;
@@ -87,6 +88,9 @@
// Returns a pointer to the UpdateManager singleton.
virtual chromeos_update_manager::UpdateManager* update_manager() = 0;
+ // DBus proxies. Mocked during test.
+ virtual org::chromium::PowerManagerProxyInterface* power_manager_proxy() = 0;
+
// If true, this is the first instance of the update engine since the system
// restarted. Important for tracking whether you are running instance of the
// update engine on first boot or due to a crash/restart.
diff --git a/test_utils.cc b/test_utils.cc
index 0c02279..b4a8074 100644
--- a/test_utils.cc
+++ b/test_utils.cc
@@ -19,8 +19,8 @@
#include <base/files/file_util.h>
#include <base/logging.h>
-#include <base/strings/stringprintf.h>
#include <base/strings/string_util.h>
+#include <base/strings/stringprintf.h>
#include "update_engine/file_writer.h"
#include "update_engine/utils.h"
@@ -302,19 +302,6 @@
return true;
}
-GValue* GValueNewString(const char* str) {
- GValue* gval = g_new0(GValue, 1);
- g_value_init(gval, G_TYPE_STRING);
- g_value_set_string(gval, str);
- return gval;
-}
-
-void GValueFree(gpointer arg) {
- auto gval = reinterpret_cast<GValue*>(arg);
- g_value_unset(gval);
- g_free(gval);
-}
-
base::FilePath GetBuildArtifactsPath() {
base::FilePath exe_path;
base::ReadSymbolicLink(base::FilePath("/proc/self/exe"), &exe_path);
diff --git a/test_utils.h b/test_utils.h
index 13bbd41..84aeae1 100644
--- a/test_utils.h
+++ b/test_utils.h
@@ -18,7 +18,6 @@
#include <base/callback.h>
#include <base/files/file_path.h>
-#include <glib-object.h>
#include <gtest/gtest.h>
#include "update_engine/action.h"
@@ -183,12 +182,6 @@
// This WILL cross filesystem boundaries.
bool RecursiveUnlinkDir(const std::string& path);
-// Allocates, initializes and returns a string GValue object.
-GValue* GValueNewString(const char* str);
-
-// Frees a GValue object and its allocated resources.
-void GValueFree(gpointer arg);
-
// Returns the path where the build artifacts are stored. This is the directory
// where the unittest executable is being run from.
base::FilePath GetBuildArtifactsPath();
diff --git a/testrunner.cc b/testrunner.cc
index 3defa63..dfd8a73 100644
--- a/testrunner.cc
+++ b/testrunner.cc
@@ -7,19 +7,13 @@
#include <base/at_exit.h>
#include <base/command_line.h>
#include <chromeos/test_helpers.h>
-#include <dbus/dbus-glib.h>
-#include <dbus/dbus-glib-bindings.h>
-#include <dbus/dbus-glib-lowlevel.h>
#include <glib.h>
-#include <glib-object.h>
#include <gtest/gtest.h>
#include "update_engine/terminator.h"
int main(int argc, char **argv) {
LOG(INFO) << "started";
- ::g_type_init();
- dbus_threads_init_default();
base::AtExitManager exit_manager;
// TODO(garnold) temporarily cause the unittest binary to exit with status
// code 2 upon catching a SIGTERM. This will help diagnose why the unittest
diff --git a/update_attempter.cc b/update_attempter.cc
index 762a7e5..05fbde8 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -32,10 +32,8 @@
#include "update_engine/clock_interface.h"
#include "update_engine/constants.h"
#include "update_engine/dbus_service.h"
-#include "update_engine/dbus_wrapper_interface.h"
#include "update_engine/download_action.h"
#include "update_engine/filesystem_verifier_action.h"
-#include "update_engine/glib_utils.h"
#include "update_engine/hardware_interface.h"
#include "update_engine/libcurl_http_fetcher.h"
#include "update_engine/metrics.h"
@@ -134,18 +132,23 @@
return code;
}
-UpdateAttempter::UpdateAttempter(SystemState* system_state,
- DBusWrapperInterface* dbus_iface)
- : UpdateAttempter(system_state, dbus_iface, kUpdateCompletedMarker) {}
+UpdateAttempter::UpdateAttempter(
+ SystemState* system_state,
+ LibCrosProxy* libcros_proxy,
+ org::chromium::debugdProxyInterface* debugd_proxy)
+ : UpdateAttempter(system_state, libcros_proxy, debugd_proxy,
+ kUpdateCompletedMarker) {}
-UpdateAttempter::UpdateAttempter(SystemState* system_state,
- DBusWrapperInterface* dbus_iface,
- const string& update_completed_marker)
+UpdateAttempter::UpdateAttempter(
+ SystemState* system_state,
+ LibCrosProxy* libcros_proxy,
+ org::chromium::debugdProxyInterface* debugd_proxy,
+ const string& update_completed_marker)
: processor_(new ActionProcessor()),
system_state_(system_state),
- dbus_iface_(dbus_iface),
- chrome_proxy_resolver_(dbus_iface),
- update_completed_marker_(update_completed_marker) {
+ chrome_proxy_resolver_(libcros_proxy),
+ update_completed_marker_(update_completed_marker),
+ debugd_proxy_(debugd_proxy) {
if (!update_completed_marker_.empty() &&
utils::FileExists(update_completed_marker_.c_str())) {
status_ = UPDATE_STATUS_UPDATED_NEED_REBOOT;
@@ -879,35 +882,17 @@
}
bool UpdateAttempter::RequestPowerManagerReboot() {
- GError* error = nullptr;
- DBusGConnection* bus = dbus_iface_->BusGet(DBUS_BUS_SYSTEM, &error);
- if (!bus) {
- LOG(ERROR) << "Failed to get system bus: "
- << utils::GetAndFreeGError(&error);
+ org::chromium::PowerManagerProxyInterface* power_manager_proxy =
+ system_state_->power_manager_proxy();
+ if (!power_manager_proxy) {
+ LOG(WARNING) << "No PowerManager proxy defined, skipping reboot.";
return false;
}
-
LOG(INFO) << "Calling " << power_manager::kPowerManagerInterface << "."
<< power_manager::kRequestRestartMethod;
- DBusGProxy* proxy = dbus_iface_->ProxyNewForName(
- bus,
- power_manager::kPowerManagerServiceName,
- power_manager::kPowerManagerServicePath,
- power_manager::kPowerManagerInterface);
- const gboolean success = dbus_iface_->ProxyCall_1_0(
- proxy,
- power_manager::kRequestRestartMethod,
- &error,
- power_manager::REQUEST_RESTART_FOR_UPDATE);
- dbus_iface_->ProxyUnref(proxy);
-
- if (!success) {
- LOG(ERROR) << "Failed to call " << power_manager::kRequestRestartMethod
- << ": " << utils::GetAndFreeGError(&error);
- return false;
- }
-
- return true;
+ chromeos::ErrorPtr error;
+ return power_manager_proxy->RequestRestart(
+ power_manager::REQUEST_RESTART_FOR_UPDATE, &error);
}
bool UpdateAttempter::RebootDirectly() {
@@ -1649,26 +1634,11 @@
// Official images in devmode are allowed a custom update source iff the
// debugd dev tools are enabled.
- GError* error = nullptr;
- DBusGConnection* bus = dbus_iface_->BusGet(DBUS_BUS_SYSTEM, &error);
- if (!bus) {
- LOG(ERROR) << "Failed to get system bus: "
- << utils::GetAndFreeGError(&error);
+ if (!debugd_proxy_)
return false;
- }
-
- gint dev_features = debugd::DEV_FEATURES_DISABLED;
- DBusGProxy* proxy = dbus_iface_->ProxyNewForName(
- bus,
- debugd::kDebugdServiceName,
- debugd::kDebugdServicePath,
- debugd::kDebugdInterface);
- const gboolean success = dbus_iface_->ProxyCall_0_1(
- proxy,
- debugd::kQueryDevFeatures,
- &error,
- &dev_features);
- dbus_iface_->ProxyUnref(proxy);
+ int32_t dev_features = debugd::DEV_FEATURES_DISABLED;
+ chromeos::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.
diff --git a/update_attempter.h b/update_attempter.h
index ea32fe4..180b45e 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -20,6 +20,7 @@
#include "update_engine/action_processor.h"
#include "update_engine/chrome_browser_proxy_resolver.h"
#include "update_engine/download_action.h"
+#include "update_engine/libcros_proxy.h"
#include "update_engine/omaha_request_params.h"
#include "update_engine/omaha_response_handler_action.h"
#include "update_engine/proxy_resolver.h"
@@ -36,7 +37,6 @@
namespace chromeos_update_engine {
class UpdateEngineAdaptor;
-class DBusWrapperInterface;
enum UpdateStatus {
UPDATE_STATUS_IDLE = 0,
@@ -59,7 +59,8 @@
static const int kMaxDeltaUpdateFailures;
UpdateAttempter(SystemState* system_state,
- DBusWrapperInterface* dbus_iface);
+ LibCrosProxy* libcros_proxy,
+ org::chromium::debugdProxyInterface* debugd_proxy);
~UpdateAttempter() override;
// Further initialization to be done post construction.
@@ -222,7 +223,8 @@
// Special ctor + friend declarations for testing purposes.
UpdateAttempter(SystemState* system_state,
- DBusWrapperInterface* dbus_iface,
+ LibCrosProxy* libcros_proxy,
+ org::chromium::debugdProxyInterface* debugd_proxy,
const std::string& update_completed_marker);
friend class UpdateAttempterUnderTest;
@@ -395,9 +397,6 @@
// carved out separately to mock out easily in unit tests.
SystemState* system_state_;
- // Interface for getting D-Bus connections.
- DBusWrapperInterface* dbus_iface_ = nullptr;
-
// If non-null, this UpdateAttempter will send status updates over this
// dbus service.
UpdateEngineAdaptor* dbus_adaptor_ = nullptr;
@@ -507,6 +506,8 @@
std::string forced_app_version_;
std::string forced_omaha_url_;
+ org::chromium::debugdProxyInterface* debugd_proxy_;
+
DISALLOW_COPY_AND_ASSIGN(UpdateAttempter);
};
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 941f2b2..9965def 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -11,6 +11,7 @@
#include <base/files/file_util.h>
#include <chromeos/bind_lambda.h>
#include <chromeos/dbus/service_constants.h>
+#include <chromeos/make_unique_ptr.h>
#include <chromeos/message_loops/glib_message_loop.h>
#include <chromeos/message_loops/message_loop.h>
#include <chromeos/message_loops/message_loop_utils.h>
@@ -25,7 +26,6 @@
#include "update_engine/install_plan.h"
#include "update_engine/mock_action.h"
#include "update_engine/mock_action_processor.h"
-#include "update_engine/mock_dbus_wrapper.h"
#include "update_engine/mock_http_fetcher.h"
#include "update_engine/mock_p2p_manager.h"
#include "update_engine/mock_payload_state.h"
@@ -38,9 +38,10 @@
using base::Time;
using base::TimeDelta;
using chromeos::MessageLoop;
+using org::chromium::LibCrosServiceInterfaceProxyMock;
+using org::chromium::UpdateEngineLibcrosProxyResolvedInterfaceProxyMock;
using std::string;
using std::unique_ptr;
-using testing::A;
using testing::DoAll;
using testing::InSequence;
using testing::Ne;
@@ -50,7 +51,6 @@
using testing::ReturnPointee;
using testing::SaveArg;
using testing::SetArgumentPointee;
-using testing::StrEq;
using testing::_;
namespace chromeos_update_engine {
@@ -60,17 +60,12 @@
// methods.
class UpdateAttempterUnderTest : public UpdateAttempter {
public:
- // We always feed an explicit update completed marker name; however, unless
- // explicitly specified, we feed an empty string, which causes the
- // UpdateAttempter class to ignore / not write the marker file.
UpdateAttempterUnderTest(SystemState* system_state,
- DBusWrapperInterface* dbus_iface)
- : UpdateAttempterUnderTest(system_state, dbus_iface, "") {}
-
- UpdateAttempterUnderTest(SystemState* system_state,
- DBusWrapperInterface* dbus_iface,
+ LibCrosProxy* libcros_proxy,
+ org::chromium::debugdProxyInterface* debugd_proxy,
const string& update_completed_marker)
- : UpdateAttempter(system_state, dbus_iface, update_completed_marker) {}
+ : UpdateAttempter(system_state, libcros_proxy, debugd_proxy,
+ update_completed_marker) {}
// Wrap the update scheduling method, allowing us to opt out of scheduled
// updates for testing purposes.
@@ -99,9 +94,12 @@
class UpdateAttempterTest : public ::testing::Test {
protected:
UpdateAttempterTest()
- : attempter_(&fake_system_state_, &dbus_),
- fake_dbus_system_bus_(reinterpret_cast<DBusGConnection*>(1)),
- fake_dbus_debugd_proxy_(reinterpret_cast<DBusGProxy*>(2)) {
+ : service_interface_mock_(new LibCrosServiceInterfaceProxyMock()),
+ ue_proxy_resolved_interface_mock_(
+ new NiceMock<UpdateEngineLibcrosProxyResolvedInterfaceProxyMock>()),
+ libcros_proxy_(
+ chromeos::make_unique_ptr(service_interface_mock_),
+ chromeos::make_unique_ptr(ue_proxy_resolved_interface_mock_)) {
// Override system state members.
fake_system_state_.set_connection_manager(&mock_connection_manager);
fake_system_state_.set_update_attempter(&attempter_);
@@ -146,16 +144,6 @@
EXPECT_CALL(*fake_system_state_.mock_payload_state(),
GetUsingP2PForDownloading())
.WillRepeatedly(ReturnPointee(&actual_using_p2p_for_sharing_));
-
- // Set up mock debugd access over the system D-Bus. ProxyCall_0_1() also
- // needs to be mocked in any test using debugd to provide the desired value.
- ON_CALL(dbus_, BusGet(DBUS_BUS_SYSTEM, _))
- .WillByDefault(Return(fake_dbus_system_bus_));
- ON_CALL(dbus_, ProxyNewForName(fake_dbus_system_bus_,
- StrEq(debugd::kDebugdServiceName),
- StrEq(debugd::kDebugdServicePath),
- StrEq(debugd::kDebugdInterface)))
- .WillByDefault(Return(fake_dbus_debugd_proxy_));
}
void TearDown() override {
@@ -192,15 +180,19 @@
chromeos::GlibMessageLoop loop_;
FakeSystemState fake_system_state_;
- NiceMock<MockDBusWrapper> dbus_;
- UpdateAttempterUnderTest attempter_;
+ org::chromium::debugdProxyMock debugd_proxy_mock_;
+ LibCrosServiceInterfaceProxyMock* service_interface_mock_;
+ UpdateEngineLibcrosProxyResolvedInterfaceProxyMock*
+ ue_proxy_resolved_interface_mock_;
+ LibCrosProxy libcros_proxy_;
+ UpdateAttempterUnderTest attempter_{&fake_system_state_,
+ &libcros_proxy_,
+ &debugd_proxy_mock_,
+ ""};
+
NiceMock<MockActionProcessor>* processor_;
NiceMock<MockPrefs>* prefs_; // Shortcut to fake_system_state_->mock_prefs().
NiceMock<MockConnectionManager> mock_connection_manager;
- // fake_dbus_xxx pointers will be non-null for comparison purposes, but won't
- // be valid objects so don't try to use them.
- DBusGConnection* fake_dbus_system_bus_;
- DBusGProxy* fake_dbus_debugd_proxy_;
string test_dir_;
@@ -251,15 +243,18 @@
ASSERT_TRUE(attempter_.error_event_.get() == nullptr);
}
-TEST_F(UpdateAttempterTest, RunAsRootConstructWithUpdatedMarkerTest) {
+TEST_F(UpdateAttempterTest, ConstructWithUpdatedMarkerTest) {
string test_update_completed_marker;
CHECK(utils::MakeTempFile(
- "update_attempter_unittest-update_completed_marker-XXXXXX",
- &test_update_completed_marker, nullptr));
+ "update_attempter_unittest-update_completed_marker-XXXXXX",
+ &test_update_completed_marker,
+ nullptr));
ScopedPathUnlinker completed_marker_unlinker(test_update_completed_marker);
const base::FilePath marker(test_update_completed_marker);
EXPECT_EQ(0, base::WriteFile(marker, "", 0));
- UpdateAttempterUnderTest attempter(&fake_system_state_, &dbus_,
+ UpdateAttempterUnderTest attempter(&fake_system_state_,
+ nullptr,
+ &debugd_proxy_mock_,
test_update_completed_marker);
EXPECT_EQ(UPDATE_STATUS_UPDATED_NEED_REBOOT, attempter.status());
}
@@ -930,8 +925,10 @@
TEST_F(UpdateAttempterTest, BootTimeInUpdateMarkerFile) {
const string update_completed_marker = test_dir_ + "/update-completed-marker";
- UpdateAttempterUnderTest attempter(&fake_system_state_, &dbus_,
- update_completed_marker);
+ UpdateAttempterUnderTest attempter{&fake_system_state_,
+ nullptr, // libcros_proxy
+ &debugd_proxy_mock_,
+ update_completed_marker};
FakeClock fake_clock;
fake_clock.SetBootTime(Time::FromTimeT(42));
@@ -954,11 +951,8 @@
TEST_F(UpdateAttempterTest, AnyUpdateSourceAllowedOfficialDevmode) {
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
fake_system_state_.fake_hardware()->SetIsNormalBootMode(false);
- EXPECT_CALL(dbus_, ProxyCall_0_1(fake_dbus_debugd_proxy_,
- StrEq(debugd::kQueryDevFeatures),
- _, A<gint*>()))
- .WillRepeatedly(DoAll(SetArgumentPointee<3>(0),
- Return(true)));
+ EXPECT_CALL(debugd_proxy_mock_, QueryDevFeatures(_, _, _))
+ .WillRepeatedly(DoAll(SetArgumentPointee<0>(0), Return(true)));
EXPECT_TRUE(attempter_.IsAnyUpdateSourceAllowed());
}
@@ -966,10 +960,7 @@
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
fake_system_state_.fake_hardware()->SetIsNormalBootMode(true);
// debugd should not be queried in this case.
- EXPECT_CALL(dbus_, ProxyCall_0_1(fake_dbus_debugd_proxy_,
- StrEq(debugd::kQueryDevFeatures),
- _, A<gint*>()))
- .Times(0);
+ EXPECT_CALL(debugd_proxy_mock_, QueryDevFeatures(_, _, _)).Times(0);
EXPECT_FALSE(attempter_.IsAnyUpdateSourceAllowed());
}
@@ -977,20 +968,16 @@
using debugd::DEV_FEATURES_DISABLED;
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
fake_system_state_.fake_hardware()->SetIsNormalBootMode(false);
- EXPECT_CALL(dbus_, ProxyCall_0_1(fake_dbus_debugd_proxy_,
- StrEq(debugd::kQueryDevFeatures),
- _, A<gint*>()))
- .WillRepeatedly(DoAll(SetArgumentPointee<3>(DEV_FEATURES_DISABLED),
- Return(true)));
+ EXPECT_CALL(debugd_proxy_mock_, QueryDevFeatures(_, _, _))
+ .WillRepeatedly(
+ DoAll(SetArgumentPointee<0>(DEV_FEATURES_DISABLED), Return(true)));
EXPECT_FALSE(attempter_.IsAnyUpdateSourceAllowed());
}
TEST_F(UpdateAttempterTest, AnyUpdateSourceDisallowedDebugdFailure) {
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
fake_system_state_.fake_hardware()->SetIsNormalBootMode(false);
- EXPECT_CALL(dbus_, ProxyCall_0_1(fake_dbus_debugd_proxy_,
- StrEq(debugd::kQueryDevFeatures),
- _, A<gint*>()))
+ EXPECT_CALL(debugd_proxy_mock_, QueryDevFeatures(_, _, _))
.WillRepeatedly(Return(false));
EXPECT_FALSE(attempter_.IsAnyUpdateSourceAllowed());
}
diff --git a/update_engine.gyp b/update_engine.gyp
index 1ddb4d9..1482877 100644
--- a/update_engine.gyp
+++ b/update_engine.gyp
@@ -77,11 +77,11 @@
'type': 'static_library',
'dependencies': [
'update_metadata-protos',
+ 'update_engine-dbus-adaptor',
],
'variables': {
'exported_deps': [
'dbus-1',
- 'dbus-glib-1',
'glib-2.0',
'libchrome-<(libbase_ver)',
'libchromeos-<(libbase_ver)',
@@ -131,12 +131,12 @@
'file_descriptor.cc',
'file_writer.cc',
'filesystem_verifier_action.cc',
- 'glib_utils.cc',
'hardware.cc',
'http_common.cc',
'http_fetcher.cc',
'hwid_override.cc',
'install_plan.cc',
+ 'libcros_proxy.cc',
'libcurl_http_fetcher.cc',
'metrics.cc',
'multi_range_http_fetcher.cc',
@@ -152,6 +152,7 @@
'prefs.cc',
'proxy_resolver.cc',
'real_system_state.cc',
+ 'shill_proxy.cc',
'subprocess.cc',
'terminator.cc',
'update_attempter.cc',
@@ -171,6 +172,25 @@
'update_manager/update_manager.cc',
'utils.cc',
],
+ 'actions': [
+ {
+ 'action_name': 'update_engine-dbus-proxies',
+ 'variables': {
+ 'dbus_service_config': '',
+ 'mock_output_file': 'include/update_engine/dbus_mocks.h',
+ 'proxy_output_file': 'include/update_engine/dbus_proxies.h'
+ },
+ 'sources': [
+ '../debugd/share/org.chromium.debugd.xml',
+ '../login_manager/org.chromium.SessionManagerInterface.xml',
+ '../power_manager/dbus_bindings/org.chromium.PowerManager.xml',
+ '../shill/dbus_bindings/org.chromium.flimflam.Manager.xml',
+ '../shill/dbus_bindings/org.chromium.flimflam.Service.xml',
+ 'dbus_bindings/org.chromium.LibCrosService.xml',
+ ],
+ 'includes': ['../common-mk/generate-dbus-proxies.gypi'],
+ },
+ ],
'conditions': [
['USE_mtd == 1', {
'sources': [
@@ -190,7 +210,6 @@
'type': 'executable',
'dependencies': [
'libupdate_engine',
- 'update_engine-dbus-adaptor',
],
'sources': [
'main.cc',
@@ -219,10 +238,10 @@
],
'actions': [
{
- 'action_name': 'update_engine-dbus-proxies',
+ 'action_name': 'update_engine_client-dbus-proxies',
'variables': {
'dbus_service_config': 'dbus_bindings/dbus-service-config.json',
- 'proxy_output_file': 'include/update_engine/dbus_proxies.h'
+ 'proxy_output_file': 'include/update_engine/client_dbus_proxies.h'
},
'sources': [
'dbus_bindings/org.chromium.UpdateEngineInterface.xml',
@@ -373,6 +392,7 @@
'download_action_unittest.cc',
'extent_writer_unittest.cc',
'fake_prefs.cc',
+ 'fake_shill_proxy.cc',
'fake_system_state.cc',
'file_writer_unittest.cc',
'filesystem_verifier_action_unittest.cc',
diff --git a/update_engine_client.cc b/update_engine_client.cc
index d0fee7a..a2470f3 100644
--- a/update_engine_client.cc
+++ b/update_engine_client.cc
@@ -17,8 +17,8 @@
#include <chromeos/flag_helper.h>
#include <dbus/bus.h>
+#include "update_engine/client_dbus_proxies.h"
#include "update_engine/dbus_constants.h"
-#include "update_engine/dbus_proxies.h"
using chromeos_update_engine::kAttemptUpdateFlagNonInteractive;
using chromeos_update_engine::kUpdateEngineServiceName;
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