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>
@@ skipping 11 matching lines @@
 #include "third_party/khronos/GLES2/gl2.h"
 #include "third_party/khronos/GLES2/gl2ext.h"
 #include "ui/gfx/geometry/size_conversions.h"
 
 namespace cc {
 
 namespace {
 
 const ResourceFormat kRGBResourceFormat = RGBA_8888;
 
+// This key is used to mark the userdata in VideoFrame to indicate that its
+// content has been cached in VideoResourceUpdater.
+const char kResourceUploadedKey[] = "ResourceUploadedFlag";
+
 VideoFrameExternalResources::ResourceType ResourceTypeForVideoFrame(
     media::VideoFrame* video_frame) {
   switch (video_frame->format()) {
     case media::PIXEL_FORMAT_ARGB:
     case media::PIXEL_FORMAT_XRGB:
     case media::PIXEL_FORMAT_UYVY:
       switch (video_frame->mailbox_holder(0).texture_target) {
         case GL_TEXTURE_2D:
           return (video_frame->format() == media::PIXEL_FORMAT_XRGB)
                      ? VideoFrameExternalResources::RGB_RESOURCE
@@ skipping 85 matching lines @@
       frame_ptr(nullptr),
       plane_index(0u) {
 }
 
 bool VideoResourceUpdater::PlaneResourceMatchesUniqueID(
     const PlaneResource& plane_resource,
     const media::VideoFrame* video_frame,
     size_t plane_index) {
   return plane_resource.frame_ptr == video_frame &&
          plane_resource.plane_index == plane_index &&
-         plane_resource.timestamp == video_frame->timestamp();
+         plane_resource.timestamp == video_frame->timestamp() &&

[danakj, 2016/02/23 23:55:09]

Can you instead just return false if the timestamps are 0?

[xjz, 2016/02/24 00:01:28]

The actual timestamp could be 0. It is difficult to check whether the timestamp
is correctly set (unique for each frame) here.

[danakj, 2016/02/24 00:07:08]

Oh :/ I don't really like cc setting things on a VideoFrame for this here, it
feels very much like a layering violation.

Can the video decoders set some incrementing id on VideoFrames that they
generate or something, and it could use that, since there is this new need to
tell when a VideoFrame is repeated? That feels like something the VideoFrame
should provide then, rather than hacking in this general thing where cc sets a
flag only it knows about.

[xjz, 2016/02/24 21:57:58]

This sounds equivalent to asking decoders correctly set the timestamps.  

+         video_frame->GetUserData(kResourceUploadedKey);
 }
 
 void VideoResourceUpdater::SetPlaneResourceUniqueId(
-    const media::VideoFrame* video_frame,
+    media::VideoFrame* video_frame,
     size_t plane_index,
     PlaneResource* plane_resource) {
   plane_resource->frame_ptr = video_frame;
   plane_resource->plane_index = plane_index;
   plane_resource->timestamp = video_frame->timestamp();
+  video_frame->SetUserData(kResourceUploadedKey,
+                           new base::SupportsUserData::Data());
 }
 
 VideoFrameExternalResources::VideoFrameExternalResources()
     : type(NONE),
       read_lock_fences_enabled(false),
       offset(0.0f),
       multiplier(1.0f) {}
 
 VideoFrameExternalResources::~VideoFrameExternalResources() {}
 
@@ skipping 524 matching lines @@
     resource_it->ref_count = 0;
     updater->DeleteResource(resource_it);
     return;
   }
 
   --resource_it->ref_count;
   DCHECK_GE(resource_it->ref_count, 0);
 }
 
 }  // namespace cc