Fixed not-unique event 'id' in TRACE_EVENT from GLARBTimerTrace

The GLARBTimerTrace implementation previously used the 'this' pointer for it's
event 'id'.  The 'this' pointers are not unique since memory is recycled with
repeated alloc & free.  Trace output could appear jumbled in chrome://tracing
as in BUG=331161
.

Moved the event 'id' assignment into a TraceOutputter which manages output to
TRACE_EVENT and maintains an incrementing event 'id'.

Added a unit test for the GLARBTimerTrace.

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

[vmiura, 2014-01-03 23:59:32]

Please take a look.

[jbauman, 2014-01-04 00:24:11]

lgtm

https://codereview.chromium.org/122723002/diff/130001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gpu_tracer.cc (right):

https://codereview.chromium.org/122723002/diff/130001/gpu/command_buffer/serv...
gpu/command_buffer/service/gpu_tracer.cc:22: g_outputter_thread = new
TraceOutputter(name);
I'm not sure if this is really useful as is. We could remove it at some point.

https://codereview.chromium.org/122723002/diff/130001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gpu_tracer_unittest.cc (right):

https://codereview.chromium.org/122723002/diff/130001/gpu/command_buffer/serv...
gpu/command_buffer/service/gpu_tracer_unittest.cc:136: // Delegate queri APIs
used by GLARBTimerTrace to a GlFakeQueries
"query"

[vmiura, 2014-01-07 21:57:20]

https://codereview.chromium.org/122723002/diff/130001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gpu_tracer_unittest.cc (right):

https://codereview.chromium.org/122723002/diff/130001/gpu/command_buffer/serv...
gpu/command_buffer/service/gpu_tracer_unittest.cc:136: // Delegate queri APIs
used by GLARBTimerTrace to a GlFakeQueries
On 2014/01/04 00:24:12, jbauman wrote:
> "query"

Done.

[commit-bot: I haz the power, 2014-01-07 22:01:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vmiura@chromium.org/122723002/290001

[commit-bot: I haz the power, 2014-01-07 22:22:57]

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

[Ken Russell (switch to Gerrit), 2014-01-07 22:24:25]

piman: OWNERS review please.

[piman, 2014-01-07 22:47:48]

https://codereview.chromium.org/122723002/diff/290001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gpu_tracer.cc (right):

https://codereview.chromium.org/122723002/diff/290001/gpu/command_buffer/serv...
gpu/command_buffer/service/gpu_tracer.cc:24: g_outputter_thread->Stop();
Are we really creating a thread just to get a unique id? :/

https://codereview.chromium.org/122723002/diff/290001/gpu/command_buffer/serv...
gpu/command_buffer/service/gpu_tracer.cc:39: int64 start_time,
nit: indentation ('int64' needs to be right under 'const')

https://codereview.chromium.org/122723002/diff/290001/gpu/command_buffer/serv...
gpu/command_buffer/service/gpu_tracer.cc:42: local_trace_id_, thread_id(),
start_time);
nit: indentation for the line continuation (either +4, or aligned under "gpu").

Maybe try git cl format.

https://codereview.chromium.org/122723002/diff/290001/gpu/command_buffer/serv...
gpu/command_buffer/service/gpu_tracer.cc:44: local_trace_id_, thread_id(),
end_time);
nit: indentation

https://codereview.chromium.org/122723002/diff/290001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gpu_tracer.h (right):

https://codereview.chromium.org/122723002/diff/290001/gpu/command_buffer/serv...
gpu/command_buffer/service/gpu_tracer.h:54: : public Outputter, private
base::Thread {
Instead of private inheritance, can you make a base::Thread member?

[vmiura, 2014-01-07 23:26:46]

https://codereview.chromium.org/122723002/diff/290001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gpu_tracer.cc (right):

https://codereview.chromium.org/122723002/diff/290001/gpu/command_buffer/serv...
gpu/command_buffer/service/gpu_tracer.cc:24: g_outputter_thread->Stop();
On 2014/01/07 22:47:49, piman (OOO back 2014-1-7) wrote:
> Are we really creating a thread just to get a unique id? :/

Yep :/.  This thread use is not part of my change btw.

I think it was intended to give a name to the trace in about:tracing (but it
doesn't actually work).

I'd like to remove this later with some updates in about:tracing.

https://codereview.chromium.org/122723002/diff/290001/gpu/command_buffer/serv...
gpu/command_buffer/service/gpu_tracer.cc:39: int64 start_time,
On 2014/01/07 22:47:49, piman (OOO back 2014-1-7) wrote:
> nit: indentation ('int64' needs to be right under 'const')

Done.

https://codereview.chromium.org/122723002/diff/290001/gpu/command_buffer/serv...
gpu/command_buffer/service/gpu_tracer.cc:42: local_trace_id_, thread_id(),
start_time);
On 2014/01/07 22:47:49, piman (OOO back 2014-1-7) wrote:
> nit: indentation for the line continuation (either +4, or aligned under
"gpu").
> 
> Maybe try git cl format.

Done.

https://codereview.chromium.org/122723002/diff/290001/gpu/command_buffer/serv...
gpu/command_buffer/service/gpu_tracer.cc:44: local_trace_id_, thread_id(),
end_time);
On 2014/01/07 22:47:49, piman (OOO back 2014-1-7) wrote:
> nit: indentation

Done.

https://codereview.chromium.org/122723002/diff/290001/gpu/command_buffer/serv...
File gpu/command_buffer/service/gpu_tracer.h (right):

https://codereview.chromium.org/122723002/diff/290001/gpu/command_buffer/serv...
gpu/command_buffer/service/gpu_tracer.h:54: : public Outputter, private
base::Thread {
On 2014/01/07 22:47:49, piman (OOO back 2014-1-7) wrote:
> Instead of private inheritance, can you make a base::Thread member?

Done.

[piman, 2014-01-08 01:15:56]

lgtm

[commit-bot: I haz the power, 2014-01-08 19:03:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vmiura@chromium.org/122723002/670001

[commit-bot: I haz the power, 2014-01-08 21:50:51]

Change committed as 243657