Convert the SkiaBenchmarkingExtension to a gin::Wrappable class

Convert the SkiaBenchmarkingExtension to a gin::Wrappable class

BUG=334679
R=fmalita@chromium.org,nduca@chromium.org

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

[jochen (gone - plz use gerrit), 2014-01-20 15:01:14]

i couldn't find any test that uses this, so I couldn't verify that it still
works.

Is this still used or should we delete the extension instead?

[jochen (gone - plz use gerrit), 2014-01-20 19:47:22]

I ran the steps from
http://dev.chromium.org/developers/how-tos/trace-event-profiling-tool/frame-v...
and as far as I can tell, it still works.

There's a lot of complaining about not being able to rasterize images, but I get
the same errors when compiling without my patch.

[f(malita), 2014-01-21 14:43:40]

Change looks OK to me, but I'll defer to owners.

(I'm also hoping the new wrapper format makes mocking/unit testing easier - this
extension could certainly use some).

https://codereview.chromium.org/142143005/diff/1/content/renderer/skia_benchm...
File content/renderer/skia_benchmarking_extension.cc (left):

https://codereview.chromium.org/142143005/diff/1/content/renderer/skia_benchm...
content/renderer/skia_benchmarking_extension.cc:68: "  /* "
Can we preserve the API docs in some form? Even if not exposed to JS, they may
come in handy for someone trying to figure out how to use the extension.

https://codereview.chromium.org/142143005/diff/1/content/renderer/skia_benchm...
File content/renderer/skia_benchmarking_extension.cc (right):

https://codereview.chromium.org/142143005/diff/1/content/renderer/skia_benchm...
content/renderer/skia_benchmarking_extension.cc:111: .SetMethod("getOpTiminsg",
&SkiaBenchmarking::GetOpTimings)
'getOpTimings'

[jochen (gone - plz use gerrit), 2014-01-21 14:56:32]

done

[piman, 2014-01-21 20:59:31]

https://codereview.chromium.org/142143005/diff/140001/content/renderer/render...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/142143005/diff/140001/content/renderer/render...
content/renderer/render_view_impl.cc:3651:
StatsCollectionController::Install(frame);
I'm not sure what this does. Did you mean to include it the if (world_id == 0)?
It sounds like a change that's unrelated to the skia benchmarking.

https://codereview.chromium.org/142143005/diff/140001/content/renderer/skia_b...
File content/renderer/skia_benchmarking_extension.cc (right):

https://codereview.chromium.org/142143005/diff/140001/content/renderer/skia_b...
content/renderer/skia_benchmarking_extension.cc:213: if
(args->PeekNext().IsEmpty())
nit: the previous code used to fail if the arg count was not exactly 1. New code
only fails if arg count is less than one. Does it matter/do we care?

https://codereview.chromium.org/142143005/diff/140001/content/renderer/skia_b...
content/renderer/skia_benchmarking_extension.cc:256: if
(args->PeekNext().IsEmpty())
same nit here.

https://codereview.chromium.org/142143005/diff/140001/content/renderer/skia_b...
content/renderer/skia_benchmarking_extension.cc:296: if
(args->PeekNext().IsEmpty())
and here

[nduca, 2014-01-22 08:29:47]

sorry there's no unit test for this jochen. 
www.chromium.org/developers/how-tos/trace-event-profiling-tool/frame-viewer uses
it, if that works using the steps before/after, then your'e good. Though if you
break it, its mainly our fault since we don't have tests... :)

[jochen (gone - plz use gerrit), 2014-01-22 12:17:05]

https://codereview.chromium.org/142143005/diff/140001/content/renderer/render...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/142143005/diff/140001/content/renderer/render...
content/renderer/render_view_impl.cc:3651:
StatsCollectionController::Install(frame);
On 2014/01/21 20:59:31, piman wrote:
> I'm not sure what this does. Did you mean to include it the if (world_id ==
0)?
> It sounds like a change that's unrelated to the skia benchmarking.

yes, I forgot about moving the stats controller under the if in my previous
change here.

https://codereview.chromium.org/142143005/diff/140001/content/renderer/skia_b...
File content/renderer/skia_benchmarking_extension.cc (right):

https://codereview.chromium.org/142143005/diff/140001/content/renderer/skia_b...
content/renderer/skia_benchmarking_extension.cc:213: if
(args->PeekNext().IsEmpty())
On 2014/01/21 20:59:31, piman wrote:
> nit: the previous code used to fail if the arg count was not exactly 1. New
code
> only fails if arg count is less than one. Does it matter/do we care?

I don't think it matters.

[piman, 2014-01-22 19:08:19]

lgtm

[commit-bot: I haz the power, 2014-01-22 21:59:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jochen@chromium.org/142143005/270001

[commit-bot: I haz the power, 2014-01-23 00:06:54]

Change committed as 246453