Merge cherrypicks of [2367392, 2367426, 2367393] into oc-dr1-release
Change-Id: Idd4025620601cce84304b3e5190e4b5a088c7c59
diff --git a/UpdateEngine.conf b/UpdateEngine.conf
index 192e6ab..58cca09 100644
--- a/UpdateEngine.conf
+++ b/UpdateEngine.conf
@@ -53,9 +53,6 @@
send_member="SetUpdateOverCellularPermission"/>
<allow send_destination="org.chromium.UpdateEngine"
send_interface="org.chromium.UpdateEngineInterface"
- send_member="SetUpdateOverCellularTarget"/>
- <allow send_destination="org.chromium.UpdateEngine"
- send_interface="org.chromium.UpdateEngineInterface"
send_member="GetUpdateOverCellularPermission"/>
<allow send_destination="org.chromium.UpdateEngine"
send_interface="org.chromium.UpdateEngineInterface"
diff --git a/binder_bindings/android/brillo/IUpdateEngine.aidl b/binder_bindings/android/brillo/IUpdateEngine.aidl
index 7893548..b1a1b4f 100644
--- a/binder_bindings/android/brillo/IUpdateEngine.aidl
+++ b/binder_bindings/android/brillo/IUpdateEngine.aidl
@@ -33,8 +33,6 @@
void SetP2PUpdatePermission(in boolean enabled);
boolean GetP2PUpdatePermission();
void SetUpdateOverCellularPermission(in boolean enabled);
- void SetUpdateOverCellularTarget(in String target_version,
- in long target_size);
boolean GetUpdateOverCellularPermission();
long GetDurationSinceUpdate();
String GetPrevVersion();
diff --git a/binder_service_brillo.cc b/binder_service_brillo.cc
index 14df725..5e74159 100644
--- a/binder_service_brillo.cc
+++ b/binder_service_brillo.cc
@@ -151,14 +151,6 @@
&UpdateEngineService::SetUpdateOverCellularPermission, enabled);
}
-Status BinderUpdateEngineBrilloService::SetUpdateOverCellularTarget(
- const String16& target_version,
- int64_t target_size) {
- return CallCommonHandler(
- &UpdateEngineService::SetUpdateOverCellularTarget,
- NormalString(target_version), target_size);
-}
-
Status BinderUpdateEngineBrilloService::GetUpdateOverCellularPermission(
bool* out_cellular_permission) {
return CallCommonHandler(
diff --git a/binder_service_brillo.h b/binder_service_brillo.h
index 4cc007b..982c7b1 100644
--- a/binder_service_brillo.h
+++ b/binder_service_brillo.h
@@ -76,9 +76,6 @@
bool* out_p2p_permission) override;
android::binder::Status SetUpdateOverCellularPermission(
bool enabled) override;
- android::binder::Status SetUpdateOverCellularTarget(
- const android::String16& target_version,
- int64_t target_size) override;
android::binder::Status GetUpdateOverCellularPermission(
bool* out_cellular_permission) override;
android::binder::Status GetDurationSinceUpdate(
diff --git a/client_library/include/update_engine/update_status.h b/client_library/include/update_engine/update_status.h
index 2f6322a..3e9af5b 100644
--- a/client_library/include/update_engine/update_status.h
+++ b/client_library/include/update_engine/update_status.h
@@ -23,9 +23,6 @@
IDLE = 0,
CHECKING_FOR_UPDATE,
UPDATE_AVAILABLE,
- // Broadcast this state when an update aborts because user preferences does
- // not allow update over cellular.
- NEED_PERMISSION_TO_UPDATE,
DOWNLOADING,
VERIFYING,
FINALIZING,
diff --git a/common/constants.cc b/common/constants.cc
index d6d1c6d..88d0445 100644
--- a/common/constants.cc
+++ b/common/constants.cc
@@ -74,10 +74,6 @@
const char kPrefsUpdateFirstSeenAt[] = "update-first-seen-at";
const char kPrefsUpdateOverCellularPermission[] =
"update-over-cellular-permission";
-const char kPrefsUpdateOverCellularTargetVersion[] =
- "update-over-cellular-target-version";
-const char kPrefsUpdateOverCellularTargetSize[] =
- "update-over-cellular-target-size";
const char kPrefsUpdateServerCertificate[] = "update-server-cert";
const char kPrefsUpdateStateNextDataLength[] = "update-state-next-data-length";
const char kPrefsUpdateStateNextDataOffset[] = "update-state-next-data-offset";
diff --git a/common/constants.h b/common/constants.h
index c571403..ab66921 100644
--- a/common/constants.h
+++ b/common/constants.h
@@ -75,8 +75,6 @@
extern const char kPrefsUpdateDurationUptime[];
extern const char kPrefsUpdateFirstSeenAt[];
extern const char kPrefsUpdateOverCellularPermission[];
-extern const char kPrefsUpdateOverCellularTargetVersion[];
-extern const char kPrefsUpdateOverCellularTargetSize[];
extern const char kPrefsUpdateServerCertificate[];
extern const char kPrefsUpdateStateNextDataLength[];
extern const char kPrefsUpdateStateNextDataOffset[];
diff --git a/common/error_code.h b/common/error_code.h
index bcf541c..e08ec46 100644
--- a/common/error_code.h
+++ b/common/error_code.h
@@ -73,7 +73,6 @@
kFilesystemVerifierError = 47,
kUserCanceled = 48,
kNonCriticalUpdateInOOBE = 49,
- kOmahaUpdateIgnoredOverCellular = 50,
// VERY IMPORTANT! When adding new error codes:
//
diff --git a/common/error_code_utils.cc b/common/error_code_utils.cc
index 0169ab8..ad4aeeb 100644
--- a/common/error_code_utils.cc
+++ b/common/error_code_utils.cc
@@ -144,8 +144,6 @@
return "ErrorCode::kUserCanceled";
case ErrorCode::kNonCriticalUpdateInOOBE:
return "ErrorCode::kNonCriticalUpdateInOOBE";
- case ErrorCode::kOmahaUpdateIgnoredOverCellular:
- return "ErrorCode::kOmahaUpdateIgnoredOverCellular";
// Don't add a default case to let the compiler warn about newly added
// error codes which should be added here.
}
diff --git a/common_service.cc b/common_service.cc
index ee890e1..1a41b63 100644
--- a/common_service.cc
+++ b/common_service.cc
@@ -16,6 +16,7 @@
#include "update_engine/common_service.h"
+#include <set>
#include <string>
#include <base/location.h>
@@ -40,6 +41,7 @@
using base::StringPrintf;
using brillo::ErrorPtr;
using brillo::string_utils::ToString;
+using std::set;
using std::string;
namespace chromeos_update_engine {
@@ -246,12 +248,24 @@
bool UpdateEngineService::SetUpdateOverCellularPermission(ErrorPtr* error,
bool in_allowed) {
- ConnectionManagerInterface* connection_manager =
- system_state_->connection_manager();
+ set<string> allowed_types;
+ const policy::DevicePolicy* device_policy = system_state_->device_policy();
+
+ // The device_policy is loaded in a lazy way before an update check. Load it
+ // now from the libbrillo cache if it wasn't already loaded.
+ if (!device_policy) {
+ UpdateAttempter* update_attempter = system_state_->update_attempter();
+ if (update_attempter) {
+ update_attempter->RefreshDevicePolicy();
+ device_policy = system_state_->device_policy();
+ }
+ }
// Check if this setting is allowed by the device policy.
- if (connection_manager->IsAllowedConnectionTypesForUpdateSet()) {
- LogAndSetError(error, FROM_HERE,
+ if (device_policy &&
+ device_policy->GetAllowedConnectionTypesForUpdate(&allowed_types)) {
+ LogAndSetError(error,
+ FROM_HERE,
"Ignoring the update over cellular setting since there's "
"a device policy enforcing this setting.");
return false;
@@ -262,9 +276,9 @@
PrefsInterface* prefs = system_state_->prefs();
- if (!prefs ||
- !prefs->SetBoolean(kPrefsUpdateOverCellularPermission, in_allowed)) {
- LogAndSetError(error, FROM_HERE,
+ if (!prefs->SetBoolean(kPrefsUpdateOverCellularPermission, in_allowed)) {
+ LogAndSetError(error,
+ FROM_HERE,
string("Error setting the update over cellular to ") +
(in_allowed ? "true" : "false"));
return false;
@@ -272,63 +286,24 @@
return true;
}
-bool UpdateEngineService::SetUpdateOverCellularTarget(
- brillo::ErrorPtr* error, const std::string &target_version,
- int64_t target_size) {
- ConnectionManagerInterface* connection_manager =
- system_state_->connection_manager();
-
- // Check if this setting is allowed by the device policy.
- if (connection_manager->IsAllowedConnectionTypesForUpdateSet()) {
- LogAndSetError(error, FROM_HERE,
- "Ignoring the update over cellular setting since there's "
- "a device policy enforcing this setting.");
- return false;
- }
-
- // If the policy wasn't loaded yet, then it is still OK to change the local
- // setting because the policy will be checked again during the update check.
-
- PrefsInterface* prefs = system_state_->prefs();
-
- if (!prefs ||
- !prefs->SetString(kPrefsUpdateOverCellularTargetVersion,
- target_version) ||
- !prefs->SetInt64(kPrefsUpdateOverCellularTargetSize, target_size)) {
- LogAndSetError(error, FROM_HERE,
- "Error setting the target for update over cellular.");
- return false;
- }
- return true;
-}
-
-bool UpdateEngineService::GetUpdateOverCellularPermission(ErrorPtr* error,
+bool UpdateEngineService::GetUpdateOverCellularPermission(ErrorPtr* /* error */,
bool* out_allowed) {
- ConnectionManagerInterface* connection_manager =
- system_state_->connection_manager();
+ ConnectionManagerInterface* cm = system_state_->connection_manager();
- if (connection_manager->IsAllowedConnectionTypesForUpdateSet()) {
- // We have device policy, so ignore the user preferences.
- *out_allowed = connection_manager->IsUpdateAllowedOver(
- ConnectionType::kCellular, ConnectionTethering::kUnknown);
- } else {
- PrefsInterface* prefs = system_state_->prefs();
-
- if (!prefs || !prefs->Exists(kPrefsUpdateOverCellularPermission)) {
- // Update is not allowed as user preference is not set or not available.
- *out_allowed = false;
- return true;
- }
-
- bool is_allowed;
-
- if (!prefs->GetBoolean(kPrefsUpdateOverCellularPermission, &is_allowed)) {
- LogAndSetError(error, FROM_HERE,
- "Error getting the update over cellular preference.");
- return false;
- }
- *out_allowed = is_allowed;
+ // The device_policy is loaded in a lazy way before an update check and is
+ // used to determine if an update is allowed over cellular. Load the device
+ // policy now from the libbrillo cache if it wasn't already loaded.
+ if (!system_state_->device_policy()) {
+ UpdateAttempter* update_attempter = system_state_->update_attempter();
+ if (update_attempter)
+ update_attempter->RefreshDevicePolicy();
}
+
+ // Return the current setting based on the same logic used while checking for
+ // updates. A log message could be printed as the result of this test.
+ LOG(INFO) << "Checking if updates over cellular networks are allowed:";
+ *out_allowed = cm->IsUpdateAllowedOver(ConnectionType::kCellular,
+ ConnectionTethering::kUnknown);
return true;
}
diff --git a/common_service.h b/common_service.h
index 4320d09..69368fb 100644
--- a/common_service.h
+++ b/common_service.h
@@ -115,12 +115,6 @@
bool SetUpdateOverCellularPermission(brillo::ErrorPtr* error,
bool in_allowed);
- // If there's no device policy installed, sets the update over cellular
- // target. Otherwise, this method returns with an error.
- bool SetUpdateOverCellularTarget(brillo::ErrorPtr* error,
- const std::string& target_version,
- int64_t target_size);
-
// Returns the current value of the update over cellular network setting,
// either forced by the device policy if the device is enrolled or the current
// user preference otherwise.
diff --git a/connection_manager.cc b/connection_manager.cc
index f0a2c92..f72d9e8 100644
--- a/connection_manager.cc
+++ b/connection_manager.cc
@@ -30,7 +30,6 @@
#include "update_engine/connection_utils.h"
#include "update_engine/shill_proxy.h"
#include "update_engine/system_state.h"
-#include "update_engine/update_attempter.h"
using org::chromium::flimflam::ManagerProxyInterface;
using org::chromium::flimflam::ServiceProxyInterface;
@@ -59,23 +58,15 @@
case ConnectionType::kCellular: {
set<string> allowed_types;
-
const policy::DevicePolicy* device_policy =
system_state_->device_policy();
- // The device_policy is loaded in a lazy way before an update check. Load
- // it now from the libbrillo cache if it wasn't already loaded.
+ // A device_policy is loaded in a lazy way right before an update check,
+ // so the device_policy should be already loaded at this point. If it's
+ // not, return a safe value for this setting.
if (!device_policy) {
- UpdateAttempter* update_attempter = system_state_->update_attempter();
- if (update_attempter) {
- update_attempter->RefreshDevicePolicy();
- device_policy = system_state_->device_policy();
- }
- }
-
- if (!device_policy) {
- LOG(INFO) << "Disabling updates over cellular as device policy "
- "fails to be loaded.";
+ LOG(INFO) << "Disabling updates over cellular networks as there's no "
+ "device policy loaded yet.";
return false;
}
@@ -83,17 +74,38 @@
// The update setting is enforced by the device policy.
if (!ContainsKey(allowed_types, shill::kTypeCellular)) {
- LOG(INFO) << "Disabling updates over cellular per device policy.";
+ 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;
+ } else {
+ // There's no update setting in the device policy, using the local user
+ // setting.
+ PrefsInterface* prefs = system_state_->prefs();
- // If there's no update setting in the device policy, we do not check
- // the local user setting here, which should be checked by
- // |OmahaRequestAction| during checking for update.
- return true;
+ if (!prefs || !prefs->Exists(kPrefsUpdateOverCellularPermission)) {
+ LOG(INFO) << "Disabling updates over cellular connection as there's "
+ "no device policy setting nor user preference present.";
+ return false;
+ }
+
+ bool stored_value;
+ if (!prefs->GetBoolean(kPrefsUpdateOverCellularPermission,
+ &stored_value)) {
+ return false;
+ }
+
+ if (!stored_value) {
+ LOG(INFO) << "Disabling updates over cellular connection per user "
+ "setting.";
+ return false;
+ }
+ LOG(INFO) << "Allowing updates over cellular per user setting.";
+ return true;
+ }
}
default:
@@ -108,22 +120,6 @@
}
}
-bool ConnectionManager::IsAllowedConnectionTypesForUpdateSet() const {
- const policy::DevicePolicy* device_policy = system_state_->device_policy();
- if (!device_policy) {
- LOG(INFO) << "There's no device policy loaded yet.";
- return false;
- }
-
- set<string> allowed_types;
- if (!device_policy->GetAllowedConnectionTypesForUpdate(
- &allowed_types)) {
- return false;
- }
-
- return true;
-}
-
bool ConnectionManager::GetConnectionProperties(
ConnectionType* out_type, ConnectionTethering* out_tethering) {
dbus::ObjectPath default_service_path;
diff --git a/connection_manager.h b/connection_manager.h
index 864985e..e5a9d49 100644
--- a/connection_manager.h
+++ b/connection_manager.h
@@ -43,7 +43,6 @@
ConnectionTethering* out_tethering) override;
bool IsUpdateAllowedOver(ConnectionType type,
ConnectionTethering tethering) const override;
- bool IsAllowedConnectionTypesForUpdateSet() const override;
private:
// Returns (via out_path) the default network path, or empty string if
diff --git a/connection_manager_android.cc b/connection_manager_android.cc
index c594b53..2dd824a 100644
--- a/connection_manager_android.cc
+++ b/connection_manager_android.cc
@@ -34,9 +34,5 @@
ConnectionType type, ConnectionTethering tethering) const {
return true;
}
-bool ConnectionManagerAndroid::IsAllowedConnectionTypesForUpdateSet() const {
- return false;
-}
-
} // namespace chromeos_update_engine
diff --git a/connection_manager_android.h b/connection_manager_android.h
index 006f4ea..0cd5e73 100644
--- a/connection_manager_android.h
+++ b/connection_manager_android.h
@@ -34,7 +34,6 @@
ConnectionTethering* out_tethering) override;
bool IsUpdateAllowedOver(ConnectionType type,
ConnectionTethering tethering) const override;
- bool IsAllowedConnectionTypesForUpdateSet() const override;
DISALLOW_COPY_AND_ASSIGN(ConnectionManagerAndroid);
};
diff --git a/connection_manager_interface.h b/connection_manager_interface.h
index 2faeb80..df8eb4b 100644
--- a/connection_manager_interface.h
+++ b/connection_manager_interface.h
@@ -46,10 +46,6 @@
virtual bool IsUpdateAllowedOver(ConnectionType type,
ConnectionTethering tethering) const = 0;
- // Returns true if the allowed connection types for update is set in the
- // device policy. Otherwise, returns false.
- virtual bool IsAllowedConnectionTypesForUpdateSet() const = 0;
-
protected:
ConnectionManagerInterface() = default;
diff --git a/connection_manager_unittest.cc b/connection_manager_unittest.cc
index f814667..0bb5547 100644
--- a/connection_manager_unittest.cc
+++ b/connection_manager_unittest.cc
@@ -277,24 +277,16 @@
ConnectionTethering::kConfirmed));
}
-TEST_F(ConnectionManagerTest, AllowUpdatesOverCellularByDefaultTest) {
- policy::MockDevicePolicy device_policy;
- // Set an empty device policy.
- fake_system_state_.set_device_policy(&device_policy);
-
- EXPECT_TRUE(cmut_.IsUpdateAllowedOver(ConnectionType::kCellular,
- ConnectionTethering::kUnknown));
+TEST_F(ConnectionManagerTest, BlockUpdatesOverCellularByDefaultTest) {
+ EXPECT_FALSE(cmut_.IsUpdateAllowedOver(ConnectionType::kCellular,
+ ConnectionTethering::kUnknown));
}
-TEST_F(ConnectionManagerTest, AllowUpdatesOverTetheredNetworkByDefaultTest) {
- policy::MockDevicePolicy device_policy;
- // Set an empty device policy.
- fake_system_state_.set_device_policy(&device_policy);
-
- EXPECT_TRUE(cmut_.IsUpdateAllowedOver(ConnectionType::kWifi,
- ConnectionTethering::kConfirmed));
- EXPECT_TRUE(cmut_.IsUpdateAllowedOver(ConnectionType::kEthernet,
- ConnectionTethering::kConfirmed));
+TEST_F(ConnectionManagerTest, BlockUpdatesOverTetheredNetworkByDefaultTest) {
+ EXPECT_FALSE(cmut_.IsUpdateAllowedOver(ConnectionType::kWifi,
+ ConnectionTethering::kConfirmed));
+ EXPECT_FALSE(cmut_.IsUpdateAllowedOver(ConnectionType::kEthernet,
+ ConnectionTethering::kConfirmed));
EXPECT_TRUE(cmut_.IsUpdateAllowedOver(ConnectionType::kWifi,
ConnectionTethering::kSuspected));
}
@@ -319,20 +311,62 @@
ConnectionTethering::kUnknown));
}
-TEST_F(ConnectionManagerTest, AllowUpdatesOver3GIfPolicyIsNotSet) {
- policy::MockDevicePolicy device_policy;
+TEST_F(ConnectionManagerTest, BlockUpdatesOver3GIfErrorInPolicyFetchTest) {
+ policy::MockDevicePolicy allow_3g_policy;
- fake_system_state_.set_device_policy(&device_policy);
+ fake_system_state_.set_device_policy(&allow_3g_policy);
+
+ set<string> allowed_set;
+ allowed_set.insert(StringForConnectionType(ConnectionType::kCellular));
// Return false for GetAllowedConnectionTypesForUpdate and see
- // that updates are allowed as device policy is not set. Further
- // check is left to |OmahaRequestAction|.
- EXPECT_CALL(device_policy, GetAllowedConnectionTypesForUpdate(_))
+ // 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(SetArgPointee<0>(allowed_set), Return(false)));
+
+ EXPECT_FALSE(cmut_.IsUpdateAllowedOver(ConnectionType::kCellular,
+ ConnectionTethering::kUnknown));
+}
+
+TEST_F(ConnectionManagerTest, UseUserPrefForUpdatesOverCellularIfNoPolicyTest) {
+ policy::MockDevicePolicy no_policy;
+ testing::NiceMock<MockPrefs>* prefs = fake_system_state_.mock_prefs();
+
+ fake_system_state_.set_device_policy(&no_policy);
+
+ // No setting enforced by the device policy, user prefs should be used.
+ EXPECT_CALL(no_policy, GetAllowedConnectionTypesForUpdate(_))
+ .Times(3)
+ .WillRepeatedly(Return(false));
+
+ // No user pref: block.
+ EXPECT_CALL(*prefs, Exists(kPrefsUpdateOverCellularPermission))
.Times(1)
.WillOnce(Return(false));
+ EXPECT_FALSE(cmut_.IsUpdateAllowedOver(ConnectionType::kCellular,
+ ConnectionTethering::kUnknown));
+ // Allow per user pref.
+ EXPECT_CALL(*prefs, Exists(kPrefsUpdateOverCellularPermission))
+ .Times(1)
+ .WillOnce(Return(true));
+ EXPECT_CALL(*prefs, GetBoolean(kPrefsUpdateOverCellularPermission, _))
+ .Times(1)
+ .WillOnce(DoAll(SetArgPointee<1>(true), Return(true)));
EXPECT_TRUE(cmut_.IsUpdateAllowedOver(ConnectionType::kCellular,
ConnectionTethering::kUnknown));
+
+ // Block per user pref.
+ EXPECT_CALL(*prefs, Exists(kPrefsUpdateOverCellularPermission))
+ .Times(1)
+ .WillOnce(Return(true));
+ EXPECT_CALL(*prefs, GetBoolean(kPrefsUpdateOverCellularPermission, _))
+ .Times(1)
+ .WillOnce(DoAll(SetArgPointee<1>(false), Return(true)));
+ EXPECT_FALSE(cmut_.IsUpdateAllowedOver(ConnectionType::kCellular,
+ ConnectionTethering::kUnknown));
}
TEST_F(ConnectionManagerTest, StringForConnectionTypeTest) {
diff --git a/dbus_bindings/org.chromium.UpdateEngineInterface.dbus-xml b/dbus_bindings/org.chromium.UpdateEngineInterface.dbus-xml
index a20f33f..848f775 100644
--- a/dbus_bindings/org.chromium.UpdateEngineInterface.dbus-xml
+++ b/dbus_bindings/org.chromium.UpdateEngineInterface.dbus-xml
@@ -67,10 +67,6 @@
<method name="SetUpdateOverCellularPermission">
<arg type="b" name="allowed" direction="in" />
</method>
- <method name="SetUpdateOverCellularTarget">
- <arg type="s" name="target_version" direction="in" />
- <arg type="x" name="target_size" direction="in" />
- </method>
<method name="GetUpdateOverCellularPermission">
<arg type="b" name="allowed" direction="out" />
</method>
diff --git a/dbus_service.cc b/dbus_service.cc
index 731d489..0a7ad5b 100644
--- a/dbus_service.cc
+++ b/dbus_service.cc
@@ -123,14 +123,6 @@
return common_->SetUpdateOverCellularPermission(error, in_allowed);
}
-bool DBusUpdateEngineService::SetUpdateOverCellularTarget(
- brillo::ErrorPtr* error,
- const std::string& target_version,
- int64_t target_size) {
- return common_->SetUpdateOverCellularTarget(error, target_version,
- target_size);
-}
-
bool DBusUpdateEngineService::GetUpdateOverCellularPermission(
ErrorPtr* error, bool* out_allowed) {
return common_->GetUpdateOverCellularPermission(error, out_allowed);
diff --git a/dbus_service.h b/dbus_service.h
index 644dcee..2b36ae9 100644
--- a/dbus_service.h
+++ b/dbus_service.h
@@ -114,12 +114,6 @@
bool SetUpdateOverCellularPermission(brillo::ErrorPtr* error,
bool in_allowed) override;
- // If there's no device policy installed, sets the update over cellular
- // target. Otherwise, this method returns with an error.
- bool SetUpdateOverCellularTarget(brillo::ErrorPtr* error,
- const std::string& target_version,
- int64_t target_size) override;
-
// Returns the current value of the update over cellular network setting,
// either forced by the device policy if the device is enrolled or the current
// user preference otherwise.
diff --git a/metrics_utils.cc b/metrics_utils.cc
index 15e3a8a..263bacd 100644
--- a/metrics_utils.cc
+++ b/metrics_utils.cc
@@ -109,7 +109,6 @@
case ErrorCode::kPostinstallPowerwashError:
case ErrorCode::kUpdateCanceledByChannelChange:
case ErrorCode::kOmahaRequestXMLHasEntityDecl:
- case ErrorCode::kOmahaUpdateIgnoredOverCellular:
return metrics::AttemptResult::kInternalError;
// Special flags. These can't happen (we mask them out above) but
@@ -210,7 +209,6 @@
case ErrorCode::kOmahaRequestXMLHasEntityDecl:
case ErrorCode::kFilesystemVerifierError:
case ErrorCode::kUserCanceled:
- case ErrorCode::kOmahaUpdateIgnoredOverCellular:
break;
// Special flags. These can't happen (we mask them out above) but
diff --git a/mock_connection_manager.h b/mock_connection_manager.h
index 2fff68c..e37460b 100644
--- a/mock_connection_manager.h
+++ b/mock_connection_manager.h
@@ -36,7 +36,6 @@
MOCK_CONST_METHOD2(IsUpdateAllowedOver,
bool(ConnectionType type, ConnectionTethering tethering));
- MOCK_CONST_METHOD0(IsAllowedConnectionTypesForUpdateSet, bool());
};
} // namespace chromeos_update_engine
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index ed3c716..b06de09 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -1001,11 +1001,9 @@
output_object.update_exists = true;
SetOutputObject(output_object);
- ErrorCode error = ErrorCode::kSuccess;
- if (ShouldIgnoreUpdate(&error, output_object)) {
- // No need to change output_object.update_exists here, since the value
- // has been output to the pipe.
- completer.set_code(error);
+ if (ShouldIgnoreUpdate(output_object)) {
+ output_object.update_exists = false;
+ completer.set_code(ErrorCode::kOmahaUpdateIgnoredPerPolicy);
return;
}
@@ -1463,7 +1461,6 @@
break;
case ErrorCode::kOmahaUpdateIgnoredPerPolicy:
- case ErrorCode::kOmahaUpdateIgnoredOverCellular:
result = metrics::CheckResult::kUpdateAvailable;
reaction = metrics::CheckReaction::kIgnored;
break;
@@ -1498,7 +1495,7 @@
}
bool OmahaRequestAction::ShouldIgnoreUpdate(
- ErrorCode* error, const OmahaResponse& response) const {
+ const OmahaResponse& response) const {
// Note: policy decision to not update to a version we rolled back from.
string rollback_version =
system_state_->payload_state()->GetRollbackVersion();
@@ -1506,12 +1503,11 @@
LOG(INFO) << "Detected previous rollback from version " << rollback_version;
if (rollback_version == response.version) {
LOG(INFO) << "Received version that we rolled back from. Ignoring.";
- *error = ErrorCode::kOmahaUpdateIgnoredPerPolicy;
return true;
}
}
- if (!IsUpdateAllowedOverCurrentConnection(error, response)) {
+ if (!IsUpdateAllowedOverCurrentConnection()) {
LOG(INFO) << "Update is not allowed over current connection.";
return true;
}
@@ -1526,59 +1522,7 @@
return false;
}
-bool OmahaRequestAction::IsUpdateAllowedOverCellularByPrefs(
- const OmahaResponse& response) const {
- PrefsInterface* prefs = system_state_->prefs();
-
- if (!prefs) {
- LOG(INFO) << "Disabling updates over cellular as the preferences are "
- "not available.";
- return false;
- }
-
- bool is_allowed;
-
- if (prefs->Exists(kPrefsUpdateOverCellularPermission) &&
- prefs->GetBoolean(kPrefsUpdateOverCellularPermission, &is_allowed) &&
- is_allowed) {
- LOG(INFO) << "Allowing updates over cellular as permission preference is "
- "set to true.";
- return true;
- }
-
- if (!prefs->Exists(kPrefsUpdateOverCellularTargetVersion) ||
- !prefs->Exists(kPrefsUpdateOverCellularTargetSize)) {
- LOG(INFO) << "Disabling updates over cellular as permission preference is "
- "set to false or does not exist while target does not exist.";
- return false;
- }
-
- std::string target_version;
- int64_t target_size;
-
- if (!prefs->GetString(kPrefsUpdateOverCellularTargetVersion,
- &target_version) ||
- !prefs->GetInt64(kPrefsUpdateOverCellularTargetSize,
- &target_size)) {
- LOG(INFO) << "Disabling updates over cellular as the target version or "
- "size is not accessible.";
- return false;
- }
-
- if (target_version == response.version && target_size == response.size) {
- LOG(INFO) << "Allowing updates over cellular as the target matches the"
- "omaha response.";
- return true;
- } else {
- LOG(INFO) << "Disabling updates over cellular as the target does not"
- "match the omaha response.";
- return false;
- }
-}
-
-bool OmahaRequestAction::IsUpdateAllowedOverCurrentConnection(
- ErrorCode* error,
- const OmahaResponse& response) const {
+bool OmahaRequestAction::IsUpdateAllowedOverCurrentConnection() const {
ConnectionType type;
ConnectionTethering tethering;
ConnectionManagerInterface* connection_manager =
@@ -1588,30 +1532,7 @@
<< "Defaulting to allow updates.";
return true;
}
-
bool is_allowed = connection_manager->IsUpdateAllowedOver(type, tethering);
- bool is_device_policy_set = connection_manager->
- IsAllowedConnectionTypesForUpdateSet();
- // Treats tethered connection as if it is cellular connection.
- bool is_over_cellular = type == ConnectionType::kCellular ||
- tethering == ConnectionTethering::kConfirmed;
-
- if (!is_over_cellular) {
- // There's no need to further check user preferences as we are not over
- // cellular connection.
- if (!is_allowed)
- *error = ErrorCode::kOmahaUpdateIgnoredPerPolicy;
- } else if (is_device_policy_set) {
- // There's no need to further check user preferences as the device policy
- // is set regarding updates over cellular.
- if (!is_allowed)
- *error = ErrorCode::kOmahaUpdateIgnoredPerPolicy;
- } else if (!IsUpdateAllowedOverCellularByPrefs(response)) {
- // The user prefereces does not allow updates over cellular.
- is_allowed = false;
- *error = ErrorCode::kOmahaUpdateIgnoredOverCellular;
- }
-
LOG(INFO) << "We are connected via "
<< connection_utils::StringForConnectionType(type)
<< ", Updates allowed: " << (is_allowed ? "Yes" : "No");
diff --git a/omaha_request_action.h b/omaha_request_action.h
index 7d35c23..2915a6a 100644
--- a/omaha_request_action.h
+++ b/omaha_request_action.h
@@ -299,17 +299,11 @@
void OnLookupPayloadViaP2PCompleted(const std::string& url);
// Returns true if the current update should be ignored.
- bool ShouldIgnoreUpdate(ErrorCode* error,
- const OmahaResponse& response) const;
-
- // Return true if updates are allowed by user preferences.
- bool IsUpdateAllowedOverCellularByPrefs(const OmahaResponse& response) const;
+ bool ShouldIgnoreUpdate(const OmahaResponse& response) const;
// Returns true if updates are allowed over the current type of connection.
// False otherwise.
- bool IsUpdateAllowedOverCurrentConnection(
- ErrorCode* error,
- const OmahaResponse& response) const;
+ bool IsUpdateAllowedOverCurrentConnection() const;
// Global system context.
SystemState* system_state_;
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index cb3ef71..1c1d25c 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -518,185 +518,6 @@
EXPECT_FALSE(response.update_exists);
}
-TEST_F(OmahaRequestActionTest, ValidUpdateOverCellularAllowedByDevicePolicy) {
- // This test tests that update over cellular is allowed as device policy
- // says yes.
- OmahaResponse response;
- MockConnectionManager mock_cm;
-
- fake_system_state_.set_connection_manager(&mock_cm);
-
- EXPECT_CALL(mock_cm, GetConnectionProperties(_, _))
- .WillRepeatedly(
- DoAll(SetArgumentPointee<0>(ConnectionType::kCellular),
- SetArgumentPointee<1>(ConnectionTethering::kUnknown),
- Return(true)));
- EXPECT_CALL(mock_cm, IsAllowedConnectionTypesForUpdateSet())
- .WillRepeatedly(Return(true));
- EXPECT_CALL(mock_cm, IsUpdateAllowedOver(ConnectionType::kCellular, _))
- .WillRepeatedly(Return(true));
-
- ASSERT_TRUE(
- TestUpdateCheck(nullptr, // request_params
- fake_update_response_.GetUpdateResponse(),
- -1,
- false, // ping_only
- ErrorCode::kSuccess,
- metrics::CheckResult::kUpdateAvailable,
- metrics::CheckReaction::kUpdating,
- metrics::DownloadErrorCode::kUnset,
- &response,
- nullptr));
- EXPECT_TRUE(response.update_exists);
-}
-
-TEST_F(OmahaRequestActionTest, ValidUpdateOverCellularBlockedByDevicePolicy) {
- // This test tests that update over cellular is blocked as device policy
- // says no.
- OmahaResponse response;
- MockConnectionManager mock_cm;
-
- fake_system_state_.set_connection_manager(&mock_cm);
-
- EXPECT_CALL(mock_cm, GetConnectionProperties(_, _))
- .WillRepeatedly(
- DoAll(SetArgumentPointee<0>(ConnectionType::kCellular),
- SetArgumentPointee<1>(ConnectionTethering::kUnknown),
- Return(true)));
- EXPECT_CALL(mock_cm, IsAllowedConnectionTypesForUpdateSet())
- .WillRepeatedly(Return(true));
- EXPECT_CALL(mock_cm, IsUpdateAllowedOver(ConnectionType::kCellular, _))
- .WillRepeatedly(Return(false));
-
- ASSERT_FALSE(
- TestUpdateCheck(nullptr, // request_params
- fake_update_response_.GetUpdateResponse(),
- -1,
- false, // ping_only
- ErrorCode::kOmahaUpdateIgnoredPerPolicy,
- metrics::CheckResult::kUpdateAvailable,
- metrics::CheckReaction::kIgnored,
- metrics::DownloadErrorCode::kUnset,
- &response,
- nullptr));
- EXPECT_FALSE(response.update_exists);
-}
-
-TEST_F(OmahaRequestActionTest,
- ValidUpdateOverCellularAllowedByUserPermissionTrue) {
- // This test tests that, when device policy is not set, update over cellular
- // is allowed as permission for update over cellular is set to true.
- OmahaResponse response;
- MockConnectionManager mock_cm;
-
- fake_prefs_.SetBoolean(kPrefsUpdateOverCellularPermission, true);
- fake_system_state_.set_connection_manager(&mock_cm);
-
- EXPECT_CALL(mock_cm, GetConnectionProperties(_, _))
- .WillRepeatedly(
- DoAll(SetArgumentPointee<0>(ConnectionType::kCellular),
- SetArgumentPointee<1>(ConnectionTethering::kUnknown),
- Return(true)));
- EXPECT_CALL(mock_cm, IsAllowedConnectionTypesForUpdateSet())
- .WillRepeatedly(Return(false));
- EXPECT_CALL(mock_cm, IsUpdateAllowedOver(ConnectionType::kCellular, _))
- .WillRepeatedly(Return(true));
-
- ASSERT_TRUE(
- TestUpdateCheck(nullptr, // request_params
- fake_update_response_.GetUpdateResponse(),
- -1,
- false, // ping_only
- ErrorCode::kSuccess,
- metrics::CheckResult::kUpdateAvailable,
- metrics::CheckReaction::kUpdating,
- metrics::DownloadErrorCode::kUnset,
- &response,
- nullptr));
- EXPECT_TRUE(response.update_exists);
-}
-
-TEST_F(OmahaRequestActionTest,
- ValidUpdateOverCellularBlockedByUpdateTargetNotMatch) {
- // This test tests that, when device policy is not set and permission for
- // update over cellular is set to false or does not exist, update over
- // cellular is blocked as update target does not match the omaha response.
- OmahaResponse response;
- MockConnectionManager mock_cm;
- // A version different from the version in omaha response.
- string diff_version = "99.99.99";
- // A size different from the size in omaha response.
- int64_t diff_size = 999;
-
- fake_prefs_.SetString(kPrefsUpdateOverCellularTargetVersion, diff_version);
- fake_prefs_.SetInt64(kPrefsUpdateOverCellularTargetSize, diff_size);
- // This test tests cellular (3G) being the only connection type being allowed.
- fake_system_state_.set_connection_manager(&mock_cm);
-
- EXPECT_CALL(mock_cm, GetConnectionProperties(_, _))
- .WillRepeatedly(
- DoAll(SetArgumentPointee<0>(ConnectionType::kCellular),
- SetArgumentPointee<1>(ConnectionTethering::kUnknown),
- Return(true)));
- EXPECT_CALL(mock_cm, IsAllowedConnectionTypesForUpdateSet())
- .WillRepeatedly(Return(false));
- EXPECT_CALL(mock_cm, IsUpdateAllowedOver(ConnectionType::kCellular, _))
- .WillRepeatedly(Return(true));
-
- ASSERT_FALSE(
- TestUpdateCheck(nullptr, // request_params
- fake_update_response_.GetUpdateResponse(),
- -1,
- false, // ping_only
- ErrorCode::kOmahaUpdateIgnoredOverCellular,
- metrics::CheckResult::kUpdateAvailable,
- metrics::CheckReaction::kIgnored,
- metrics::DownloadErrorCode::kUnset,
- &response,
- nullptr));
- EXPECT_FALSE(response.update_exists);
-}
-
-TEST_F(OmahaRequestActionTest,
- ValidUpdateOverCellularAllowedByUpdateTargetMatch) {
- // This test tests that, when device policy is not set and permission for
- // update over cellular is set to false or does not exist, update over
- // cellular is allowed as update target matches the omaha response.
- OmahaResponse response;
- MockConnectionManager mock_cm;
- // A version same as the version in omaha response.
- string new_version = fake_update_response_.version;
- // A size same as the size in omaha response.
- int64_t new_size = fake_update_response_.size;
-
- fake_prefs_.SetString(kPrefsUpdateOverCellularTargetVersion, new_version);
- fake_prefs_.SetInt64(kPrefsUpdateOverCellularTargetSize, new_size);
- fake_system_state_.set_connection_manager(&mock_cm);
-
- EXPECT_CALL(mock_cm, GetConnectionProperties(_, _))
- .WillRepeatedly(
- DoAll(SetArgumentPointee<0>(ConnectionType::kCellular),
- SetArgumentPointee<1>(ConnectionTethering::kUnknown),
- Return(true)));
- EXPECT_CALL(mock_cm, IsAllowedConnectionTypesForUpdateSet())
- .WillRepeatedly(Return(false));
- EXPECT_CALL(mock_cm, IsUpdateAllowedOver(ConnectionType::kCellular, _))
- .WillRepeatedly(Return(true));
-
- ASSERT_TRUE(
- TestUpdateCheck(nullptr, // request_params
- fake_update_response_.GetUpdateResponse(),
- -1,
- false, // ping_only
- ErrorCode::kSuccess,
- metrics::CheckResult::kUpdateAvailable,
- metrics::CheckReaction::kUpdating,
- metrics::DownloadErrorCode::kUnset,
- &response,
- nullptr));
- EXPECT_TRUE(response.update_exists);
-}
-
TEST_F(OmahaRequestActionTest, ValidUpdateBlockedByRollback) {
string rollback_version = "1234.0.0";
OmahaResponse response;
diff --git a/payload_state.cc b/payload_state.cc
index 846f901..1da472f 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -348,7 +348,6 @@
case ErrorCode::kOmahaRequestXMLHasEntityDecl:
case ErrorCode::kFilesystemVerifierError:
case ErrorCode::kUserCanceled:
- case ErrorCode::kOmahaUpdateIgnoredOverCellular:
LOG(INFO) << "Not incrementing URL index or failure count for this error";
break;
diff --git a/update_attempter.cc b/update_attempter.cc
index 6a7853d..8e25819 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -49,7 +49,6 @@
#include "update_engine/common/prefs_interface.h"
#include "update_engine/common/subprocess.h"
#include "update_engine/common/utils.h"
-#include "update_engine/connection_manager_interface.h"
#include "update_engine/libcurl_http_fetcher.h"
#include "update_engine/metrics.h"
#include "update_engine/omaha_request_action.h"
@@ -1004,20 +1003,9 @@
consecutive_failed_update_checks_ = 0;
}
- const OmahaResponse& omaha_response =
- omaha_request_action->GetOutputObject();
// Store the server-dictated poll interval, if any.
server_dictated_poll_interval_ =
- std::max(0, omaha_response.poll_interval);
-
- // This update is ignored by omaha request action because update over
- // cellular connection is not allowed. Needs to ask for user's permissions
- // to update.
- if (code == ErrorCode::kOmahaUpdateIgnoredOverCellular) {
- new_version_ = omaha_response.version;
- new_payload_size_ = omaha_response.size;
- SetStatusAndNotify(UpdateStatus::NEED_PERMISSION_TO_UPDATE);
- }
+ std::max(0, omaha_request_action->GetOutputObject().poll_interval);
}
}
if (code != ErrorCode::kSuccess) {
diff --git a/update_manager/chromeos_policy.cc b/update_manager/chromeos_policy.cc
index e3893d3..ffd378c 100644
--- a/update_manager/chromeos_policy.cc
+++ b/update_manager/chromeos_policy.cc
@@ -134,7 +134,6 @@
case ErrorCode::kOmahaRequestXMLHasEntityDecl:
case ErrorCode::kFilesystemVerifierError:
case ErrorCode::kUserCanceled:
- case ErrorCode::kOmahaUpdateIgnoredOverCellular:
LOG(INFO) << "Not changing URL index or failure count due to error "
<< chromeos_update_engine::utils::ErrorCodeToString(err_code)
<< " (" << static_cast<int>(err_code) << ")";
diff --git a/update_status_utils.cc b/update_status_utils.cc
index 5de3381..ff039b8 100644
--- a/update_status_utils.cc
+++ b/update_status_utils.cc
@@ -30,8 +30,6 @@
return update_engine::kUpdateStatusCheckingForUpdate;
case UpdateStatus::UPDATE_AVAILABLE:
return update_engine::kUpdateStatusUpdateAvailable;
- case UpdateStatus::NEED_PERMISSION_TO_UPDATE:
- return update_engine::kUpdateStatusNeedPermissionToUpdate;
case UpdateStatus::DOWNLOADING:
return update_engine::kUpdateStatusDownloading;
case UpdateStatus::VERIFYING:
@@ -63,9 +61,6 @@
} else if (s == update_engine::kUpdateStatusUpdateAvailable) {
*status = UpdateStatus::UPDATE_AVAILABLE;
return true;
- } else if (s == update_engine::kUpdateStatusNeedPermissionToUpdate) {
- *status = UpdateStatus::NEED_PERMISSION_TO_UPDATE;
- return true;
} else if (s == update_engine::kUpdateStatusDownloading) {
*status = UpdateStatus::DOWNLOADING;
return true;
