Added tracing to data-manager's loading of resources.

Added tracing to data-manager's loading of resources.


BUG=
TEST=


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

[Kevin Greer, 2012-04-11 21:08:00]

Please review the tracing I've added to the chrome data-source.  This makes
resource loading visible and helps when profiling WebUI code.

[Evan Stade, 2012-04-11 21:27:30]

I can't really comment on whether this is an correct usage of the TRACE_EVENT
macros

[Kevin Greer, 2012-04-11 21:32:26]

On 2012/04/11 21:27:30, Evan Stade wrote:
> I can't really comment on whether this is an correct usage of the TRACE_EVENT
> macros

It isn't actually.  Ideally I would provide the URL as the third argument in the
BEGIN and END calls, but because the loading is asynchronous and multiple loads
are run concurrently, they won't match up properly and this causes the tracing
system to crash.  To get around this I instead include the URL information in a
separate INSTANCE traces which bracket the BEGIN/END traces.

[Kevin Greer, 2012-04-11 21:39:22]

Adding jbates, who git blames for the TRACE_EVENT_INSTANT_ETW macro, as a
reviewer.

[jbates, 2012-04-11 23:08:13]

On 2012/04/11 21:39:22, Kevin Greer wrote:
> Adding jbates, who git blames for the TRACE_EVENT_INSTANT_ETW macro, as a
> reviewer.

Do you need ETW for some reason? We have a nice tracing UI built into chrome now
for all platforms (go to about:tracing). Your use case would show up nicely in
about:tracing with the ASYNC_BEGIN and ASYNC_END tracing macros from
trace_event.h:

TRACE_EVENT_ASYNC_BEGIN1("browser", "DataManager:Request", this, "URL",
request_->url().possibly_invalid_spec());

TRACE_EVENT_ASYNC_END0("browser", "DataManager:Request", this);

The URL would only need to be on the BEGIN because the BEGIN/END pair is
associated by the name ("DataManager:Request") and this pointer.

[Kevin Greer, 2012-04-12 14:27:55]

Thanks jbates, this works a lot better.

[flackr, 2012-04-12 15:04:05]

This looks good with the async tracing code, though I'm not really a tracing
expert either.

https://chromiumcodereview.appspot.com/10020052/diff/5001/chrome/browser/ui/w...
File chrome/browser/ui/webui/chrome_url_data_manager_backend.cc (right):

https://chromiumcodereview.appspot.com/10020052/diff/5001/chrome/browser/ui/w...
chrome/browser/ui/webui/chrome_url_data_manager_backend.cc:263:
TRACE_EVENT_ASYNC_END0("browser", "DataManager:Request", this);
If there's any significant amount of work done in this method (i.e. by
CompleteRead) then maybe this signal should be at the end of the method.

[Kevin Greer, 2012-04-12 15:52:05]

On 2012/04/12 15:04:05, flackr wrote:
> then maybe this signal should be at the end of the method.
It looks like the amount of work it does is pretty insignificant, but I've moved
it until after we start the load just to ensure that we don't delay the start of
loading even a little.

[jbates, 2012-04-12 16:30:37]

lgtm

[Evan Stade, 2012-04-14 00:47:20]

rubber stamp lgtm

[commit-bot: I haz the power, 2012-04-14 01:09:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kgr@chromium.org/10020052/9001

[commit-bot: I haz the power, 2012-04-14 03:07:12]

Change committed as 132318