Query the preferred readback config in CopyFromBackingStore

Query the preferred readback config in CopyFromBackingStore

As of now we are using the default format for both android/aura to read the snapshot, but as we have different format readbacks supported we should be able to decide the format before calling copyFromCompsitingSurface
based on some criteria.

BUG=323150
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=256812

[sivag, 2014-03-06 13:02:31]

PTAL..

[piman, 2014-03-06 22:01:52]

https://codereview.chromium.org/188633002/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

https://codereview.chromium.org/188633002/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_widget_host_impl.cc:735: preffered_format =
gl_helper->PreferredReadbackFormat();
This code is out-of-place.
1- Obviously we can't access Android things on non-Android.
2- Some platforms (e.g. mac) don't have a GLHelper, and a GLHelper is not
involved in the readback

Maybe what you want is ask view_ what is the preferred format, and these can
delegate to their GLHelper if appropriate?

[sivag, 2014-03-10 15:48:17]

PTAL..

https://codereview.chromium.org/188633002/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

https://codereview.chromium.org/188633002/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_widget_host_impl.cc:735: preffered_format =
gl_helper->PreferredReadbackFormat();
On 2014/03/06 22:01:52, piman wrote:
> This code is out-of-place.
> 1- Obviously we can't access Android things on non-Android.
> 2- Some platforms (e.g. mac) don't have a GLHelper, and a GLHelper is not
> involved in the readback
> 
> Maybe what you want is ask view_ what is the preferred format, and these can
> delegate to their GLHelper if appropriate?

Done.

[piman, 2014-03-10 18:27:20]

In many of the call sites, the code is only capable of receiving 8888 bitmaps.
In those cases, we can't blindly use the preferred format, we need to force it
to 8888 like it was before.

https://codereview.chromium.org/188633002/diff/20001/chrome/browser/extension...
File chrome/browser/extensions/api/capture_web_contents_function.cc (right):

https://codereview.chromium.org/188633002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/api/capture_web_contents_function.cc:82:
preferred_format);
The receiving code only accepts 8888.

https://codereview.chromium.org/188633002/diff/20001/chrome/browser/thumbnail...
File chrome/browser/thumbnails/thumbnail_tab_helper.cc (right):

https://codereview.chromium.org/188633002/diff/20001/chrome/browser/thumbnail...
chrome/browser/thumbnails/thumbnail_tab_helper.cc:127: preferred_format);
I believe the receiving path expects 8888 bitmaps.

https://codereview.chromium.org/188633002/diff/20001/content/browser/frame_ho...
File content/browser/frame_host/navigation_entry_screenshot_manager.cc (right):

https://codereview.chromium.org/188633002/diff/20001/content/browser/frame_ho...
content/browser/frame_host/navigation_entry_screenshot_manager.cc:137:
SkBitmap::Config preffered_format = host->PreferredReadbackFormat();
typo: preferred_format.

https://codereview.chromium.org/188633002/diff/20001/content/browser/media/ca...
File content/browser/media/capture/web_contents_video_capture_device.cc (right):

https://codereview.chromium.org/188633002/diff/20001/content/browser/media/ca...
content/browser/media/capture/web_contents_video_capture_device.cc:669:
SkBitmap::Config preffered_format = rwh->PreferredReadbackFormat();
typo: preferred_format

https://codereview.chromium.org/188633002/diff/20001/content/browser/media/ca...
content/browser/media/capture/web_contents_video_capture_device.cc:678:
preffered_format);
The receiving code only accepts RGBA8888, see RenderVideoFrame. So we can't
blindly use the preferred format, we should force it to 8888.

https://codereview.chromium.org/188633002/diff/20001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

https://codereview.chromium.org/188633002/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_impl.cc:36: #include
"content/browser/renderer_host/image_transport_factory_android.h"
nit: remove

https://codereview.chromium.org/188633002/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_impl.cc:49: #include
"content/common/gpu/client/gl_helper.h"
nit: remove

https://codereview.chromium.org/188633002/diff/20001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_view_android.cc (right):

https://codereview.chromium.org/188633002/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_android.cc:1391: // Define
the criteria here. If say the 16 texture readbback is
typo: readbback->readback

[sivag, 2014-03-11 14:41:49]

PTAL..

https://codereview.chromium.org/188633002/diff/20001/chrome/browser/extension...
File chrome/browser/extensions/api/capture_web_contents_function.cc (right):

https://codereview.chromium.org/188633002/diff/20001/chrome/browser/extension...
chrome/browser/extensions/api/capture_web_contents_function.cc:82:
preferred_format);
On 2014/03/10 18:27:20, piman wrote:
> The receiving code only accepts 8888.

Done.

https://codereview.chromium.org/188633002/diff/20001/chrome/browser/thumbnail...
File chrome/browser/thumbnails/thumbnail_tab_helper.cc (right):

