plumbing for skia trace events into Chrome's about://tracing framework

plumbing for skia trace events into Chrome's about://tracing framework

BUG=

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

[humper, 2014-02-03 19:58:58]

PTAL; I've tested this with some local hacks to third_party/skia and it appears
to work.

[humper, 2014-02-03 20:05:31]

+reed, +bsalomon for owners in skia/ext

[bsalomon, 2014-02-03 21:44:29]

lgtm

Aside: Maybe is RendererMain a place where we could stick a call to
SkGraphics:Init()?

[piman, 2014-02-03 22:25:02]

https://codereview.chromium.org/153063002/diff/1/content/renderer/renderer_ma...
File content/renderer/renderer_main.cc (right):

https://codereview.chromium.org/153063002/diff/1/content/renderer/renderer_ma...
content/renderer/renderer_main.cc:244: SkEventTracer::SetInstance(new
skia::SkChromiumEventTracer());
nit: should we do this in the RenderThreadImpl initialization? That'd be more
consistent (and allow it to work in --single-process etc.)

[nduca, 2014-02-04 01:15:19]

https://codereview.chromium.org/153063002/diff/1/content/renderer/renderer_ma...
File content/renderer/renderer_main.cc (right):

https://codereview.chromium.org/153063002/diff/1/content/renderer/renderer_ma...
content/renderer/renderer_main.cc:244: SkEventTracer::SetInstance(new
skia::SkChromiumEventTracer());
yea definitely reder thread impl

https://codereview.chromium.org/153063002/diff/1/skia/ext/trace_event.h
File skia/ext/trace_event.h (right):

https://codereview.chromium.org/153063002/diff/1/skia/ext/trace_event.h#newcode5
skia/ext/trace_event.h:5: #ifndef SKIA_EXT_TRACE_EVENT_H_
unless skia/ folder has its own subclassing conventions, i'd suggest

skia/ext/event_tracer_impl.h

with just a single global function, InitSkiaEventTracer or something similarly
named. Nobody in the chrome side of things is intended to see this subclass, you
see, so you can declare the subclass in event_tracer_impl.cc and register it
right there.

[humper, 2014-02-04 19:58:07]

https://codereview.chromium.org/153063002/diff/1/content/renderer/renderer_ma...
File content/renderer/renderer_main.cc (right):

https://codereview.chromium.org/153063002/diff/1/content/renderer/renderer_ma...
content/renderer/renderer_main.cc:244: SkEventTracer::SetInstance(new
skia::SkChromiumEventTracer());
On 2014/02/03 22:25:02, piman wrote:
> nit: should we do this in the RenderThreadImpl initialization? That'd be more
> consistent (and allow it to work in --single-process etc.)

Sure, I'd be happy to move it -- this place was suggested by Nat, but if there's
a better place I'm all for it.

https://codereview.chromium.org/153063002/diff/1/content/renderer/renderer_ma...
content/renderer/renderer_main.cc:244: SkEventTracer::SetInstance(new
skia::SkChromiumEventTracer());
On 2014/02/04 01:15:19, nduca wrote:
> yea definitely reder thread impl

Done.

https://codereview.chromium.org/153063002/diff/1/skia/ext/trace_event.h
File skia/ext/trace_event.h (right):

https://codereview.chromium.org/153063002/diff/1/skia/ext/trace_event.h#newcode5
skia/ext/trace_event.h:5: #ifndef SKIA_EXT_TRACE_EVENT_H_
On 2014/02/04 01:15:19, nduca wrote:
> unless skia/ folder has its own subclassing conventions, i'd suggest
> 
> skia/ext/event_tracer_impl.h
> 
> with just a single global function, InitSkiaEventTracer or something similarly
> named. Nobody in the chrome side of things is intended to see this subclass,
you
> see, so you can declare the subclass in event_tracer_impl.cc and register it
> right there.

Done.

[nduca, 2014-02-04 20:01:37]

https://codereview.chromium.org/153063002/diff/100001/skia/ext/event_tracer_i...
File skia/ext/event_tracer_impl.cc (right):

https://codereview.chromium.org/153063002/diff/100001/skia/ext/event_tracer_i...
skia/ext/event_tracer_impl.cc:75: // Initialize the binding to Skia's tracing
events. Skia will
lgtm

should you dcheck against multiple initialization?

[humper, 2014-02-04 20:20:25]

On 2014/02/04 20:01:37, nduca wrote:
>
https://codereview.chromium.org/153063002/diff/100001/skia/ext/event_tracer_i...
> File skia/ext/event_tracer_impl.cc (right):
> 
>
https://codereview.chromium.org/153063002/diff/100001/skia/ext/event_tracer_i...
> skia/ext/event_tracer_impl.cc:75: // Initialize the binding to Skia's tracing
> events. Skia will
> lgtm
> 
> should you dcheck against multiple initialization?

Actually, multiple initialization is fine; skia will delete the existing object
(if any) and replace it with the new one, so even if this initialization routine
gets called more than once from the same thread, it should work fine.

[piman, 2014-02-04 21:40:28]

LGTM for content/

[humper, 2014-02-04 21:41:42]

The CQ bit was checked by humper@google.com

[commit-bot: I haz the power, 2014-02-04 22:36:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/humper@google.com/153063002/100001

[commit-bot: I haz the power, 2014-02-04 23:51:40]

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

[commit-bot: I haz the power, 2014-02-04 23:51:40]

Retried try job too often on android_clang_dbg for step(s) slave_steps
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...

[humper, 2014-02-05 17:25:45]

The CQ bit was checked by humper@google.com

[commit-bot: I haz the power, 2014-02-05 17:29:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/humper@google.com/153063002/410001

[humper, 2014-02-05 19:04:26]

The CQ bit was checked by humper@google.com

[commit-bot: I haz the power, 2014-02-05 19:11:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/humper@google.com/153063002/720001

[commit-bot: I haz the power, 2014-02-05 21:59:24]

Change committed as 249128