Work around missing resources during Display Compositor rendering.

Under certain circumstances, we may try to render quads with missing
resources in GLRenderer. Currently, this leads to a crash.

This change causes us to handle this more gracefully (skip drawing
these quads) rather than crashing. This is a temporary fix until we
figure out why these resources are missing in the first place.

Bug: 811858
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: Ie01891f7816f7797178a63989b301ad276984671
Reviewed-on: https://chromium-review.googlesource.com/920145Reviewed-by: enne <enne@chromium.org>
Commit-Queue: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546039}

cc/resources/display_resource_provider.cc

@@ -80,6 +80,12 @@ void DisplayResourceProvider::SendPromotionHints(
       continue;
 
     const viz::internal::Resource* resource = LockForRead(id);
+    // TODO(ericrk): We should never fail LockForRead, but we appear to be
+    // doing so on Android in rare cases. Handle this gracefully until a better
+    // solution can be found. https://crbug.com/811858
+    if (!resource)
+      return;
+
     DCHECK(resource->wants_promotion_hint);
 
     // Insist that this is backed by a GPU texture.
@@ -121,8 +127,11 @@ size_t DisplayResourceProvider::CountPromotionHintRequestsForTesting() {
 
 #endif
 bool DisplayResourceProvider::IsOverlayCandidate(viz::ResourceId id) {
-  viz::internal::Resource* resource = GetResource(id);
-  return resource->is_overlay_candidate;
+  viz::internal::Resource* resource = TryGetResource(id);
+  // TODO(ericrk): We should never fail TryGetResource, but we appear to
+  // be doing so on Android in rare cases. Handle this gracefully until a
+  // better solution can be found. https://crbug.com/811858
+  return resource && resource->is_overlay_candidate;
 }
 
 viz::ResourceType DisplayResourceProvider::GetResourceType(viz::ResourceId id) {
@@ -135,7 +144,12 @@ gfx::BufferFormat DisplayResourceProvider::GetBufferFormat(viz::ResourceId id) {
 }
 
 void DisplayResourceProvider::WaitSyncToken(viz::ResourceId id) {
-  viz::internal::Resource* resource = GetResource(id);
+  viz::internal::Resource* resource = TryGetResource(id);
+  // TODO(ericrk): We should never fail TryGetResource, but we appear to
+  // be doing so on Android in rare cases. Handle this gracefully until a
+  // better solution can be found. https://crbug.com/811858
+  if (!resource)
+    return;
   WaitSyncTokenInternal(resource);
 #if defined(OS_ANDROID)
   // Now that the resource is synced, we may send it a promotion hint.  We could
@@ -453,7 +467,12 @@ GLenum DisplayResourceProvider::BindForSampling(viz::ResourceId resource_id,
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   GLES2Interface* gl = ContextGL();
   ResourceMap::iterator it = resources_.find(resource_id);
-  DCHECK(it != resources_.end());
+  // TODO(ericrk): We should never fail to find resource_id, but we appear to
+  // be doing so on Android in rare cases. Handle this gracefully until a
+  // better solution can be found. https://crbug.com/811858
+  if (it == resources_.end())
+    return GL_TEXTURE_2D;
+
   viz::internal::Resource* resource = &it->second;
   DCHECK(resource->lock_for_read_count);
   // TODO(xing.xu): remove locked_for_write.
@@ -544,6 +563,12 @@ DisplayResourceProvider::ScopedReadLockGL::ScopedReadLockGL(
     : resource_provider_(resource_provider), resource_id_(resource_id) {
   const viz::internal::Resource* resource =
       resource_provider->LockForRead(resource_id);
+  // TODO(ericrk): We should never fail LockForRead, but we appear to be
+  // doing so on Android in rare cases. Handle this gracefully until a better
+  // solution can be found. https://crbug.com/811858
+  if (!resource)
+    return;
+
   texture_id_ = resource->gl_id;
   target_ = resource->target;
   size_ = resource->size;
@@ -552,7 +577,13 @@ DisplayResourceProvider::ScopedReadLockGL::ScopedReadLockGL(
 
 const viz::internal::Resource* DisplayResourceProvider::LockForRead(
     viz::ResourceId id) {
-  viz::internal::Resource* resource = GetResource(id);
+  // TODO(ericrk): We should never fail TryGetResource, but we appear to be
+  // doing so on Android in rare cases. Handle this gracefully until a better
+  // solution can be found. https://crbug.com/811858
+  viz::internal::Resource* resource = TryGetResource(id);
+  if (!resource)
+    return nullptr;
+
   // TODO(xing.xu): remove locked_for_write.
   DCHECK(!resource->locked_for_write)
       << "locked for write: " << resource->locked_for_write;
@@ -600,7 +631,11 @@ const viz::internal::Resource* DisplayResourceProvider::LockForRead(
 void DisplayResourceProvider::UnlockForRead(viz::ResourceId id) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   ResourceMap::iterator it = resources_.find(id);
-  CHECK(it != resources_.end());
+  // TODO(ericrk): We should never fail to find id, but we appear to be
+  // doing so on Android in rare cases. Handle this gracefully until a better
+  // solution can be found. https://crbug.com/811858
+  if (it == resources_.end())
+    return;
 
   viz::internal::Resource* resource = &it->second;
   DCHECK_GT(resource->lock_for_read_count, 0);
@@ -660,6 +695,7 @@ DisplayResourceProvider::ScopedReadLockSkImage::ScopedReadLockSkImage(
     : resource_provider_(resource_provider), resource_id_(resource_id) {
   const viz::internal::Resource* resource =
       resource_provider->LockForRead(resource_id);
+  DCHECK(resource);
   if (resource_provider_->resource_sk_image_.find(resource_id) !=
       resource_provider_->resource_sk_image_.end()) {
     // Use cached sk_image.
@@ -704,6 +740,7 @@ DisplayResourceProvider::ScopedReadLockSoftware::ScopedReadLockSoftware(
     : resource_provider_(resource_provider), resource_id_(resource_id) {
   const viz::internal::Resource* resource =
       resource_provider->LockForRead(resource_id);
+  DCHECK(resource);
   resource_provider->PopulateSkBitmapWithResource(&sk_bitmap_, resource);
 }
 

cc/resources/display_resource_provider.h

@@ -83,8 +83,8 @@ class CC_EXPORT DisplayResourceProvider : public ResourceProvider {
     DisplayResourceProvider* const resource_provider_;
     const viz::ResourceId resource_id_;
 
-    GLuint texture_id_;
-    GLenum target_;
+    GLuint texture_id_ = 0;
+    GLenum target_ = GL_TEXTURE_2D;
     gfx::Size size_;
     gfx::ColorSpace color_space_;
 

cc/resources/resource_provider.cc

@@ -149,11 +149,19 @@ viz::internal::Resource* ResourceProvider::InsertResource(
 
 viz::internal::Resource* ResourceProvider::GetResource(viz::ResourceId id) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  // TODO(ericrk): Changing the DCHECKs in this function to CHECKs to debug
-  // https://crbug.com/811858.
-  CHECK(id);
+  DCHECK(id);
   ResourceMap::iterator it = resources_.find(id);
-  CHECK(it != resources_.end());
+  DCHECK(it != resources_.end());
+  return &it->second;
+}
+
+viz::internal::Resource* ResourceProvider::TryGetResource(viz::ResourceId id) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  if (!id)
+    return nullptr;
+  ResourceMap::iterator it = resources_.find(id);
+  if (it == resources_.end())
+    return nullptr;
   return &it->second;
 }
 

cc/resources/resource_provider.h

@@ -92,6 +92,11 @@ class CC_EXPORT ResourceProvider
                                           viz::internal::Resource resource);
   viz::internal::Resource* GetResource(viz::ResourceId id);
 
+  // TODO(ericrk): TryGetResource is part of a temporary workaround for cases
+  // where resources which should be available are missing. This version may
+  // return nullptr if a resource is not found. https://crbug.com/811858
+  viz::internal::Resource* TryGetResource(viz::ResourceId id);
+
   void PopulateSkBitmapWithResource(SkBitmap* sk_bitmap,
                                     const viz::internal::Resource* resource);