vaapi vda: Delete owned objects on worker thread in Cleanup()

This CL adds a SEQUENCE_CHECKER to Vaapi*Accelerator classes, and
posts the destruction of those objects to the appropriate thread on
Cleanup().

Also makes {H264,VP8,VP9}Picture RefCountedThreadSafe, see miu@
comment in
https://chromium-review.googlesource.com/c/chromium/src/+/794091#message-a64bed985cfaf8a19499a517bb110a7ce581dc0f

TEST=play back VP9/VP8/H264 w/ simplechrome on soraka, Release build
unstripped, let video play for a few seconds then navigate back; no
crashes. Unittests as before:
video_decode_accelerator_unittest --test_video_data=test-25fps.vp9:320:240:250:250:35:150:12
video_decode_accelerator_unittest --test_video_data=test-25fps.vp8:320:240:250:250:35:150:11
video_decode_accelerator_unittest --test_video_data=test-25fps.h264:320:240:250:258:35:150:1

Bug: 789160
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I7d96aaf89c92bf46f00c8b8b36798e057a842ed2
Reviewed-on: https://chromium-review.googlesource.com/794091Reviewed-by: Pawel Osciak <posciak@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523372}

media/gpu/h264_dpb.h

@@ -25,7 +25,8 @@ class VaapiH264Picture;
 
 // A picture (a frame or a field) in the H.264 spec sense.
 // See spec at http://www.itu.int/rec/T-REC-H.264
-class MEDIA_GPU_EXPORT H264Picture : public base::RefCounted<H264Picture> {
+class MEDIA_GPU_EXPORT H264Picture
+    : public base::RefCountedThreadSafe<H264Picture> {
  public:
   using Vector = std::vector<scoped_refptr<H264Picture>>;
 
@@ -91,7 +92,7 @@ class MEDIA_GPU_EXPORT H264Picture : public base::RefCounted<H264Picture> {
   gfx::Rect visible_rect;
 
  protected:
-  friend class base::RefCounted<H264Picture>;
+  friend class base::RefCountedThreadSafe<H264Picture>;
   virtual ~H264Picture();
 
  private:

media/gpu/vaapi/vaapi_video_decode_accelerator.cc

@@ -171,6 +171,8 @@ class VaapiVideoDecodeAccelerator::VaapiH264Accelerator
   VaapiWrapper* vaapi_wrapper_;
   VaapiVideoDecodeAccelerator* vaapi_dec_;
 
+  SEQUENCE_CHECKER(sequence_checker_);
+
   DISALLOW_COPY_AND_ASSIGN(VaapiH264Accelerator);
 };
 
@@ -218,6 +220,8 @@ class VaapiVideoDecodeAccelerator::VaapiVP8Accelerator
   VaapiWrapper* vaapi_wrapper_;
   VaapiVideoDecodeAccelerator* vaapi_dec_;
 
+  SEQUENCE_CHECKER(sequence_checker_);
+
   DISALLOW_COPY_AND_ASSIGN(VaapiVP8Accelerator);
 };
 
@@ -270,6 +274,8 @@ class VaapiVideoDecodeAccelerator::VaapiVP9Accelerator
   VaapiWrapper* vaapi_wrapper_;
   VaapiVideoDecodeAccelerator* vaapi_dec_;
 
+  SEQUENCE_CHECKER(sequence_checker_);
+
   DISALLOW_COPY_AND_ASSIGN(VaapiVP9Accelerator);
 };
 
