Monitor VideoResourceUpdater reusing destructed resource in Debug mode.

cc/resources/video_resource_updater.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/resources/video_resource_updater.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/bit_cast.h"
+#include "base/single_thread_task_runner.h"
+#include "base/thread_task_runner_handle.h"
 #include "base/trace_event/trace_event.h"
 #include "cc/base/math_util.h"
 #include "cc/output/gl_renderer.h"
 #include "cc/resources/resource_provider.h"
 #include "gpu/GLES2/gl2extchromium.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
 #include "media/base/video_frame.h"
 #include "media/renderers/skcanvas_video_renderer.h"
 #include "third_party/khronos/GLES2/gl2.h"
 #include "third_party/khronos/GLES2/gl2ext.h"
@@ skipping 81 matching lines @@
         sync_token_.Clear();
       }
     }
   }
 
  private:
   gpu::gles2::GLES2Interface* gl_;
   gpu::SyncToken sync_token_;
 };
 
+void OnVideoFrameDestruct(
+    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+    const base::Closure& task) {
+  task_runner->PostTask(FROM_HERE, task);

[miu, 2016/02/19 02:47:46]

This will introduce an unknown time delay before the VideoFrame is invalidated. 
It'd be rare, but not impossible for a new VideoFrame to be created with the
same pointer in the meantime...and even harder bug to catch!  ;-)

Instead of this approach, how about this idea:

1. Add a new VideoFrameMetadata field called RESOURCE_UPDATER_FLAG.

2. When a VideoFrame is passed in a call to SetPlaneResourceUniqueId(), set the
VideoFrame's RESOURCE_UPDATER_FLAG metadata to PlaneResource::resource_id.

3. When a VideoFrame is passed in a call to PlaneResourceMatchesUniqueID(), it
should also check that the RESOURCE_UPDATER_FLAG field is set to the same value
as PlaneResource::resource_id.  (and add a comment about why the
pointer+timestamp sometimes won't work as a unique identifier).

That way, if a VideoFrame is destroyed and re-created with the same pointer, AND
the client always sets the timestamp to zero; it's still possible for
VideoResourceUpdater to determine that this is a new VideoFrame (because it's
FLAG value will be missing).

[xjz, 2016/02/20 00:40:47]

Done. Since each VideoFrame can have three resource_ids, instead of setting
VideoFrame's RESOURCE_UPDATER_FLAG metadata to PlaneResource::resource_id,
setting it to true in SetPlaneResourceUniqueId().

+}
+
 }  // namespace
 
 VideoResourceUpdater::PlaneResource::PlaneResource(
     unsigned int resource_id,
     const gfx::Size& resource_size,
     ResourceFormat resource_format,
     gpu::Mailbox mailbox)
     : resource_id(resource_id),
       resource_size(resource_size),
       resource_format(resource_format),
@@ skipping 221 matching lines @@
         }
       }
     }
 
     // Check if we need to allocate a new resource.
     if (resource_it == all_resources_.end()) {
       const bool is_immutable = true;
       resource_it =
           AllocateResource(output_plane_resource_size, output_resource_format,
                            !software_compositor, is_immutable);
+      if (resource_it != all_resources_.end()) {
+        video_frame->AddDestructionObserver(base::Bind(
+            &OnVideoFrameDestruct, base::ThreadTaskRunnerHandle::Get(),
+            base::Bind(&VideoResourceUpdater::InvalidateResource, AsWeakPtr(),
+                       resource_it->resource_id)));
+      }
     }
     if (resource_it == all_resources_.end())
       break;
 
     ++resource_it->ref_count;
     plane_resources.push_back(resource_it);
   }
 
   if (plane_resources.size() != output_plane_count) {
     // Allocation failed, nothing will be returned so restore reference counts.
@@ skipping 312 matching lines @@
   if (lost_resource) {
     resource_it->ref_count = 0;
     updater->DeleteResource(resource_it);
     return;
   }
 
   --resource_it->ref_count;
   DCHECK_GE(resource_it->ref_count, 0);
 }
 
+// static
+void VideoResourceUpdater::InvalidateResource(
+    base::WeakPtr<VideoResourceUpdater> updater,
+    ResourceId resource_id) {
+  if (!updater.get()) {
+    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;
+
+  resource_it->ref_count = 0;
+  updater->DeleteResource(resource_it);
+}
+
 }  // namespace cc