UpdateEngine-side changes to allow updates over 3G based on device policy.
Some enterprise chromebooks have only 3G and hence they need the ability
to update over 3G if the enterprise policy allows that. This CL adds
the support in update_engine to enable that.
BUG=chromium-os:31099
TEST=Tested E2E on 3G, added unit tests and did regression testing.
CQ-DEPEND=I1a55a392f3dc0f12d917eb45dcf0456b57735514
Change-Id: I121bda35e54fa6c35e002a76db198d13b72b650e
Reviewed-on: https://gerrit.chromium.org/gerrit/25470
Commit-Ready: Jay Srinivasan <jaysri@chromium.org>
Reviewed-by: Jay Srinivasan <jaysri@chromium.org>
Tested-by: Jay Srinivasan <jaysri@chromium.org>
diff --git a/SConstruct b/SConstruct
index 432bdb9..62c6470 100644
--- a/SConstruct
+++ b/SConstruct
@@ -244,6 +244,7 @@
certificate_checker.cc
chrome_browser_proxy_resolver.cc
chrome_proxy_resolver.cc
+ connection_manager.cc
cycle_breaker.cc
dbus_service.cc
delta_diff_generator.cc
@@ -256,7 +257,6 @@
filesystem_iterator.cc
file_descriptor.cc
file_writer.cc
- flimflam_proxy.cc
full_update_generator.cc
gpio_handler.cc
graph_utils.cc
@@ -276,6 +276,7 @@
proxy_resolver.cc
simple_key_value_store.cc
subprocess.cc
+ system_state.cc
tarjan.cc
terminator.cc
topological_sort.cc
@@ -292,6 +293,7 @@
certificate_checker_unittest.cc
chrome_browser_proxy_resolver_unittest.cc
chrome_proxy_resolver_unittest.cc
+ connection_manager_unittest.cc
cycle_breaker_unittest.cc
delta_diff_generator_unittest.cc
delta_performer_unittest.cc
@@ -302,7 +304,6 @@
file_writer_unittest.cc
filesystem_copier_action_unittest.cc
filesystem_iterator_unittest.cc
- flimflam_proxy_unittest.cc
full_update_generator_unittest.cc
gpio_handler_unittest.cc
gpio_mock_udev_interface.cc
diff --git a/flimflam_proxy.cc b/connection_manager.cc
similarity index 64%
rename from flimflam_proxy.cc
rename to connection_manager.cc
index 63bd623..438ed41 100644
--- a/flimflam_proxy.cc
+++ b/connection_manager.cc
@@ -1,17 +1,21 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 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/flimflam_proxy.h"
+#include "update_engine/connection_manager.h"
#include <string>
+#include <base/stl_util.h>
#include <base/string_util.h>
+#include <chromeos/dbus/service_constants.h>
#include <dbus/dbus-glib.h>
#include <glib.h>
+#include "update_engine/system_state.h"
#include "update_engine/utils.h"
+using std::set;
using std::string;
namespace chromeos_update_engine {
@@ -33,7 +37,7 @@
return false;
}
proxy = dbus_iface->ProxyNewForNameOwner(bus,
- kFlimFlamDbusService,
+ flimflam::kFlimflamServiceName,
path,
interface,
&error);
@@ -85,8 +89,8 @@
GHashTable* hash_table = NULL;
TEST_AND_RETURN_FALSE(GetProperties(dbus_iface,
- kFlimFlamDbusManagerPath,
- kFlimFlamDbusManagerInterface,
+ flimflam::kFlimflamServicePath,
+ flimflam::kFlimflamManagerInterface,
&hash_table));
GValue* value = reinterpret_cast<GValue*>(g_hash_table_lookup(hash_table,
@@ -105,15 +109,15 @@
}
NetworkConnectionType ParseConnectionType(const char* type_str) {
- if (!strcmp(type_str, kFlimFlamNetTypeEthernet)) {
+ if (!strcmp(type_str, flimflam::kTypeEthernet)) {
return kNetEthernet;
- } else if (!strcmp(type_str, kFlimFlamNetTypeWifi)) {
+ } else if (!strcmp(type_str, flimflam::kTypeWifi)) {
return kNetWifi;
- } else if (!strcmp(type_str, kFlimFlamNetTypeWimax)) {
+ } else if (!strcmp(type_str, flimflam::kTypeWimax)) {
return kNetWimax;
- } else if (!strcmp(type_str, kFlimFlamNetTypeBluetooth)) {
+ } else if (!strcmp(type_str, flimflam::kTypeBluetooth)) {
return kNetBluetooth;
- } else if (!strcmp(type_str, kFlimFlamNetTypeCellular)) {
+ } else if (!strcmp(type_str, flimflam::kTypeCellular)) {
return kNetCellular;
}
return kNetUnknown;
@@ -126,7 +130,7 @@
TEST_AND_RETURN_FALSE(GetProperties(dbus_iface,
path.c_str(),
- kFlimFlamDbusServiceInterface,
+ flimflam::kFlimflamServiceInterface,
&hash_table));
GValue* value = (GValue*)g_hash_table_lookup(hash_table, "Type");
@@ -142,20 +146,61 @@
} // namespace {}
-const char* FlimFlamProxy::StringForConnectionType(NetworkConnectionType type) {
- static const char* const kValues[] = {kFlimFlamNetTypeEthernet,
- kFlimFlamNetTypeWifi,
- kFlimFlamNetTypeWimax,
- kFlimFlamNetTypeBluetooth,
- kFlimFlamNetTypeCellular};
+ConnectionManager::ConnectionManager(SystemState *system_state)
+ : system_state_(system_state) {}
+
+bool ConnectionManager::IsUpdateAllowedOver(NetworkConnectionType type) const {
+ switch (type) {
+ case kNetBluetooth:
+ return false;
+
+ case kNetCellular: {
+ set<string> allowed_types;
+ const policy::DevicePolicy* device_policy =
+ system_state_->GetDevicePolicy();
+ if (!device_policy) {
+ LOG(INFO) << "Disabling updates over cellular connection as there's no "
+ "device policy object present";
+ return false;
+ }
+
+ if (!device_policy->GetAllowedConnectionTypesForUpdate(&allowed_types)) {
+ LOG(INFO) << "Disabling updates over cellular connection as there's no "
+ "allowed connection types from policy";
+ return false;
+ }
+
+ if (!ContainsKey(allowed_types, flimflam::kTypeCellular)) {
+ LOG(INFO) << "Disabling updates over cellular connection as it's not "
+ "allowed in the device policy.";
+ return false;
+ }
+
+ LOG(INFO) << "Allowing updates over cellular per device policy";
+ return true;
+ }
+
+ default:
+ return true;
+ }
+}
+
+const char* ConnectionManager::StringForConnectionType(
+ NetworkConnectionType type) const {
+ static const char* const kValues[] = {flimflam::kTypeEthernet,
+ flimflam::kTypeWifi,
+ flimflam::kTypeWimax,
+ flimflam::kTypeBluetooth,
+ flimflam::kTypeCellular};
if (type < 0 || type >= static_cast<int>(arraysize(kValues))) {
return "Unknown";
}
return kValues[type];
}
-bool FlimFlamProxy::GetConnectionType(DbusGlibInterface* dbus_iface,
- NetworkConnectionType* out_type) {
+bool ConnectionManager::GetConnectionType(
+ DbusGlibInterface* dbus_iface,
+ NetworkConnectionType* out_type) const {
string default_service_path;
TEST_AND_RETURN_FALSE(GetDefaultServicePath(dbus_iface,
&default_service_path));
@@ -165,15 +210,4 @@
return true;
}
-const char* kFlimFlamDbusService = "org.chromium.flimflam";
-const char* kFlimFlamDbusManagerInterface = "org.chromium.flimflam.Manager";
-const char* kFlimFlamDbusManagerPath = "/";
-const char* kFlimFlamDbusServiceInterface = "org.chromium.flimflam.Service";
-
-const char* kFlimFlamNetTypeEthernet = "ethernet";
-const char* kFlimFlamNetTypeWifi = "wifi";
-const char* kFlimFlamNetTypeWimax = "wimax";
-const char* kFlimFlamNetTypeBluetooth = "bluetooth";
-const char* kFlimFlamNetTypeCellular = "cellular";
-
} // namespace chromeos_update_engine
diff --git a/connection_manager.h b/connection_manager.h
new file mode 100644
index 0000000..6cc279e
--- /dev/null
+++ b/connection_manager.h
@@ -0,0 +1,57 @@
+// Copyright (c) 2012 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 CHROMEOS_PLATFORM_UPDATE_ENGINE_CONNECTION_MANAGER_H_
+#define CHROMEOS_PLATFORM_UPDATE_ENGINE_CONNECTION_MANAGER_H_
+
+#include <base/basictypes.h>
+
+#include "update_engine/dbus_interface.h"
+
+namespace chromeos_update_engine {
+
+enum NetworkConnectionType {
+ kNetEthernet = 0,
+ kNetWifi,
+ kNetWimax,
+ kNetBluetooth,
+ kNetCellular,
+ kNetUnknown
+};
+
+class SystemState;
+
+// This class exposes a generic interface to the connection manager
+// (e.g FlimFlam, Shill, etc.) to consolidate all connection-related
+// logic in update_engine.
+class ConnectionManager {
+ public:
+ // Constructs a new ConnectionManager object initialized with the
+ // given system state.
+ explicit ConnectionManager(SystemState* system_state);
+
+ // Populates |out_type| with the type of the network connection
+ // that we are currently connected. The dbus_iface is used to
+ // query the real connection manager (e.g shill).
+ virtual bool GetConnectionType(DbusGlibInterface* dbus_iface,
+ NetworkConnectionType* out_type) const;
+
+ // Returns true if we're allowed to update the system when we're
+ // connected to the internet through the given network connection type.
+ virtual bool IsUpdateAllowedOver(NetworkConnectionType type) const;
+
+ // Returns the string representation corresponding to the given
+ // connection type.
+ virtual const char* StringForConnectionType(NetworkConnectionType type) const;
+
+ private:
+ // The global context for update_engine
+ SystemState* system_state_;
+
+ DISALLOW_COPY_AND_ASSIGN(ConnectionManager);
+};
+
+} // namespace chromeos_update_engine
+
+#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_CONNECTION_MANAGER_H_
diff --git a/connection_manager_unittest.cc b/connection_manager_unittest.cc
new file mode 100644
index 0000000..5499d2f
--- /dev/null
+++ b/connection_manager_unittest.cc
@@ -0,0 +1,309 @@
+// Copyright (c) 2012 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 <base/logging.h>
+#include <chromeos/dbus/service_constants.h>
+#include <gtest/gtest.h>
+#include <string>
+
+#include "update_engine/connection_manager.h"
+#include "update_engine/mock_dbus_interface.h"
+#include "update_engine/mock_system_state.h"
+
+using std::set;
+using std::string;
+using testing::_;
+using testing::AnyNumber;
+using testing::Return;
+using testing::SetArgumentPointee;
+using testing::StrEq;
+
+namespace chromeos_update_engine {
+
+class ConnectionManagerTest : public ::testing::Test {
+ public:
+ ConnectionManagerTest()
+ : kMockFlimFlamManagerProxy_(NULL),
+ kMockFlimFlamServiceProxy_(NULL),
+ kServicePath_(NULL),
+ cmut_(&mock_system_state_) {
+ mock_system_state_.SetConnectionManager(&cmut_);
+ }
+
+ protected:
+ void SetupMocks(const char* service_path);
+ void SetManagerReply(gconstpointer value, const GType& type);
+ void SetServiceReply(const char* service_type);
+ void TestWithServiceType(
+ const char* service_type, NetworkConnectionType expected_type);
+
+ static const char* kGetPropertiesMethod;
+ DBusGProxy* kMockFlimFlamManagerProxy_;
+ DBusGProxy* kMockFlimFlamServiceProxy_;
+ DBusGConnection* kMockSystemBus_;
+ const char* kServicePath_;
+ MockDbusGlib dbus_iface_;
+ ConnectionManager cmut_; // ConnectionManager under test.
+ MockSystemState mock_system_state_;
+};
+
+// static
+const char* ConnectionManagerTest::kGetPropertiesMethod = "GetProperties";
+
+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_, reinterpret_cast<DBusGConnection*>(NULL));
+
+ kServicePath_ = service_path;
+
+ ON_CALL(dbus_iface_, BusGet(DBUS_BUS_SYSTEM, _))
+ .WillByDefault(Return(kMockSystemBus_));
+ EXPECT_CALL(dbus_iface_, BusGet(DBUS_BUS_SYSTEM, _))
+ .Times(AnyNumber());
+}
+
+void ConnectionManagerTest::SetManagerReply(gconstpointer reply_value,
+ const GType& reply_type) {
+ // Initialize return value for D-Bus call to Manager object.
+ // TODO (jaysri): Free the objects allocated here.
+ GHashTable* manager_hash_table = g_hash_table_new(g_str_hash, g_str_equal);
+
+ GArray* array = g_array_new(FALSE, FALSE, sizeof(const char*));
+ ASSERT_TRUE(array != NULL);
+
+ EXPECT_EQ(array, g_array_append_val(array, 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);
+ 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(kMockFlimFlamManagerProxy_,
+ StrEq(kGetPropertiesMethod),
+ _,
+ G_TYPE_INVALID,
+ dbus_g_type_get_map("GHashTable",
+ G_TYPE_STRING,
+ G_TYPE_VALUE),
+ _,
+ G_TYPE_INVALID))
+ .WillOnce(DoAll(SetArgumentPointee<5>(manager_hash_table), Return(TRUE)));
+
+ // Set other expectations.
+ EXPECT_CALL(dbus_iface_,
+ ProxyNewForNameOwner(kMockSystemBus_,
+ StrEq(flimflam::kFlimflamServiceName),
+ StrEq(flimflam::kFlimflamServicePath),
+ StrEq(flimflam::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) {
+ // Initialize return value for D-Bus call to Service object.
+ // TODO (jaysri): Free the objects allocated here.
+ GHashTable* service_hash_table = g_hash_table_new(g_str_hash, g_str_equal);
+
+ GValue* service_type_value = g_new0(GValue, 1);
+ EXPECT_EQ(service_type_value,
+ g_value_init(service_type_value, G_TYPE_STRING));
+ g_value_set_static_string(service_type_value, service_type);
+
+ g_hash_table_insert(service_hash_table,
+ const_cast<char*>("Type"),
+ service_type_value);
+
+ // Plumb return value into mock object.
+ EXPECT_CALL(dbus_iface_, ProxyCall(kMockFlimFlamServiceProxy_,
+ StrEq(kGetPropertiesMethod),
+ _,
+ G_TYPE_INVALID,
+ dbus_g_type_get_map("GHashTable",
+ G_TYPE_STRING,
+ G_TYPE_VALUE),
+ _,
+ G_TYPE_INVALID))
+ .WillOnce(DoAll(SetArgumentPointee<5>(service_hash_table), Return(TRUE)));
+
+ // Set other expectations.
+ EXPECT_CALL(dbus_iface_,
+ ProxyNewForNameOwner(kMockSystemBus_,
+ StrEq(flimflam::kFlimflamServiceName),
+ StrEq(kServicePath_),
+ StrEq(flimflam::kFlimflamServiceInterface),
+ _))
+ .WillOnce(Return(kMockFlimFlamServiceProxy_));
+ EXPECT_CALL(dbus_iface_, ProxyUnref(kMockFlimFlamServiceProxy_));
+ EXPECT_CALL(dbus_iface_, BusGet(DBUS_BUS_SYSTEM, _))
+ .RetiresOnSaturation();
+}
+
+void ConnectionManagerTest::TestWithServiceType(
+ const char* service_type,
+ NetworkConnectionType expected_type) {
+
+ SetupMocks("/service/guest-network");
+ SetManagerReply(kServicePath_, DBUS_TYPE_G_OBJECT_PATH_ARRAY);
+ SetServiceReply(service_type);
+
+ NetworkConnectionType type;
+ EXPECT_TRUE(cmut_.GetConnectionType(&dbus_iface_, &type));
+ EXPECT_EQ(expected_type, type);
+}
+
+TEST_F(ConnectionManagerTest, SimpleTest) {
+ TestWithServiceType(flimflam::kTypeEthernet, kNetEthernet);
+ TestWithServiceType(flimflam::kTypeWifi, kNetWifi);
+ TestWithServiceType(flimflam::kTypeWimax, kNetWimax);
+ TestWithServiceType(flimflam::kTypeBluetooth, kNetBluetooth);
+ TestWithServiceType(flimflam::kTypeCellular, kNetCellular);
+}
+
+TEST_F(ConnectionManagerTest, UnknownTest) {
+ TestWithServiceType("foo", kNetUnknown);
+}
+
+TEST_F(ConnectionManagerTest, AllowUpdatesOverEthernetTest) {
+ EXPECT_CALL(mock_system_state_, GetDevicePolicy()).Times(0);
+
+ // Updates over Ethernet are allowed even if there's no policy.
+ EXPECT_TRUE(cmut_.IsUpdateAllowedOver(kNetEthernet));
+}
+
+TEST_F(ConnectionManagerTest, AllowUpdatesOverWifiTest) {
+ EXPECT_CALL(mock_system_state_, GetDevicePolicy()).Times(0);
+ EXPECT_TRUE(cmut_.IsUpdateAllowedOver(kNetWifi));
+}
+
+TEST_F(ConnectionManagerTest, AllowUpdatesOverWimaxTest) {
+ EXPECT_CALL(mock_system_state_, GetDevicePolicy()).Times(0);
+ EXPECT_TRUE(cmut_.IsUpdateAllowedOver(kNetWimax));
+}
+
+TEST_F(ConnectionManagerTest, BlockUpdatesOverBluetoothTest) {
+ EXPECT_CALL(mock_system_state_, GetDevicePolicy()).Times(0);
+ EXPECT_FALSE(cmut_.IsUpdateAllowedOver(kNetBluetooth));
+}
+
+TEST_F(ConnectionManagerTest, AllowUpdatesOnlyOver3GPerPolicyTest) {
+ policy::MockDevicePolicy allow_3g_policy;
+
+ EXPECT_CALL(mock_system_state_, GetDevicePolicy())
+ .Times(1)
+ .WillOnce(Return(&allow_3g_policy));
+
+ // This test tests 3G being the only connection type being allowed.
+ set<string> allowed_set;
+ allowed_set.insert(cmut_.StringForConnectionType(kNetCellular));
+
+ EXPECT_CALL(allow_3g_policy, GetAllowedConnectionTypesForUpdate(_))
+ .Times(1)
+ .WillOnce(DoAll(SetArgumentPointee<0>(allowed_set), Return(true)));
+
+ EXPECT_TRUE(cmut_.IsUpdateAllowedOver(kNetCellular));
+}
+
+TEST_F(ConnectionManagerTest, AllowUpdatesOver3GAndOtherTypesPerPolicyTest) {
+ policy::MockDevicePolicy allow_3g_policy;
+
+ EXPECT_CALL(mock_system_state_, GetDevicePolicy())
+ .Times(1)
+ .WillOnce(Return(&allow_3g_policy));
+
+ // This test tests multiple connection types being allowed, with
+ // 3G one among them.
+ set<string> allowed_set;
+ allowed_set.insert(cmut_.StringForConnectionType(kNetEthernet));
+ allowed_set.insert(cmut_.StringForConnectionType(kNetCellular));
+ allowed_set.insert(cmut_.StringForConnectionType(kNetWifi));
+
+ EXPECT_CALL(allow_3g_policy, GetAllowedConnectionTypesForUpdate(_))
+ .Times(1)
+ .WillOnce(DoAll(SetArgumentPointee<0>(allowed_set), Return(true)));
+
+ EXPECT_TRUE(cmut_.IsUpdateAllowedOver(kNetCellular));
+}
+
+TEST_F(ConnectionManagerTest, BlockUpdatesOver3GByDefaultTest) {
+ EXPECT_CALL(mock_system_state_, GetDevicePolicy()).Times(1);
+ EXPECT_FALSE(cmut_.IsUpdateAllowedOver(kNetCellular));
+}
+
+TEST_F(ConnectionManagerTest, BlockUpdatesOver3GPerPolicyTest) {
+ policy::MockDevicePolicy block_3g_policy;
+
+ EXPECT_CALL(mock_system_state_, GetDevicePolicy())
+ .Times(1)
+ .WillOnce(Return(&block_3g_policy));
+
+ // Test that updates for 3G are blocked while updates are allowed
+ // over several other types.
+ set<string> allowed_set;
+ allowed_set.insert(cmut_.StringForConnectionType(kNetEthernet));
+ allowed_set.insert(cmut_.StringForConnectionType(kNetWifi));
+ allowed_set.insert(cmut_.StringForConnectionType(kNetWimax));
+
+ EXPECT_CALL(block_3g_policy, GetAllowedConnectionTypesForUpdate(_))
+ .Times(1)
+ .WillOnce(DoAll(SetArgumentPointee<0>(allowed_set), Return(true)));
+
+ EXPECT_FALSE(cmut_.IsUpdateAllowedOver(kNetCellular));
+}
+
+TEST_F(ConnectionManagerTest, BlockUpdatesOver3GIfErrorInPolicyFetchTest) {
+ policy::MockDevicePolicy allow_3g_policy;
+
+ EXPECT_CALL(mock_system_state_, GetDevicePolicy())
+ .Times(1)
+ .WillOnce(Return(&allow_3g_policy));
+
+ set<string> allowed_set;
+ allowed_set.insert(cmut_.StringForConnectionType(kNetCellular));
+
+ // Return false for GetAllowedConnectionTypesForUpdate and see
+ // that updates are still blocked for 3G despite the value being in
+ // the string set above.
+ EXPECT_CALL(allow_3g_policy, GetAllowedConnectionTypesForUpdate(_))
+ .Times(1)
+ .WillOnce(DoAll(SetArgumentPointee<0>(allowed_set), Return(false)));
+
+ EXPECT_FALSE(cmut_.IsUpdateAllowedOver(kNetCellular));
+}
+
+TEST_F(ConnectionManagerTest, StringForConnectionTypeTest) {
+ EXPECT_STREQ(flimflam::kTypeEthernet,
+ cmut_.StringForConnectionType(kNetEthernet));
+ EXPECT_STREQ(flimflam::kTypeWifi,
+ cmut_.StringForConnectionType(kNetWifi));
+ EXPECT_STREQ(flimflam::kTypeWimax,
+ cmut_.StringForConnectionType(kNetWimax));
+ EXPECT_STREQ(flimflam::kTypeBluetooth,
+ cmut_.StringForConnectionType(kNetBluetooth));
+ EXPECT_STREQ(flimflam::kTypeCellular,
+ cmut_.StringForConnectionType(kNetCellular));
+ EXPECT_STREQ("Unknown",
+ cmut_.StringForConnectionType(kNetUnknown));
+ EXPECT_STREQ("Unknown",
+ cmut_.StringForConnectionType(
+ static_cast<NetworkConnectionType>(999999)));
+}
+
+TEST_F(ConnectionManagerTest, MalformedServiceList) {
+ SetupMocks("/service/guest-network");
+ string service_name(kServicePath_);
+ SetManagerReply(&service_name, DBUS_TYPE_G_STRING_ARRAY);
+
+ NetworkConnectionType type;
+ EXPECT_FALSE(cmut_.GetConnectionType(&dbus_iface_, &type));
+}
+
+} // namespace chromeos_update_engine
diff --git a/flimflam_proxy.h b/flimflam_proxy.h
deleted file mode 100644
index b571100..0000000
--- a/flimflam_proxy.h
+++ /dev/null
@@ -1,53 +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 CHROMEOS_PLATFORM_UPDATE_ENGINE_FLIMFLAM_PROXY_H__
-#define CHROMEOS_PLATFORM_UPDATE_ENGINE_FLIMFLAM_PROXY_H__
-
-// This class interfaces with FlimFlam to find out data about connectivity.
-
-#include <base/basictypes.h>
-
-#include "update_engine/dbus_interface.h"
-
-namespace chromeos_update_engine {
-
-extern const char* kFlimFlamDbusService;
-extern const char* kFlimFlamDbusManagerInterface;
-extern const char* kFlimFlamDbusManagerPath;
-extern const char* kFlimFlamDbusServiceInterface;
-
-extern const char* kFlimFlamNetTypeEthernet;
-extern const char* kFlimFlamNetTypeWifi;
-extern const char* kFlimFlamNetTypeWimax;
-extern const char* kFlimFlamNetTypeBluetooth;
-extern const char* kFlimFlamNetTypeCellular;
-
-enum NetworkConnectionType {
- kNetEthernet = 0,
- kNetWifi,
- kNetWimax,
- kNetBluetooth,
- kNetCellular,
- kNetUnknown
-};
-
-class FlimFlamProxy {
- public:
- static bool GetConnectionType(DbusGlibInterface* dbus_iface,
- NetworkConnectionType* out_type);
-
- static bool IsExpensiveConnectionType(NetworkConnectionType type) {
- return type == kNetBluetooth || type == kNetCellular;
- }
- static const char* StringForConnectionType(NetworkConnectionType type);
-
- private:
- // Should never be allocated
- DISALLOW_IMPLICIT_CONSTRUCTORS(FlimFlamProxy);
-};
-
-} // namespace chromeos_update_engine
-
-#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_FLIMFLAM_PROXY_H__
diff --git a/flimflam_proxy_unittest.cc b/flimflam_proxy_unittest.cc
deleted file mode 100644
index fbc5b86..0000000
--- a/flimflam_proxy_unittest.cc
+++ /dev/null
@@ -1,212 +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.
-
-#include <base/logging.h>
-#include <gtest/gtest.h>
-#include <string>
-
-#include "update_engine/flimflam_proxy.h"
-#include "update_engine/mock_dbus_interface.h"
-
-using ::testing::_;
-using ::testing::AnyNumber;
-using ::testing::Return;
-using ::testing::SetArgumentPointee;
-using ::testing::StrEq;
-
-namespace chromeos_update_engine {
-
-template<typename T, void F(T)>
-class ScopedRelease {
- public:
- ScopedRelease(T obj) : obj_(obj) {}
- ~ScopedRelease() {
- F(obj_);
- }
-
- private:
- T obj_;
-};
-
-class FlimFlamProxyTest : public ::testing::Test {
- public:
- FlimFlamProxyTest()
- : kMockFlimFlamManagerProxy_(NULL),
- kMockFlimFlamServiceProxy_(NULL),
- kServicePath_(NULL) {}
-
- protected:
- void SetupMocks(const char* service_path);
- void SetManagerReply(gconstpointer value, const GType& type);
- void SetServiceReply(const char* service_type);
- void TestWithServiceType(
- const char* service_type, NetworkConnectionType expected_type);
-
- static const char* kGetPropertiesMethod;
- DBusGProxy* kMockFlimFlamManagerProxy_;
- DBusGProxy* kMockFlimFlamServiceProxy_;
- DBusGConnection* kMockSystemBus_;
- const char* kServicePath_;
- MockDbusGlib dbus_iface_;
-};
-
-// static
-const char* FlimFlamProxyTest::kGetPropertiesMethod = "GetProperties";
-
-void FlimFlamProxyTest::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_, reinterpret_cast<DBusGConnection*>(NULL));
-
- kServicePath_ = service_path;
-
- ON_CALL(dbus_iface_, BusGet(DBUS_BUS_SYSTEM, _))
- .WillByDefault(Return(kMockSystemBus_));
- EXPECT_CALL(dbus_iface_, BusGet(DBUS_BUS_SYSTEM, _))
- .Times(AnyNumber());
-}
-
-void FlimFlamProxyTest::SetManagerReply(gconstpointer reply_value,
- const GType &reply_type) {
- // Initialize return value for D-Bus call to Manager object.
- GHashTable* manager_hash_table = g_hash_table_new(g_str_hash, g_str_equal);
-
- GArray* array = g_array_new(FALSE, FALSE, sizeof(const char *));
- ASSERT_TRUE(array != NULL);
-
- EXPECT_EQ(array, g_array_append_val(array, 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);
- 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(kMockFlimFlamManagerProxy_,
- StrEq(kGetPropertiesMethod),
- _,
- G_TYPE_INVALID,
- dbus_g_type_get_map("GHashTable",
- G_TYPE_STRING,
- G_TYPE_VALUE),
- _,
- G_TYPE_INVALID))
- .WillOnce(DoAll(SetArgumentPointee<5>(manager_hash_table), Return(TRUE)));
-
- // Set other expectations.
- EXPECT_CALL(dbus_iface_,
- ProxyNewForNameOwner(kMockSystemBus_,
- StrEq(kFlimFlamDbusService),
- StrEq(kFlimFlamDbusManagerPath),
- StrEq(kFlimFlamDbusManagerInterface),
- _))
- .WillOnce(Return(kMockFlimFlamManagerProxy_));
- EXPECT_CALL(dbus_iface_, ProxyUnref(kMockFlimFlamManagerProxy_));
- EXPECT_CALL(dbus_iface_, BusGet(DBUS_BUS_SYSTEM, _))
- .RetiresOnSaturation();
-}
-
-void FlimFlamProxyTest::SetServiceReply(const char *service_type) {
- // Initialize return value for D-Bus call to Service object.
- GHashTable* service_hash_table = g_hash_table_new(g_str_hash, g_str_equal);
-
- GValue* service_type_value = g_new0(GValue, 1);
- EXPECT_EQ(service_type_value,
- g_value_init(service_type_value, G_TYPE_STRING));
- g_value_set_static_string(service_type_value, service_type);
-
- g_hash_table_insert(service_hash_table,
- const_cast<char*>("Type"),
- service_type_value);
-
- // Plumb return value into mock object.
- EXPECT_CALL(dbus_iface_, ProxyCall(kMockFlimFlamServiceProxy_,
- StrEq(kGetPropertiesMethod),
- _,
- G_TYPE_INVALID,
- dbus_g_type_get_map("GHashTable",
- G_TYPE_STRING,
- G_TYPE_VALUE),
- _,
- G_TYPE_INVALID))
- .WillOnce(DoAll(SetArgumentPointee<5>(service_hash_table), Return(TRUE)));
-
- // Set other expectations.
- EXPECT_CALL(dbus_iface_,
- ProxyNewForNameOwner(kMockSystemBus_,
- StrEq(kFlimFlamDbusService),
- StrEq(kServicePath_),
- StrEq(kFlimFlamDbusServiceInterface),
- _))
- .WillOnce(Return(kMockFlimFlamServiceProxy_));
- EXPECT_CALL(dbus_iface_, ProxyUnref(kMockFlimFlamServiceProxy_));
- EXPECT_CALL(dbus_iface_, BusGet(DBUS_BUS_SYSTEM, _))
- .RetiresOnSaturation();
-}
-
-void FlimFlamProxyTest::TestWithServiceType(
- const char* service_type,
- NetworkConnectionType expected_type) {
-
- SetupMocks("/service/guest-network");
- SetManagerReply(kServicePath_, DBUS_TYPE_G_OBJECT_PATH_ARRAY);
- SetServiceReply(service_type);
-
- NetworkConnectionType type;
- EXPECT_TRUE(FlimFlamProxy::GetConnectionType(&dbus_iface_, &type));
- EXPECT_EQ(expected_type, type);
-}
-
-TEST_F(FlimFlamProxyTest, SimpleTest) {
- TestWithServiceType(kFlimFlamNetTypeEthernet, kNetEthernet);
- TestWithServiceType(kFlimFlamNetTypeWifi, kNetWifi);
- TestWithServiceType(kFlimFlamNetTypeWimax, kNetWimax);
- TestWithServiceType(kFlimFlamNetTypeBluetooth, kNetBluetooth);
- TestWithServiceType(kFlimFlamNetTypeCellular, kNetCellular);
-}
-
-TEST_F(FlimFlamProxyTest, UnknownTest) {
- TestWithServiceType("foo", kNetUnknown);
-}
-
-TEST_F(FlimFlamProxyTest, ExpensiveConnectionsTest) {
- EXPECT_FALSE(FlimFlamProxy::IsExpensiveConnectionType(kNetEthernet));
- EXPECT_FALSE(FlimFlamProxy::IsExpensiveConnectionType(kNetWifi));
- EXPECT_FALSE(FlimFlamProxy::IsExpensiveConnectionType(kNetWimax));
- EXPECT_TRUE(FlimFlamProxy::IsExpensiveConnectionType(kNetBluetooth));
- EXPECT_TRUE(FlimFlamProxy::IsExpensiveConnectionType(kNetCellular));
-}
-
-TEST_F(FlimFlamProxyTest, StringForConnectionTypeTest) {
- EXPECT_STREQ(kFlimFlamNetTypeEthernet,
- FlimFlamProxy::StringForConnectionType(kNetEthernet));
- EXPECT_STREQ(kFlimFlamNetTypeWifi,
- FlimFlamProxy::StringForConnectionType(kNetWifi));
- EXPECT_STREQ(kFlimFlamNetTypeWimax,
- FlimFlamProxy::StringForConnectionType(kNetWimax));
- EXPECT_STREQ(kFlimFlamNetTypeBluetooth,
- FlimFlamProxy::StringForConnectionType(kNetBluetooth));
- EXPECT_STREQ(kFlimFlamNetTypeCellular,
- FlimFlamProxy::StringForConnectionType(kNetCellular));
- EXPECT_STREQ("Unknown",
- FlimFlamProxy::StringForConnectionType(kNetUnknown));
- EXPECT_STREQ("Unknown",
- FlimFlamProxy::StringForConnectionType(
- static_cast<NetworkConnectionType>(999999)));
-}
-
-TEST_F(FlimFlamProxyTest, MalformedServiceList) {
- SetupMocks("/service/guest-network");
- std::string service_name(kServicePath_);
- SetManagerReply(&service_name, DBUS_TYPE_G_STRING_ARRAY);
-
- NetworkConnectionType type;
- EXPECT_FALSE(FlimFlamProxy::GetConnectionType(&dbus_iface_, &type));
-}
-
-} // namespace chromeos_update_engine
diff --git a/http_fetcher.h b/http_fetcher.h
index 2ca6fe2..0078a49 100644
--- a/http_fetcher.h
+++ b/http_fetcher.h
@@ -16,6 +16,7 @@
#include "http_common.h"
#include "update_engine/proxy_resolver.h"
+#include "update_engine/system_state.h"
// This class is a simple wrapper around an HTTP library (libcurl). We can
// easily mock out this interface for testing.
@@ -33,14 +34,15 @@
// |proxy_resolver| is the resolver that will be consulted for proxy
// settings. It may be null, in which case direct connections will
// be used. Does not take ownership of the resolver.
- explicit HttpFetcher(ProxyResolver* proxy_resolver)
+ HttpFetcher(ProxyResolver* proxy_resolver, SystemState* system_state)
: post_data_set_(false),
http_response_code_(0),
delegate_(NULL),
proxies_(1, kNoProxy),
proxy_resolver_(proxy_resolver),
no_resolver_idle_id_(0),
- callback_(NULL) {}
+ callback_(NULL),
+ system_state_(system_state) {}
virtual ~HttpFetcher();
void set_delegate(HttpFetcherDelegate* delegate) { delegate_ = delegate; }
@@ -59,7 +61,7 @@
// Returns true on success.
bool ResolveProxiesForUrl(const std::string& url,
google::protobuf::Closure* callback);
-
+
void SetProxies(const std::deque<std::string>& proxies) {
proxies_ = proxies;
}
@@ -106,9 +108,12 @@
ProxyResolver* proxy_resolver() const { return proxy_resolver_; }
// These are used for testing:
- virtual void SetConnectionAsExpensive(bool is_expensive) {}
virtual void SetBuildType(bool is_official) {}
+ SystemState* GetSystemState() {
+ return system_state_;
+ }
+
protected:
// The URL we're actively fetching from
std::string url_;
@@ -137,6 +142,9 @@
// Callback for when we are resolving proxies
google::protobuf::Closure* callback_;
+ // Global system context.
+ SystemState* system_state_;
+
private:
// Callback from the proxy resolver
void ProxiesResolved(const std::deque<std::string>& proxies);
@@ -144,7 +152,7 @@
void* data) {
reinterpret_cast<HttpFetcher*>(data)->ProxiesResolved(proxies);
}
-
+
DISALLOW_COPY_AND_ASSIGN(HttpFetcher);
};
diff --git a/http_fetcher_unittest.cc b/http_fetcher_unittest.cc
index 2a0d5a6..f2f1272 100644
--- a/http_fetcher_unittest.cc
+++ b/http_fetcher_unittest.cc
@@ -12,12 +12,14 @@
#include <base/memory/scoped_ptr.h>
#include <base/string_util.h>
#include <base/stringprintf.h>
+#include <chromeos/dbus/service_constants.h>
#include <glib.h>
#include <gtest/gtest.h>
#include "update_engine/http_common.h"
#include "update_engine/http_fetcher_unittest.h"
#include "update_engine/libcurl_http_fetcher.h"
+#include "update_engine/mock_connection_manager.h"
#include "update_engine/mock_http_fetcher.h"
#include "update_engine/mock_system_state.h"
#include "update_engine/multi_range_http_fetcher.h"
@@ -29,6 +31,11 @@
using std::string;
using std::vector;
+using testing::_;
+using testing::SetArgumentPointee;
+using testing::DoAll;
+using testing::Return;
+
namespace {
const int kBigLength = 100000;
@@ -44,10 +51,9 @@
static const char *kUnusedUrl = "unused://unused";
static inline string LocalServerUrlForPath(const string& path) {
- return (string("http://127.0.0.1:") + base::StringPrintf("%d", kServerPort) + path);
+ return base::StringPrintf("http://127.0.0.1:%d%s", kServerPort, path.c_str());
}
-
//
// Class hierarchy for HTTP server implementations.
//
@@ -131,15 +137,18 @@
bool validate_quit_;
};
-
-
//
// Class hierarchy for HTTP fetcher test wrappers.
//
class AnyHttpFetcherTest {
public:
- virtual HttpFetcher* NewLargeFetcher(size_t num_proxies) = 0;
+ AnyHttpFetcherTest()
+ : mock_connection_manager_(&mock_system_state_) {
+ mock_system_state_.SetConnectionManager(&mock_connection_manager_);
+ }
+
+ virtual HttpFetcher* NewLargeFetcher(size_t num_proxies) = 0;
HttpFetcher* NewLargeFetcher() {
return NewLargeFetcher(1);
}
@@ -162,6 +171,8 @@
protected:
DirectProxyResolver proxy_resolver_;
+ MockSystemState mock_system_state_;
+ MockConnectionManager mock_connection_manager_;
};
class MockHttpFetcherTest : public AnyHttpFetcherTest {
@@ -210,7 +221,6 @@
// Speed up test execution.
ret->set_idle_seconds(1);
ret->set_retry_seconds(1);
- ret->SetConnectionAsExpensive(false);
ret->SetBuildType(false);
return ret;
}
@@ -243,8 +253,6 @@
virtual HttpServer *CreateServer() {
return new PythonHttpServer;
}
-
- MockSystemState mock_system_state_;
};
class MultiRangeHttpFetcherTest : public LibcurlHttpFetcherTest {
@@ -263,7 +271,6 @@
// Speed up test execution.
ret->set_idle_seconds(1);
ret->set_retry_seconds(1);
- ret->SetConnectionAsExpensive(false);
ret->SetBuildType(false);
return ret;
}
@@ -368,6 +375,15 @@
scoped_ptr<HttpFetcher> fetcher(this->test_.NewSmallFetcher());
fetcher->set_delegate(&delegate);
+ MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
+ fetcher->GetSystemState()->GetConnectionManager());
+ EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
+ .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWifi), Return(true)));
+ EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWifi))
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mock_cm, StringForConnectionType(kNetWifi))
+ .WillRepeatedly(Return(flimflam::kTypeWifi));
+
scoped_ptr<HttpServer> server(this->test_.CreateServer());
ASSERT_TRUE(server->started_);
@@ -387,6 +403,15 @@
scoped_ptr<HttpFetcher> fetcher(this->test_.NewLargeFetcher());
fetcher->set_delegate(&delegate);
+ MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
+ fetcher->GetSystemState()->GetConnectionManager());
+ EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
+ .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetEthernet), Return(true)));
+ EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetEthernet))
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mock_cm, StringForConnectionType(kNetEthernet))
+ .WillRepeatedly(Return(flimflam::kTypeEthernet));
+
scoped_ptr<HttpServer> server(this->test_.CreateServer());
ASSERT_TRUE(server->started_);
@@ -414,6 +439,15 @@
scoped_ptr<HttpFetcher> fetcher(this->test_.NewSmallFetcher());
fetcher->set_delegate(&delegate);
+ MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
+ fetcher->GetSystemState()->GetConnectionManager());
+ EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
+ .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWimax), Return(true)));
+ EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWimax))
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mock_cm, StringForConnectionType(kNetWimax))
+ .WillRepeatedly(Return(flimflam::kTypeWimax));
+
scoped_ptr<HttpServer> server(this->test_.CreateServer());
ASSERT_TRUE(server->started_);
@@ -484,6 +518,15 @@
delegate.fetcher_ = fetcher.get();
fetcher->set_delegate(&delegate);
+ MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
+ fetcher->GetSystemState()->GetConnectionManager());
+ EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
+ .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetCellular), Return(true)));
+ EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetCellular))
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mock_cm, StringForConnectionType(kNetCellular))
+ .WillRepeatedly(Return(flimflam::kTypeCellular));
+
scoped_ptr<HttpServer> server(this->test_.CreateServer());
ASSERT_TRUE(server->started_);
@@ -551,6 +594,15 @@
delegate.loop_ = loop;
delegate.fetcher_->set_delegate(&delegate);
+ MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
+ delegate.fetcher_->GetSystemState()->GetConnectionManager());
+ EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
+ .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWifi), Return(true)));
+ EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWifi))
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mock_cm, StringForConnectionType(kNetWifi))
+ .WillRepeatedly(Return(flimflam::kTypeWifi));
+
scoped_ptr<HttpServer> server(this->test_.CreateServer());
this->test_.IgnoreServerAborting(server.get());
ASSERT_TRUE(server->started_);
@@ -600,6 +652,16 @@
scoped_ptr<HttpFetcher> fetcher(this->test_.NewSmallFetcher());
fetcher->set_delegate(&delegate);
+ MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
+ fetcher->GetSystemState()->GetConnectionManager());
+ EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
+ .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWifi), Return(true)));
+ EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWifi))
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mock_cm, StringForConnectionType(kNetWifi))
+ .WillRepeatedly(Return(flimflam::kTypeWifi));
+
+
scoped_ptr<HttpServer> server(this->test_.CreateServer());
ASSERT_TRUE(server->started_);
@@ -661,6 +723,16 @@
scoped_ptr<HttpFetcher> fetcher(this->test_.NewSmallFetcher());
fetcher->set_delegate(&delegate);
+ MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
+ fetcher->GetSystemState()->GetConnectionManager());
+ EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
+ .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetEthernet), Return(true)));
+ EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetEthernet))
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mock_cm, StringForConnectionType(kNetEthernet))
+ .WillRepeatedly(Return(flimflam::kTypeEthernet));
+
+
StartTransferArgs start_xfer_args = {
fetcher.get(),
LocalServerUrlForPath(this->test_.SmallUrl())
@@ -743,6 +815,15 @@
scoped_ptr<HttpFetcher> fetcher(http_fetcher);
fetcher->set_delegate(&delegate);
+ MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
+ fetcher->GetSystemState()->GetConnectionManager());
+ EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
+ .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetEthernet), Return(true)));
+ EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetEthernet))
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mock_cm, StringForConnectionType(kNetEthernet))
+ .WillRepeatedly(Return(flimflam::kTypeEthernet));
+
StartTransferArgs start_xfer_args =
{ fetcher.get(), LocalServerUrlForPath(url) };
@@ -850,6 +931,16 @@
MultiHttpFetcherTestDelegate delegate(expected_response_code);
delegate.loop_ = loop;
delegate.fetcher_.reset(fetcher_in);
+
+ MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
+ fetcher_in->GetSystemState()->GetConnectionManager());
+ EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
+ .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWifi), Return(true)));
+ EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWifi))
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*mock_cm, StringForConnectionType(kNetWifi))
+ .WillRepeatedly(Return(flimflam::kTypeWifi));
+
MultiRangeHttpFetcher* multi_fetcher =
dynamic_cast<MultiRangeHttpFetcher*>(fetcher_in);
ASSERT_TRUE(multi_fetcher);
@@ -866,7 +957,6 @@
}
LOG(INFO) << "added range: " << tmp_str;
}
- multi_fetcher->SetConnectionAsExpensive(false);
multi_fetcher->SetBuildType(false);
multi_fetcher->set_delegate(&delegate);
@@ -1035,15 +1125,21 @@
BlockedTransferTestDelegate delegate;
delegate.loop_ = loop;
+ bool is_allowed = (i != 0);
scoped_ptr<HttpFetcher> fetcher(this->test_.NewLargeFetcher());
- LibcurlHttpFetcher* curl_fetcher =
- dynamic_cast<LibcurlHttpFetcher*>(fetcher.get());
- bool is_expensive_connection = (i == 0);
+ MockConnectionManager* mock_cm = dynamic_cast<MockConnectionManager*>(
+ fetcher->GetSystemState()->GetConnectionManager());
+ EXPECT_CALL(*mock_cm, GetConnectionType(_,_))
+ .WillRepeatedly(DoAll(SetArgumentPointee<1>(kNetWifi), Return(true)));
+ EXPECT_CALL(*mock_cm, IsUpdateAllowedOver(kNetWifi))
+ .WillRepeatedly(Return(is_allowed));
+ EXPECT_CALL(*mock_cm, StringForConnectionType(kNetWifi))
+ .WillRepeatedly(Return(flimflam::kTypeWifi));
+
bool is_official_build = (i == 1);
- LOG(INFO) << "is_expensive_connection: " << is_expensive_connection;
+ LOG(INFO) << "is_update_allowed_over_connection: " << is_allowed;
LOG(INFO) << "is_official_build: " << is_official_build;
- curl_fetcher->SetConnectionAsExpensive(is_expensive_connection);
- curl_fetcher->SetBuildType(is_official_build);
+ fetcher->SetBuildType(is_official_build);
fetcher->set_delegate(&delegate);
StartTransferArgs start_xfer_args =
diff --git a/libcurl_http_fetcher.cc b/libcurl_http_fetcher.cc
index 206daca..6153f4c 100644
--- a/libcurl_http_fetcher.cc
+++ b/libcurl_http_fetcher.cc
@@ -13,7 +13,6 @@
#include "update_engine/certificate_checker.h"
#include "update_engine/chrome_proxy_resolver.h"
#include "update_engine/dbus_interface.h"
-#include "update_engine/flimflam_proxy.h"
#include "update_engine/utils.h"
using google::protobuf::NewCallback;
@@ -42,15 +41,17 @@
}
// On error, returns false.
-bool LibcurlHttpFetcher::ConnectionIsExpensive() const {
- if (force_connection_type_)
- return forced_expensive_connection_;
+bool LibcurlHttpFetcher::IsUpdateAllowedOverCurrentConnection() const {
NetworkConnectionType type;
ConcreteDbusGlib dbus_iface;
- TEST_AND_RETURN_FALSE(FlimFlamProxy::GetConnectionType(&dbus_iface, &type));
+ ConnectionManager* connection_manager = system_state_->GetConnectionManager();
+ TEST_AND_RETURN_FALSE(connection_manager->GetConnectionType(&dbus_iface,
+ &type));
+ bool is_allowed = connection_manager->IsUpdateAllowedOver(type);
LOG(INFO) << "We are connected via "
- << FlimFlamProxy::StringForConnectionType(type);
- return FlimFlamProxy::IsExpensiveConnectionType(type);
+ << connection_manager->StringForConnectionType(type)
+ << ", Updates allowed: " << (is_allowed ? "Yes" : "No");
+ return is_allowed;
}
bool LibcurlHttpFetcher::IsOfficialBuild() const {
@@ -140,9 +141,9 @@
StaticLibcurlWrite), CURLE_OK);
string url_to_use(url_);
- if (ConnectionIsExpensive()) {
- LOG(INFO) << "Not initiating HTTP connection b/c we are on an expensive"
- << " connection";
+ if (!IsUpdateAllowedOverCurrentConnection()) {
+ LOG(INFO) << "Not initiating HTTP connection b/c updates are disabled "
+ << "over this connection";
url_to_use = ""; // Sabotage the URL
}
diff --git a/libcurl_http_fetcher.h b/libcurl_http_fetcher.h
index cd7c858..e317819 100644
--- a/libcurl_http_fetcher.h
+++ b/libcurl_http_fetcher.h
@@ -14,7 +14,10 @@
#include <glib.h>
#include "update_engine/certificate_checker.h"
+#include "update_engine/connection_manager.h"
#include "update_engine/http_fetcher.h"
+#include "update_engine/system_state.h"
+
// This is a concrete implementation of HttpFetcher that uses libcurl to do the
// http work.
@@ -27,10 +30,9 @@
static const int kMaxRetryCountOobeComplete;
static const int kMaxRetryCountOobeNotComplete;
- explicit LibcurlHttpFetcher(ProxyResolver* proxy_resolver,
- SystemState* system_state)
- : HttpFetcher(proxy_resolver),
- system_state_(system_state),
+ LibcurlHttpFetcher(ProxyResolver* proxy_resolver,
+ SystemState* system_state)
+ : HttpFetcher(proxy_resolver, system_state),
curl_multi_handle_(NULL),
curl_handle_(NULL),
curl_http_headers_(NULL),
@@ -46,8 +48,6 @@
no_network_retry_count_(0),
no_network_max_retries_(0),
idle_seconds_(1),
- force_connection_type_(false),
- forced_expensive_connection_(false),
force_build_type_(false),
forced_official_build_(false),
in_write_callback_(false),
@@ -94,11 +94,6 @@
no_network_max_retries_ = retries;
}
- void SetConnectionAsExpensive(bool is_expensive) {
- force_connection_type_ = true;
- forced_expensive_connection_ = is_expensive;
- }
-
void SetBuildType(bool is_official) {
force_build_type_ = true;
forced_official_build_ = is_official;
@@ -117,7 +112,7 @@
// Callback for when proxy resolution has completed. This begins the
// transfer.
void ProxiesResolved();
-
+
// Asks libcurl for the http response code and stores it in the object.
void GetHttpResponseCode();
@@ -187,16 +182,13 @@
// be destroyed.
void ForceTransferTermination();
- // Returns whether or not the current network connection is considered
- // expensive.
- bool ConnectionIsExpensive() const;
+ // Returns true if updates are allowed over the current type of connection.
+ // False otherwise.
+ bool IsUpdateAllowedOverCurrentConnection() const;
// Returns whether or not the current build is official.
bool IsOfficialBuild() const;
- // External state of the system
- SystemState* system_state_;
-
// Handles for the libcurl library
CURLM *curl_multi_handle_;
CURL *curl_handle_;
@@ -245,11 +237,6 @@
// Seconds to wait before asking libcurl to "perform".
int idle_seconds_;
- // If true, assume the network is expensive or not, according to
- // forced_expensive_connection_. (Useful for testing).
- bool force_connection_type_;
- bool forced_expensive_connection_;
-
// If true, assume the build is official or not, according to
// forced_official_build_. Useful for testing.
bool force_build_type_;
diff --git a/main.cc b/main.cc
index 213bb7d..84b6c11 100644
--- a/main.cc
+++ b/main.cc
@@ -26,6 +26,7 @@
#include "update_engine/terminator.h"
#include "update_engine/update_attempter.h"
#include "update_engine/update_check_scheduler.h"
+#include "update_engine/system_state.h"
#include "update_engine/utils.h"
extern "C" {
diff --git a/mock_connection_manager.h b/mock_connection_manager.h
new file mode 100644
index 0000000..522e1f6
--- /dev/null
+++ b/mock_connection_manager.h
@@ -0,0 +1,33 @@
+// Copyright (c) 2012 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 CHROMEOS_PLATFORM_UPDATE_ENGINE_MOCK_CONNECTION_MANAGER_H_
+#define CHROMEOS_PLATFORM_UPDATE_ENGINE_MOCK_CONNECTION_MANAGER_H_
+
+#include <gmock/gmock.h>
+
+#include "update_engine/connection_manager.h"
+
+namespace chromeos_update_engine {
+
+// This class mocks the generic interface to the connection manager
+// (e.g FlimFlam, Shill, etc.) to consolidate all connection-related
+// logic in update_engine.
+class MockConnectionManager : public ConnectionManager {
+ public:
+ MockConnectionManager(SystemState* system_state)
+ : ConnectionManager(system_state) {}
+
+ MOCK_CONST_METHOD2(GetConnectionType, bool(DbusGlibInterface* dbus_iface,
+ NetworkConnectionType* out_type));
+
+ MOCK_CONST_METHOD1(IsUpdateAllowedOver, bool(NetworkConnectionType type));
+
+ MOCK_CONST_METHOD1(StringForConnectionType,
+ const char*(NetworkConnectionType type));
+};
+
+} // namespace chromeos_update_engine
+
+#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_MOCK_CONNECTION_MANAGER_H_
diff --git a/mock_http_fetcher.cc b/mock_http_fetcher.cc
index 5452d5d..81f9225 100644
--- a/mock_http_fetcher.cc
+++ b/mock_http_fetcher.cc
@@ -9,7 +9,6 @@
#include <base/logging.h>
#include <gtest/gtest.h>
-
// This is a mock implementation of HttpFetcher which is useful for testing.
using std::min;
diff --git a/mock_http_fetcher.h b/mock_http_fetcher.h
index 31171f1..d3cc75a 100644
--- a/mock_http_fetcher.h
+++ b/mock_http_fetcher.h
@@ -11,6 +11,8 @@
#include <glib.h>
#include "update_engine/http_fetcher.h"
+#include "update_engine/mock_connection_manager.h"
+#include "update_engine/mock_system_state.h"
// This is a mock implementation of HttpFetcher which is useful for testing.
// All data must be passed into the ctor. When started, MockHttpFetcher will
@@ -31,13 +33,15 @@
MockHttpFetcher(const char* data,
size_t size,
ProxyResolver* proxy_resolver)
- : HttpFetcher(proxy_resolver),
+ : HttpFetcher(proxy_resolver, &mock_system_state_),
sent_size_(0),
timeout_source_(NULL),
timout_tag_(0),
paused_(false),
fail_transfer_(false),
- never_use_(false) {
+ never_use_(false),
+ mock_connection_manager_(&mock_system_state_) {
+ mock_system_state_.SetConnectionManager(&mock_connection_manager_);
data_.insert(data_.end(), data, data + size);
}
@@ -124,6 +128,9 @@
// Set to true if BeginTransfer should EXPECT fail.
bool never_use_;
+ MockSystemState mock_system_state_;
+ MockConnectionManager mock_connection_manager_;
+
DISALLOW_COPY_AND_ASSIGN(MockHttpFetcher);
};
diff --git a/mock_system_state.h b/mock_system_state.h
index 0d9f1fd..7f71fa9 100644
--- a/mock_system_state.h
+++ b/mock_system_state.h
@@ -7,15 +7,32 @@
#include <gmock/gmock.h>
-#include "update_engine/utils.h"
+#include <policy/mock_device_policy.h>
+#include "update_engine/system_state.h"
namespace chromeos_update_engine {
// Mock the SystemStateInterface so that we could lie that
// OOBE is completed even when there's no such marker file, etc.
class MockSystemState : public SystemState {
- public:
- MOCK_METHOD0(IsOOBEComplete, bool());
+ public:
+ MockSystemState() {}
+ virtual ~MockSystemState() {}
+
+ MOCK_METHOD0(IsOOBEComplete, bool());
+ MOCK_METHOD1(SetDevicePolicy, void(const policy::DevicePolicy*));
+ MOCK_CONST_METHOD0(GetDevicePolicy, const policy::DevicePolicy*());
+
+ void SetConnectionManager(ConnectionManager* connection_manager) {
+ connection_manager_ = connection_manager;
+ }
+
+ virtual ConnectionManager* GetConnectionManager() {
+ return connection_manager_;
+ }
+
+ private:
+ ConnectionManager* connection_manager_;
};
} // namespeace chromeos_update_engine
diff --git a/multi_range_http_fetcher.h b/multi_range_http_fetcher.h
index 10829cc..5251bfe 100644
--- a/multi_range_http_fetcher.h
+++ b/multi_range_http_fetcher.h
@@ -34,7 +34,8 @@
public:
// Takes ownership of the passed in fetcher.
explicit MultiRangeHttpFetcher(HttpFetcher* base_fetcher)
- : HttpFetcher(base_fetcher->proxy_resolver()),
+ : HttpFetcher(base_fetcher->proxy_resolver(),
+ base_fetcher->GetSystemState()),
base_fetcher_(base_fetcher),
base_fetcher_active_(false),
pending_transfer_ended_(false),
@@ -78,9 +79,6 @@
virtual void set_retry_seconds(int seconds) {
base_fetcher_->set_retry_seconds(seconds);
}
- virtual void SetConnectionAsExpensive(bool is_expensive) {
- base_fetcher_->SetConnectionAsExpensive(is_expensive);
- }
virtual void SetBuildType(bool is_official) {
base_fetcher_->SetBuildType(is_official);
}
@@ -114,11 +112,11 @@
};
typedef std::vector<Range> RangesVect;
-
+
// State change: Stopped or Downloading -> Downloading
void StartTransfer();
-// State change: Downloading -> Downloading or Pending transfer ended
+ // State change: Downloading -> Downloading or Pending transfer ended
virtual void ReceivedBytes(HttpFetcher* fetcher,
const char* bytes,
int length);
@@ -139,7 +137,7 @@
// If true, the next fetcher needs to be started when TransferTerminated is
// received from the current fetcher.
bool pending_transfer_ended_;
-
+
// True if we are waiting for base fetcher to terminate b/c we are
// ourselves terminating.
bool terminating_;
diff --git a/system_state.cc b/system_state.cc
new file mode 100644
index 0000000..d2bd6f1
--- /dev/null
+++ b/system_state.cc
@@ -0,0 +1,32 @@
+// Copyright (c) 2012 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 <base/file_util.h>
+
+#include "update_engine/system_state.h"
+
+namespace chromeos_update_engine {
+
+static const char kOOBECompletedMarker[] = "/home/chronos/.oobe_completed";
+
+RealSystemState::RealSystemState() : connection_manager_(this) {}
+
+bool RealSystemState::IsOOBEComplete() {
+ return file_util::PathExists(FilePath(kOOBECompletedMarker));
+}
+
+void RealSystemState::SetDevicePolicy(
+ const policy::DevicePolicy* device_policy) {
+ device_policy_ = device_policy;
+}
+
+const policy::DevicePolicy* RealSystemState::GetDevicePolicy() const {
+ return device_policy_;
+}
+
+ConnectionManager* RealSystemState::GetConnectionManager() {
+ return &connection_manager_;
+}
+
+} // namespace chromeos_update_engine
diff --git a/system_state.h b/system_state.h
new file mode 100644
index 0000000..a7fc68c
--- /dev/null
+++ b/system_state.h
@@ -0,0 +1,64 @@
+// Copyright (c) 2012 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 CHROMEOS_PLATFORM_UPDATE_ENGINE_SYSTEM_STATE_H_
+#define CHROMEOS_PLATFORM_UPDATE_ENGINE_SYSTEM_STATE_H_
+
+#include <policy/device_policy.h>
+#include <policy/libpolicy.h>
+
+#include <update_engine/connection_manager.h>
+
+namespace chromeos_update_engine {
+
+// An interface to global system context, including platform resources,
+// the current state of the system, high-level objects, system interfaces, etc.
+// Carved out separately so it can be mocked for unit tests.
+// Currently it has only one method, but we should start migrating other
+// methods to use this as and when needed to unit test them.
+// TODO (jaysri): Consider renaming this to something like GlobalContext.
+class SystemState {
+ public:
+ // Destructs this object.
+ virtual ~SystemState() {}
+
+ // Returns true if the OOBE process has been completed and EULA accepted.
+ // False otherwise.
+ virtual bool IsOOBEComplete() = 0;
+
+ // Sets or gets the latest device policy.
+ virtual void SetDevicePolicy(const policy::DevicePolicy* device_policy) = 0;
+ virtual const policy::DevicePolicy* GetDevicePolicy() const = 0;
+
+ // Gets the connection manager object.
+ virtual ConnectionManager* GetConnectionManager() = 0;
+};
+
+// A real implementation of the SystemStateInterface which is
+// used by the actual product code.
+class RealSystemState : public SystemState {
+public:
+ // Constructors and destructors.
+ RealSystemState();
+ virtual ~RealSystemState() {}
+
+ virtual bool IsOOBEComplete();
+
+ virtual void SetDevicePolicy(const policy::DevicePolicy* device_policy);
+ virtual const policy::DevicePolicy* GetDevicePolicy() const;
+
+ virtual ConnectionManager* GetConnectionManager();
+
+private:
+ // The latest device policy object from the policy provider.
+ const policy::DevicePolicy* device_policy_;
+
+ // The connection manager object that makes download
+ // decisions depending on the current type of connection.
+ ConnectionManager connection_manager_;
+};
+
+} // namespace chromeos_update_engine
+
+#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_UTILS_H_
diff --git a/update_attempter.cc b/update_attempter.cc
index 52d6e4c..68c1417 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -32,6 +32,7 @@
#include "update_engine/postinstall_runner_action.h"
#include "update_engine/prefs_interface.h"
#include "update_engine/subprocess.h"
+#include "update_engine/system_state.h"
#include "update_engine/update_check_scheduler.h"
using base::TimeDelta;
@@ -39,6 +40,7 @@
using google::protobuf::NewPermanentCallback;
using std::make_pair;
using std::tr1::shared_ptr;
+using std::set;
using std::string;
using std::vector;
@@ -217,6 +219,21 @@
if (new_scatter_factor_in_secs < 0) // sanitize input, just in case.
new_scatter_factor_in_secs = 0;
scatter_factor_ = TimeDelta::FromSeconds(new_scatter_factor_in_secs);
+
+ system_state_->SetDevicePolicy(&device_policy);
+
+ set<string> allowed_types;
+ string allowed_types_str;
+ if (device_policy.GetAllowedConnectionTypesForUpdate(&allowed_types)) {
+ set<string>::const_iterator iter;
+ for (iter = allowed_types.begin(); iter != allowed_types.end(); ++iter)
+ allowed_types_str += *iter + " ";
+ }
+
+ LOG(INFO) << "Networks over which updates are allowed per policy : "
+ << (allowed_types_str.empty() ? "all" : allowed_types_str);
+ } else {
+ LOG(INFO) << "No device policies present.";
}
bool is_scatter_enabled = false;
diff --git a/update_attempter.h b/update_attempter.h
index deb0ff2..7a1855f 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -22,6 +22,7 @@
#include "update_engine/omaha_request_params.h"
#include "update_engine/omaha_response_handler_action.h"
#include "update_engine/proxy_resolver.h"
+#include "update_engine/system_state.h"
class MetricsLibraryInterface;
struct UpdateEngineService;
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 424ca18..0173dea 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -43,7 +43,12 @@
class UpdateAttempterTest : public ::testing::Test {
protected:
- UpdateAttempterTest() : attempter_(&dbus_), loop_(NULL) {}
+ UpdateAttempterTest()
+ : attempter_(&dbus_),
+ mock_connection_manager(&mock_system_state_),
+ loop_(NULL) {
+ mock_system_state_.SetConnectionManager(&mock_connection_manager);
+ }
virtual void SetUp() {
EXPECT_EQ(NULL, attempter_.dbus_service_);
EXPECT_EQ(NULL, attempter_.prefs_);
@@ -103,6 +108,7 @@
ActionProcessorMock* processor_;
NiceMock<PrefsMock> prefs_;
MockSystemState mock_system_state_;
+ MockConnectionManager mock_connection_manager;
GMainLoop* loop_;
};
diff --git a/update_check_scheduler.cc b/update_check_scheduler.cc
index 148fd1b..be599ab 100644
--- a/update_check_scheduler.cc
+++ b/update_check_scheduler.cc
@@ -6,6 +6,7 @@
#include "update_engine/certificate_checker.h"
#include "update_engine/http_common.h"
+#include "update_engine/system_state.h"
#include "update_engine/utils.h"
namespace chromeos_update_engine {
diff --git a/update_check_scheduler.h b/update_check_scheduler.h
index 16f831e..8cfcc46 100644
--- a/update_check_scheduler.h
+++ b/update_check_scheduler.h
@@ -10,6 +10,7 @@
#include <gtest/gtest_prod.h> // for FRIEND_TEST
#include "update_engine/update_attempter.h"
+#include "update_engine/system_state.h"
namespace chromeos_update_engine {
diff --git a/utils.cc b/utils.cc
index 769d7fe..f4b3b24 100644
--- a/utils.cc
+++ b/utils.cc
@@ -41,12 +41,6 @@
namespace chromeos_update_engine {
-static const char kOOBECompletedMarker[] = "/home/chronos/.oobe_completed";
-
-bool RealSystemState::IsOOBEComplete() {
- return file_util::PathExists(FilePath(kOOBECompletedMarker));
-}
-
namespace utils {
static const char kDevImageMarker[] = "/root/.dev_mode";
diff --git a/utils.h b/utils.h
index fd9782a..628602f 100644
--- a/utils.h
+++ b/utils.h
@@ -269,24 +269,6 @@
} // namespace utils
-// An interface to get the state of the current system.
-// Carved out separately so it can be mocked for unit tests.
-// Currently it has only one method, but we should start migrating other
-// methods to use this as and when needed to unit test them.
-class SystemState {
- public:
- // Returns true if the OOBE process has been completed and EULA accepted.
- // False otherwise.
- virtual bool IsOOBEComplete() = 0;
-};
-
-// A real implementation of the SystemStateInterface which is
-// used by the actual product code.
-class RealSystemState : public SystemState {
- public:
- virtual bool IsOOBEComplete();
-};
-
// Class to unmount FS when object goes out of scope
class ScopedFilesystemUnmounter {
public: