added gpu_benchmarking_extension boilerplate and hooked it up to a new switch.

added gpu_benchmarking_extension boilerplate and hooked it up to a new switch.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=141710

[nduca, 2012-06-08 00:36:03]

+jam for OWNERS review. @jam, this is based on our discussions in my smooth
scrolling patch to put the extension code directly inside content. This does the
setup, then we'll be adding a bunch of stuff over time.

https://chromiumcodereview.appspot.com/10532058/diff/1/content/browser/render...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://chromiumcodereview.appspot.com/10532058/diff/1/content/browser/render...
content/browser/renderer_host/render_process_host_impl.cc:680:
switches::kEnableGpuBenchmarking,
Looks like we use all-caps GPU here for this kind of enum. When you find
inconsistency, you should try to follow the convention established in adjacent
code.

https://chromiumcodereview.appspot.com/10532058/diff/1/content/renderer/gpu_b...
File content/renderer/gpu_benchmarking_extension.cc (right):

https://chromiumcodereview.appspot.com/10532058/diff/1/content/renderer/gpu_b...
content/renderer/gpu_benchmarking_extension.cc:16:
v8::Extension(kGpuBenchmarkingExtensionName, "") {}
lets put an empty object in here to establish the namespace. E.g. the benchmark
extension puts things into chrome.benchmarking, so we should probably establish
a chrome.gpuBenchmarking object.

[jam, 2012-06-08 00:52:04]

just nits

http://codereview.chromium.org/10532058/diff/1/content/renderer/gpu_benchmark...
File content/renderer/gpu_benchmarking_extension.h (right):

http://codereview.chromium.org/10532058/diff/1/content/renderer/gpu_benchmark...
content/renderer/gpu_benchmarking_extension.h:23: #endif /*
CONTENT_RENDERER_GPU_BENCHMARKING_EXTENSION_H_ */
please follow the style guide for this, i.e. 

#endif  // CONTENT_RENDERER_GPU_BENCHMARKING_EXTENSION_H_

http://codereview.chromium.org/10532058/diff/1/content/renderer/render_thread...
File content/renderer/render_thread_impl.cc (right):

http://codereview.chromium.org/10532058/diff/1/content/renderer/render_thread...
content/renderer/render_thread_impl.cc:56: #include
"content/renderer/gpu_benchmarking_extension.h"
should this code be under content/renderer/gpu/?

http://codereview.chromium.org/10532058/diff/1/content/renderer/render_thread...
content/renderer/render_thread_impl.cc:247:
RegisterExtension(content::GpuBenchmarkingExtension::Get());
nit: 2 spaces

[dmurph, 2012-06-08 18:36:21]

https://chromiumcodereview.appspot.com/10532058/diff/1/content/browser/render...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://chromiumcodereview.appspot.com/10532058/diff/1/content/browser/render...
content/browser/renderer_host/render_process_host_impl.cc:680:
switches::kEnableGpuBenchmarking,
On 2012/06/08 00:36:03, nduca wrote:
> Looks like we use all-caps GPU here for this kind of enum. When you find
> inconsistency, you should try to follow the convention established in adjacent
> code.

Hm, well all of the switches in content_switches use 'Gpu', those two are from
gl_switches, where there are two 'GPU's and four 'Gpu's.  Since there's a
universal majority of 'Gpu', I'll leave it as that, but let me know if I should
change it still.

https://chromiumcodereview.appspot.com/10532058/diff/1/content/renderer/gpu_b...
File content/renderer/gpu_benchmarking_extension.cc (right):

https://chromiumcodereview.appspot.com/10532058/diff/1/content/renderer/gpu_b...
content/renderer/gpu_benchmarking_extension.cc:16:
v8::Extension(kGpuBenchmarkingExtensionName, "") {}
On 2012/06/08 00:36:03, nduca wrote:
> lets put an empty object in here to establish the namespace. E.g. the
benchmark
> extension puts things into chrome.benchmarking, so we should probably
establish
> a chrome.gpuBenchmarking object.

Done.

https://chromiumcodereview.appspot.com/10532058/diff/1/content/renderer/gpu_b...
File content/renderer/gpu_benchmarking_extension.h (right):

https://chromiumcodereview.appspot.com/10532058/diff/1/content/renderer/gpu_b...
content/renderer/gpu_benchmarking_extension.h:23: #endif /*
CONTENT_RENDERER_GPU_BENCHMARKING_EXTENSION_H_ */
On 2012/06/08 00:52:04, John Abd-El-Malek wrote:
> please follow the style guide for this, i.e. 
> 
> #endif  // CONTENT_RENDERER_GPU_BENCHMARKING_EXTENSION_H_

Done.

https://chromiumcodereview.appspot.com/10532058/diff/1/content/renderer/rende...
File content/renderer/render_thread_impl.cc (right):

https://chromiumcodereview.appspot.com/10532058/diff/1/content/renderer/rende...
content/renderer/render_thread_impl.cc:56: #include
"content/renderer/gpu_benchmarking_extension.h"
On 2012/06/08 00:52:04, John Abd-El-Malek wrote:
> should this code be under content/renderer/gpu/?

following the alphabetical order of 'renderer' (which is the only order that
makes sense here), 'gpu' comes before 'gpu_'.

https://chromiumcodereview.appspot.com/10532058/diff/1/content/renderer/rende...
content/renderer/render_thread_impl.cc:247:
RegisterExtension(content::GpuBenchmarkingExtension::Get());
On 2012/06/08 00:52:04, John Abd-El-Malek wrote:
> nit: 2 spaces

Done.

[jam, 2012-06-08 18:39:13]

https://chromiumcodereview.appspot.com/10532058/diff/1/content/renderer/rende...
File content/renderer/render_thread_impl.cc (right):

https://chromiumcodereview.appspot.com/10532058/diff/1/content/renderer/rende...
content/renderer/render_thread_impl.cc:56: #include
"content/renderer/gpu_benchmarking_extension.h"
On 2012/06/08 18:36:22, dmurph wrote:
> On 2012/06/08 00:52:04, John Abd-El-Malek wrote:
> > should this code be under content/renderer/gpu/?
> 
> following the alphabetical order of 'renderer' (which is the only order that
> makes sense here), 'gpu' comes before 'gpu_'.  

I meant should the file itself be located in content/renderer/gpu/

[dmurph, 2012-06-08 19:30:13]

https://chromiumcodereview.appspot.com/10532058/diff/1/content/renderer/rende...
File content/renderer/render_thread_impl.cc (right):

https://chromiumcodereview.appspot.com/10532058/diff/1/content/renderer/rende...
content/renderer/render_thread_impl.cc:56: #include
"content/renderer/gpu_benchmarking_extension.h"
On 2012/06/08 18:39:13, John Abd-El-Malek wrote:
> On 2012/06/08 18:36:22, dmurph wrote:
> > On 2012/06/08 00:52:04, John Abd-El-Malek wrote:
> > > should this code be under content/renderer/gpu/?
> > 
> > following the alphabetical order of 'renderer' (which is the only order that
> > makes sense here), 'gpu' comes before 'gpu_'.  
> 
> I meant should the file itself be located in content/renderer/gpu/

Ah.  Yes, it should.  Moved.

[jam, 2012-06-08 20:36:01]

lgtm

[nduca, 2012-06-12 17:21:01]

LGTM

[dmurph, 2012-06-12 18:24:59]

fixed merge issues.  Can someone push the commit queue button?

[commit-bot: I haz the power, 2012-06-12 18:27:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dmurph@chromium.org/10532058/2005

[commit-bot: I haz the power, 2012-06-12 19:40:15]

Change committed as 141710