Fix errors in destruction sequence of GpuVideoEncodeAccelerator::OnWillDestroyStub()

media/gpu/ipc/service/gpu_video_encode_accelerator.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/gpu/ipc/service/gpu_video_encode_accelerator.h"
 
 #include <memory>
 
 #include "base/callback.h"
 #include "base/command_line.h"
@@ skipping 153 matching lines @@
     : host_route_id_(host_route_id),
       stub_(stub),
       input_format_(PIXEL_FORMAT_UNKNOWN),
       output_buffer_size_(0),
       filter_removed_(base::WaitableEvent::ResetPolicy::MANUAL,
                       base::WaitableEvent::InitialState::NOT_SIGNALED),
       encoder_worker_thread_("EncoderWorkerThread"),
       main_task_runner_(base::ThreadTaskRunnerHandle::Get()),
       io_task_runner_(io_task_runner),
       encode_task_runner_(main_task_runner_),
-      weak_this_factory_for_encoder_worker_(this),
+      weak weak_this_factory_for_encoder_worker_(this),

[sandersd (OOO until July 31), 2017/04/28 18:22:44]

Presumably does not compile.

[emircan, 2017/04/29 00:10:46]

Done.

       weak_this_factory_(this) {
+  weak_this_for_encoder_worker_ =
+      weak_this_factory_for_encoder_worker_.GetWeakPtr();
+  weak_this_ = weak_this_factory_.GetWeakPtr();
   stub_->AddDestructionObserver(this);
   make_context_current_ =
       base::Bind(&MakeDecoderContextCurrent, stub_->AsWeakPtr());
 }
 
 GpuVideoEncodeAccelerator::~GpuVideoEncodeAccelerator() {
   // This class can only be self-deleted from OnWillDestroyStub(), which means
   // the VEA has already been destroyed in there.
   DCHECK(!encoder_);
-  if (encoder_worker_thread_.IsRunning()) {
-    encoder_worker_task_runner_->PostTask(
-        FROM_HERE,
-        base::Bind(&GpuVideoEncodeAccelerator::DestroyOnEncoderWorker,
-                   weak_this_factory_for_encoder_worker_.GetWeakPtr()));
-    encoder_worker_thread_.Stop();
-  }
+  DCHECK(!encoder_worker_thread_.IsRunning());
 }
 
 bool GpuVideoEncodeAccelerator::Initialize(VideoPixelFormat input_format,
                                            const gfx::Size& input_visible_size,
                                            VideoCodecProfile output_profile,
                                            uint32_t initial_bitrate) {
   DCHECK(main_task_runner_->BelongsToCurrentThread());
   DVLOG(1) << __func__
            << " input_format=" << VideoPixelFormatToString(input_format)
            << ", input_visible_size=" << input_visible_size.ToString()
@@ skipping 20 matching lines @@
   // Try all possible encoders and use the first successful encoder.
   for (const auto& factory_function : GetVEAFactoryFunctions(gpu_preferences)) {
     encoder_ = factory_function.Run();
     if (encoder_ &&
         encoder_->Initialize(input_format, input_visible_size, output_profile,
                              initial_bitrate, this)) {
       input_format_ = input_format;
       input_visible_size_ = input_visible_size;
       // Attempt to set up performing encoding tasks on IO thread, if supported
       // by the VEA.
-      if (encoder_->TryToSetupEncodeOnSeparateThread(
-              weak_this_factory_.GetWeakPtr(), io_task_runner_)) {
+      if (encoder_->TryToSetupEncodeOnSeparateThread(weak_this_,
+                                                     io_task_runner_)) {
         filter_ = new MessageFilter(this, host_route_id_);
         stub_->channel()->AddFilter(filter_.get());
         encode_task_runner_ = io_task_runner_;
       }
 
       if (!encoder_worker_thread_.Start()) {
         DLOG(ERROR) << "Failed spawning encoder worker thread.";
         return false;
       }
       encoder_worker_task_runner_ = encoder_worker_thread_.task_runner();
@@ skipping 74 matching lines @@
   // removed however, since we cannot service incoming messages with VEA gone.
   // We cannot simply check for existence of VEA on IO thread though, because
   // we don't want to synchronize the IO thread with the ChildThread.
   // So we have to wait for the RemoveFilter callback here instead and remove
   // the VEA after it arrives and before returning.
   if (filter_) {
     stub_->channel()->RemoveFilter(filter_.get());
     filter_removed_.Wait();
   }
 
+  // We should stop |encoder_worker_thread_| before releasing |encoder_|, see
+  // crbug.com/715759.
+  if (encoder_worker_thread_.IsRunning()) {
+    encoder_worker_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(&GpuVideoEncodeAccelerator::DestroyOnEncoderWorker,
+                   weak_this_for_encoder_worker_));
+    encoder_worker_thread_.Stop();
+  }
+
   stub_->channel()->RemoveRoute(host_route_id_);
   stub_->RemoveDestructionObserver(this);
   encoder_.reset();
   delete this;
 }
 
 // static
 gpu::VideoEncodeAcceleratorSupportedProfiles
 GpuVideoEncodeAccelerator::GetSupportedProfiles(
     const gpu::GpuPreferences& gpu_preferences) {
@@ skipping 59 matching lines @@
 
   if (params.frame_id < 0) {
     DLOG(ERROR) << __func__ << " invalid frame_id=" << params.frame_id;
     NotifyError(VideoEncodeAccelerator::kPlatformFailureError);
     return;
   }
 
   encoder_worker_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&GpuVideoEncodeAccelerator::CreateEncodeFrameOnEncoderWorker,
-                 weak_this_factory_for_encoder_worker_.GetWeakPtr(), params));
+                 weak_this_for_encoder_worker_, params));
 }
 
 void GpuVideoEncodeAccelerator::OnUseOutputBitstreamBuffer(
     int32_t buffer_id,
     base::SharedMemoryHandle buffer_handle,
     uint32_t buffer_size) {
   DVLOG(3) << __func__ << " buffer_id=" << buffer_id
            << ", buffer_size=" << buffer_size;
   DCHECK(CheckIfCalledOnCorrectThread());
   if (!encoder_)
@@ skipping 40 matching lines @@
   const uint32_t aligned_offset =
       params.buffer_offset % base::SysInfo::VMAllocationGranularity();
   base::CheckedNumeric<off_t> map_offset = params.buffer_offset;
   map_offset -= aligned_offset;
   base::CheckedNumeric<size_t> map_size = params.buffer_size;
   map_size += aligned_offset;
 
   if (!map_offset.IsValid() || !map_size.IsValid()) {
     DLOG(ERROR) << __func__ << "  invalid map_offset or map_size";
     encode_task_runner_->PostTask(
-        FROM_HERE, base::Bind(&GpuVideoEncodeAccelerator::NotifyError,
-                              weak_this_factory_.GetWeakPtr(),
-                              VideoEncodeAccelerator::kPlatformFailureError));
+        FROM_HERE,
+        base::Bind(&GpuVideoEncodeAccelerator::NotifyError, weak_this_,
+                   VideoEncodeAccelerator::kPlatformFailureError));
     return;
   }
 
   if (!shm->MapAt(map_offset.ValueOrDie(), map_size.ValueOrDie())) {
     DLOG(ERROR) << __func__ << " could not map frame_id=" << params.frame_id;
     encode_task_runner_->PostTask(
-        FROM_HERE, base::Bind(&GpuVideoEncodeAccelerator::NotifyError,
-                              weak_this_factory_.GetWeakPtr(),
-                              VideoEncodeAccelerator::kPlatformFailureError));
+        FROM_HERE,
+        base::Bind(&GpuVideoEncodeAccelerator::NotifyError, weak_this_,
+                   VideoEncodeAccelerator::kPlatformFailureError));
     return;
   }
 
   uint8_t* shm_memory =
       reinterpret_cast<uint8_t*>(shm->memory()) + aligned_offset;
   scoped_refptr<VideoFrame> frame = VideoFrame::WrapExternalSharedMemory(
       input_format_, input_coded_size_, gfx::Rect(input_visible_size_),
       input_visible_size_, shm_memory, params.buffer_size, params.buffer_handle,
       params.buffer_offset, params.timestamp);
   if (!frame) {
     DLOG(ERROR) << __func__ << " could not create a frame";
     encode_task_runner_->PostTask(
-        FROM_HERE, base::Bind(&GpuVideoEncodeAccelerator::NotifyError,
-                              weak_this_factory_.GetWeakPtr(),
-                              VideoEncodeAccelerator::kPlatformFailureError));
+        FROM_HERE,
+        base::Bind(&GpuVideoEncodeAccelerator::NotifyError, weak_this_,
+                   VideoEncodeAccelerator::kPlatformFailureError));
     return;
   }
 
   // We wrap |shm| in a callback and add it as a destruction observer, so it
   // stays alive and mapped until |frame| goes out of scope.
   frame->AddDestructionObserver(
       base::Bind(&DropSharedMemory, base::Passed(&shm)));
   encode_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&GpuVideoEncodeAccelerator::OnEncodeFrameCreated,
