update_engine: Pass Action ownership to ActionProcessor

Currently, an object that uses an ActionProcessor for processing one or more
actions has to own the Actions. This is problematic, because if we want to
create an action on the fly and use an ActionProcessor to perform it, we have to
own the Action until it is finished. Furthermore, if someone forget to own the
action, there will be memory leaks because ActionProcessor does not delete the
Action.

This patch passes the ownership of the Actions to the ActionProcessor through
unique pointers. If an object wants to have access to the Action, it can get it
when ActionComplete() is called.

BUG=chromium:807976
TEST=unittests
TEST=cros flash
TEST=precq

Change-Id: I28f7e9fd3425f17cc51b4db4a4abc130a7d6ef8f
Reviewed-on: https://chromium-review.googlesource.com/1065113
Commit-Ready: Amin Hassani <ahassani@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Xiaochu Liu <xiaochu@chromium.org>

1 files changed, 12 insertions, 7 deletions

diff --git a/common/action_processor.h b/common/action_processor.h
index c9c179ea..f651b8e3 100644
--- a/common/action_processor.h
+++ b/common/action_processor.h
@@ -18,6 +18,8 @@
 #define UPDATE_ENGINE_COMMON_ACTION_PROCESSOR_H_
 
 #include <deque>
+#include <memory>
+#include <vector>
 
 #include <base/macros.h>
 #include <brillo/errors/error.h>
@@ -69,10 +71,10 @@ class ActionProcessor {
 
   // Returns true iff the processing was started but not yet completed nor
   // stopped.
-  bool IsRunning() const { return current_action_ != nullptr || suspended_; }
+  bool IsRunning() const;
 
   // Adds another Action to the end of the queue.
-  virtual void EnqueueAction(AbstractAction* action);
+  virtual void EnqueueAction(std::unique_ptr<AbstractAction> action);
 
   // Sets/gets the current delegate. Set to null to remove a delegate.
   ActionProcessorDelegate* delegate() const { return delegate_; }
@@ -81,14 +83,17 @@ class ActionProcessor {
   }
 
   // Returns a pointer to the current Action that's processing.
-  AbstractAction* current_action() const {
-    return current_action_;
-  }
+  AbstractAction* current_action() const { return current_action_.get(); }
 
   // Called by an action to notify processor that it's done. Caller passes self.
+  // But this call deletes the action if there no other object has a reference
+  // to it, so in that case, the caller should not try to access any of its
+  // member variables after this call.
   void ActionComplete(AbstractAction* actionptr, ErrorCode code);
 
  private:
+  FRIEND_TEST(ActionProcessorTest, ChainActionsTest);
+
   // 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.
@@ -96,10 +101,10 @@ class ActionProcessor {
 
   // Actions that have not yet begun processing, in the order in which
   // they'll be processed.
-  std::deque<AbstractAction*> actions_;
+  std::deque<std::unique_ptr<AbstractAction>> actions_;
 
   // A pointer to the currently processing Action, if any.
-  AbstractAction* current_action_{nullptr};
+  std::unique_ptr<AbstractAction> current_action_;
 
   // 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