DevTools: expose last composited frame metadata on RenderWidgetHostViewPort.

DevTools: expose last composited frame metadata on RenderWidgetHostViewPort.

R=joi@chromium.org, sievers@chromium.org, skyostil@chromium.org, vsevik@chromium.org

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

[Sami, 2013-08-28 15:42:50]

https://codereview.chromium.org/23706005/diff/1/content/browser/devtools/rend...
File content/browser/devtools/renderer_overrides_handler.cc (right):

https://codereview.chromium.org/23706005/diff/1/content/browser/devtools/rend...
content/browser/devtools/renderer_overrides_handler.cc:309:
view_port->GetCompositorFrameMetadata()));
I think reading the screenshot and the frame metadata separately is going to be
too racy and lead to problems down the road. Let's take another shot at making
the compositor give back both pieces of data atomically. WDYT?

[pfeldman, 2013-08-29 13:57:59]

PTAL

[Sami, 2013-08-29 14:36:14]

RenderWidgetHostView parts lgtm. +sievers@ in case he has a better idea about
how to retrieve the compositor frame metadata, since this method is still a bit
racy for the individual screenshot case.

Is the front-end side prepared to potentially get zero frame metadata back for
the first frame if we hadn't rendered anything yet?

[pfeldman, 2013-08-29 14:46:32]

> Is the front-end side prepared to potentially get zero frame metadata back for
> the first frame if we hadn't rendered anything yet?

Fixed that. Am now conditionally sending viewport data in case of non-empty
CompositorFrameMetadata. Marked these values optional in the protocol upstream
so that clients expected it.

[vsevik, 2013-08-29 15:59:45]

lgtm

devtools lgtm

[pfeldman, 2013-08-29 16:07:18]

+joi for owners of content/port.

[Jói, 2013-08-29 16:20:05]

//content/port LGTM.

[no sievers, 2013-08-29 20:24:52]

lgtm for android

https://chromiumcodereview.appspot.com/23706005/diff/8001/content/browser/dev...
File content/browser/devtools/renderer_overrides_handler.cc (right):

https://chromiumcodereview.appspot.com/23706005/diff/8001/content/browser/dev...
content/browser/devtools/renderer_overrides_handler.cc:154: void
RendererOverridesHandler::OnSwapCompositorFrame(
Can you even save the metadata in here instead of putting it into RWHVBase?

[no sievers, 2013-08-29 20:27:06]

On 2013/08/29 20:24:52, sievers wrote:
> lgtm for android
> 
>
https://chromiumcodereview.appspot.com/23706005/diff/8001/content/browser/dev...
> File content/browser/devtools/renderer_overrides_handler.cc (right):
> 
>
https://chromiumcodereview.appspot.com/23706005/diff/8001/content/browser/dev...
> content/browser/devtools/renderer_overrides_handler.cc:154: void
> RendererOverridesHandler::OnSwapCompositorFrame(
> Can you even save the metadata in here instead of putting it into RWHVBase?

+piman for content/browser/renderer_host/

[pfeldman, 2013-08-29 20:32:42]

I guess I did not want to parse the IPC message twice and was thinking that it
might be useful to other clients. Let me do what you suggest.

[commit-bot: I haz the power, 2013-08-30 07:33:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pfeldman@chromium.org/23706005/50001

[pfeldman, 2013-08-30 08:55:00]

Committed patchset #5 manually as r220534 (presubmit successful).