-                            weak_this_factory_.GetWeakPtr(), params.frame_id,
-                            params.force_keyframe, frame));
+      FROM_HERE,
+      base::Bind(&GpuVideoEncodeAccelerator::OnEncodeFrameCreated, weak_this_,
+                 params.frame_id, params.force_keyframe, frame));
 }
 
 void GpuVideoEncodeAccelerator::DestroyOnEncoderWorker() {
   DCHECK(encoder_worker_task_runner_->BelongsToCurrentThread());
   weak_this_factory_for_encoder_worker_.InvalidateWeakPtrs();
 }
 
 void GpuVideoEncodeAccelerator::OnEncodeFrameCreated(
     int32_t frame_id,
     bool force_keyframe,
     const scoped_refptr<media::VideoFrame>& frame) {
   DVLOG(3) << __func__;
   DCHECK(CheckIfCalledOnCorrectThread());
 
+  if (filter_removed_.IsSignaled())
+    return;
+
   if (!frame) {
     DLOG(ERROR) << __func__ << " could not create a frame";
     NotifyError(VideoEncodeAccelerator::kPlatformFailureError);
     return;
   }
 
   frame->AddDestructionObserver(BindToCurrentLoop(
       base::Bind(&GpuVideoEncodeAccelerator::EncodeFrameFinished,
                  weak_this_factory_.GetWeakPtr(), frame_id)));
   encoder_->Encode(frame, force_keyframe);
 }
 
 void GpuVideoEncodeAccelerator::EncodeFrameFinished(int32_t frame_id) {
   DCHECK(CheckIfCalledOnCorrectThread());
   if (!Send(new AcceleratedVideoEncoderHostMsg_NotifyInputDone(host_route_id_,
                                                                frame_id))) {
     DLOG(ERROR) << __func__ << " failed.";
   }
 }
 
 bool GpuVideoEncodeAccelerator::CheckIfCalledOnCorrectThread() {
   return (filter_ && io_task_runner_->BelongsToCurrentThread()) ||
          (!filter_ && main_task_runner_->BelongsToCurrentThread());
 }
 
 }  // namespace media