https://codereview.chromium.org/188633002/diff/20001/chrome/browser/thumbnail...
chrome/browser/thumbnails/thumbnail_tab_helper.cc:127: preferred_format);
On 2014/03/10 18:27:20, piman wrote:
> I believe the receiving path expects 8888 bitmaps.

Done.

https://codereview.chromium.org/188633002/diff/20001/content/browser/frame_ho...
File content/browser/frame_host/navigation_entry_screenshot_manager.cc (right):

https://codereview.chromium.org/188633002/diff/20001/content/browser/frame_ho...
content/browser/frame_host/navigation_entry_screenshot_manager.cc:137:
SkBitmap::Config preffered_format = host->PreferredReadbackFormat();
On 2014/03/10 18:27:20, piman wrote:
> typo: preferred_format.

Done.

https://codereview.chromium.org/188633002/diff/20001/content/browser/media/ca...
File content/browser/media/capture/web_contents_video_capture_device.cc (right):

https://codereview.chromium.org/188633002/diff/20001/content/browser/media/ca...
content/browser/media/capture/web_contents_video_capture_device.cc:669:
SkBitmap::Config preffered_format = rwh->PreferredReadbackFormat();
On 2014/03/10 18:27:20, piman wrote:
> typo: preferred_format

Done.

https://codereview.chromium.org/188633002/diff/20001/content/browser/media/ca...
content/browser/media/capture/web_contents_video_capture_device.cc:678:
preffered_format);
On 2014/03/10 18:27:20, piman wrote:
> The receiving code only accepts RGBA8888, see RenderVideoFrame. So we can't
> blindly use the preferred format, we should force it to 8888.

Done.

https://codereview.chromium.org/188633002/diff/20001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_impl.cc (right):

https://codereview.chromium.org/188633002/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_impl.cc:36: #include
"content/browser/renderer_host/image_transport_factory_android.h"
On 2014/03/10 18:27:20, piman wrote:
> nit: remove

Done.

https://codereview.chromium.org/188633002/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_impl.cc:49: #include
"content/common/gpu/client/gl_helper.h"
On 2014/03/10 18:27:20, piman wrote:
> nit: remove

Done.

https://codereview.chromium.org/188633002/diff/20001/content/browser/renderer...
File content/browser/renderer_host/render_widget_host_view_android.cc (right):

https://codereview.chromium.org/188633002/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_android.cc:1391: // Define
the criteria here. If say the 16 texture readbback is
On 2014/03/10 18:27:20, piman wrote:
> typo: readbback->readback

Done.

https://codereview.chromium.org/188633002/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_widget_host_view_android.cc:1391: // Define
the criteria here. If say the 16 texture readbback is
On 2014/03/10 18:27:20, piman wrote:
> typo: readbback->readback

Done.

[piman, 2014-03-11 21:59:26]

lgtm

[sivag, 2014-03-12 05:40:54]

The CQ bit was checked by siva.gunturi@samsung.com

[commit-bot: I haz the power, 2014-03-12 05:42:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/siva.gunturi@samsung.com/188633002/40001

[sivag, 2014-03-12 06:00:52]

PTAL..

+ Mek, Mike

chrome/browser/extensions/api/capture_web_contents_function.cc

+ sky, thakis,

chrome/browser/thumbnails/thumbnail_tab_helper.cc

[commit-bot: I haz the power, 2014-03-12 06:07:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-12 06:07:54]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[sky, 2014-03-12 16:22:32]

chrome/browser/thumbnails/thumbnail_tab_helper.cc LGTM

[Marijn Kruisselbrink, 2014-03-12 16:40:33]

chrome/browser/extensions/api/capture_web_contents_function.cc LGTM

[miket_OOO, 2014-03-12 16:56:25]

c/b/e/ LGTM

[sivag, 2014-03-12 18:12:45]

The CQ bit was checked by siva.gunturi@samsung.com

[commit-bot: I haz the power, 2014-03-12 18:13:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/siva.gunturi@samsung.com/188633002/40001

[commit-bot: I haz the power, 2014-03-12 19:03:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-12 19:03:26]

Retried try job too often on android_dbg for step(s) slave_steps
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[sivag, 2014-03-13 08:23:50]

The CQ bit was checked by siva.gunturi@samsung.com

[commit-bot: I haz the power, 2014-03-13 08:23:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/siva.gunturi@samsung.com/188633002/60001

[commit-bot: I haz the power, 2014-03-13 08:36:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-13 08:36:55]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_clang_dbg

[sivag, 2014-03-13 10:17:52]

The CQ bit was checked by siva.gunturi@samsung.com

[commit-bot: I haz the power, 2014-03-13 10:18:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/siva.gunturi@samsung.com/188633002/80001

[commit-bot: I haz the power, 2014-03-13 13:19:38]

Change committed as 256812