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 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 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 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 worker; + }; + // Stores Job entries for delayed construction of Job object. + std::vector job_entries_; std::vector> 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()