Plumb cc::CCRenderingStats from gpu_benchmarking to cc

Plumb cc::CCRenderingStats from gpu_benchmarking to cc

This is a workaround to avoid having to edit WebKit code every time we want to add a rendering stats field.  Eventually, the rendering stats will not go through WebKit, at which point this workaround should be removed.

BUG=151456

[jamesr, 2012-10-19 05:09:18]

https://codereview.chromium.org/11189081/diff/1/webkit/glue/compositor_render...
File webkit/glue/compositor_rendering_stats.h (right):

https://codereview.chromium.org/11189081/diff/1/webkit/glue/compositor_render...
webkit/glue/compositor_rendering_stats.h:21: // simplifies the addition of new
fields to this type.
you really do not need this.  Just have this be a struct with fields

[brianderson, 2012-10-19 17:11:20]

https://codereview.chromium.org/11189081/diff/1/webkit/glue/compositor_render...
File webkit/glue/compositor_rendering_stats.h (right):

https://codereview.chromium.org/11189081/diff/1/webkit/glue/compositor_render...
webkit/glue/compositor_rendering_stats.h:21: // simplifies the addition of new
fields to this type.
On 2012/10/19 05:09:18, jamesr wrote:
> you really do not need this.  Just have this be a struct with fields

Do we want to get rid of all the enumerators?
https://chromiumcodereview.appspot.com/11027009 enabled them.

Using the Enumerator is a little more verbose and round about, but it also
reduces the number of files we need to edit to add a field. I could go either
way on this.

[jamesr, 2012-10-19 17:15:37]

On 2012/10/19 17:11:20, Brian Anderson wrote:
>
https://codereview.chromium.org/11189081/diff/1/webkit/glue/compositor_render...
> File webkit/glue/compositor_rendering_stats.h (right):
> 
>
https://codereview.chromium.org/11189081/diff/1/webkit/glue/compositor_render...
> webkit/glue/compositor_rendering_stats.h:21: // simplifies the addition of new
> fields to this type.
> On 2012/10/19 05:09:18, jamesr wrote:
> > you really do not need this.  Just have this be a struct with fields
> 
> Do we want to get rid of all the enumerators?
> https://chromiumcodereview.appspot.com/11027009 enabled them.
> 

Yes I do.  The point of them was to avoid having to patch two repositories
(WebKit and chromium) to add a field.   That's not a problem here, we're only
patching chromium to add a field.  Without that consideration, the Enumerator is
just overhead.  Adding a field should just be:

1.) Add the field to the struct
2.) Update cc/ code to update field
3.) Update gpu_benchmarking_extension to map the struct field to JS name

(3) is important to have as an explicit step since the fields we expose to the
GPU benchmarking extension need to be somewhat stable - code that runs on bots,
etc, are going to be relying on the values.

base/values.h is the way to do generic bag-of-values in chromium, but I don't
think we really need that here.

[brianderson, 2012-10-19 18:32:35]

This includes https://codereview.chromium.org/11229007, so we can get that in
first and I'll remove it from this patch, or we can get this one in all at once.

[jamesr, 2012-10-22 21:39:36]

lgtm

https://codereview.chromium.org/11189081/diff/10001/content/renderer/render_w...
File content/renderer/render_widget.h (right):

https://codereview.chromium.org/11189081/diff/10001/content/renderer/render_w...
content/renderer/render_widget.h:582: double total_paint_time_in_seconds_;
note: this would be better as a base::TimeDelta to avoid unnecessary int->double
conversions and to make it less likely to have unit mixups. you can fix it as a
follow up if you prefer

[commit-bot: I haz the power, 2012-10-22 21:50:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/brianderson@chromium.org/11189081/10001

[commit-bot: I haz the power, 2012-10-22 22:26:39]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-10-22 22:28:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/brianderson@chromium.org/11189081/7011

[commit-bot: I haz the power, 2012-10-22 23:17:58]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[jamesr, 2012-10-23 02:00:17]

Hmm, might have to embed WebKit::WebRenderingStats instead of subclassing it
here to make the windows component build happy.

[brianderson, 2012-10-23 19:27:41]

On 2012/10/23 02:00:17, jamesr wrote:
> Hmm, might have to embed WebKit::WebRenderingStats instead of subclassing it
> here to make the windows component build happy.

I'm not sure if that will work, as we'll strip away the ability to downcast to
cc::RendererStats.

Taking a completely different plan of attack, how about we:
1) Add CompositorRenderingStats as a class with all the stats we care about.
2) Change WebKit to use CompositorRenderingStats instead of
WebKit::WebRenderingStats.
3) Change chromium to use CompositorRenderingStats instead of
cc::RenderingStats.

It'll take more steps and will still result in us only having to edit chromium
code to add a statistic, but will not require any downcasting.

[jamesr, 2012-10-23 19:31:30]

On 2012/10/23 19:27:41, Brian Anderson wrote:
> On 2012/10/23 02:00:17, jamesr wrote:
> > Hmm, might have to embed WebKit::WebRenderingStats instead of subclassing it
> > here to make the windows component build happy.
> 
> I'm not sure if that will work, as we'll strip away the ability to downcast to
> cc::RendererStats.

Ah, good point.

> 
> Taking a completely different plan of attack, how about we:
> 1) Add CompositorRenderingStats as a class with all the stats we care about.
> 2) Change WebKit to use CompositorRenderingStats instead of
> WebKit::WebRenderingStats.
> 3) Change chromium to use CompositorRenderingStats instead of
> cc::RenderingStats.
> 
> It'll take more steps and will still result in us only having to edit chromium
> code to add a statistic, but will not require any downcasting.

I think time would be better spent piping the call to not go through WebKit at
all.

[brianderson, 2012-10-23 19:55:59]

> I think time would be better spent piping the call to not go through WebKit at
all.

Ok. I don't think I'm going to be able to do this the right way before I leave
tomorrow, so I'm going to focus on other items and revisit this when I return at
the end of November.