Add interactive override update restriction flags
am: 081c023b53
Change-Id: I14fb472a007a7ca09841ab47ba9a744ca4808c91
diff --git a/binder_bindings/android/brillo/IUpdateEngine.aidl b/binder_bindings/android/brillo/IUpdateEngine.aidl
index c85c0dc..e549a4d 100644
--- a/binder_bindings/android/brillo/IUpdateEngine.aidl
+++ b/binder_bindings/android/brillo/IUpdateEngine.aidl
@@ -21,7 +21,7 @@
interface IUpdateEngine {
void SetUpdateAttemptFlags(in int flags);
- void AttemptUpdate(in String app_version, in String omaha_url, in int flags);
+ boolean AttemptUpdate(in String app_version, in String omaha_url, in int flags);
void AttemptRollback(in boolean powerwash);
boolean CanRollback();
void ResetStatus();
diff --git a/binder_service_brillo.cc b/binder_service_brillo.cc
index c57561c..3f01e42 100644
--- a/binder_service_brillo.cc
+++ b/binder_service_brillo.cc
@@ -63,11 +63,15 @@
}
Status BinderUpdateEngineBrilloService::AttemptUpdate(
- const String16& app_version, const String16& omaha_url, int flags) {
+ const String16& app_version,
+ const String16& omaha_url,
+ int flags,
+ bool* out_result) {
return CallCommonHandler(&UpdateEngineService::AttemptUpdate,
NormalString(app_version),
NormalString(omaha_url),
- flags);
+ flags,
+ out_result);
}
Status BinderUpdateEngineBrilloService::AttemptRollback(bool powerwash) {
diff --git a/binder_service_brillo.h b/binder_service_brillo.h
index 81a4e4c..c802fca 100644
--- a/binder_service_brillo.h
+++ b/binder_service_brillo.h
@@ -54,7 +54,8 @@
android::binder::Status SetUpdateAttemptFlags(int flags) override;
android::binder::Status AttemptUpdate(const android::String16& app_version,
const android::String16& omaha_url,
- int flags) override;
+ int flags,
+ bool* out_result) override;
android::binder::Status AttemptRollback(bool powerwash) override;
android::binder::Status CanRollback(bool* out_can_rollback) override;
android::binder::Status ResetStatus() override;
diff --git a/client_library/client_binder.cc b/client_library/client_binder.cc
index b4625e4..54b33ed 100644
--- a/client_library/client_binder.cc
+++ b/client_library/client_binder.cc
@@ -48,11 +48,13 @@
bool BinderUpdateEngineClient::AttemptUpdate(const string& in_app_version,
const string& in_omaha_url,
bool at_user_request) {
+ bool started;
return service_
->AttemptUpdate(
String16{in_app_version.c_str()},
String16{in_omaha_url.c_str()},
- at_user_request ? 0 : UpdateAttemptFlags::kFlagNonInteractive)
+ at_user_request ? 0 : UpdateAttemptFlags::kFlagNonInteractive,
+ &started)
.isOk();
}
diff --git a/common_service.cc b/common_service.cc
index 38a3096..370587b 100644
--- a/common_service.cc
+++ b/common_service.cc
@@ -88,16 +88,20 @@
bool UpdateEngineService::AttemptUpdate(ErrorPtr* /* error */,
const string& in_app_version,
const string& in_omaha_url,
- int32_t in_flags_as_int) {
+ int32_t in_flags_as_int,
+ bool* out_result) {
auto flags = static_cast<UpdateAttemptFlags>(in_flags_as_int);
bool interactive = !(flags & UpdateAttemptFlags::kFlagNonInteractive);
+ bool restrict_downloads = (flags & UpdateAttemptFlags::kFlagRestrictDownload);
LOG(INFO) << "Attempt update: app_version=\"" << in_app_version << "\" "
<< "omaha_url=\"" << in_omaha_url << "\" "
<< "flags=0x" << std::hex << flags << " "
- << "interactive=" << (interactive ? "yes" : "no");
- system_state_->update_attempter()->CheckForUpdate(
- in_app_version, in_omaha_url, interactive);
+ << "interactive=" << (interactive ? "yes " : "no ")
+ << "RestrictDownload=" << (restrict_downloads ? "yes " : "no ");
+
+ *out_result = system_state_->update_attempter()->CheckForUpdate(
+ in_app_version, in_omaha_url, flags);
return true;
}
diff --git a/common_service.h b/common_service.h
index 261c9e8..544dd93 100644
--- a/common_service.h
+++ b/common_service.h
@@ -49,7 +49,8 @@
bool AttemptUpdate(brillo::ErrorPtr* error,
const std::string& in_app_version,
const std::string& in_omaha_url,
- int32_t in_flags_as_int);
+ int32_t in_flags_as_int,
+ bool* out_result);
bool AttemptRollback(brillo::ErrorPtr* error, bool in_powerwash);
diff --git a/common_service_unittest.cc b/common_service_unittest.cc
index 71e42d0..c124466 100644
--- a/common_service_unittest.cc
+++ b/common_service_unittest.cc
@@ -57,12 +57,32 @@
};
TEST_F(UpdateEngineServiceTest, AttemptUpdate) {
- EXPECT_CALL(*mock_update_attempter_, CheckForUpdate(
- "app_ver", "url", false /* interactive */));
- // The update is non-interactive when we pass the non-interactive flag.
- EXPECT_TRUE(common_service_.AttemptUpdate(
- &error_, "app_ver", "url", UpdateAttemptFlags::kFlagNonInteractive));
+ EXPECT_CALL(
+ *mock_update_attempter_,
+ CheckForUpdate("app_ver", "url", UpdateAttemptFlags::kFlagNonInteractive))
+ .WillOnce(Return(true));
+
+ // The non-interactive flag needs to be passed through to CheckForUpdate.
+ bool result = false;
+ EXPECT_TRUE(
+ common_service_.AttemptUpdate(&error_,
+ "app_ver",
+ "url",
+ UpdateAttemptFlags::kFlagNonInteractive,
+ &result));
EXPECT_EQ(nullptr, error_);
+ EXPECT_TRUE(result);
+}
+
+TEST_F(UpdateEngineServiceTest, AttemptUpdateReturnsFalse) {
+ EXPECT_CALL(*mock_update_attempter_,
+ CheckForUpdate("app_ver", "url", UpdateAttemptFlags::kNone))
+ .WillOnce(Return(false));
+ bool result = true;
+ EXPECT_TRUE(common_service_.AttemptUpdate(
+ &error_, "app_ver", "url", UpdateAttemptFlags::kNone, &result));
+ EXPECT_EQ(nullptr, error_);
+ EXPECT_FALSE(result);
}
// SetChannel is allowed when there's no device policy (the device is not
diff --git a/mock_update_attempter.h b/mock_update_attempter.h
index d64dcbe..d88b840 100644
--- a/mock_update_attempter.h
+++ b/mock_update_attempter.h
@@ -44,9 +44,10 @@
MOCK_METHOD0(GetCurrentUpdateAttemptFlags, UpdateAttemptFlags(void));
- MOCK_METHOD3(CheckForUpdate, void(const std::string& app_version,
- const std::string& omaha_url,
- bool is_interactive));
+ MOCK_METHOD3(CheckForUpdate,
+ bool(const std::string& app_version,
+ const std::string& omaha_url,
+ UpdateAttemptFlags flags));
MOCK_METHOD0(RefreshDevicePolicy, void(void));
diff --git a/update_attempter.cc b/update_attempter.cc
index e40dfe7..a6f9fa5 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -773,9 +773,20 @@
return BootControlInterface::kInvalidSlot;
}
-void UpdateAttempter::CheckForUpdate(const string& app_version,
+bool UpdateAttempter::CheckForUpdate(const string& app_version,
const string& omaha_url,
- bool interactive) {
+ UpdateAttemptFlags flags) {
+ bool interactive = !(flags & UpdateAttemptFlags::kFlagNonInteractive);
+
+ if (interactive && status_ != UpdateStatus::IDLE) {
+ // An update check is either in-progress, or an update has completed and the
+ // system is in UPDATED_NEED_REBOOT. Either way, don't do an interactive
+ // update at this time
+ LOG(INFO) << "Refusing to do an interactive update with an update already "
+ "in progress";
+ return false;
+ }
+
LOG(INFO) << "Forced update check requested.";
forced_app_version_.clear();
forced_omaha_url_.clear();
@@ -797,12 +808,22 @@
forced_omaha_url_ = constants::kOmahaDefaultAUTestURL;
}
+ if (interactive) {
+ // Use the passed-in update attempt flags for this update attempt instead
+ // of the previously set ones.
+ current_update_attempt_flags_ = flags;
+ // Note: The caching for non-interactive update checks happens in
+ // OnUpdateScheduled().
+ }
+
if (forced_update_pending_callback_.get()) {
// Make sure that a scheduling request is made prior to calling the forced
// update pending callback.
ScheduleUpdates();
forced_update_pending_callback_->Run(true, interactive);
}
+
+ return true;
}
bool UpdateAttempter::RebootIfNeeded() {
@@ -860,9 +881,12 @@
<< (params.is_interactive ? "interactive" : "periodic")
<< " update.";
- // Cache the update attempt flags that will be used by this update attempt
- // so that they can't be changed mid-way through.
- current_update_attempt_flags_ = update_attempt_flags_;
+ if (!params.is_interactive) {
+ // Cache the update attempt flags that will be used by this update attempt
+ // so that they can't be changed mid-way through.
+ current_update_attempt_flags_ = update_attempt_flags_;
+ }
+
LOG(INFO) << "Update attempt flags in use = 0x" << std::hex
<< current_update_attempt_flags_;
diff --git a/update_attempter.h b/update_attempter.h
index 758494a..d2fe3ec 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -148,9 +148,11 @@
// This is the internal entry point for going through an
// update. If the current status is idle invokes Update.
// This is called by the DBus implementation.
- virtual void CheckForUpdate(const std::string& app_version,
+ // This returns true if an update check was started, false if a check or an
+ // update was already in progress.
+ virtual bool CheckForUpdate(const std::string& app_version,
const std::string& omaha_url,
- bool is_interactive);
+ UpdateAttemptFlags flags);
// This is the internal entry point for going through a rollback. This will
// attempt to run the postinstall on the non-active partition and set it as
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index d54c867..bdc0196 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -1049,14 +1049,14 @@
TEST_F(UpdateAttempterTest, CheckForUpdateAUTest) {
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
fake_system_state_.fake_hardware()->SetAreDevFeaturesEnabled(false);
- attempter_.CheckForUpdate("", "autest", true);
+ attempter_.CheckForUpdate("", "autest", UpdateAttemptFlags::kNone);
EXPECT_EQ(constants::kOmahaDefaultAUTestURL, attempter_.forced_omaha_url());
}
TEST_F(UpdateAttempterTest, CheckForUpdateScheduledAUTest) {
fake_system_state_.fake_hardware()->SetIsOfficialBuild(true);
fake_system_state_.fake_hardware()->SetAreDevFeaturesEnabled(false);
- attempter_.CheckForUpdate("", "autest-scheduled", true);
+ attempter_.CheckForUpdate("", "autest-scheduled", UpdateAttemptFlags::kNone);
EXPECT_EQ(constants::kOmahaDefaultAUTestURL, attempter_.forced_omaha_url());
}
@@ -1135,4 +1135,25 @@
attempter_.GetCurrentUpdateAttemptFlags());
}
+TEST_F(UpdateAttempterTest, InteractiveUpdateUsesPassedRestrictions) {
+ attempter_.SetUpdateAttemptFlags(UpdateAttemptFlags::kFlagRestrictDownload);
+
+ attempter_.CheckForUpdate("", "", UpdateAttemptFlags::kNone);
+ EXPECT_EQ(UpdateAttemptFlags::kNone,
+ attempter_.GetCurrentUpdateAttemptFlags());
+}
+
+TEST_F(UpdateAttempterTest, NonInteractiveUpdateUsesSetRestrictions) {
+ attempter_.SetUpdateAttemptFlags(UpdateAttemptFlags::kNone);
+
+ // This tests that when CheckForUpdate() is called with the non-interactive
+ // flag set, that it doesn't change the current UpdateAttemptFlags.
+ attempter_.CheckForUpdate("",
+ "",
+ UpdateAttemptFlags::kFlagNonInteractive |
+ UpdateAttemptFlags::kFlagRestrictDownload);
+ EXPECT_EQ(UpdateAttemptFlags::kNone,
+ attempter_.GetCurrentUpdateAttemptFlags());
+}
+
} // namespace chromeos_update_engine