update_engine: Remove Action object from FilesystemVerifierActionDelegate

in CL:1065113 we forgot to remove the FileSystemVerfierAction instance from the
FilesystemVerifierActionDelegate. Hence, it will be removed from memory while
FilesystemVerifierActionDelegate still keeps a pointer to it and access it. This
causes a possible race condition on ExitMainLoop() function. This patch fixes
the issue by removing that instance and the unnecessary checks for the
cleanup. Once the Action is deleted by the ActionProcessor it will be cleaned up
(which will happen when the action is terminated too). So there is not really a
need for testing the cleanup method.

BUG=chromium:867815
TEST=unittests pass

Change-Id: Id9d3f361eb916007e95dbb1d0adb9603ceae00b9
Reviewed-on: https://chromium-review.googlesource.com/1151902
Commit-Ready: Lann Martin <lannm@chromium.org>
Tested-by: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Sen Jiang <senj@chromium.org>

1 files changed, 7 insertions, 20 deletions

diff --git a/payload_consumer/filesystem_verifier_action_unittest.cc b/payload_consumer/filesystem_verifier_action_unittest.cc
index db8b664d..895d816f 100644
--- a/payload_consumer/filesystem_verifier_action_unittest.cc
+++ b/payload_consumer/filesystem_verifier_action_unittest.cc
@@ -64,27 +64,14 @@ class FilesystemVerifierActionTest : public ::testing::Test {
 
 class FilesystemVerifierActionTestDelegate : public ActionProcessorDelegate {
  public:
-  explicit FilesystemVerifierActionTestDelegate(
-      FilesystemVerifierAction* action)
-      : action_(action), ran_(false), code_(ErrorCode::kError) {}
-  void ExitMainLoop() {
-    // We need to wait for the Action to call Cleanup.
-    if (action_->IsCleanupPending()) {
-      LOG(INFO) << "Waiting for Cleanup() to be called.";
-      MessageLoop::current()->PostDelayedTask(
-          FROM_HERE,
-          base::Bind(&FilesystemVerifierActionTestDelegate::ExitMainLoop,
-                     base::Unretained(this)),
-          base::TimeDelta::FromMilliseconds(100));
-    } else {
-      MessageLoop::current()->BreakLoop();
-    }
-  }
+  FilesystemVerifierActionTestDelegate()
+      : ran_(false), code_(ErrorCode::kError) {}
+
   void ProcessingDone(const ActionProcessor* processor, ErrorCode code) {
-    ExitMainLoop();
+    MessageLoop::current()->BreakLoop();
   }
   void ProcessingStopped(const ActionProcessor* processor) {
-    ExitMainLoop();
+    MessageLoop::current()->BreakLoop();
   }
   void ActionCompleted(ActionProcessor* processor,
                        AbstractAction* action,
@@ -92,6 +79,7 @@ class FilesystemVerifierActionTestDelegate : public ActionProcessorDelegate {
     if (action->Type() == FilesystemVerifierAction::StaticType()) {
       ran_ = true;
       code_ = code;
+      EXPECT_FALSE(static_cast<FilesystemVerifierAction*>(action)->src_stream_);
     } else if (action->Type() ==
                ObjectCollectorAction<InstallPlan>::StaticType()) {
       auto collector_action =
@@ -105,7 +93,6 @@ class FilesystemVerifierActionTestDelegate : public ActionProcessorDelegate {
   std::unique_ptr<InstallPlan> install_plan_;
 
  private:
-  FilesystemVerifierAction* action_;
   bool ran_;
   ErrorCode code_;
 };
@@ -174,7 +161,7 @@ bool FilesystemVerifierActionTest::DoTest(bool terminate_early,
   BondActions(copier_action.get(), collector_action.get());
 
   ActionProcessor processor;
-  FilesystemVerifierActionTestDelegate delegate(copier_action.get());
+  FilesystemVerifierActionTestDelegate delegate;
   processor.set_delegate(&delegate);
   processor.EnqueueAction(std::move(feeder_action));
   processor.EnqueueAction(std::move(copier_action));