cc: Use ResourceProvider::Resouce::type to check the type of the given resource.

cc/resources/resource_provider.cc

Index: cc/resources/resource_provider.cc
diff --git a/cc/resources/resource_provider.cc b/cc/resources/resource_provider.cc
index b43164847a0ad7ac62db9c801c499cd1448d7745..3d8b275535526567eca15371a8869a711bf22cf1 100644
--- a/cc/resources/resource_provider.cc
+++ b/cc/resources/resource_provider.cc
@@ -190,7 +190,7 @@ ResourceProvider::Resource::Resource()
       wrap_mode(0),
       lost(false),
       hint(TextureUsageAny),
-      type(static_cast<ResourceType>(0)),
+      type(InvalidType),
       format(RGBA_8888),
       shared_bitmap(NULL) {}
 
@@ -198,6 +198,7 @@ ResourceProvider::Resource::~Resource() {}
 
 ResourceProvider::Resource::Resource(GLuint texture_id,
                                      gfx::Size size,
+                                     Origin origin,
                                      GLenum target,
                                      GLenum filter,
                                      GLenum texture_pool,
@@ -214,7 +215,7 @@ ResourceProvider::Resource::Resource(GLuint texture_id,
       imported_count(0),
       exported_count(0),
       locked_for_write(false),
-      origin(Internal),
+      origin(origin),
       marked_for_deletion(false),
       pending_set_pixels(false),
       set_pixels_completion_forced(false),
@@ -236,11 +237,13 @@ ResourceProvider::Resource::Resource(GLuint texture_id,
       format(format),
       shared_bitmap(NULL) {
   DCHECK(wrap_mode == GL_CLAMP_TO_EDGE || wrap_mode == GL_REPEAT);
+  DCHECK_EQ(origin == Internal, !!texture_pool);
 }
 
 ResourceProvider::Resource::Resource(uint8_t* pixels,
                                      SharedBitmap* bitmap,
                                      gfx::Size size,
+                                     Origin origin,
                                      GLenum filter,
                                      GLint wrap_mode)
     : child_id(0),
@@ -253,7 +256,7 @@ ResourceProvider::Resource::Resource(uint8_t* pixels,
       imported_count(0),
       exported_count(0),
       locked_for_write(false),
-      origin(Internal),
+      origin(origin),
       marked_for_deletion(false),
       pending_set_pixels(false),
       set_pixels_completion_forced(false),
@@ -275,6 +278,7 @@ ResourceProvider::Resource::Resource(uint8_t* pixels,
       format(RGBA_8888),
       shared_bitmap(bitmap) {
   DCHECK(wrap_mode == GL_CLAMP_TO_EDGE || wrap_mode == GL_REPEAT);
+  DCHECK(pixels);

[danakj, 2014/01/30 21:50:31]

Reviewing a software ubercomp CL (https://codereview.chromium.org/148243013),
this will not be true for software resources that are received in a renderer
process from a child renderer. The child does not get access to the pixels, so
the current code will receive the resource and set the pixels field to NULL.

So I guess it's better to DCHECK this in places that will actually access the
pixels instead.

[dshwang, 2014/01/31 17:25:34]

I still think pixel is always non-null.
See below comment

 }
 
 ResourceProvider::Child::Child() : marked_for_deletion(false) {}
@@ -392,8 +396,15 @@ ResourceProvider::ResourceId ResourceProvider::CreateGLTexture(
   DCHECK(thread_checker_.CalledOnValidThread());
 
   ResourceId id = next_id_++;
-  Resource resource(
-      0, size, target, GL_LINEAR, texture_pool, wrap_mode, hint, format);
+  Resource resource(0,
+                    size,
+                    Resource::Internal,
+                    target,
+                    GL_LINEAR,
+                    texture_pool,
+                    wrap_mode,
+                    hint,
+                    format);
   resource.allocated = false;
   resources_[id] = resource;
   return id;
@@ -415,7 +426,7 @@ ResourceProvider::ResourceId ResourceProvider::CreateBitmap(
 
   ResourceId id = next_id_++;
   Resource resource(
-      pixels, bitmap.release(), size, GL_LINEAR, wrap_mode);
+      pixels, bitmap.release(), size, Resource::Internal, GL_LINEAR, wrap_mode);
   resource.allocated = true;
   resources_[id] = resource;
   return id;
@@ -427,6 +438,8 @@ ResourceProvider::CreateResourceFromExternalTexture(
     GLuint texture_id) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  DCHECK(texture_target);
+  DCHECK(texture_id);
   GLES2Interface* gl = ContextGL();
   DCHECK(gl);
   GLC(gl, gl->BindTexture(texture_target, texture_id));
@@ -440,13 +453,13 @@ ResourceProvider::CreateResourceFromExternalTexture(
   ResourceId id = next_id_++;
   Resource resource(texture_id,
                     gfx::Size(),
+                    Resource::External,
                     texture_target,
                     GL_LINEAR,
                     0,
                     GL_CLAMP_TO_EDGE,
                     TextureUsageAny,
                     RGBA_8888);
-  resource.origin = Resource::External;
   resource.allocated = true;
   resources_[id] = resource;
   return id;
@@ -463,6 +476,7 @@ ResourceProvider::ResourceId ResourceProvider::CreateResourceFromTextureMailbox(
   if (mailbox.IsTexture()) {
     resource = Resource(0,
                         gfx::Size(),
+                        Resource::External,
                         mailbox.target(),
                         GL_LINEAR,
                         0,
@@ -482,10 +496,10 @@ ResourceProvider::ResourceId ResourceProvider::CreateResourceFromTextureMailbox(
     resource = Resource(pixels,
                         shared_bitmap.release(),
                         mailbox.shared_memory_size(),
+                        Resource::External,
                         GL_LINEAR,
                         GL_CLAMP_TO_EDGE);
   }
-  resource.origin = Resource::External;
   resource.allocated = true;
   resource.mailbox = mailbox;
   resource.release_callback =
@@ -543,7 +557,8 @@ void ResourceProvider::DeleteResourceInternal(ResourceMap::iterator it,
   }
   if (resource->mailbox.IsValid() && resource->origin == Resource::External) {
     GLuint sync_point = resource->mailbox.sync_point();
-    if (resource->mailbox.IsTexture()) {
+    if (resource->type == GLTexture) {
+      DCHECK(resource->mailbox.IsTexture());
       lost_resource |= lost_output_surface_;
       GLES2Interface* gl = ContextGL();
       DCHECK(gl);
@@ -573,6 +588,7 @@ void ResourceProvider::DeleteResourceInternal(ResourceMap::iterator it,
   }
   if (resource->shared_bitmap) {
     DCHECK(resource->origin != Resource::External);
+    DCHECK_EQ(Bitmap, resource->type);
     delete resource->shared_bitmap;
     resource->pixels = NULL;
   }
@@ -609,7 +625,8 @@ void ResourceProvider::SetPixels(ResourceId id,
   DCHECK(ReadLockFenceHasPassed(resource));
   LazyAllocate(resource);
 
-  if (resource->gl_id) {
+  if (resource->type == GLTexture) {
+    DCHECK(resource->gl_id);
     DCHECK(!resource->pending_set_pixels);
     DCHECK_EQ(resource->target, static_cast<GLenum>(GL_TEXTURE_2D));
     GLES2Interface* gl = ContextGL();
@@ -622,9 +639,8 @@ void ResourceProvider::SetPixels(ResourceId id,
                               dest_offset,
                               resource->format,
                               resource->size);
-  }
-
-  if (resource->pixels) {
+  } else {
+    DCHECK_EQ(Bitmap, resource->type);
     DCHECK(resource->allocated);
     DCHECK_EQ(RGBA_8888, resource->format);
     SkBitmap src_full;
@@ -747,8 +763,9 @@ const ResourceProvider::Resource* ResourceProvider::LockForRead(ResourceId id) {
 
   LazyCreate(resource);
 
-  if (!resource->gl_id && resource->mailbox.IsTexture()) {
+  if (resource->type == GLTexture && !resource->gl_id) {
     DCHECK(resource->origin != Resource::Internal);
+    DCHECK(resource->mailbox.IsTexture());
     GLES2Interface* gl = ContextGL();
     DCHECK(gl);
     if (resource->mailbox.sync_point()) {
@@ -1087,11 +1104,16 @@ void ResourceProvider::ReceiveFromChild(
     ResourceId local_id = next_id_++;
     Resource& resource = resources_[local_id];
     if (it->is_software) {
-      resource = Resource(
-          pixels, bitmap.release(), it->size, GL_LINEAR, GL_CLAMP_TO_EDGE);
+      resource = Resource(pixels,
+                          bitmap.release(),
+                          it->size,
+                          Resource::Delegated,
+                          GL_LINEAR,
+                          GL_CLAMP_TO_EDGE);
     } else {
       resource = Resource(0,
                           it->size,
+                          Resource::Delegated,
                           it->target,
                           it->filter,
                           0,
@@ -1102,7 +1124,6 @@ void ResourceProvider::ReceiveFromChild(
           TextureMailbox(it->mailbox, it->target, it->sync_point);
     }
     resource.child_id = child;
-    resource.origin = Resource::Delegated;
     // Don't allocate a texture for a child.
     resource.allocated = true;
     resource.imported_count = 1;
@@ -1272,7 +1293,8 @@ void ResourceProvider::TransferResource(GLES2Interface* gl,
   resource->filter = source->filter;
   resource->size = source->size;
 
-  if (source->shared_bitmap) {
+  if (source->type == Bitmap) {
+    // Currently, this routine is used by only unittests.

[danakj, 2014/01/30 21:50:31]

I don't think you need this, we'll just have to remove it later.

[dshwang, 2014/01/31 17:25:34]

indeed, i removed it.

     resource->mailbox = source->shared_bitmap->id();
     resource->is_software = true;
   } else if (!source->mailbox.IsValid()) {
@@ -1337,7 +1359,8 @@ void ResourceProvider::DeleteAndReturnUnusedResourcesToChild(
     DCHECK(child_info->child_to_parent_map.count(child_id));
 
     bool is_lost =
-        resource.lost || (!resource.shared_bitmap && lost_output_surface_);
+        resource.lost ||
+        (resource.type == GLTexture && lost_output_surface_);
     if (resource.exported_count > 0) {
       if (style != ForShutdown) {
         // Defer this until we receive the resource back from the parent.
@@ -1367,7 +1390,7 @@ void ResourceProvider::DeleteAndReturnUnusedResourcesToChild(
     ReturnedResource returned;
     returned.id = child_id;
     returned.sync_point = resource.mailbox.sync_point();
-    if (!returned.sync_point && !resource.shared_bitmap)
+    if (!returned.sync_point && resource.type == GLTexture)
       need_sync_point = true;
     returned.count = resource.imported_count;
     returned.lost = is_lost;
@@ -1418,9 +1441,8 @@ void ResourceProvider::AcquirePixelBuffer(ResourceId id) {
                    NULL,
                    GL_DYNAMIC_DRAW);
     gl->BindBuffer(GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM, 0);
-  }
-
-  if (resource->pixels) {
+  } else {
+    DCHECK_EQ(Bitmap, resource->type);
     if (resource->pixel_buffer)
       return;
 
@@ -1456,9 +1478,8 @@ void ResourceProvider::ReleasePixelBuffer(ResourceId id) {
     gl->BufferData(
         GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM, 0, NULL, GL_DYNAMIC_DRAW);
     gl->BindBuffer(GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM, 0);
-  }
-
-  if (resource->pixels) {
+  } else {
+    DCHECK_EQ(Bitmap, resource->type);
     if (!resource->pixel_buffer)
       return;
     delete[] resource->pixel_buffer;
@@ -1485,11 +1506,8 @@ uint8_t* ResourceProvider::MapPixelBuffer(ResourceId id) {
     CHECK(!(reinterpret_cast<intptr_t>(image) & 3));
     return image;
   }
-
-  if (resource->pixels)
-    return resource->pixel_buffer;
-
-  return NULL;
+  DCHECK_EQ(Bitmap, resource->type);
+  return resource->pixel_buffer;
 }
 
 void ResourceProvider::UnmapPixelBuffer(ResourceId id) {
@@ -1541,6 +1559,7 @@ void ResourceProvider::BeginSetPixels(ResourceId id) {
   DCHECK(!resource->pending_set_pixels);
 
   LazyCreate(resource);
+  DCHECK(resource->origin == Resource::Internal);
   DCHECK(resource->gl_id || resource->allocated);
   DCHECK(ReadLockFenceHasPassed(resource));
   DCHECK(!resource->image_id);
@@ -1549,7 +1568,8 @@ void ResourceProvider::BeginSetPixels(ResourceId id) {
   resource->allocated = true;
   LockForWrite(id);
 
-  if (resource->gl_id) {
+  if (resource->type == GLTexture) {
+    DCHECK(resource->gl_id);
     GLES2Interface* gl = ContextGL();
     DCHECK(gl);
     DCHECK(resource->gl_pixel_buffer_id);
@@ -1584,9 +1604,8 @@ void ResourceProvider::BeginSetPixels(ResourceId id) {
     }
     gl->EndQueryEXT(GL_ASYNC_PIXEL_UNPACK_COMPLETED_CHROMIUM);
     gl->BindBuffer(GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM, 0);
-  }
-
-  if (resource->pixels) {
+  } else {
+    DCHECK_EQ(Bitmap, resource->type);
     DCHECK(!resource->mailbox.IsValid());
     DCHECK(resource->pixel_buffer);
     DCHECK_EQ(RGBA_8888, resource->format);
@@ -1648,13 +1667,13 @@ GLenum ResourceProvider::TargetForTesting(ResourceId id) {
 }
 
 void ResourceProvider::LazyCreate(Resource* resource) {
-  if (resource->type != GLTexture || resource->gl_id != 0)
+  if (resource->type != GLTexture || resource->origin != Resource::Internal)
     return;
 
-  // Early out for resources that don't require texture creation.
-  if (resource->texture_pool == 0)
+  if (resource->gl_id)
     return;
 
+  DCHECK(resource->texture_pool);
   DCHECK(resource->origin == Resource::Internal);
   DCHECK(!resource->mailbox.IsValid());
   resource->gl_id = texture_id_allocator_->NextId();
@@ -1788,17 +1807,15 @@ uint8_t* ResourceProvider::MapImage(ResourceId id) {
   DCHECK(resource->origin == Resource::Internal);
   DCHECK_EQ(resource->exported_count, 0);
 
-  if (resource->image_id) {
+  if (resource->type == GLTexture) {
+    DCHECK(resource->image_id);
     GLES2Interface* gl = ContextGL();
     DCHECK(gl);
     return static_cast<uint8_t*>(
         gl->MapImageCHROMIUM(resource->image_id, GL_READ_WRITE));
   }
-
-  if (resource->pixels)
-    return resource->pixels;
-
-  return NULL;
+  DCHECK_EQ(Bitmap, resource->type);
+  return resource->pixels;
 }
 
 void ResourceProvider::UnmapImage(ResourceId id) {