Allow to Suspend/Resume the ActionProcessor.

This patch implements the core functionality of suspend/resume actions
from the ActionProcessor. No actions support suspend/resume yet.

Bug: 27047026
TEST=Added unittets, tested on edison-eng.

Change-Id: Ib9600098dbccf05fc30f10f0add4a5bc87892b66

7 files changed, 266 insertions, 106 deletions

diff --git a/common/action.h b/common/action.h
index d8049ac0..6c88216e 100644
--- a/common/action.h
+++ b/common/action.h
@@ -124,6 +124,15 @@ class AbstractAction {
   // Only the ActionProcessor should call this.
   virtual void TerminateProcessing() {}
 
+  // Called on asynchronous actions if the processing is suspended and resumed,
+  // respectively. These methods are called by the ActionProcessor and should
+  // not be explicitly called.
+  // The action may still call ActionCompleted() once the action is completed
+  // while the processing is suspended, for example if suspend/resume is not
+  // implemented for the given action.
+  virtual void SuspendAction() {}
+  virtual void ResumeAction() {}
+
   // These methods are useful for debugging. TODO(adlr): consider using
   // std::type_info for this?
   // Type() returns a string of the Action type. I.e., for DownloadAction,
diff --git a/common/action_processor.cc b/common/action_processor.cc
index 7ccdfbd3..2618e4e7 100644
--- a/common/action_processor.cc
+++ b/common/action_processor.cc
@@ -21,14 +21,12 @@
 #include <base/logging.h>
 
 #include "update_engine/common/action.h"
+#include "update_engine/common/error_code_utils.h"
 
 using std::string;
 
 namespace chromeos_update_engine {
 
-ActionProcessor::ActionProcessor()
-    : current_action_(nullptr), delegate_(nullptr) {}
-
 ActionProcessor::~ActionProcessor() {
   if (IsRunning())
     StopProcessing();
@@ -45,8 +43,7 @@ void ActionProcessor::StartProcessing() {
   CHECK(!IsRunning());
   if (!actions_.empty()) {
     current_action_ = actions_.front();
-    LOG(INFO) << "ActionProcessor::StartProcessing: "
-              << current_action_->Type();
+    LOG(INFO) << "ActionProcessor: starting " << current_action_->Type();
     actions_.pop_front();
     current_action_->PerformAction();
   }
@@ -54,17 +51,55 @@ void ActionProcessor::StartProcessing() {
 
 void ActionProcessor::StopProcessing() {
   CHECK(IsRunning());
-  CHECK(current_action_);
-  current_action_->TerminateProcessing();
-  CHECK(current_action_);
-  current_action_->SetProcessor(nullptr);
-  LOG(INFO) << "ActionProcessor::StopProcessing: aborted "
-            << current_action_->Type();
+  if (current_action_) {
+    current_action_->TerminateProcessing();
+    current_action_->SetProcessor(nullptr);
+  }
+  LOG(INFO) << "ActionProcessor: aborted "
+            << (current_action_ ? current_action_->Type() : "")
+            << (suspended_ ? " while suspended" : "");
   current_action_ = nullptr;
+  suspended_ = false;
   if (delegate_)
     delegate_->ProcessingStopped(this);
 }
 
+void ActionProcessor::SuspendProcessing() {
+  // No current_action_ when not suspended means that the action processor was
+  // never started or already finished.
+  if (suspended_ || !current_action_) {
+    LOG(WARNING) << "Called SuspendProcessing while not processing.";
+    return;
+  }
+  suspended_ = true;
+
+  // If there's a current action we should notify it that it should suspend, but
+  // the action can ignore that and terminate at any point.
+  LOG(INFO) << "ActionProcessor: suspending " << current_action_->Type();
+  current_action_->SuspendAction();
+}
+
+void ActionProcessor::ResumeProcessing() {
+  if (!suspended_) {
+    LOG(WARNING) << "Called ResumeProcessing while not suspended.";
+    return;
+  }
+  suspended_ = false;
+  if (current_action_) {
+    // The current_action_ did not call ActionComplete while suspended, so we
+    // should notify it of the resume operation.
+    LOG(INFO) << "ActionProcessor: resuming " << current_action_->Type();
+    current_action_->ResumeAction();
+  } else {
+    // The last action called ActionComplete while suspended, so there is
+    // already a log message with the type of the finished action. We simply
+    // state that we are resuming processing and the next function will log the
+    // start of the next action or processing completion.
+    LOG(INFO) << "ActionProcessor: resuming processing";
+    StartNextActionOrFinish(suspended_error_code_);
+  }
+}
+
 void ActionProcessor::ActionComplete(AbstractAction* actionptr,
                                      ErrorCode code) {
   CHECK_EQ(actionptr, current_action_);
@@ -74,17 +109,26 @@ void ActionProcessor::ActionComplete(AbstractAction* actionptr,
   current_action_->ActionCompleted(code);
   current_action_->SetProcessor(nullptr);
   current_action_ = nullptr;
-  if (actions_.empty()) {
-    LOG(INFO) << "ActionProcessor::ActionComplete: finished last action of"
-                 " type " << old_type;
-  } else if (code != ErrorCode::kSuccess) {
-    LOG(INFO) << "ActionProcessor::ActionComplete: " << old_type
-              << " action failed. Aborting processing.";
+  LOG(INFO) << "ActionProcessor: finished "
+            << (actions_.empty() ? "last action " : "") << old_type
+            << (suspended_ ? " while suspended" : "")
+            << " with code " << utils::ErrorCodeToString(code);
+  if (!actions_.empty() && code != ErrorCode::kSuccess) {
+    LOG(INFO) << "ActionProcessor: Aborting processing due to failure.";
     actions_.clear();
   }
+  if (suspended_) {
+    // If an action finished while suspended we don't start the next action (or
+    // terminate the processing) until the processor is resumed. This condition
+    // will be flagged by a nullptr current_action_ while suspended_ is true.
+    suspended_error_code_ = code;
+    return;
+  }
+  StartNextActionOrFinish(code);
+}
+
+void ActionProcessor::StartNextActionOrFinish(ErrorCode code) {
   if (actions_.empty()) {
-    LOG(INFO) << "ActionProcessor::ActionComplete: finished last action of"
-                 " type " << old_type;
     if (delegate_) {
       delegate_->ProcessingDone(this, code);
     }
@@ -92,8 +136,7 @@ void ActionProcessor::ActionComplete(AbstractAction* actionptr,
   }
   current_action_ = actions_.front();
   actions_.pop_front();
-  LOG(INFO) << "ActionProcessor::ActionComplete: finished " << old_type
-            << ", starting " << current_action_->Type();
+  LOG(INFO) << "ActionProcessor: starting " << current_action_->Type();
   current_action_->PerformAction();
 }
 
diff --git a/common/action_processor.h b/common/action_processor.h
index d61e12d7..050eee97 100644
--- a/common/action_processor.h
+++ b/common/action_processor.h
@@ -20,6 +20,7 @@
 #include <deque>
 
 #include <base/macros.h>
+#include <brillo/errors/error.h>
 
 #include "update_engine/common/error_code.h"
 
@@ -40,7 +41,7 @@ class ActionProcessorDelegate;
 
 class ActionProcessor {
  public:
-  ActionProcessor();
+  ActionProcessor() = default;
 
   virtual ~ActionProcessor();
 
@@ -54,8 +55,20 @@ class ActionProcessor {
   // will be lost and must be re-enqueued if this Processor is to use it.
   void StopProcessing();
 
-  // Returns true iff an Action is currently processing.
-  bool IsRunning() const { return nullptr != current_action_; }
+  // Suspend the processing. If an Action is running, it will have the
+  // SuspendProcessing() called on it, and it should suspend operations until
+  // ResumeProcessing() is called on this class to continue. While suspended,
+  // no new actions will be started. Calling SuspendProcessing while the
+  // processing is suspended or not running this method performs no action.
+  void SuspendProcessing();
+
+  // Resume the suspended processing. If the ActionProcessor is not suspended
+  // or not running on the first place this method performs no action.
+  void ResumeProcessing();
+
+  // Returns true iff the processing was started but not yet completed nor
+  // stopped.
+  bool IsRunning() const { return current_action_ != nullptr || suspended_; }
 
   // Adds another Action to the end of the queue.
   virtual void EnqueueAction(AbstractAction* action);
@@ -75,15 +88,29 @@ class ActionProcessor {
   void ActionComplete(AbstractAction* actionptr, ErrorCode code);
 
  private:
+  // Continue processing actions (if any) after the last action terminated with
+  // the passed error code. If there are no more actions to process, the
+  // processing will terminate.
+  void StartNextActionOrFinish(ErrorCode code);
+
   // Actions that have not yet begun processing, in the order in which
   // they'll be processed.
   std::deque<AbstractAction*> actions_;
 
   // A pointer to the currently processing Action, if any.
-  AbstractAction* current_action_;
+  AbstractAction* current_action_{nullptr};
+
+  // The ErrorCode reported by an action that was suspended but finished while
+  // being suspended. This error code is stored here to be reported back to the
+  // delegate once the processor is resumed.
+  ErrorCode suspended_error_code_{ErrorCode::kSuccess};
+
+  // Whether the action processor is or should be suspended.
+  bool suspended_{false};
 
   // A pointer to the delegate, or null if none.
-  ActionProcessorDelegate *delegate_;
+  ActionProcessorDelegate* delegate_{nullptr};
+
   DISALLOW_COPY_AND_ASSIGN(ActionProcessor);
 };
 
diff --git a/common/action_processor_unittest.cc b/common/action_processor_unittest.cc
index 8285470d..631e42d1 100644
--- a/common/action_processor_unittest.cc
+++ b/common/action_processor_unittest.cc
@@ -16,9 +16,12 @@
 
 #include "update_engine/common/action_processor.h"
 
-#include <gtest/gtest.h>
 #include <string>
+
+#include <gtest/gtest.h>
+
 #include "update_engine/common/action.h"
+#include "update_engine/common/mock_action.h"
 
 using std::string;
 
@@ -51,26 +54,6 @@ class ActionProcessorTestAction : public Action<ActionProcessorTestAction> {
   string Type() const { return "ActionProcessorTestAction"; }
 };
 
-class ActionProcessorTest : public ::testing::Test { };
-
-// This test creates two simple Actions and sends a message via an ActionPipe
-// from one to the other.
-TEST(ActionProcessorTest, SimpleTest) {
-  ActionProcessorTestAction action;
-  ActionProcessor action_processor;
-  EXPECT_FALSE(action_processor.IsRunning());
-  action_processor.EnqueueAction(&action);
-  EXPECT_FALSE(action_processor.IsRunning());
-  EXPECT_FALSE(action.IsRunning());
-  action_processor.StartProcessing();
-  EXPECT_TRUE(action_processor.IsRunning());
-  EXPECT_TRUE(action.IsRunning());
-  EXPECT_EQ(action_processor.current_action(), &action);
-  action.CompleteAction();
-  EXPECT_FALSE(action_processor.IsRunning());
-  EXPECT_FALSE(action.IsRunning());
-}
-
 namespace {
 class MyActionProcessorDelegate : public ActionProcessorDelegate {
  public:
@@ -109,53 +92,79 @@ class MyActionProcessorDelegate : public ActionProcessorDelegate {
 };
 }  // namespace
 
-TEST(ActionProcessorTest, DelegateTest) {
-  ActionProcessorTestAction action;
-  ActionProcessor action_processor;
-  MyActionProcessorDelegate delegate(&action_processor);
-  action_processor.set_delegate(&delegate);
-
-  action_processor.EnqueueAction(&action);
-  action_processor.StartProcessing();
-  action.CompleteAction();
-  action_processor.set_delegate(nullptr);
-  EXPECT_TRUE(delegate.processing_done_called_);
-  EXPECT_TRUE(delegate.action_completed_called_);
+class ActionProcessorTest : public ::testing::Test {
+  void SetUp() override {
+    action_processor_.set_delegate(&delegate_);
+    // Silence Type() calls used for logging.
+    EXPECT_CALL(mock_action_, Type()).Times(testing::AnyNumber());
+  }
+
+  void TearDown() override {
+    action_processor_.set_delegate(nullptr);
+  }
+
+ protected:
+  // The ActionProcessor under test.
+  ActionProcessor action_processor_;
+
+  MyActionProcessorDelegate delegate_{&action_processor_};
+
+  // Common actions used during most tests.
+  testing::StrictMock<MockAction> mock_action_;
+  ActionProcessorTestAction action_;
+};
+
+TEST_F(ActionProcessorTest, SimpleTest) {
+  EXPECT_FALSE(action_processor_.IsRunning());
+  action_processor_.EnqueueAction(&action_);
+  EXPECT_FALSE(action_processor_.IsRunning());
+  EXPECT_FALSE(action_.IsRunning());
+  action_processor_.StartProcessing();
+  EXPECT_TRUE(action_processor_.IsRunning());
+  EXPECT_TRUE(action_.IsRunning());
+  EXPECT_EQ(action_processor_.current_action(), &action_);
+  action_.CompleteAction();
+  EXPECT_FALSE(action_processor_.IsRunning());
+  EXPECT_FALSE(action_.IsRunning());
+}
+
+TEST_F(ActionProcessorTest, DelegateTest) {
+  action_processor_.EnqueueAction(&action_);
+  action_processor_.StartProcessing();
+  action_.CompleteAction();
+  EXPECT_TRUE(delegate_.processing_done_called_);
+  EXPECT_TRUE(delegate_.action_completed_called_);
 }
 
-TEST(ActionProcessorTest, StopProcessingTest) {
-  ActionProcessorTestAction action;
-  ActionProcessor action_processor;
-  MyActionProcessorDelegate delegate(&action_processor);
-  action_processor.set_delegate(&delegate);
-
-  action_processor.EnqueueAction(&action);
-  action_processor.StartProcessing();
-  action_processor.StopProcessing();
-  action_processor.set_delegate(nullptr);
-  EXPECT_TRUE(delegate.processing_stopped_called_);
-  EXPECT_FALSE(delegate.action_completed_called_);
-  EXPECT_FALSE(action_processor.IsRunning());
-  EXPECT_EQ(nullptr, action_processor.current_action());
+TEST_F(ActionProcessorTest, StopProcessingTest) {
+  action_processor_.EnqueueAction(&action_);
+  action_processor_.StartProcessing();
+  action_processor_.StopProcessing();
+  EXPECT_TRUE(delegate_.processing_stopped_called_);
+  EXPECT_FALSE(delegate_.action_completed_called_);
+  EXPECT_FALSE(action_processor_.IsRunning());
+  EXPECT_EQ(nullptr, action_processor_.current_action());
 }
 
-TEST(ActionProcessorTest, ChainActionsTest) {
+TEST_F(ActionProcessorTest, ChainActionsTest) {
+  // This test doesn't use a delegate since it terminates several actions.
+  action_processor_.set_delegate(nullptr);
+
   ActionProcessorTestAction action1, action2;
-  ActionProcessor action_processor;
-  action_processor.EnqueueAction(&action1);
-  action_processor.EnqueueAction(&action2);
-  action_processor.StartProcessing();
-  EXPECT_EQ(&action1, action_processor.current_action());
-  EXPECT_TRUE(action_processor.IsRunning());
+  action_processor_.EnqueueAction(&action1);
+  action_processor_.EnqueueAction(&action2);
+  action_processor_.StartProcessing();
+  EXPECT_EQ(&action1, action_processor_.current_action());
+  EXPECT_TRUE(action_processor_.IsRunning());
   action1.CompleteAction();
-  EXPECT_EQ(&action2, action_processor.current_action());
-  EXPECT_TRUE(action_processor.IsRunning());
+  EXPECT_EQ(&action2, action_processor_.current_action());
+  EXPECT_TRUE(action_processor_.IsRunning());
   action2.CompleteAction();
-  EXPECT_EQ(nullptr, action_processor.current_action());
-  EXPECT_FALSE(action_processor.IsRunning());
+  EXPECT_EQ(nullptr, action_processor_.current_action());
+  EXPECT_FALSE(action_processor_.IsRunning());
 }
 
-TEST(ActionProcessorTest, DtorTest) {
+TEST_F(ActionProcessorTest, DtorTest) {
   ActionProcessorTestAction action1, action2;
   {
     ActionProcessor action_processor;
@@ -169,22 +178,87 @@ TEST(ActionProcessorTest, DtorTest) {
   EXPECT_FALSE(action2.IsRunning());
 }
 
-TEST(ActionProcessorTest, DefaultDelegateTest) {
+TEST_F(ActionProcessorTest, DefaultDelegateTest) {
   // Just make sure it doesn't crash
-  ActionProcessorTestAction action;
-  ActionProcessor action_processor;
-  ActionProcessorDelegate delegate;
-  action_processor.set_delegate(&delegate);
+  action_processor_.EnqueueAction(&action_);
+  action_processor_.StartProcessing();
+  action_.CompleteAction();
+
+  action_processor_.EnqueueAction(&action_);
+  action_processor_.StartProcessing();
+  action_processor_.StopProcessing();
+}
+
+// This test suspends and resume the action processor while running one action_.
+TEST_F(ActionProcessorTest, SuspendResumeTest) {
+  action_processor_.EnqueueAction(&mock_action_);
+
+  testing::InSequence s;
+  EXPECT_CALL(mock_action_, PerformAction());
+  action_processor_.StartProcessing();
+
+  EXPECT_CALL(mock_action_, SuspendAction());
+  action_processor_.SuspendProcessing();
+  // Suspending the processor twice should not suspend the action twice.
+  action_processor_.SuspendProcessing();
+
+  // IsRunning should return whether there's is an action doing some work, even
+  // if it is suspended.
+  EXPECT_TRUE(action_processor_.IsRunning());
+  EXPECT_EQ(&mock_action_, action_processor_.current_action());
+
+  EXPECT_CALL(mock_action_, ResumeAction());
+  action_processor_.ResumeProcessing();
+
+  // Calling ResumeProcessing twice should not affect the action_.
+  action_processor_.ResumeProcessing();
+
+  action_processor_.ActionComplete(&mock_action_, ErrorCode::kSuccess);
+}
+
+// This test suspends an action that presumably doesn't support suspend/resume
+// and it finished before being resumed.
+TEST_F(ActionProcessorTest, ActionCompletedWhileSuspendedTest) {
+  action_processor_.EnqueueAction(&mock_action_);
+
+  testing::InSequence s;
+  EXPECT_CALL(mock_action_, PerformAction());
+  action_processor_.StartProcessing();
+
+  EXPECT_CALL(mock_action_, SuspendAction());
+  action_processor_.SuspendProcessing();
+
+  // Simulate the action completion while suspended. No other call to
+  // |mock_action_| is expected at this point.
+  action_processor_.ActionComplete(&mock_action_, ErrorCode::kSuccess);
+
+  // The processing should not be done since the ActionProcessor is suspended
+  // and the processing is considered to be still running until resumed.
+  EXPECT_FALSE(delegate_.processing_done_called_);
+  EXPECT_TRUE(action_processor_.IsRunning());
+
+  action_processor_.ResumeProcessing();
+  EXPECT_TRUE(delegate_.processing_done_called_);
+  EXPECT_FALSE(delegate_.processing_stopped_called_);
+}
 
-  action_processor.EnqueueAction(&action);
-  action_processor.StartProcessing();
-  action.CompleteAction();
+TEST_F(ActionProcessorTest, StoppedWhileSuspendedTest) {
+  action_processor_.EnqueueAction(&mock_action_);
 
-  action_processor.EnqueueAction(&action);
-  action_processor.StartProcessing();
-  action_processor.StopProcessing();
+  testing::InSequence s;
+  EXPECT_CALL(mock_action_, PerformAction());
+  action_processor_.StartProcessing();
+  EXPECT_CALL(mock_action_, SuspendAction());
+  action_processor_.SuspendProcessing();
 
-  action_processor.set_delegate(nullptr);
+  EXPECT_CALL(mock_action_, TerminateProcessing());
+  action_processor_.StopProcessing();
+  // Stopping the processing should abort the current execution no matter what.
+  EXPECT_TRUE(delegate_.processing_stopped_called_);
+  EXPECT_FALSE(delegate_.processing_done_called_);
+  EXPECT_FALSE(delegate_.action_completed_called_);
+  EXPECT_FALSE(action_processor_.IsRunning());
+  EXPECT_EQ(nullptr, action_processor_.current_action());
 }
 
 }  // namespace chromeos_update_engine
diff --git a/mock_action.h b/common/mock_action.h
index 0ba796d0..06acad1b 100644
--- a/mock_action.h
+++ b/common/mock_action.h
@@ -14,8 +14,8 @@
 // limitations under the License.
 //
 
-#ifndef UPDATE_ENGINE_MOCK_ACTION_H_
-#define UPDATE_ENGINE_MOCK_ACTION_H_
+#ifndef UPDATE_ENGINE_COMMON_MOCK_ACTION_H_
+#define UPDATE_ENGINE_COMMON_MOCK_ACTION_H_
 
 #include <string>
 
@@ -27,7 +27,7 @@ namespace chromeos_update_engine {
 
 class MockAction;
 
-template<>
+template <>
 class ActionTraits<MockAction> {
  public:
   typedef NoneType OutputObjectType;
@@ -36,10 +36,17 @@ class ActionTraits<MockAction> {
 
 class MockAction : public Action<MockAction> {
  public:
+  MockAction() {
+    ON_CALL(*this, Type()).WillByDefault(testing::Return("MockAction"));
+  }
+
   MOCK_METHOD0(PerformAction, void());
+  MOCK_METHOD0(TerminateProcessing, void());
+  MOCK_METHOD0(SuspendAction, void());
+  MOCK_METHOD0(ResumeAction, void());
   MOCK_CONST_METHOD0(Type, std::string());
 };
 
 }  // namespace chromeos_update_engine
 
-#endif  // UPDATE_ENGINE_MOCK_ACTION_H_
+#endif  // UPDATE_ENGINE_COMMON_MOCK_ACTION_H_
diff --git a/mock_action_processor.h b/common/mock_action_processor.h
index c98ff3c9..04275c1f 100644
--- a/mock_action_processor.h
+++ b/common/mock_action_processor.h
@@ -14,8 +14,8 @@
 // limitations under the License.
 //
 
-#ifndef UPDATE_ENGINE_MOCK_ACTION_PROCESSOR_H_
-#define UPDATE_ENGINE_MOCK_ACTION_PROCESSOR_H_
+#ifndef UPDATE_ENGINE_COMMON_MOCK_ACTION_PROCESSOR_H_
+#define UPDATE_ENGINE_COMMON_MOCK_ACTION_PROCESSOR_H_
 
 #include <gmock/gmock.h>
 
@@ -31,4 +31,4 @@ class MockActionProcessor : public ActionProcessor {
 
 }  // namespace chromeos_update_engine
 
-#endif  // UPDATE_ENGINE_MOCK_ACTION_PROCESSOR_H_
+#endif  // UPDATE_ENGINE_COMMON_MOCK_ACTION_PROCESSOR_H_
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index a0977b17..35bd2063 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -38,6 +38,8 @@
 #include "libcros/dbus-proxy-mocks.h"
 #include "update_engine/common/fake_clock.h"
 #include "update_engine/common/fake_prefs.h"
+#include "update_engine/common/mock_action.h"
+#include "update_engine/common/mock_action_processor.h"
 #include "update_engine/common/mock_http_fetcher.h"
 #include "update_engine/common/mock_prefs.h"
 #include "update_engine/common/platform_constants.h"
@@ -45,8 +47,6 @@
 #include "update_engine/common/test_utils.h"
 #include "update_engine/common/utils.h"
 #include "update_engine/fake_system_state.h"
-#include "update_engine/mock_action.h"
-#include "update_engine/mock_action_processor.h"
 #include "update_engine/mock_p2p_manager.h"
 #include "update_engine/mock_payload_state.h"
 #include "update_engine/payload_consumer/filesystem_verifier_action.h"