Fixed GPU tracing so the categories do not get mixed.

Fixed GPU tracing so the categories do not get mixed.

Because of how the chromium tracing macros work, it is not possible
to use the same line for multiple tracing categories. I have moved
the GPU categories to a separate trace arg "gl_categories" instead,
and changed the trace categories back to gpu.device and gpu.service.

GroupPushMarkerEXT calls can no longer be disabled by their category.
We currently use these to trace top level GPU traces which are
constructed before the tracing categories are initialized.

There were also some other issues related to having multiple channels
in the GPU Tracer. Now functions that query the current trace take
in a channel argument as well to differentiate the channels.

R=vmiura@chromium.org
BUG=None
TEST=trybots

Committed: https://crrev.com/4bb4328938b0bfa4d2fa59403ce3de5639952e16
Cr-Commit-Position: refs/heads/master@{#310426}

[vmiura, 2014-12-16 23:40:11]

https://codereview.chromium.org/813573003/diff/1/gpu/command_buffer/service/g...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/813573003/diff/1/gpu/command_buffer/service/g...
gpu/command_buffer/service/gles2_cmd_decoder.cc:11109:
TRACE_EVENT_COPY_ASYNC_END0(category.c_str(), name.c_str(), this);
This seems like it might have the same problem, where we are having a variable
category passed through the same line.

Would it make sense to remove these traces from
HandleTraceBeginCHROMIUM/DoTraceEndCHROMIUM?  We have the 'gpu.service' trace
inside GpuTracer.

[vmiura, 2014-12-16 23:41:08]

vmiura@chromium.org changed reviewers:
+ nduca@chromium.org

[vmiura, 2014-12-16 23:41:08]

nduca@chromium.org: Please review changes in 
  base/debug/trace_event.h

[David Yen, 2014-12-17 18:29:35]

https://codereview.chromium.org/813573003/diff/1/gpu/command_buffer/service/g...
File gpu/command_buffer/service/gles2_cmd_decoder.cc (right):

https://codereview.chromium.org/813573003/diff/1/gpu/command_buffer/service/g...
gpu/command_buffer/service/gles2_cmd_decoder.cc:11109:
TRACE_EVENT_COPY_ASYNC_END0(category.c_str(), name.c_str(), this);
On 2014/12/16 23:40:11, vmiura wrote:
> This seems like it might have the same problem, where we are having a variable
> category passed through the same line.
> 
> Would it make sense to remove these traces from
> HandleTraceBeginCHROMIUM/DoTraceEndCHROMIUM?  We have the 'gpu.service' trace
> inside GpuTracer.

Done.

[vmiura, 2014-12-18 02:24:15]

Looks good.  Could you add some tests for
GPUTracer::CurrentCategory/CurrentName?

Also we need the integrated browser-test to validate the events are getting the
correct categories.  Are you planning to do that as a separate CL?

[David Yen, 2014-12-18 19:32:22]

On 2014/12/18 02:24:15, vmiura wrote:
> Looks good.  Could you add some tests for
> GPUTracer::CurrentCategory/CurrentName?
> 
> Also we need the integrated browser-test to validate the events are getting
the
> correct categories.  Are you planning to do that as a separate CL?

I couldn't find any unit tests that tested GPUTracer itself so I took the
opportunity to add a whole suite of tests for GPUTracer inside of
gpu_tracer_unittests.cc including ones for CurrentCategory() and CurrentName(),
PTAL.

I plan on adding browser tests after the GL_CHROMIUM_trace_marker extension gets
plumbed through blink.

[vmiura, 2014-12-19 00:30:15]

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

https://codereview.chromium.org/813573003/diff/100001/gpu/command_buffer/serv...
gpu/command_buffer/service/gpu_tracer_unittest.cc:224: class MockGPUTracer :
public GPUTracer {
This is not really a "Mock", I guess.

I think you could use BaseGpuTraceTest which sets up fake GL Apis, and run with
one of the timer extensions enabled.

This would allow more code coverage, and testing for example that
BeginDecoding/EndDecoding do the right things for pausing & resuming traces.

[David Yen, 2015-01-07 23:24:28]

All unit test revamps has been taken out and landed in a separate CL:
https://codereview.chromium.org/794673007/

This CL now only contains fixes for the GPU Tracer so categories properly show
up for each trace source.

I've also moved the "CurrentName()" and "CurrentCategory()" tests to a later
time after all the sources have pushed a marker. This will test that each source
can be queried independently from one another.

[vmiura, 2015-01-08 00:24:32]

LGTM for gpu/command_buffer/.

[David Yen, 2015-01-08 00:40:50]

dyen@chromium.org changed reviewers:
+ dsinclair@chromium.org

[David Yen, 2015-01-08 00:40:50]

+dsinclair to review trace_event.h

[dsinclair, 2015-01-08 01:03:56]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2015-01-08 01:03:56]

lgtm

[commit-bot: I haz the power, 2015-01-08 01:04:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/813573003/140001

[commit-bot: I haz the power, 2015-01-08 01:10:28]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-01-08 01:11:43]

Patchset 8 (id:??) landed as
https://crrev.com/4bb4328938b0bfa4d2fa59403ce3de5639952e16
Cr-Commit-Position: refs/heads/master@{#310426}