From 2d5e2b09036075825ecbbed071e121d546eafbea Mon Sep 17 00:00:00 2001
From: KongQun Yang <kqyang@google.com>
Date: Tue, 6 Feb 2018 11:47:41 -0800
Subject: [PATCH] Fix DCHECK of SimpleThread on invalid command line options

- Construct SimpleThreads only after all workers have been successfully
  initialized

b/72999680

Change-Id: I4016321a4bdd27b1de746c50c5f1d7a60d7ae9f1
---
 packager/app/job_manager.cc        | 25 +++++++++++++------------
 packager/app/job_manager.h         | 11 ++++++-----
 packager/app/test/packager_test.py |  8 ++++++++
 3 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/packager/app/job_manager.cc b/packager/app/job_manager.cc
index f20c0fe836..576566a302 100644
--- a/packager/app/job_manager.cc
+++ b/packager/app/job_manager.cc
@@ -14,14 +14,10 @@ namespace media {
 
 Job::Job(const std::string& name, std::shared_ptr<OriginHandler> work)
     : SimpleThread(name),
-      work_(work),
+      work_(std::move(work)),
       wait_(base::WaitableEvent::ResetPolicy::MANUAL,
             base::WaitableEvent::InitialState::NOT_SIGNALED) {
-  DCHECK(work);
-}
-
-void Job::Initialize() {
-  status_ = work_->Initialize();
+  DCHECK(work_);
 }
 
 void Job::Cancel() {
@@ -35,17 +31,22 @@ void Job::Run() {
 
 void JobManager::Add(const std::string& name,
                      std::shared_ptr<OriginHandler> handler) {
-  jobs_.emplace_back(new Job(name, std::move(handler)));
+  // Stores Job entries for delayed construction of Job objects, to avoid
+  // setting up SimpleThread until we know all workers can be initialized
+  // successfully.
+  job_entries_.push_back({name, std::move(handler)});
 }
 
 Status JobManager::InitializeJobs() {
   Status status;
+  for (const JobEntry& job_entry : job_entries_)
+    status.Update(job_entry.worker->Initialize());
+  if (!status.ok())
+    return status;
 
-  for (auto& job : jobs_) {
-    job->Initialize();
-    status.Update(job->status());
-  }
-
+  // Create Job objects after successfully initialized all workers.
+  for (const JobEntry& job_entry : job_entries_)
+    jobs_.emplace_back(new Job(job_entry.name, std::move(job_entry.worker)));
   return status;
 }
 
diff --git a/packager/app/job_manager.h b/packager/app/job_manager.h
index 7201f18246..258f85dee5 100644
--- a/packager/app/job_manager.h
+++ b/packager/app/job_manager.h
@@ -24,11 +24,6 @@ class Job : public base::SimpleThread {
  public:
   Job(const std::string& name, std::shared_ptr<OriginHandler> work);
 
-  // Initialize the chain of handlers that make up this job. This only
-  // initializes the handlers, it does not execute the job. If
-  // initialization fails, |status| will return a non-ok status.
-  void Initialize();
-
   // Request that the job stops executing. This is only a request and
   // will not block. If you want to wait for the job to complete, use
   // |wait|.
@@ -84,6 +79,12 @@ class JobManager {
   JobManager(const JobManager&) = delete;
   JobManager& operator=(const JobManager&) = delete;
 
+  struct JobEntry {
+    std::string name;
+    std::shared_ptr<OriginHandler> worker;
+  };
+  // Stores Job entries for delayed construction of Job object.
+  std::vector<JobEntry> job_entries_;
   std::vector<std::unique_ptr<Job>> jobs_;
 };
 
diff --git a/packager/app/test/packager_test.py b/packager/app/test/packager_test.py
index 98d2d5c086..64ed16c8c3 100755
--- a/packager/app/test/packager_test.py
+++ b/packager/app/test/packager_test.py
@@ -1415,6 +1415,14 @@ class PackagerCommandParsingTest(PackagerAppTest):
         self._GetStreams(['audio', 'video']), flags)
     self.assertEqual(packaging_result, 1)
 
+  def testPackageAudioVideoWithNotExistText(self):
+    audio_video_stream = self._GetStreams(['audio', 'video'])
+    text_stream = self._GetStreams(['text'], test_files=['not-exist.vtt'])
+    packaging_result = self.packager.Package(audio_video_stream + text_stream,
+                                             self._GetFlags())
+    # Expect the test to fail but we do not expect a crash.
+    self.assertEqual(packaging_result, 1)
+
 
 if __name__ == '__main__':
   unittest.main()