Update from https://crrev.com/308331

cc/resources/video_resource_updater.cc

Index: cc/resources/video_resource_updater.cc
diff --git a/cc/resources/video_resource_updater.cc b/cc/resources/video_resource_updater.cc
index 2c9a688619adeb0087018be964281b8f9d17dcb8..edbea9b53c92f110b28024a3814f31bfabe2ad46 100644
--- a/cc/resources/video_resource_updater.cc
+++ b/cc/resources/video_resource_updater.cc
@@ -4,6 +4,8 @@
 
 #include "cc/resources/video_resource_updater.h"
 
+#include <algorithm>
+
 #include "base/bind.h"
 #include "base/debug/trace_event.h"
 #include "cc/output/gl_renderer.h"
@@ -48,6 +50,7 @@ VideoResourceUpdater::PlaneResource::PlaneResource(
       resource_size(resource_size),
       resource_format(resource_format),
       mailbox(mailbox),
+      ref_count(0),
       frame_ptr(nullptr),
       plane_index(0) {
 }
@@ -81,17 +84,43 @@ VideoResourceUpdater::VideoResourceUpdater(ContextProvider* context_provider,
 }
 
 VideoResourceUpdater::~VideoResourceUpdater() {
-  while (!all_resources_.empty()) {
-    resource_provider_->DeleteResource(all_resources_.back());
-    all_resources_.pop_back();
+  for (const PlaneResource& plane_resource : all_resources_)
+    resource_provider_->DeleteResource(plane_resource.resource_id);
+}
+
+VideoResourceUpdater::ResourceList::iterator
+VideoResourceUpdater::AllocateResource(const gfx::Size& plane_size,
+                                       ResourceFormat format,
+                                       bool has_mailbox) {
+  // TODO(danakj): Abstract out hw/sw resource create/delete from
+  // ResourceProvider and stop using ResourceProvider in this class.
+  const ResourceProvider::ResourceId resource_id =
+      resource_provider_->CreateResource(plane_size, GL_CLAMP_TO_EDGE,
+                                         ResourceProvider::TextureHintImmutable,
+                                         format);
+  if (resource_id == 0)
+    return all_resources_.end();
+
+  gpu::Mailbox mailbox;
+  if (has_mailbox) {
+    DCHECK(context_provider_);
+
+    gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL();
+
+    GLC(gl, gl->GenMailboxCHROMIUM(mailbox.name));
+    ResourceProvider::ScopedWriteLockGL lock(resource_provider_, resource_id);
+    GLC(gl, gl->ProduceTextureDirectCHROMIUM(lock.texture_id(), GL_TEXTURE_2D,
+                                             mailbox.name));
   }
+  all_resources_.push_front(
+      PlaneResource(resource_id, plane_size, format, mailbox));
+  return all_resources_.begin();
 }
 
-void VideoResourceUpdater::DeleteResource(unsigned resource_id) {
-  resource_provider_->DeleteResource(resource_id);
-  all_resources_.erase(std::remove(all_resources_.begin(),
-                                   all_resources_.end(),
-                                   resource_id));
+void VideoResourceUpdater::DeleteResource(ResourceList::iterator resource_it) {
+  DCHECK_EQ(resource_it->ref_count, 0);
+  resource_provider_->DeleteResource(resource_it->resource_id);
+  all_resources_.erase(resource_it);
 }
 
 VideoFrameExternalResources VideoResourceUpdater::
@@ -157,19 +186,15 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
 #endif  // defined(VIDEO_HOLE)
 
   // Only YUV software video frames are supported.
-  DCHECK(input_frame_format == media::VideoFrame::YV12 ||
-         input_frame_format == media::VideoFrame::I420 ||
-         input_frame_format == media::VideoFrame::YV12A ||
-         input_frame_format == media::VideoFrame::YV12J ||
-         input_frame_format == media::VideoFrame::YV16 ||
-         input_frame_format == media::VideoFrame::YV24);
   if (input_frame_format != media::VideoFrame::YV12 &&
       input_frame_format != media::VideoFrame::I420 &&
       input_frame_format != media::VideoFrame::YV12A &&
       input_frame_format != media::VideoFrame::YV12J &&
       input_frame_format != media::VideoFrame::YV16 &&
-      input_frame_format != media::VideoFrame::YV24)
+      input_frame_format != media::VideoFrame::YV24) {
+    NOTREACHED() << input_frame_format;
     return VideoFrameExternalResources();
+  }
 
   bool software_compositor = context_provider_ == NULL;
 
@@ -186,81 +211,69 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
     output_plane_count = 1;
   }
 
-  int max_resource_size = resource_provider_->max_texture_size();
-  std::vector<PlaneResource> plane_resources;
-  bool allocation_success = true;
+  // Drop recycled resources that are the wrong format.
+  for (auto it = all_resources_.begin(); it != all_resources_.end();) {
+    if (it->ref_count == 0 && it->resource_format != output_resource_format)
+      DeleteResource(it++);
+    else
+      ++it;
+  }
 
+  const int max_resource_size = resource_provider_->max_texture_size();
+  std::vector<ResourceList::iterator> plane_resources;
   for (size_t i = 0; i < output_plane_count; ++i) {
     gfx::Size output_plane_resource_size =
         SoftwarePlaneDimension(video_frame, software_compositor, i);
     if (output_plane_resource_size.IsEmpty() ||
         output_plane_resource_size.width() > max_resource_size ||
         output_plane_resource_size.height() > max_resource_size) {
-      allocation_success = false;
       break;
     }
 
     // Try recycle a previously-allocated resource.
-    auto recycled_it = recycled_resources_.end();
-    for (auto it = recycled_resources_.begin(); it != recycled_resources_.end();
-         ++it) {
-      const bool resource_matches =
-          it->resource_format == output_resource_format &&
-          it->resource_size == output_plane_resource_size;
-      const bool in_use = software_compositor &&
-                          resource_provider_->InUseByConsumer(it->resource_id);
-      if (resource_matches && !in_use) {
-        // We found a recycled resource with the allocation size and format we
-        // are looking for.
-        recycled_it = it;
-        // Keep looking for a recycled resource that also contains the data we
-        // are planning to put in it.
-        if (PlaneResourceMatchesUniqueID(*it, video_frame.get(), i))
+    ResourceList::iterator resource_it = all_resources_.end();
+    for (auto it = all_resources_.begin(); it != all_resources_.end(); ++it) {
+      if (it->resource_size == output_plane_resource_size &&
+          it->resource_format == output_resource_format) {
+        if (PlaneResourceMatchesUniqueID(*it, video_frame.get(), i)) {
+          // Bingo, we found a resource that already contains the data we are
+          // planning to put in it. It's safe to reuse it even if
+          // resource_provider_ holds some references to it, because those
+          // references are read-only.
+          resource_it = it;
           break;
+        }
+
+        // This extra check is needed because resources backed by SharedMemory
+        // are not ref-counted, unlike mailboxes. Full discussion in
+        // codereview.chromium.org/145273021.
+        const bool in_use =
+            software_compositor &&
+            resource_provider_->InUseByConsumer(it->resource_id);
+        if (it->ref_count == 0 && !in_use) {
+          // We found a resource with the correct size that we can overwrite.
+          resource_it = it;
+        }
       }
     }
 
-    // Check if we can avoid allocating a new resource.
-    if (recycled_it != recycled_resources_.end()) {
-      plane_resources.push_back(*recycled_it);
-      recycled_resources_.erase(recycled_it);
-      continue;
+    // Check if we need to allocate a new resource.
+    if (resource_it == all_resources_.end()) {
+      resource_it =
+          AllocateResource(output_plane_resource_size, output_resource_format,
+                           !software_compositor);
     }
-
-    // TODO(danakj): Abstract out hw/sw resource create/delete from
-    // ResourceProvider and stop using ResourceProvider in this class.
-    const ResourceProvider::ResourceId resource_id =
-        resource_provider_->CreateResource(
-            output_plane_resource_size, GL_CLAMP_TO_EDGE,
-            ResourceProvider::TextureHintImmutable, output_resource_format);
-    if (resource_id == 0) {
-      allocation_success = false;
+    if (resource_it == all_resources_.end())
       break;
-    }
-    all_resources_.push_back(resource_id);
-
-    gpu::Mailbox mailbox;
-    if (!software_compositor) {
-      DCHECK(context_provider_);
 
-      gpu::gles2::GLES2Interface* gl = context_provider_->ContextGL();
-
-      GLC(gl, gl->GenMailboxCHROMIUM(mailbox.name));
-      ResourceProvider::ScopedWriteLockGL lock(resource_provider_, resource_id);
-      GLC(gl, gl->ProduceTextureDirectCHROMIUM(lock.texture_id(), GL_TEXTURE_2D,
-                                               mailbox.name));
-    }
-
-    DCHECK(software_compositor || !mailbox.IsZero());
-    plane_resources.push_back(PlaneResource(resource_id,
-                                            output_plane_resource_size,
-                                            output_resource_format,
-                                            mailbox));
+    ++resource_it->ref_count;
+    plane_resources.push_back(resource_it);
   }
 
-  if (!allocation_success) {
-    for (size_t i = 0; i < plane_resources.size(); ++i)
-      DeleteResource(plane_resources[i].resource_id);
+  if (plane_resources.size() != output_plane_count) {
+    // Allocation failed, nothing will be returned so restore reference counts.
+    for (ResourceList::iterator resource_it : plane_resources)
+      --resource_it->ref_count;
     return VideoFrameExternalResources();
   }
 
@@ -268,54 +281,52 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
 
   if (software_compositor) {
     DCHECK_EQ(plane_resources.size(), 1u);
-    DCHECK_EQ(plane_resources[0].resource_format, kRGBResourceFormat);
-    DCHECK(plane_resources[0].mailbox.IsZero());
+    PlaneResource& plane_resource = *plane_resources[0];
+    DCHECK_EQ(plane_resource.resource_format, kRGBResourceFormat);
+    DCHECK(plane_resource.mailbox.IsZero());
 
-    if (!PlaneResourceMatchesUniqueID(plane_resources[0], video_frame.get(),
-                                      0)) {
+    if (!PlaneResourceMatchesUniqueID(plane_resource, video_frame.get(), 0)) {
       // We need to transfer data from |video_frame| to the plane resource.
       if (!video_renderer_)
         video_renderer_.reset(new media::SkCanvasVideoRenderer);
 
       ResourceProvider::ScopedWriteLockSoftware lock(
-          resource_provider_, plane_resources[0].resource_id);
+          resource_provider_, plane_resource.resource_id);
       SkCanvas canvas(lock.sk_bitmap());
       video_renderer_->Copy(video_frame, &canvas);
-      SetPlaneResourceUniqueId(video_frame.get(), 0, &plane_resources[0]);
+      SetPlaneResourceUniqueId(video_frame.get(), 0, &plane_resource);
     }
 
-    external_resources.software_resources.push_back(
-        plane_resources[0].resource_id);
+    external_resources.software_resources.push_back(plane_resource.resource_id);
     external_resources.software_release_callback =
-        base::Bind(&RecycleResource, AsWeakPtr(), plane_resources[0]);
+        base::Bind(&RecycleResource, AsWeakPtr(), plane_resource.resource_id);
     external_resources.type = VideoFrameExternalResources::SOFTWARE_RESOURCE;
-
     return external_resources;
   }
 
   for (size_t i = 0; i < plane_resources.size(); ++i) {
+    PlaneResource& plane_resource = *plane_resources[i];
     // Update each plane's resource id with its content.
-    DCHECK_EQ(plane_resources[i].resource_format,
+    DCHECK_EQ(plane_resource.resource_format,
               resource_provider_->yuv_resource_format());
 
-    if (!PlaneResourceMatchesUniqueID(plane_resources[i], video_frame.get(),
-                                      i)) {
+    if (!PlaneResourceMatchesUniqueID(plane_resource, video_frame.get(), i)) {
       // We need to transfer data from |video_frame| to the plane resource.
       const uint8_t* input_plane_pixels = video_frame->data(i);
 
       gfx::Rect image_rect(0, 0, video_frame->stride(i),
-                           plane_resources[i].resource_size.height());
-      gfx::Rect source_rect(plane_resources[i].resource_size);
-      resource_provider_->SetPixels(plane_resources[i].resource_id,
+                           plane_resource.resource_size.height());
+      gfx::Rect source_rect(plane_resource.resource_size);
+      resource_provider_->SetPixels(plane_resource.resource_id,
                                     input_plane_pixels, image_rect, source_rect,
                                     gfx::Vector2d());
-      SetPlaneResourceUniqueId(video_frame.get(), i, &plane_resources[i]);
+      SetPlaneResourceUniqueId(video_frame.get(), i, &plane_resource);
     }
 
     external_resources.mailboxes.push_back(
-        TextureMailbox(plane_resources[i].mailbox, GL_TEXTURE_2D, 0));
+        TextureMailbox(plane_resource.mailbox, GL_TEXTURE_2D, 0));
     external_resources.release_callbacks.push_back(
-        base::Bind(&RecycleResource, AsWeakPtr(), plane_resources[i]));
+        base::Bind(&RecycleResource, AsWeakPtr(), plane_resource.resource_id));
   }
 
   external_resources.type = VideoFrameExternalResources::YUV_RESOURCE;
@@ -382,7 +393,7 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForHardwarePlanes(
 // static
 void VideoResourceUpdater::RecycleResource(
     base::WeakPtr<VideoResourceUpdater> updater,
-    PlaneResource data,
+    ResourceProvider::ResourceId resource_id,
     uint32 sync_point,
     bool lost_resource,
     BlockingTaskRunner* main_thread_task_runner) {
@@ -391,6 +402,14 @@ void VideoResourceUpdater::RecycleResource(
     return;
   }
 
+  const ResourceList::iterator resource_it = std::find_if(
+      updater->all_resources_.begin(), updater->all_resources_.end(),
+      [resource_id](const PlaneResource& plane_resource) {
+        return plane_resource.resource_id == resource_id;
+      });
+  if (resource_it == updater->all_resources_.end())
+    return;
+
   ContextProvider* context_provider = updater->context_provider_;
   if (context_provider && sync_point) {
     GLC(context_provider->ContextGL(),
@@ -398,19 +417,13 @@ void VideoResourceUpdater::RecycleResource(
   }
 
   if (lost_resource) {
-    updater->DeleteResource(data.resource_id);
+    resource_it->ref_count = 0;
+    updater->DeleteResource(resource_it);
     return;
   }
 
-  // Drop recycled resources that are the wrong format.
-  while (!updater->recycled_resources_.empty() &&
-         updater->recycled_resources_.back().resource_format !=
-             data.resource_format) {
-    updater->DeleteResource(updater->recycled_resources_.back().resource_id);
-    updater->recycled_resources_.pop_back();
-  }
-
-  updater->recycled_resources_.push_back(data);
+  --resource_it->ref_count;
+  DCHECK_GE(resource_it->ref_count, 0);
 }
 
 }  // namespace cc