Group AVDA output surface into AVDASurfaceBundle.

media/gpu/avda_codec_allocator.cc

Index: media/gpu/avda_codec_allocator.cc
diff --git a/media/gpu/avda_codec_allocator.cc b/media/gpu/avda_codec_allocator.cc
index bbf96156ea4d85280e8bba9b69dbbd319f80b7c0..52bde7bb28bb582a9ff3f683fc3a5944339ba666 100644
--- a/media/gpu/avda_codec_allocator.cc
+++ b/media/gpu/avda_codec_allocator.cc
@@ -41,6 +41,11 @@ void DeleteMediaCodecAndSignal(std::unique_ptr<VideoCodecBridge> codec,
     done_event->Signal();
 }
 
+void DropReferenceToSurfaceBundle(
+    scoped_refptr<AVDASurfaceBundle> surface_bundle) {
+  // Do nothing.  Let |surface_bundle| go out of scope.
+}
+
 }  // namespace
 
 CodecConfig::CodecConfig() {}
@@ -248,8 +253,8 @@ std::unique_ptr<VideoCodecBridge> AVDACodecAllocator::CreateMediaCodecSync(
   std::unique_ptr<VideoCodecBridge> codec(VideoCodecBridge::CreateDecoder(
       codec_config->codec, codec_config->needs_protected_surface,
       codec_config->initial_expected_coded_size,
-      codec_config->surface.j_surface().obj(), media_crypto, codec_config->csd0,
-      codec_config->csd1, true, require_software_codec));
+      codec_config->surface_bundle->surface.j_surface().obj(), media_crypto,
+      codec_config->csd0, codec_config->csd1, true, require_software_codec));
 
   return codec;
 }
@@ -257,55 +262,89 @@ std::unique_ptr<VideoCodecBridge> AVDACodecAllocator::CreateMediaCodecSync(
 void AVDACodecAllocator::CreateMediaCodecAsync(
     base::WeakPtr<AVDACodecAllocatorClient> client,
     scoped_refptr<CodecConfig> codec_config) {
+  // Allocate the codec on the appropriate thread, and reply to this one with
+  // the result.  If |client| is gone by then, we handle cleanup.
   base::PostTaskAndReplyWithResult(
       TaskRunnerFor(codec_config->task_type).get(), FROM_HERE,
       base::Bind(&AVDACodecAllocator::CreateMediaCodecSync,
                  base::Unretained(this), codec_config),
-      base::Bind(&AVDACodecAllocatorClient::OnCodecConfigured, client));
+      base::Bind(&AVDACodecAllocator::ForwardOrDropCodec,
+                 base::Unretained(this), client, codec_config->task_type,
+                 codec_config->surface_bundle));
+}
+
+void AVDACodecAllocator::ForwardOrDropCodec(
+    base::WeakPtr<AVDACodecAllocatorClient> client,
+    TaskType task_type,
+    scoped_refptr<AVDASurfaceBundle> surface_bundle,
+    std::unique_ptr<VideoCodecBridge> media_codec) {
+  if (!client) {
+    // |client| has been destroyed.  Free |media_codec| on the right thread.
+    // Note that this also preserves |surface_bundle| until |media_codec| has
+    // been released, in case our ref to it is the last one.
+    ReleaseMediaCodec(std::move(media_codec), task_type, surface_bundle);
+    return;
+  }
+
+  // The client is still around, so (presumably) it has a reference to
+  // |surface_bundle| already.

[watk, 2017/02/22 20:38:56]

This comment seems out of place. It seems to be justifying why OnCodecConfigured
doesn't accept the bundle as a param? Can be deleted IMO.

[liberato (no reviews please), 2017/02/23 18:18:46]

Done.

+  client->OnCodecConfigured(std::move(media_codec));
 }
 
 void AVDACodecAllocator::ReleaseMediaCodec(
     std::unique_ptr<VideoCodecBridge> media_codec,
     TaskType task_type,
-    int surface_id) {
+    const scoped_refptr<AVDASurfaceBundle>& surface_bundle) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(media_codec);
 
-  // No need to track the release if it's a SurfaceTexture.
-  if (surface_id == SurfaceManager::kNoSurfaceID) {
-    TaskRunnerFor(task_type)->PostTask(
+  // No need to track the release if it's a SurfaceTexture.  We still forward
+  // the reference to |surface_bundle|, though, so that the SurfaceTexture
+  // lasts at least as long as the codec.
+  if (surface_bundle->surface_id == SurfaceManager::kNoSurfaceID) {
+    TaskRunnerFor(task_type)->PostTaskAndReply(
         FROM_HERE, base::Bind(&DeleteMediaCodecAndSignal,
-                              base::Passed(std::move(media_codec)), nullptr));
+                              base::Passed(std::move(media_codec)), nullptr),
+        base::Bind(&DropReferenceToSurfaceBundle, surface_bundle));
     return;
   }
 
+  DCHECK(!surface_bundle->surface_texture);
   pending_codec_releases_.emplace(
-      std::piecewise_construct, std::forward_as_tuple(surface_id),
+      std::piecewise_construct,
+      std::forward_as_tuple(surface_bundle->surface_id),
       std::forward_as_tuple(base::WaitableEvent::ResetPolicy::MANUAL,
                             base::WaitableEvent::InitialState::NOT_SIGNALED));
   base::WaitableEvent* released =
-      &pending_codec_releases_.find(surface_id)->second;
+      &pending_codec_releases_.find(surface_bundle->surface_id)->second;
 
+  // Note that we forward |surface_bundle|, too, so that the surface outlasts
+  // the codec.  This doesn't matter so much for CVV surfaces, since they don't
+  // auto-release when they're dropped.  However, for surface owners, this will
+  // become important, so we still handle it.  Plus, it makes sense.
   TaskRunnerFor(task_type)->PostTaskAndReply(
       FROM_HERE, base::Bind(&DeleteMediaCodecAndSignal,
                             base::Passed(std::move(media_codec)), released),
       base::Bind(&AVDACodecAllocator::OnMediaCodecAndSurfaceReleased,
-                 base::Unretained(this), surface_id));
+                 base::Unretained(this), surface_bundle));
 }
 
-void AVDACodecAllocator::OnMediaCodecAndSurfaceReleased(int surface_id) {
+void AVDACodecAllocator::OnMediaCodecAndSurfaceReleased(
+    scoped_refptr<AVDASurfaceBundle> surface_bundle) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  pending_codec_releases_.erase(surface_id);
-  if (!surface_owners_.count(surface_id))
+  pending_codec_releases_.erase(surface_bundle->surface_id);
+  if (!surface_owners_.count(surface_bundle->surface_id))
     return;
 
-  OwnerRecord& record = surface_owners_[surface_id];
+  OwnerRecord& record = surface_owners_[surface_bundle->surface_id];
   if (!record.owner && record.waiter) {
     record.owner = record.waiter;
     record.waiter = nullptr;
     record.owner->OnSurfaceAvailable(true);
   }
+
+  // Also note that |surface_bundle| lasted at least as long as the codec.
 }
 
 // Returns a hint about whether the construction thread has hung for