Restrict the number of software_resource in cc::VideoLaiyerImpl to one

VideoLayerImpl used to hold the software ResourceID as a vector, so that
it can hold more than one resource. However, it actually doesn't hold
more than one, and causes multiple usage of ReleaseCallback, which should
be used only once.

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I79e4164bd889bf1a8b46ae55f03660f96e65835d
Reviewed-on: https://chromium-review.googlesource.com/765611
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517034}

cc/layers/video_layer_impl.cc

@@ -113,7 +113,8 @@ bool VideoLayerImpl::WillDraw(DrawMode draw_mode,
 
   if (external_resources.type ==
       VideoFrameExternalResources::SOFTWARE_RESOURCE) {
-    software_resources_ = external_resources.software_resources;
+    DCHECK_GT(external_resources.software_resource, viz::kInvalidResourceId);
+    software_resource_ = external_resources.software_resource;
     software_release_callback_ =
         external_resources.software_release_callback;
     return true;
@@ -201,9 +202,7 @@ void VideoLayerImpl::AppendQuads(viz::RenderPass* render_pass,
     // TODO(danakj): Remove this, hide it in the hardware path.
     case VideoFrameExternalResources::SOFTWARE_RESOURCE: {
       DCHECK_EQ(frame_resources_.size(), 0u);
-      DCHECK_EQ(software_resources_.size(), 1u);
-      if (software_resources_.size() < 1u)
-        break;
+      DCHECK_GT(software_resource_, viz::kInvalidResourceId);
       bool premultiplied_alpha = true;
       gfx::PointF uv_top_left(0.f, 0.f);
       gfx::PointF uv_bottom_right(tex_width_scale, tex_height_scale);
@@ -212,11 +211,10 @@ void VideoLayerImpl::AppendQuads(viz::RenderPass* render_pass,
       bool nearest_neighbor = false;
       auto* texture_quad =
           render_pass->CreateAndAppendDrawQuad<viz::TextureDrawQuad>();
-      texture_quad->SetNew(shared_quad_state, quad_rect, visible_quad_rect,
-                           needs_blending, software_resources_[0],
-                           premultiplied_alpha, uv_top_left, uv_bottom_right,
-                           SK_ColorTRANSPARENT, opacity, flipped,
-                           nearest_neighbor, false);
+      texture_quad->SetNew(
+          shared_quad_state, quad_rect, visible_quad_rect, needs_blending,
+          software_resource_, premultiplied_alpha, uv_top_left, uv_bottom_right,
+          SK_ColorTRANSPARENT, opacity, flipped, nearest_neighbor, false);
       ValidateQuadResources(texture_quad);
       break;
     }
@@ -337,11 +335,10 @@ void VideoLayerImpl::DidDraw(LayerTreeResourceProvider* resource_provider) {
 
   if (frame_resource_type_ ==
       VideoFrameExternalResources::SOFTWARE_RESOURCE) {
-    for (size_t i = 0; i < software_resources_.size(); ++i) {
-      software_release_callback_.Run(gpu::SyncToken(), false);
-    }
+    DCHECK_GT(software_resource_, viz::kInvalidResourceId);
+    software_release_callback_.Run(gpu::SyncToken(), false);
 
-    software_resources_.clear();
+    software_resource_ = viz::kInvalidResourceId;
     software_release_callback_.Reset();
   } else {
     for (size_t i = 0; i < frame_resources_.size(); ++i)

cc/layers/video_layer_impl.h

@@ -83,7 +83,7 @@ class CC_EXPORT VideoLayerImpl : public LayerImpl {
 
   // TODO(danakj): Remove these, hide software path inside ResourceProvider and
   // ExternalResource (aka TextureMailbox) classes.
-  std::vector<unsigned> software_resources_;
+  unsigned software_resource_ = viz::kInvalidResourceId;
   using SoftwareReleaseCallback =
       VideoFrameExternalResources::SoftwareReleaseCallback;
   // Called once for software_resources_.

cc/resources/resource_provider.cc

@@ -181,6 +181,7 @@ ResourceProvider::ResourceProvider(
       tracing_id_(g_next_resource_provider_tracing_id.GetNext()) {
   DCHECK(resource_settings.texture_id_allocation_chunk_size);
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  DCHECK_GT(next_id_, viz::kInvalidResourceId);
 
   // In certain cases, ThreadTaskRunnerHandle isn't set (Android Webview).
   // Don't register a dump provider in these cases.

cc/resources/video_resource_updater.cc

@@ -401,6 +401,8 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
     DCHECK_EQ(plane_resources.size(), 1u);
     PlaneResource& plane_resource = *plane_resources[0];
     DCHECK_EQ(plane_resource.resource_format(), kRGBResourceFormat);
+    DCHECK(!software_compositor ||
+           plane_resource.resource_id() > viz::kInvalidResourceId);
     DCHECK_EQ(software_compositor, plane_resource.mailbox().IsZero());
 
     if (!plane_resource.Matches(video_frame->unique_id(), 0)) {
@@ -433,8 +435,7 @@ VideoFrameExternalResources VideoResourceUpdater::CreateForSoftwarePlanes(
     }
 
     if (software_compositor) {
-      external_resources.software_resources.push_back(
-          plane_resource.resource_id());
+      external_resources.software_resource = plane_resource.resource_id();
       external_resources.software_release_callback =
           base::Bind(&RecycleResource, weak_ptr_factory_.GetWeakPtr(),
                      plane_resource.resource_id());

cc/resources/video_resource_updater.h

@@ -20,6 +20,7 @@
 #include "components/viz/common/quads/texture_mailbox.h"
 #include "components/viz/common/resources/release_callback.h"
 #include "components/viz/common/resources/resource_format.h"
+#include "components/viz/common/resources/resource_id.h"
 #include "ui/gfx/buffer_types.h"
 #include "ui/gfx/geometry/size.h"
 
@@ -63,7 +64,7 @@ class CC_EXPORT VideoFrameExternalResources {
   using SoftwareReleaseCallback =
       base::RepeatingCallback<void(const gpu::SyncToken& sync_token,
                                    bool is_lost)>;
-  std::vector<unsigned> software_resources;
+  unsigned software_resource = viz::kInvalidResourceId;
   SoftwareReleaseCallback software_release_callback;
 
   // Used by hardware textures which do not return values in the 0-1 range.

cc/resources/video_resource_updater_unittest.cc

@@ -390,7 +390,7 @@ TEST_F(VideoResourceUpdaterTest, ReuseResource) {
   EXPECT_EQ(VideoFrameExternalResources::YUV_RESOURCE, resources.type);
   EXPECT_EQ(size_t(3), resources.mailboxes.size());
   EXPECT_EQ(size_t(3), resources.release_callbacks.size());
-  EXPECT_EQ(size_t(0), resources.software_resources.size());
+  EXPECT_EQ(viz::kInvalidResourceId, resources.software_resource);
   // Expect exactly three texture uploads, one for each plane.
   EXPECT_EQ(3, context3d_->UploadCount());
 
@@ -424,7 +424,7 @@ TEST_F(VideoResourceUpdaterTest, ReuseResourceNoDelete) {
   EXPECT_EQ(VideoFrameExternalResources::YUV_RESOURCE, resources.type);
   EXPECT_EQ(size_t(3), resources.mailboxes.size());
   EXPECT_EQ(size_t(3), resources.release_callbacks.size());
-  EXPECT_EQ(size_t(0), resources.software_resources.size());
+  EXPECT_EQ(viz::kInvalidResourceId, resources.software_resource);
   // Expect exactly three texture uploads, one for each plane.
   EXPECT_EQ(3, context3d_->UploadCount());
 
@@ -463,7 +463,7 @@ TEST_F(VideoResourceUpdaterTest, ReuseResourceSoftwareCompositor) {
   EXPECT_EQ(VideoFrameExternalResources::SOFTWARE_RESOURCE, resources.type);
   EXPECT_EQ(size_t(0), resources.mailboxes.size());
   EXPECT_EQ(size_t(0), resources.release_callbacks.size());
-  EXPECT_EQ(size_t(1), resources.software_resources.size());
+  EXPECT_LT(viz::kInvalidResourceId, resources.software_resource);
   // Expect exactly one allocated shared bitmap.
   EXPECT_EQ(1, shared_bitmap_manager_->AllocationCount());
 
@@ -477,7 +477,7 @@ TEST_F(VideoResourceUpdaterTest, ReuseResourceSoftwareCompositor) {
   EXPECT_EQ(VideoFrameExternalResources::SOFTWARE_RESOURCE, resources.type);
   EXPECT_EQ(size_t(0), resources.mailboxes.size());
   EXPECT_EQ(size_t(0), resources.release_callbacks.size());
-  EXPECT_EQ(size_t(1), resources.software_resources.size());
+  EXPECT_LT(viz::kInvalidResourceId, resources.software_resource);
   // The data should be reused so expect no new allocations.
   EXPECT_EQ(0, shared_bitmap_manager_->AllocationCount());
 }
@@ -496,7 +496,7 @@ TEST_F(VideoResourceUpdaterTest, ReuseResourceNoDeleteSoftwareCompositor) {
   EXPECT_EQ(VideoFrameExternalResources::SOFTWARE_RESOURCE, resources.type);
   EXPECT_EQ(size_t(0), resources.mailboxes.size());
   EXPECT_EQ(size_t(0), resources.release_callbacks.size());
-  EXPECT_EQ(size_t(1), resources.software_resources.size());
+  EXPECT_LT(viz::kInvalidResourceId, resources.software_resource);
   // Expect exactly one allocated shared bitmap.
   EXPECT_EQ(1, shared_bitmap_manager_->AllocationCount());
 
@@ -506,7 +506,7 @@ TEST_F(VideoResourceUpdaterTest, ReuseResourceNoDeleteSoftwareCompositor) {
   EXPECT_EQ(VideoFrameExternalResources::SOFTWARE_RESOURCE, resources.type);
   EXPECT_EQ(size_t(0), resources.mailboxes.size());
   EXPECT_EQ(size_t(0), resources.release_callbacks.size());
-  EXPECT_EQ(size_t(1), resources.software_resources.size());
+  EXPECT_NE(viz::kInvalidResourceId, resources.software_resource);
   // The data should be reused so expect no new allocations.
   EXPECT_EQ(0, shared_bitmap_manager_->AllocationCount());
 }
@@ -526,7 +526,7 @@ TEST_F(VideoResourceUpdaterTest, CreateForHardwarePlanes) {
             resources.type);
   EXPECT_EQ(1u, resources.mailboxes.size());
   EXPECT_EQ(1u, resources.release_callbacks.size());
-  EXPECT_EQ(0u, resources.software_resources.size());
+  EXPECT_EQ(viz::kInvalidResourceId, resources.software_resource);
 
   video_frame = CreateTestYuvHardwareVideoFrame(media::PIXEL_FORMAT_I420, 3,
                                                 GL_TEXTURE_RECTANGLE_ARB);
@@ -535,7 +535,7 @@ TEST_F(VideoResourceUpdaterTest, CreateForHardwarePlanes) {
   EXPECT_EQ(VideoFrameExternalResources::YUV_RESOURCE, resources.type);
   EXPECT_EQ(3u, resources.mailboxes.size());
   EXPECT_EQ(3u, resources.release_callbacks.size());
-  EXPECT_EQ(0u, resources.software_resources.size());
+  EXPECT_EQ(viz::kInvalidResourceId, resources.software_resource);
   EXPECT_FALSE(resources.read_lock_fences_enabled);
 
   video_frame = CreateTestYuvHardwareVideoFrame(media::PIXEL_FORMAT_I420, 3,
@@ -563,7 +563,7 @@ TEST_F(VideoResourceUpdaterTest, CreateForHardwarePlanes_StreamTexture) {
   EXPECT_EQ(1u, resources.mailboxes.size());
   EXPECT_EQ((GLenum)GL_TEXTURE_EXTERNAL_OES, resources.mailboxes[0].target());
   EXPECT_EQ(1u, resources.release_callbacks.size());
-  EXPECT_EQ(0u, resources.software_resources.size());
+  EXPECT_EQ(viz::kInvalidResourceId, resources.software_resource);
   EXPECT_EQ(0, context3d_->TextureCreationCount());
 
   // A copied stream texture should return an RGBA resource in a new
@@ -576,7 +576,7 @@ TEST_F(VideoResourceUpdaterTest, CreateForHardwarePlanes_StreamTexture) {
   EXPECT_EQ(1u, resources.mailboxes.size());
   EXPECT_EQ((GLenum)GL_TEXTURE_2D, resources.mailboxes[0].target());
   EXPECT_EQ(1u, resources.release_callbacks.size());
-  EXPECT_EQ(0u, resources.software_resources.size());
+  EXPECT_EQ(viz::kInvalidResourceId, resources.software_resource);
   EXPECT_EQ(1, context3d_->TextureCreationCount());
 }
 
@@ -596,7 +596,7 @@ TEST_F(VideoResourceUpdaterTest, CreateForHardwarePlanes_TextureQuad) {
   EXPECT_EQ(1u, resources.mailboxes.size());
   EXPECT_EQ((GLenum)GL_TEXTURE_EXTERNAL_OES, resources.mailboxes[0].target());
   EXPECT_EQ(1u, resources.release_callbacks.size());
-  EXPECT_EQ(0u, resources.software_resources.size());
+  EXPECT_EQ(viz::kInvalidResourceId, resources.software_resource);
   EXPECT_EQ(0, context3d_->TextureCreationCount());
 }
 
@@ -727,7 +727,7 @@ TEST_F(VideoResourceUpdaterTest, CreateForHardwarePlanes_DualNV12) {
   EXPECT_EQ(VideoFrameExternalResources::YUV_RESOURCE, resources.type);
   EXPECT_EQ(2u, resources.mailboxes.size());
   EXPECT_EQ(2u, resources.release_callbacks.size());
-  EXPECT_EQ(0u, resources.software_resources.size());
+  EXPECT_EQ(viz::kInvalidResourceId, resources.software_resource);
   EXPECT_EQ((GLenum)GL_TEXTURE_EXTERNAL_OES, resources.mailboxes[0].target());
   // |updater| doesn't set |buffer_format| in this case.
   EXPECT_EQ(gfx::BufferFormat::RGBA_8888, resources.buffer_format);

components/viz/common/resources/resource_id.h

@@ -13,6 +13,7 @@ namespace viz {
 
 using ResourceId = uint32_t;
 using ResourceIdSet = base::flat_set<ResourceId>;
+constexpr ResourceId kInvalidResourceId = 0;
 
 }  // namespace viz