about:tracing support for TRACE_ASYNC_START/FINISH events.

about:tracing support for TRACE_ASYNC_START/FINISH events.


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

[nduca, 2012-03-14 11:51:06]

Tadaaa.

[jbates, 2012-03-15 00:17:15]

LGTM with some nits

http://codereview.chromium.org/9706010/diff/1/chrome/browser/resources/tracin...
File chrome/browser/resources/tracing/tests/nonnested_trace.json (left):

http://codereview.chromium.org/9706010/diff/1/chrome/browser/resources/tracin...
chrome/browser/resources/tracing/tests/nonnested_trace.json:2: {"cat": "PERF",
"pid": 22630, "tid": 22630, "ts": 826, "ph": "B",
Isn't this still a valid test case?

http://codereview.chromium.org/9706010/diff/1/chrome/browser/resources/tracin...
File chrome/browser/resources/tracing/timeline_model.js (right):

http://codereview.chromium.org/9706010/diff/1/chrome/browser/resources/tracin...
chrome/browser/resources/tracing/timeline_model.js:58: *   TRACE_EVENT_END()    
             at time=0.3ms
nit: TRACE_EVENT_END0("x") so noobs aren't confused

http://codereview.chromium.org/9706010/diff/1/chrome/browser/resources/tracin...
chrome/browser/resources/tracing/timeline_model.js:99: * The asynchronous slices
ar stored in an TimelineAsyncSliceGroup object.
ar->are

http://codereview.chromium.org/9706010/diff/1/chrome/browser/resources/tracin...
chrome/browser/resources/tracing/timeline_model.js:113: "AsyncSliceGroup for " +
this.ptid);
Is "AsyncSliceGroup for ptid" the title presented to the user? If so can we
rename to something easier for end-users to read? "ptid async events" or
something like that?

http://codereview.chromium.org/9706010/diff/1/chrome/browser/resources/tracin...
chrome/browser/resources/tracing/timeline_model.js:471: * Built automatically
rebuildSubRows()
Built automatically by rebuildSubRows

http://codereview.chromium.org/9706010/diff/1/chrome/browser/resources/tracin...
File chrome/browser/resources/tracing/trace_event_importer_test.html (right):

http://codereview.chromium.org/9706010/diff/1/chrome/browser/resources/tracin...
chrome/browser/resources/tracing/trace_event_importer_test.html:581:
//oneslicetwothreads
mark these TODO?

[nduca, 2012-03-15 15:20:38]

nonnested trace support is being removed.

[jbates, 2012-03-15 15:40:35]

On 2012/03/15 15:20:38, nduca wrote:
> nonnested trace support is being removed.

I was thinking you might leave the test case to verify about:tracing doesn't
explode if the trace macros are used incorrectly. Or it could be merged with
something like bad_data.json that tests other such error cases.

[nduca, 2012-03-15 15:51:30]

On 2012/03/15 15:40:35, jbates wrote:
> I was thinking you might leave the test case to verify about:tracing doesn't
> explode if the trace macros are used incorrectly. Or it could be merged with
> something like bad_data.json that tests other such error cases.
Oh, crap. Good point. Hm.... /me clears commit bit and contemplates.

[jbates, 2012-03-15 16:20:14]

On 2012/03/15 15:51:30, nduca wrote:
> On 2012/03/15 15:40:35, jbates wrote:
> > I was thinking you might leave the test case to verify about:tracing doesn't
> > explode if the trace macros are used incorrectly. Or it could be merged with
> > something like bad_data.json that tests other such error cases.
> Oh, crap. Good point. Hm.... /me clears commit bit and contemplates.

I see, this was for an older feature. Seems good to remove the file and add some
edge-case error checking later.

[commit-bot: I haz the power, 2012-03-15 16:22:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nduca@chromium.org/9706010/3002

[commit-bot: I haz the power, 2012-03-15 16:22:41]

Presubmit check for 9706010-3002 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change has an associated bug, add BUG=[bug number].

If this change requires manual test instructions to QA team, add
TEST=[instructions].

** Presubmit Warnings **
License must match:
.*? Copyright \(c\) 2012 The Chromium Authors\. All rights reserved\.\n.*? Use
of this source code is governed by a BSD-style license that can be\n.*? found in
the LICENSE file\.(?: \*/)?\n
Found a bad license header in these files:
  chrome/browser/resources/tracing/timeline.css

Presubmit checks took 1.2s to calculate.

[commit-bot: I haz the power, 2012-03-15 16:38:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nduca@chromium.org/9706010/5002

[commit-bot: I haz the power, 2012-03-15 20:15:56]

Change committed as 126978