Use GLHelper support to validate format before processing texture.

content/browser/compositor/delegated_frame_host.cc

 // Copyright 2014 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 "content/browser/compositor/delegated_frame_host.h"
 
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "cc/output/compositor_frame.h"
 #include "cc/output/compositor_frame_ack.h"
@@ skipping 599 matching lines @@
   callback.Run(*bitmap,
                result ? content::READBACK_SUCCESS : content::READBACK_FAILED);
 }
 
 // static
 void DelegatedFrameHost::PrepareTextureCopyOutputResult(
     const gfx::Size& dst_size_in_pixel,
     const SkColorType color_type,
     ReadbackRequestCallback& callback,
     scoped_ptr<cc::CopyOutputResult> result) {
   DCHECK(result->HasTexture());

[danakj, 2015/01/08 05:56:31]

Nit: blank line between dcheck and code

[sivag, 2015/01/08 11:03:31]

Done.

+  ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
+  GLHelper* gl_helper = factory->GetGLHelper();
+  if (!gl_helper) {
+    callback.Run(SkBitmap(), content::READBACK_FAILED);
+    return;
+  }
+  if (!gl_helper->IsReadbackConfigSupported(color_type)) {
+    callback.Run(SkBitmap(), content::READBACK_FORMAT_NOT_SUPPORTED);
+    return;
+  }
+
   base::ScopedClosureRunner scoped_callback_runner(

[danakj, 2015/01/08 05:56:31]

Maybe move this to the top of the function, and just reset the scoped runner
when calling with format error?

[sivag, 2015/01/08 11:03:31]

Done.

       base::Bind(callback, SkBitmap(), content::READBACK_FAILED));
 
-  // TODO(sikugu): We should be able to validate the format here using
-  // GLHelper::IsReadbackConfigSupported before we processs the result.
-  // See crbug.com/415682.
   scoped_ptr<SkBitmap> bitmap(new SkBitmap);
   if (!bitmap->tryAllocPixels(SkImageInfo::Make(dst_size_in_pixel.width(),
                                                 dst_size_in_pixel.height(),
                                                 color_type,
                                                 kOpaque_SkAlphaType)))
     return;
 
-  ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
-  GLHelper* gl_helper = factory->GetGLHelper();
-  if (!gl_helper)
-    return;
-
   scoped_ptr<SkAutoLockPixels> bitmap_pixels_lock(
       new SkAutoLockPixels(*bitmap));
   uint8* pixels = static_cast<uint8*>(bitmap->getPixels());
 
   cc::TextureMailbox texture_mailbox;
   scoped_ptr<cc::SingleReleaseCallback> release_callback;
   result->TakeTexture(&texture_mailbox, &release_callback);
   DCHECK(texture_mailbox.IsTexture());
 
   ignore_result(scoped_callback_runner.Release());
@@ skipping 384 matching lines @@
     cc::SurfaceManager* manager = factory->GetSurfaceManager();
     new_layer->SetShowSurface(
         surface_id_, base::Bind(&SatisfyCallback, base::Unretained(manager)),
         base::Bind(&RequireCallback, base::Unretained(manager)),
         current_surface_size_, current_scale_factor_,
         current_frame_size_in_dip_);
   }
 }
 
 }  // namespace content