@@ -1061,6 +1067,18 @@ void VaapiVideoDecodeAccelerator::Cleanup() {
   client_ptr_factory_.reset();
   weak_this_factory_.InvalidateWeakPtrs();
 
+  decoder_thread_task_runner_->DeleteSoon(FROM_HERE, decoder_.release());
+  if (h264_accelerator_) {
+    decoder_thread_task_runner_->DeleteSoon(FROM_HERE,
+                                            h264_accelerator_.release());
+  } else if (vp8_accelerator_) {
+    decoder_thread_task_runner_->DeleteSoon(FROM_HERE,
+                                            vp8_accelerator_.release());
+  } else if (vp9_accelerator_) {
+    decoder_thread_task_runner_->DeleteSoon(FROM_HERE,
+                                            vp9_accelerator_.release());
+  }
+
   // Signal all potential waiters on the decoder_thread_, let them early-exit,
   // as we've just moved to the kDestroying state, and wait for all tasks
   // to finish.
@@ -1144,12 +1162,16 @@ VaapiVideoDecodeAccelerator::VaapiH264Accelerator::VaapiH264Accelerator(
     : vaapi_wrapper_(vaapi_wrapper), vaapi_dec_(vaapi_dec) {
   DCHECK(vaapi_wrapper_);
   DCHECK(vaapi_dec_);
+  DETACH_FROM_SEQUENCE(sequence_checker_);
 }
 
-VaapiVideoDecodeAccelerator::VaapiH264Accelerator::~VaapiH264Accelerator() {}
+VaapiVideoDecodeAccelerator::VaapiH264Accelerator::~VaapiH264Accelerator() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+}
 
 scoped_refptr<H264Picture>
 VaapiVideoDecodeAccelerator::VaapiH264Accelerator::CreateH264Picture() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   scoped_refptr<VaapiDecodeSurface> va_surface = vaapi_dec_->CreateSurface();
   if (!va_surface)
     return nullptr;
@@ -1172,6 +1194,7 @@ bool VaapiVideoDecodeAccelerator::VaapiH264Accelerator::SubmitFrameMetadata(
     const H264Picture::Vector& ref_pic_listb0,
     const H264Picture::Vector& ref_pic_listb1,
     const scoped_refptr<H264Picture>& pic) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   VAPictureParameterBufferH264 pic_param;
   memset(&pic_param, 0, sizeof(pic_param));
 
@@ -1286,6 +1309,7 @@ bool VaapiVideoDecodeAccelerator::VaapiH264Accelerator::SubmitSlice(
     const scoped_refptr<H264Picture>& pic,
     const uint8_t* data,
     size_t size) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   VASliceParameterBufferH264 slice_param;
   memset(&slice_param, 0, sizeof(slice_param));
 
@@ -1387,6 +1411,7 @@ bool VaapiVideoDecodeAccelerator::VaapiH264Accelerator::SubmitSlice(
 bool VaapiVideoDecodeAccelerator::VaapiH264Accelerator::SubmitDecode(
     const scoped_refptr<H264Picture>& pic) {
   VLOGF(4) << "Decoding POC " << pic->pic_order_cnt;
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   scoped_refptr<VaapiDecodeSurface> dec_surface =
       H264PictureToVaapiDecodeSurface(pic);
 
@@ -1395,6 +1420,7 @@ bool VaapiVideoDecodeAccelerator::VaapiH264Accelerator::SubmitDecode(
 
 bool VaapiVideoDecodeAccelerator::VaapiH264Accelerator::OutputPicture(
     const scoped_refptr<H264Picture>& pic) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   scoped_refptr<VaapiDecodeSurface> dec_surface =
       H264PictureToVaapiDecodeSurface(pic);
   dec_surface->set_visible_rect(pic->visible_rect);
@@ -1404,12 +1430,14 @@ bool VaapiVideoDecodeAccelerator::VaapiH264Accelerator::OutputPicture(
 }
 
 void VaapiVideoDecodeAccelerator::VaapiH264Accelerator::Reset() {
+  DETACH_FROM_SEQUENCE(sequence_checker_);
   vaapi_wrapper_->DestroyPendingBuffers();
 }
 
 scoped_refptr<VaapiVideoDecodeAccelerator::VaapiDecodeSurface>
 VaapiVideoDecodeAccelerator::VaapiH264Accelerator::
     H264PictureToVaapiDecodeSurface(const scoped_refptr<H264Picture>& pic) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   VaapiH264Picture* vaapi_pic = pic->AsVaapiH264Picture();
   CHECK(vaapi_pic);
   return vaapi_pic->dec_surface();
@@ -1418,6 +1446,7 @@ VaapiVideoDecodeAccelerator::VaapiH264Accelerator::
 void VaapiVideoDecodeAccelerator::VaapiH264Accelerator::FillVAPicture(
     VAPictureH264* va_pic,
     scoped_refptr<H264Picture> pic) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   VASurfaceID va_surface_id = VA_INVALID_SURFACE;
 
   if (!pic->nonexisting) {
@@ -1454,6 +1483,7 @@ int VaapiVideoDecodeAccelerator::VaapiH264Accelerator::FillVARefFramesFromDPB(
     const H264DPB& dpb,
     VAPictureH264* va_pics,
     int num_pics) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   H264Picture::Vector::const_reverse_iterator rit;
   int i;
 
@@ -1474,12 +1504,16 @@ VaapiVideoDecodeAccelerator::VaapiVP8Accelerator::VaapiVP8Accelerator(
     : vaapi_wrapper_(vaapi_wrapper), vaapi_dec_(vaapi_dec) {
   DCHECK(vaapi_wrapper_);
   DCHECK(vaapi_dec_);
+  DETACH_FROM_SEQUENCE(sequence_checker_);
 }
 
-VaapiVideoDecodeAccelerator::VaapiVP8Accelerator::~VaapiVP8Accelerator() {}
+VaapiVideoDecodeAccelerator::VaapiVP8Accelerator::~VaapiVP8Accelerator() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+}
 
 scoped_refptr<VP8Picture>
 VaapiVideoDecodeAccelerator::VaapiVP8Accelerator::CreateVP8Picture() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   scoped_refptr<VaapiDecodeSurface> va_surface = vaapi_dec_->CreateSurface();
   if (!va_surface)
     return nullptr;
@@ -1500,6 +1534,7 @@ bool VaapiVideoDecodeAccelerator::VaapiVP8Accelerator::SubmitDecode(
     const scoped_refptr<VP8Picture>& last_frame,
     const scoped_refptr<VP8Picture>& golden_frame,
     const scoped_refptr<VP8Picture>& alt_frame) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   VAIQMatrixBufferVP8 iq_matrix_buf;
   memset(&iq_matrix_buf, 0, sizeof(VAIQMatrixBufferVP8));
 
@@ -1682,6 +1717,7 @@ bool VaapiVideoDecodeAccelerator::VaapiVP8Accelerator::SubmitDecode(
 
 bool VaapiVideoDecodeAccelerator::VaapiVP8Accelerator::OutputPicture(
     const scoped_refptr<VP8Picture>& pic) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   scoped_refptr<VaapiDecodeSurface> dec_surface =
       VP8PictureToVaapiDecodeSurface(pic);
   dec_surface->set_visible_rect(pic->visible_rect);
@@ -1692,6 +1728,7 @@ bool VaapiVideoDecodeAccelerator::VaapiVP8Accelerator::OutputPicture(
 scoped_refptr<VaapiVideoDecodeAccelerator::VaapiDecodeSurface>
 VaapiVideoDecodeAccelerator::VaapiVP8Accelerator::
     VP8PictureToVaapiDecodeSurface(const scoped_refptr<VP8Picture>& pic) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   VaapiVP8Picture* vaapi_pic = pic->AsVaapiVP8Picture();
   CHECK(vaapi_pic);
   return vaapi_pic->dec_surface();
@@ -1703,12 +1740,16 @@ VaapiVideoDecodeAccelerator::VaapiVP9Accelerator::VaapiVP9Accelerator(
     : vaapi_wrapper_(vaapi_wrapper), vaapi_dec_(vaapi_dec) {
   DCHECK(vaapi_wrapper_);
   DCHECK(vaapi_dec_);
+  DETACH_FROM_SEQUENCE(sequence_checker_);
 }
 
-VaapiVideoDecodeAccelerator::VaapiVP9Accelerator::~VaapiVP9Accelerator() {}
+VaapiVideoDecodeAccelerator::VaapiVP9Accelerator::~VaapiVP9Accelerator() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+}
 
 scoped_refptr<VP9Picture>
 VaapiVideoDecodeAccelerator::VaapiVP9Accelerator::CreateVP9Picture() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   scoped_refptr<VaapiDecodeSurface> va_surface = vaapi_dec_->CreateSurface();
   if (!va_surface)
     return nullptr;
@@ -1722,6 +1763,7 @@ bool VaapiVideoDecodeAccelerator::VaapiVP9Accelerator::SubmitDecode(
     const Vp9LoopFilterParams& lf,
     const std::vector<scoped_refptr<VP9Picture>>& ref_pictures,
     const base::Closure& done_cb) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   // |done_cb| should be null as we return false from IsFrameContextRequired().
   DCHECK(done_cb.is_null());
 
@@ -1844,6 +1886,7 @@ bool VaapiVideoDecodeAccelerator::VaapiVP9Accelerator::SubmitDecode(
 
 bool VaapiVideoDecodeAccelerator::VaapiVP9Accelerator::OutputPicture(
     const scoped_refptr<VP9Picture>& pic) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   scoped_refptr<VaapiDecodeSurface> dec_surface =
       VP9PictureToVaapiDecodeSurface(pic);
   dec_surface->set_visible_rect(pic->visible_rect);
@@ -1854,6 +1897,7 @@ bool VaapiVideoDecodeAccelerator::VaapiVP9Accelerator::OutputPicture(
 bool VaapiVideoDecodeAccelerator::VaapiVP9Accelerator::GetFrameContext(
     const scoped_refptr<VP9Picture>& pic,
     Vp9FrameContext* frame_ctx) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   NOTIMPLEMENTED() << "Frame context update not supported";
   return false;
 }
@@ -1861,6 +1905,7 @@ bool VaapiVideoDecodeAccelerator::VaapiVP9Accelerator::GetFrameContext(
 scoped_refptr<VaapiVideoDecodeAccelerator::VaapiDecodeSurface>
 VaapiVideoDecodeAccelerator::VaapiVP9Accelerator::
     VP9PictureToVaapiDecodeSurface(const scoped_refptr<VP9Picture>& pic) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   VaapiVP9Picture* vaapi_pic = pic->AsVaapiVP9Picture();
   CHECK(vaapi_pic);
   return vaapi_pic->dec_surface();

media/gpu/vp8_picture.h

@@ -14,7 +14,7 @@ namespace media {
 class V4L2VP8Picture;
 class VaapiVP8Picture;
 
-class VP8Picture : public base::RefCounted<VP8Picture> {
+class VP8Picture : public base::RefCountedThreadSafe<VP8Picture> {
  public:
   VP8Picture();
 
@@ -25,7 +25,7 @@ class VP8Picture : public base::RefCounted<VP8Picture> {
   gfx::Rect visible_rect;
 
  protected:
-  friend class base::RefCounted<VP8Picture>;
+  friend class base::RefCountedThreadSafe<VP8Picture>;
   virtual ~VP8Picture();
 
   DISALLOW_COPY_AND_ASSIGN(VP8Picture);

media/gpu/vp9_picture.h

@@ -17,7 +17,7 @@ namespace media {
 class V4L2VP9Picture;
 class VaapiVP9Picture;
 
-class VP9Picture : public base::RefCounted<VP9Picture> {
+class VP9Picture : public base::RefCountedThreadSafe<VP9Picture> {
  public:
   VP9Picture();
 
@@ -32,7 +32,7 @@ class VP9Picture : public base::RefCounted<VP9Picture> {
   gfx::Rect visible_rect;
 
  protected:
-  friend class base::RefCounted<VP9Picture>;
+  friend class base::RefCountedThreadSafe<VP9Picture>;
   virtual ~VP9Picture();
 
   DISALLOW_COPY_AND_ASSIGN(VP9Picture);