Make tracing work in service manager

Make tracing work in service manager

A trace can be dumped using:
mojo_runner --trace-startup service:mash_session

BUG=640235
Committed: https://crrev.com/8d88f2bbbfb7e52af89e7425d80c038b0c7b287a
Cr-Commit-Position: refs/heads/master@{#426109}

[chiniforooshan, 2016-10-18 20:20:58]

chiniforooshan@chromium.org changed reviewers:
+ erg@chromium.org, rockot@chromium.org

[chiniforooshan, 2016-10-18 20:20:58]

ptal

[chiniforooshan, 2016-10-18 20:21:18]

The CQ bit was checked by chiniforooshan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-18 20:21:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ken Rockot(use gerrit already), 2016-10-18 20:30:59]

https://codereview.chromium.org/2427013002/diff/20001/services/tracing/public...
File services/tracing/public/cpp/provider.cc (right):

https://codereview.chromium.org/2427013002/diff/20001/services/tracing/public...
services/tracing/public/cpp/provider.cc:66:
InitializeWithFactoryLocked(factory);
Is the intent that this is called under the lock? If so, it's not; if not, why
the "Locked" suffix?

https://codereview.chromium.org/2427013002/diff/20001/services/tracing/public...
services/tracing/public/cpp/provider.cc:79:
InitializeWithFactoryLocked(&factory);
Should this really be calling the Locked variant?

[chiniforooshan, 2016-10-18 20:46:18]

https://codereview.chromium.org/2427013002/diff/20001/services/tracing/public...
File services/tracing/public/cpp/provider.cc (right):

https://codereview.chromium.org/2427013002/diff/20001/services/tracing/public...
services/tracing/public/cpp/provider.cc:66:
InitializeWithFactoryLocked(factory);
On 2016/10/18 20:30:59, Ken Rockot wrote:
> Is the intent that this is called under the lock? If so, it's not; if not, why
> the "Locked" suffix?

Done. Yeah, "Locked" was not a good suffix. This is called only by the first
caller who obtains the lock and not subsequent calls. Changed it to
InitializeWithFactoryInternal. Please let me know if you have a better name
suggestion.

https://codereview.chromium.org/2427013002/diff/20001/services/tracing/public...
services/tracing/public/cpp/provider.cc:79:
InitializeWithFactoryLocked(&factory);
On 2016/10/18 20:30:59, Ken Rockot wrote:
> Should this really be calling the Locked variant?

Done. Changed it to InitializeWithFactoryInternal. Please let me know if you
have a better name suggestion.

[Ken Rockot(use gerrit already), 2016-10-18 21:53:18]

Ah, makes sense now. LGTM

[Elliot Glaysher, 2016-10-18 22:15:51]

lgtm

[chiniforooshan, 2016-10-18 23:54:26]

The CQ bit was checked by chiniforooshan@chromium.org

[commit-bot: I haz the power, 2016-10-18 23:55:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-19 01:39:50]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-10-21 13:05:30]

Description was changed from

==========
Make tracing work in service manager

A trace can be dumped using:
mojo_runner --trace-startup service:mash_session

BUG=640235
==========

to

==========
Make tracing work in service manager

A trace can be dumped using:
mojo_runner --trace-startup service:mash_session

BUG=640235

Committed: https://crrev.com/8d88f2bbbfb7e52af89e7425d80c038b0c7b287a
Cr-Commit-Position: refs/heads/master@{#426109}
==========

[commit-bot: I haz the power, 2016-10-21 13:05:31]

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