DevTools: Fix for Page.captureScreenshot

DevTools: Fix for Page.captureScreenshot

from https://chromiumcodereview.appspot.com/190693002/

This patch makes Page.captureScreenshot synchronize with the renderer.
When fixed implementation hits Stable it will be possible to use Page.captureScreenshot in Telemetry instead of window.chrome.gpuBenchmarking.beginWindowSnapshotPNG and remove the latter.

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

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

[Vladislav Kaznacheev, 2014-06-16 08:25:27]

Please expand the patch description so that the choices you have made are
obvious.

https://codereview.chromium.org/311313003/diff/20001/content/browser/devtools...
File content/browser/devtools/renderer_overrides_handler_browsertest.cc (right):

https://codereview.chromium.org/311313003/diff/20001/content/browser/devtools...
content/browser/devtools/renderer_overrides_handler_browsertest.cc:113:
EXPECT_TRUE(content::ExecuteScript(
Does this test fail with the old implementation of Page.CaptureScreenshot? In
the original patch the test did 50 iterations and it made sense since the whole
thing is a bit non-deterministic.

https://codereview.chromium.org/311313003/diff/20001/ui/events/latency_info.h
File ui/events/latency_info.h (right):

https://codereview.chromium.org/311313003/diff/20001/ui/events/latency_info.h...
ui/events/latency_info.h:48: // gpuBenchmarking.beginWindowSnapshotPNG
Please add a TODO to remove this when this patch hits Stable

[vkuzkokov, 2014-06-16 10:01:24]

https://codereview.chromium.org/311313003/diff/20001/content/browser/devtools...
File content/browser/devtools/renderer_overrides_handler_browsertest.cc (right):

https://codereview.chromium.org/311313003/diff/20001/content/browser/devtools...
content/browser/devtools/renderer_overrides_handler_browsertest.cc:113:
EXPECT_TRUE(content::ExecuteScript(
On 2014/06/16 08:25:27, Vladislav Kaznacheev wrote:
> Does this test fail with the old implementation of Page.CaptureScreenshot? In
> the original patch the test did 50 iterations and it made sense since the
whole
> thing is a bit non-deterministic.

The same test with old implementation fails for me consistently.
50 iterations take 31s (9s with --enable-gpu) which I think is too much for just
one test.

https://codereview.chromium.org/311313003/diff/20001/ui/events/latency_info.h
File ui/events/latency_info.h (right):

https://codereview.chromium.org/311313003/diff/20001/ui/events/latency_info.h...
ui/events/latency_info.h:48: // gpuBenchmarking.beginWindowSnapshotPNG
On 2014/06/16 08:25:27, Vladislav Kaznacheev wrote:
> Please add a TODO to remove this when this patch hits Stable

Done.

[Vladislav Kaznacheev, 2014-06-16 12:47:33]

lgtm

[vkuzkokov, 2014-06-16 12:49:19]

The CQ bit was checked by vkuzkokov@chromium.org

[commit-bot: I haz the power, 2014-06-16 12:49:49]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vkuzkokov@chromium.org/311313003/40001

[vkuzkokov, 2014-06-16 13:14:44]

The CQ bit was unchecked by vkuzkokov@chromium.org

[vkuzkokov, 2014-06-17 17:04:14]

PTAL

ccameron:
content/browser/renderer_host/render_widget_host_view_mac.mm

jbauman:
ui/events/latency_info.h
ui/events/latency_info.cc

[ccameron, 2014-06-17 17:09:51]

lgtm

[jbauman, 2014-06-17 23:47:21]

ui/events/latency_info.* lgtm

[vkuzkokov, 2014-06-18 09:20:39]

jam, PTAL:

content/browser/renderer_host/render_widget_host_impl.h
content/browser/renderer_host/render_widget_host_impl.cc
content/common/view_messages.h
content/public/browser/render_widget_host.h
content/renderer/render_view_impl.h
content/renderer/render_view_impl.cc

[jam, 2014-06-18 16:46:35]

lgtm with comment

https://codereview.chromium.org/311313003/diff/120001/content/public/browser/...
File content/public/browser/render_widget_host.h (right):

https://codereview.chromium.org/311313003/diff/120001/content/public/browser/...
content/public/browser/render_widget_host.h:298: const base::Callback<void(const
unsigned char*, size_t)> callback) = 0;
this is only called by code inside content, so put it on RenderWidgetHostImpl
instead of the public api (see
http://www.chromium.org/developers/content-module/content-api)

[vkuzkokov, 2014-06-19 08:00:20]

https://codereview.chromium.org/311313003/diff/120001/content/public/browser/...
File content/public/browser/render_widget_host.h (right):

https://codereview.chromium.org/311313003/diff/120001/content/public/browser/...
content/public/browser/render_widget_host.h:298: const base::Callback<void(const
unsigned char*, size_t)> callback) = 0;
On 2014/06/18 16:46:34, jam wrote:
> this is only called by code inside content, so put it on RenderWidgetHostImpl
> instead of the public api (see
> http://www.chromium.org/developers/content-module/content-api)

Done.

[vkuzkokov, 2014-06-19 08:01:46]

tsepez, PTAL:

content/common/view_messages.h

[Tom Sepez, 2014-06-19 14:05:39]

Messages LGTM.

[vkuzkokov, 2014-06-19 14:07:54]

The CQ bit was checked by vkuzkokov@chromium.org

[commit-bot: I haz the power, 2014-06-19 14:08:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vkuzkokov@chromium.org/311313003/160001

[commit-bot: I haz the power, 2014-06-19 20:27:06]

Change committed as 278472

[vkuzkokov, 2014-06-20 08:54:51]

The CQ bit was checked by vkuzkokov@chromium.org

[commit-bot: I haz the power, 2014-06-20 08:56:31]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vkuzkokov@chromium.org/311313003/180001

[commit-bot: I haz the power, 2014-06-20 12:43:17]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/19488)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/23000)

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

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

[commit-bot: I haz the power, 2014-06-20 12:53:08]

Try jobs failed on following builders:
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)

[vkuzkokov, 2014-06-23 08:19:41]

The CQ bit was checked by vkuzkokov@chromium.org

[commit-bot: I haz the power, 2014-06-23 08:21:33]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vkuzkokov@chromium.org/311313003/200001

[commit-bot: I haz the power, 2014-06-23 09:23:16]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/20270)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/24099)

[commit-bot: I haz the power, 2014-06-23 09:29:39]

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

[commit-bot: I haz the power, 2014-06-23 09:29:40]

Try jobs failed on following builders:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/19...)

[vkuzkokov, 2014-06-23 10:05:21]

The CQ bit was checked by vkuzkokov@chromium.org

[commit-bot: I haz the power, 2014-06-23 10:06:38]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vkuzkokov@chromium.org/311313003/240001

[vkuzkokov, 2014-06-23 10:12:40]

The CQ bit was unchecked by vkuzkokov@chromium.org

[vkuzkokov, 2014-06-23 10:15:06]

The CQ bit was checked by vkuzkokov@chromium.org

[commit-bot: I haz the power, 2014-06-23 10:15:37]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/vkuzkokov@chromium.org/311313003/260001

[commit-bot: I haz the power, 2014-06-23 12:07:22]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)

[commit-bot: I haz the power, 2014-06-23 12:22:47]

Change committed as 279094