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

cc/resources/resource_provider.cc

 // Copyright 2012 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/resource_provider.h"
 
 #include <algorithm>
 #include <limits>
 
 #include "base/containers/hash_tables.h"
@@ skipping 172 matching lines @@
       target(0),
       original_filter(0),
       filter(0),
       image_id(0),
       bound_image_id(0),
       dirty_image(false),
       texture_pool(0),
       wrap_mode(0),
       lost(false),
       hint(TextureUsageAny),
-      type(static_cast<ResourceType>(0)),
+      type(InvalidType),
       format(RGBA_8888),
       shared_bitmap(NULL) {}
 
 ResourceProvider::Resource::~Resource() {}
 
 ResourceProvider::Resource::Resource(GLuint texture_id,
                                      gfx::Size size,
+                                     Origin origin,
                                      GLenum target,
                                      GLenum filter,
                                      GLenum texture_pool,
                                      GLint wrap_mode,
                                      TextureUsageHint hint,
                                      ResourceFormat format)
     : child_id(0),
       gl_id(texture_id),
       gl_pixel_buffer_id(0),
       gl_upload_query_id(0),
       pixels(NULL),
       pixel_buffer(NULL),
       lock_for_read_count(0),
       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),
       allocated(false),
       enable_read_lock_fences(false),
       read_lock_fence(NULL),
       size(size),
       target(target),
       original_filter(filter),
       filter(filter),
       image_id(0),
       bound_image_id(0),
       dirty_image(false),
       texture_pool(texture_pool),
       wrap_mode(wrap_mode),
       lost(false),
       hint(hint),
       type(GLTexture),
       format(format),
       shared_bitmap(NULL) {
   DCHECK(wrap_mode == GL_CLAMP_TO_EDGE || wrap_mode == GL_REPEAT);
+  DCHECK(texture_pool || origin != Internal);

[dshwang, 2014/01/28 20:35:36]

For this DCHECK, I change the constructor.

[danakj, 2014/01/28 21:59:02]

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),
       gl_id(0),
       gl_pixel_buffer_id(0),
       gl_upload_query_id(0),
       pixels(pixels),
       pixel_buffer(NULL),
       lock_for_read_count(0),
       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),
       allocated(false),
       enable_read_lock_fences(false),
       read_lock_fence(NULL),
       size(size),
       target(0),
       original_filter(filter),
       filter(filter),
       image_id(0),
       bound_image_id(0),
       dirty_image(false),
       texture_pool(0),
       wrap_mode(wrap_mode),
       lost(false),
       hint(TextureUsageAny),
       type(Bitmap),
       format(RGBA_8888),
       shared_bitmap(bitmap) {
   DCHECK(wrap_mode == GL_CLAMP_TO_EDGE || wrap_mode == GL_REPEAT);
+  DCHECK(pixels);
 }
 
 ResourceProvider::Child::Child() : marked_for_deletion(false) {}
 
 ResourceProvider::Child::~Child() {}
 
 scoped_ptr<ResourceProvider> ResourceProvider::Create(
     OutputSurface* output_surface,
     SharedBitmapManager* shared_bitmap_manager,
     int highp_threshold_min,
@@ skipping 97 matching lines @@
     GLenum target,
     GLenum texture_pool,
     GLint wrap_mode,
     TextureUsageHint hint,
     ResourceFormat format) {
   DCHECK_LE(size.width(), max_texture_size_);
   DCHECK_LE(size.height(), max_texture_size_);
   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;
 }
 
 ResourceProvider::ResourceId ResourceProvider::CreateBitmap(
     gfx::Size size, GLint wrap_mode) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   scoped_ptr<SharedBitmap> bitmap;
   if (shared_bitmap_manager_)
     bitmap = shared_bitmap_manager_->AllocateSharedBitmap(size);
 
   uint8_t* pixels;
   if (bitmap)
     pixels = bitmap->pixels();
   else
     pixels = new uint8_t[4 * size.GetArea()];
 
   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;
 }
 
 ResourceProvider::ResourceId
 ResourceProvider::CreateResourceFromExternalTexture(
     GLuint texture_target,
     GLuint texture_id) {

[danakj, 2014/01/28 21:59:02]

Can we DCHECK that texture_id is not 0 here?

   DCHECK(thread_checker_.CalledOnValidThread());
 
   GLES2Interface* gl = ContextGL();
   DCHECK(gl);
   GLC(gl, gl->BindTexture(texture_target, texture_id));
   GLC(gl, gl->TexParameteri(texture_target, GL_TEXTURE_MIN_FILTER, GL_LINEAR));
   GLC(gl, gl->TexParameteri(texture_target, GL_TEXTURE_MAG_FILTER, GL_LINEAR));
   GLC(gl,
       gl->TexParameteri(texture_target, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE));
   GLC(gl,
       gl->TexParameteri(texture_target, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE));
 
   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;
 }
 
 ResourceProvider::ResourceId ResourceProvider::CreateResourceFromTextureMailbox(
     const TextureMailbox& mailbox,
     scoped_ptr<SingleReleaseCallback> release_callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
   // Just store the information. Mailbox will be consumed in LockForRead().
   ResourceId id = next_id_++;
   DCHECK(mailbox.IsValid());
   Resource& resource = resources_[id];
   if (mailbox.IsTexture()) {
     resource = Resource(0,
                         gfx::Size(),
+                        Resource::External,
                         mailbox.target(),
                         GL_LINEAR,
                         0,
                         GL_CLAMP_TO_EDGE,
                         TextureUsageAny,
                         RGBA_8888);
   } else {
     DCHECK(mailbox.IsSharedMemory());
     base::SharedMemory* shared_memory = mailbox.shared_memory();
     DCHECK(shared_memory->memory());
     uint8_t* pixels = reinterpret_cast<uint8_t*>(shared_memory->memory());
     scoped_ptr<SharedBitmap> shared_bitmap;
     if (shared_bitmap_manager_) {
       shared_bitmap =
           shared_bitmap_manager_->GetBitmapForSharedMemory(shared_memory);
     }
     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 =
       base::Bind(&SingleReleaseCallback::Run,
                  base::Owned(release_callback.release()));
   return id;
 }
 
 void ResourceProvider::DeleteResource(ResourceId id) {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ skipping 37 matching lines @@
     GLC(gl, gl->DeleteQueriesEXT(1, &resource->gl_upload_query_id));
   }
   if (resource->gl_pixel_buffer_id) {
     DCHECK(resource->origin == Resource::Internal);
     GLES2Interface* gl = ContextGL();
     DCHECK(gl);
     GLC(gl, gl->DeleteBuffers(1, &resource->gl_pixel_buffer_id));
   }
   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);
       if (resource->gl_id) {
         GLC(gl, gl->DeleteTextures(1, &resource->gl_id));
         resource->gl_id = 0;
         if (!lost_resource)
           sync_point = gl->InsertSyncPointCHROMIUM();
       }
     } else {
       DCHECK(resource->mailbox.IsSharedMemory());
       base::SharedMemory* shared_memory = resource->mailbox.shared_memory();
       if (resource->pixels && shared_memory) {
         DCHECK(shared_memory->memory() == resource->pixels);
         resource->pixels = NULL;
         delete resource->shared_bitmap;
         resource->shared_bitmap = NULL;
       }
     }
     resource->release_callback.Run(sync_point, lost_resource);
   }
   if (resource->gl_id && resource->origin != Resource::External) {
     GLES2Interface* gl = ContextGL();
     DCHECK(gl);
     GLC(gl, gl->DeleteTextures(1, &resource->gl_id));
     resource->gl_id = 0;
   }
   if (resource->shared_bitmap) {
     DCHECK(resource->origin != Resource::External);
+    DCHECK_EQ(Bitmap, resource->type);
     delete resource->shared_bitmap;
     resource->pixels = NULL;
   }
   if (resource->pixels) {
     DCHECK(resource->origin == Resource::Internal);
     delete[] resource->pixels;
   }
   if (resource->pixel_buffer) {
     DCHECK(resource->origin == Resource::Internal);
     delete[] resource->pixel_buffer;
@@ skipping 16 matching lines @@
                                  const gfx::Rect& source_rect,
                                  gfx::Vector2d dest_offset) {
   Resource* resource = GetResource(id);
   DCHECK(!resource->locked_for_write);
   DCHECK(!resource->lock_for_read_count);
   DCHECK(resource->origin == Resource::Internal);
   DCHECK_EQ(resource->exported_count, 0);
   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();
     DCHECK(gl);
     DCHECK(texture_uploader_.get());
     gl->BindTexture(GL_TEXTURE_2D, resource->gl_id);
     texture_uploader_->Upload(image,
                               image_rect,
                               source_rect,
                               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;
     src_full.setConfig(
         SkBitmap::kARGB_8888_Config, image_rect.width(), image_rect.height());
     src_full.setPixels(const_cast<uint8_t*>(image));
     SkBitmap src_subset;
     SkIRect sk_source_rect = SkIRect::MakeXYWH(source_rect.x(),
                                                source_rect.y(),
                                                source_rect.width(),
@@ skipping 102 matching lines @@
   DCHECK(!resource->locked_for_write ||
          resource->set_pixels_completion_forced) <<
       "locked for write: " << resource->locked_for_write <<
       " pixels completion forced: " << resource->set_pixels_completion_forced;
   DCHECK_EQ(resource->exported_count, 0);
   // Uninitialized! Call SetPixels or LockForWrite first.
   DCHECK(resource->allocated);
 
   LazyCreate(resource);
 
-  if (!resource->gl_id && resource->mailbox.IsTexture()) {
+  if (!resource->gl_id && resource->type == GLTexture) {

[danakj, 2014/01/28 21:59:02]

can you reverse these 2 conditions? checking the gl_id doesn't even make sense
if you're not GLTexture type.

     DCHECK(resource->origin != Resource::Internal);
+    DCHECK(resource->mailbox.IsTexture());
     GLES2Interface* gl = ContextGL();
     DCHECK(gl);
     if (resource->mailbox.sync_point()) {
       GLC(gl, gl->WaitSyncPointCHROMIUM(resource->mailbox.sync_point()));
       resource->mailbox.ResetSyncPoint();
     }
     resource->gl_id = texture_id_allocator_->NextId();
     GLC(gl, gl->BindTexture(resource->target, resource->gl_id));
     GLC(gl,
         gl->ConsumeTextureCHROMIUM(resource->target,
@@ skipping 318 matching lines @@
       TRACE_EVENT0("cc", "ResourceProvider::ReceiveFromChild dropping invalid");
       ReturnedResourceArray to_return;
       to_return.push_back(it->ToReturnedResource());
       child_info.return_callback.Run(to_return);
       continue;
     }
 
     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,
                           GL_CLAMP_TO_EDGE,
                           TextureUsageAny,
                           it->format);
       resource.mailbox =
           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;
     child_info.parent_to_child_map[local_id] = it->id;
     child_info.child_to_parent_map[it->id] = local_id;
   }
 }
 
 void ResourceProvider::DeclareUsedResourcesFromChild(
     int child,
@@ skipping 149 matching lines @@
          source->origin == Resource::Delegated ||
          (source->origin == Resource::External && source->mailbox.IsValid()));
   DCHECK(source->allocated);
   DCHECK_EQ(source->wrap_mode, GL_CLAMP_TO_EDGE);
   resource->id = id;
   resource->format = source->format;
   resource->target = source->target;
   resource->filter = source->filter;
   resource->size = source->size;
 
-  if (source->shared_bitmap) {
+  if (source->type == Bitmap) {
+    DCHECK(source->shared_bitmap);

[danakj, 2014/01/28 21:59:02]

given the next line will crash anyway, not sure we need this dcheck?

     resource->mailbox = source->shared_bitmap->id();
     resource->is_software = true;
   } else if (!source->mailbox.IsValid()) {
     LazyCreate(source);
     DCHECK(source->gl_id);
     DCHECK(source->origin == Resource::Internal);
     GLC(gl, gl->BindTexture(resource->target, source->gl_id));
     if (source->image_id) {
       DCHECK(source->dirty_image);
       BindImageForSampling(source);
@@ skipping 44 matching lines @@
 
     DCHECK(!resource.locked_for_write);
     DCHECK(!resource.lock_for_read_count);
     DCHECK_EQ(0u, child_info->in_use_resources.count(local_id));
     DCHECK(child_info->parent_to_child_map.count(local_id));
 
     ResourceId child_id = child_info->parent_to_child_map[local_id];
     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.
         resource.marked_for_deletion = true;
         continue;
       }
 
       // We still have an exported_count, so we'll have to lose it.
       is_lost = true;
     }
 
     if (gl && resource.filter != resource.original_filter) {
       DCHECK(resource.target);
       DCHECK(resource.gl_id);
 
       GLC(gl, gl->BindTexture(resource.target, resource.gl_id));
       GLC(gl,
           gl->TexParameteri(resource.target,
                             GL_TEXTURE_MIN_FILTER,
                             resource.original_filter));
       GLC(gl,
           gl->TexParameteri(resource.target,
                             GL_TEXTURE_MAG_FILTER,
                             resource.original_filter));
     }
 
     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;
     to_return.push_back(returned);
 
     child_info->parent_to_child_map.erase(local_id);
     child_info->child_to_parent_map.erase(child_id);
     resource.imported_count = 0;
     DeleteResourceInternal(it, style);
   }
@@ skipping 30 matching lines @@
       resource->gl_pixel_buffer_id = buffer_id_allocator_->NextId();
     gl->BindBuffer(GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM,
                    resource->gl_pixel_buffer_id);
     unsigned bytes_per_pixel = BitsPerPixel(resource->format) / 8;
     gl->BufferData(GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM,
                    resource->size.height() *
                        RoundUp(bytes_per_pixel * resource->size.width(), 4u),
                    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;
 
     resource->pixel_buffer = new uint8_t[4 * resource->size.GetArea()];
   }
 }
 
 void ResourceProvider::ReleasePixelBuffer(ResourceId id) {
   Resource* resource = GetResource(id);
   DCHECK(resource->origin == Resource::Internal);
@@ skipping 15 matching lines @@
   if (resource->type == GLTexture) {
     if (!resource->gl_pixel_buffer_id)
       return;
     GLES2Interface* gl = ContextGL();
     DCHECK(gl);
     gl->BindBuffer(GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM,
                    resource->gl_pixel_buffer_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;
     resource->pixel_buffer = NULL;
   }
 }
 
 uint8_t* ResourceProvider::MapPixelBuffer(ResourceId id) {
   Resource* resource = GetResource(id);
   DCHECK(resource->origin == Resource::Internal);
   DCHECK_EQ(resource->exported_count, 0);
   DCHECK(!resource->image_id);
 
   if (resource->type == GLTexture) {
     GLES2Interface* gl = ContextGL();
     DCHECK(gl);
     DCHECK(resource->gl_pixel_buffer_id);
     gl->BindBuffer(GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM,
                    resource->gl_pixel_buffer_id);
     uint8_t* image = static_cast<uint8_t*>(gl->MapBufferCHROMIUM(
         GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM, GL_WRITE_ONLY));
     gl->BindBuffer(GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM, 0);
     // Buffer is required to be 4-byte aligned.
     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) {
   Resource* resource = GetResource(id);
   DCHECK(resource->origin == Resource::Internal);
   DCHECK_EQ(resource->exported_count, 0);
   DCHECK(!resource->image_id);
 
   if (resource->type == GLTexture) {
     GLES2Interface* gl = ContextGL();
@@ skipping 29 matching lines @@
 
   if (resource->image_id && resource->dirty_image)
     BindImageForSampling(resource);
 
   return target;
 }
 
 void ResourceProvider::BeginSetPixels(ResourceId id) {
   Resource* resource = GetResource(id);
   DCHECK(!resource->pending_set_pixels);
 

[danakj, 2014/01/28 21:59:02]

Should we DCHECK the resource has Internal Origin here?

   LazyCreate(resource);
   DCHECK(resource->gl_id || resource->allocated);
   DCHECK(ReadLockFenceHasPassed(resource));
   DCHECK(!resource->image_id);
 
   bool allocate = !resource->allocated;
   resource->allocated = true;
   LockForWrite(id);
 
-  if (resource->gl_id) {
+  if (resource->type == GLTexture) {

[dshwang, 2014/01/28 20:35:36]

For this change, I changed ResourceProvider::LazyCreate()

+    DCHECK(resource->gl_id);
     GLES2Interface* gl = ContextGL();
     DCHECK(gl);
     DCHECK(resource->gl_pixel_buffer_id);
     DCHECK_EQ(resource->target, static_cast<GLenum>(GL_TEXTURE_2D));
     gl->BindTexture(GL_TEXTURE_2D, resource->gl_id);
     gl->BindBuffer(GL_PIXEL_UNPACK_TRANSFER_BUFFER_CHROMIUM,
                    resource->gl_pixel_buffer_id);
     if (!resource->gl_upload_query_id)
       gl->GenQueriesEXT(1, &resource->gl_upload_query_id);
     gl->BeginQueryEXT(GL_ASYNC_PIXEL_UNPACK_COMPLETED_CHROMIUM,
@@ skipping 14 matching lines @@
                                      0, /* x */
                                      0, /* y */
                                      resource->size.width(),
                                      resource->size.height(),
                                      GLDataFormat(resource->format),
                                      GLDataType(resource->format),
                                      NULL);
     }
     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);
 
     std::swap(resource->pixels, resource->pixel_buffer);
     delete[] resource->pixel_buffer;
     resource->pixel_buffer = NULL;
   }
 
   resource->pending_set_pixels = true;
@@ skipping 41 matching lines @@
 void ResourceProvider::CreateForTesting(ResourceId id) {
   LazyCreate(GetResource(id));
 }
 
 GLenum ResourceProvider::TargetForTesting(ResourceId id) {
   Resource* resource = GetResource(id);
   return resource->target;
 }
 
 void ResourceProvider::LazyCreate(Resource* resource) {
-  if (resource->type != GLTexture || resource->gl_id != 0)
+  if (resource->type != GLTexture || resource->origin != Resource::Internal) {
+    DCHECK(!resource->texture_pool);

[danakj, 2014/01/28 21:59:02]

I think these DCHECKs can be covered in the constructor as I suggest in another
comment.

+    return;
+  }
+
+  if (resource->gl_id)
     return;
 
-  // Early out for resources that don't require texture creation.
-  if (resource->texture_pool == 0)
-    return;
-
+  DCHECK(resource->texture_pool);

[dshwang, 2014/01/28 20:35:36]

how about the change of LazyCreate()?

   DCHECK(resource->origin == Resource::Internal);
   DCHECK(!resource->mailbox.IsValid());
   resource->gl_id = texture_id_allocator_->NextId();
 
   GLES2Interface* gl = ContextGL();
   DCHECK(gl);
 
   // Create and set texture properties. Allocation is delayed until needed.
   GLC(gl, gl->BindTexture(resource->target, resource->gl_id));
   GLC(gl,
@@ skipping 113 matching lines @@
   resource->dirty_image = false;
   resource->allocated = false;
 }
 
 uint8_t* ResourceProvider::MapImage(ResourceId id) {
   Resource* resource = GetResource(id);
   DCHECK(ReadLockFenceHasPassed(resource));
   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) {
   Resource* resource = GetResource(id);
   DCHECK(resource->origin == Resource::Internal);
   DCHECK_EQ(resource->exported_count, 0);
 
   if (resource->image_id) {
     GLES2Interface* gl = ContextGL();
     DCHECK(gl);
@@ skipping 34 matching lines @@
   gl->GetIntegerv(GL_ACTIVE_TEXTURE, &active_unit);
   return active_unit;
 }
 
 GLES2Interface* ResourceProvider::ContextGL() const {
   ContextProvider* context_provider = output_surface_->context_provider();
   return context_provider ? context_provider->ContextGL() : NULL;
 }
 
 }  // namespace cc