Add interactive override update restriction flags

am: 081c023b53

Change-Id: I14fb472a007a7ca09841ab47ba9a744ca4808c91

11 files changed, 106 insertions, 26 deletions

diff --git a/binder_bindings/android/brillo/IUpdateEngine.aidl b/binder_bindings/android/brillo/IUpdateEngine.aidl
index c85c0dcb..e549a4d4 100644
--- a/binder_bindings/android/brillo/IUpdateEngine.aidl
+++ b/binder_bindings/android/brillo/IUpdateEngine.aidl
@@ -21,7 +21,7 @@ import android.brillo.ParcelableUpdateEngineStatus;
 
 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 c57561c3..3f01e424 100644
--- a/binder_service_brillo.cc
+++ b/binder_service_brillo.cc
@@ -63,11 +63,15 @@ Status BinderUpdateEngineBrilloService::SetUpdateAttemptFlags(int flags) {
 }
 
 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 81a4e4cf..c802fcaa 100644
--- a/binder_service_brillo.h
+++ b/binder_service_brillo.h
@@ -54,7 +54,8 @@ class BinderUpdateEngineBrilloService : public android::brillo::BnUpdateEngine,
   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 b4625e45..54b33ed7 100644
--- a/client_library/client_binder.cc
+++ b/client_library/client_binder.cc
@@ -48,11 +48,13 @@ bool BinderUpdateEngineClient::Init() {
 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 38a3096a..370587b2 100644
--- a/common_service.cc
+++ b/common_service.cc
@@ -88,16 +88,20 @@ bool UpdateEngineService::SetUpdateAttemptFlags(ErrorPtr* /* error */,
 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 261c9e84..544dd931 100644
--- a/common_service.h
+++ b/common_service.h
@@ -49,7 +49,8 @@ class UpdateEngineService {
   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 71e42d0c..c1244661 100644
--- a/common_service_unittest.cc
+++ b/common_service_unittest.cc
@@ -57,12 +57,32 @@ class UpdateEngineServiceTest : public ::testing::Test {
 };
 
 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_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::kFlagNonInteractive));
+      &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 d64dcbe5..d88b840e 100644
--- a/mock_update_attempter.h
+++ b/mock_update_attempter.h
@@ -44,9 +44,10 @@ class MockUpdateAttempter : public UpdateAttempter {
 
   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 e40dfe73..a6f9fa57 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -773,9 +773,20 @@ BootControlInterface::Slot UpdateAttempter::GetRollbackSlot() const {
   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 @@ void UpdateAttempter::CheckForUpdate(const string& app_version,
     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 @@ void UpdateAttempter::OnUpdateScheduled(EvalStatus status,
               << (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 758494ae..d2fe3ec3 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -148,9 +148,11 @@ class UpdateAttempter : public ActionProcessorDelegate,
   // 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 d54c867b..bdc01967 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -1049,14 +1049,14 @@ TEST_F(UpdateAttempterTest, AnyUpdateSourceDisallowedOfficialNormal) {
 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 @@ TEST_F(UpdateAttempterTest, UpdateAttemptFlagsCachedAtUpdateStart) {
             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