media: Ensure MediaCodecs are released before attached SurfaceTextures

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 b8b5e7b95667ed2e938007dd5279269f3bd2b4cf..2f2551742f9ef43c8ca5e277f0ce28724ea00b88 100644
--- a/media/gpu/avda_codec_allocator.cc
+++ b/media/gpu/avda_codec_allocator.cc
@@ -149,7 +149,7 @@ bool AVDACodecAllocator::AllocateSurface(AVDACodecAllocatorClient* client,
 
   // If it's not owned or being released, |client| now owns it.
   if (!surface_owners_.count(surface_id) &&
-      !pending_codec_releases_.count(surface_id)) {
+      !surface_view_codec_releases_.count(surface_id)) {
     surface_owners_[surface_id].owner = client;
     return true;
   }
@@ -178,7 +178,7 @@ void AVDACodecAllocator::DeallocateSurface(AVDACodecAllocatorClient* client,
 
   // Promote the waiter if possible.
   if (record.waiter && !record.owner &&
-      !pending_codec_releases_.count(surface_id)) {
+      !surface_view_codec_releases_.count(surface_id)) {
     record.owner = record.waiter;
     record.waiter = nullptr;
     record.owner->OnSurfaceAvailable(true);
@@ -221,7 +221,7 @@ void AVDACodecAllocator::OnSurfaceDestroyed(int surface_id) {
 
   // The codec might have been released above in OnSurfaceDestroyed(), or was
   // already pending release.
-  if (!pending_codec_releases_.count(surface_id))
+  if (!surface_view_codec_releases_.count(surface_id))
     return;
 
   // The codec is being released so we have to wait for it here. It's a
@@ -230,7 +230,7 @@ void AVDACodecAllocator::OnSurfaceDestroyed(int surface_id) {
   // occur when the UI thread is blocked for 5 seconds, so waiting for 2 seconds
   // gives us leeway to avoid an ANR. Verified no ANR on a Nexus 7.
   base::WaitableEvent& released =
-      pending_codec_releases_.find(surface_id)->second;
+      surface_view_codec_releases_.find(surface_id)->second;
   released.TimedWait(base::TimeDelta::FromSeconds(2));
   if (!released.IsSignaled())
     DLOG(WARNING) << __func__ << ": timed out waiting for MediaCodec#release()";
@@ -269,45 +269,70 @@ void AVDACodecAllocator::CreateMediaCodecAsync(
 void AVDACodecAllocator::ReleaseMediaCodec(
     std::unique_ptr<VideoCodecBridge> media_codec,
     TaskType task_type,
-    int surface_id) {
+    int surface_id,
+    gl::SurfaceTexture* surface_texture) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(media_codec);
+  DCHECK(surface_id != SurfaceManager::kNoSurfaceID || surface_texture);
 
-  // No need to track the release if it's a SurfaceTexture.
-  if (surface_id == SurfaceManager::kNoSurfaceID) {
-    TaskRunnerFor(task_type)->PostTask(
-        FROM_HERE, base::Bind(&DeleteMediaCodecAndSignal,
-                              base::Passed(std::move(media_codec)), nullptr));
-    return;
+  base::WaitableEvent* released_event = nullptr;
+  if (surface_texture) {
+    surface_texture_codec_releases_[surface_texture] = task_type;
+  } else {
+    surface_view_codec_releases_.emplace(
+        std::piecewise_construct, std::forward_as_tuple(surface_id),
+        std::forward_as_tuple(base::WaitableEvent::ResetPolicy::MANUAL,
+                              base::WaitableEvent::InitialState::NOT_SIGNALED));
+    released_event = &surface_view_codec_releases_.find(surface_id)->second;
   }
 
-  pending_codec_releases_.emplace(
-      std::piecewise_construct, std::forward_as_tuple(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;
-
   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));
+      FROM_HERE,
+      base::Bind(&DeleteMediaCodecAndSignal,
+                 base::Passed(std::move(media_codec)), released_event),
+      base::Bind(&AVDACodecAllocator::OnMediaCodecReleased,
+                 base::Unretained(this), surface_id,
+                 base::Unretained(surface_texture)));
 }
 
-void AVDACodecAllocator::OnMediaCodecAndSurfaceReleased(int surface_id) {
+void AVDACodecAllocator::OnMediaCodecReleased(
+    int surface_id,
+    gl::SurfaceTexture* surface_texture) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  pending_codec_releases_.erase(surface_id);
-  if (!surface_owners_.count(surface_id))
+  if (surface_texture) {
+    surface_texture_codec_releases_.erase(surface_texture);
+  } else {
+    surface_view_codec_releases_.erase(surface_id);
+    if (!surface_owners_.count(surface_id))

[liberato (no reviews please), 2017/01/13 17:22:04]

might want to use find rather than count + [] later to save the lookup.

+      return;
+
+    OwnerRecord& record = surface_owners_[surface_id];
+    if (!record.owner && record.waiter) {
+      record.owner = record.waiter;
+      record.waiter = nullptr;
+      record.owner->OnSurfaceAvailable(true);
+    }
+  }
+}
+
+void AVDACodecAllocator::ReleaseSurfaceTexture(
+    scoped_refptr<gl::SurfaceTexture> surface_texture) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  // If this SurfaceTexture is attached to a codec that's currently being
+  // released, defer the surface release until after the codec is released.
+  if (surface_texture_codec_releases_.count(surface_texture.get())) {

[liberato (no reviews please), 2017/01/13 17:22:04]

ditto find().

also might want to invert this to if(iter == end) { l#334, 335 return}
just to save the indentation.

+    TaskType task_type = surface_texture_codec_releases_[surface_texture.get()];
+    TaskRunnerFor(task_type)->PostTaskAndReply(
+        FROM_HERE, base::Bind(&base::DoNothing),
+        base::Bind(&gl::SurfaceTexture::ReleaseSurfaceTexture,
+                   surface_texture));
     return;
-
-  OwnerRecord& record = surface_owners_[surface_id];
-  if (!record.owner && record.waiter) {
-    record.owner = record.waiter;
-    record.waiter = nullptr;
-    record.owner->OnSurfaceAvailable(true);
   }
+
+  // Otherwise we can release immediately.
+  surface_texture->ReleaseSurfaceTexture();
 }
 
 // Returns a hint about whether the construction thread has hung for