Allow tracing to console everywhere.

Allow tracing to console everywhere.

This patch gets rid of the cc-only trace-to-vlog stuff and replaces it with a
general trace-to-console approach that can be used everywhere, even the
browser.

BUG=None

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

[nduca, 2013-06-28 19:39:02]

lgtm. lets update the tracing wiki on this.

[Ian Vollick, 2013-06-28 19:46:55]

+darin for base/ and content/ OWNERs.

[Ian Vollick, 2013-07-03 13:05:43]

-darin (unavailable)
+willchan for base/ OWNERS
+piman for content/ OWNERS

[piman, 2013-07-03 17:42:28]

https://codereview.chromium.org/18174006/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/18174006/diff/1/content/browser/renderer_host...
content/browser/renderer_host/render_process_host_impl.cc:1000: 
nit: no need for blank lines

https://codereview.chromium.org/18174006/diff/1/content/renderer/render_threa...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/18174006/diff/1/content/renderer/render_threa...
content/renderer/render_thread_impl.cc:401: if
(CommandLine::ForCurrentProcess()->HasSwitch(switches::kTraceToConsole)) {
Could we move this to ChildThread, so that it applies to all processes,
including e.g. GPU or Plugin?

[willchan no longer on Chromium, 2013-07-03 17:55:19]

base lgtm

https://codereview.chromium.org/18174006/diff/1/base/debug/trace_event_impl.cc
File base/debug/trace_event_impl.cc (right):

https://codereview.chromium.org/18174006/diff/1/base/debug/trace_event_impl.c...
base/debug/trace_event_impl.cc:72: const char*
g_category_groups[MAX_CATEGORY_GROUPS] = {
Just noticed, we ought to make this const char* const since it's probably
leading to an extra load instruction in GetCategoryGroupEnabledInternal(). Not
something you need to fix here in your CL.

https://codereview.chromium.org/18174006/diff/1/base/debug/trace_event_impl.c...
base/debug/trace_event_impl.cc:807: if
(CommandLine::ForCurrentProcess()->HasSwitch(switches::kTraceToConsole)) {
Not super fond of this, but since nduca is fine with it, I'll let it in. My
reasoning is that I view the command line as an application level concept and
not a library, and normally I'd say the library should expose a configuration
option that the application can set when it reads its command line. That said,
this way is probably more straightforward and simpler to understand.

https://codereview.chromium.org/18174006/diff/1/base/debug/trace_event_impl.c...
base/debug/trace_event_impl.cc:812: if (!category_string.size())
STL doesn't specify the algorithmic complexity of size(), but it's generally
constant-time. That said, if you wanted to be safe, you could use .empty()
instead.

[Ian Vollick, 2013-07-03 18:48:02]

https://codereview.chromium.org/18174006/diff/1/base/debug/trace_event_impl.cc
File base/debug/trace_event_impl.cc (right):

https://codereview.chromium.org/18174006/diff/1/base/debug/trace_event_impl.c...
base/debug/trace_event_impl.cc:812: if (!category_string.size())
On 2013/07/03 17:55:19, willchan wrote:
> STL doesn't specify the algorithmic complexity of size(), but it's generally
> constant-time. That said, if you wanted to be safe, you could use .empty()
> instead.
Switched to empty().

https://codereview.chromium.org/18174006/diff/1/content/browser/renderer_host...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/18174006/diff/1/content/browser/renderer_host...
content/browser/renderer_host/render_process_host_impl.cc:1000: 
On 2013/07/03 17:42:28, piman wrote:
> nit: no need for blank lines
Removed.

https://codereview.chromium.org/18174006/diff/1/content/renderer/render_threa...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/18174006/diff/1/content/renderer/render_threa...
content/renderer/render_thread_impl.cc:401: if
(CommandLine::ForCurrentProcess()->HasSwitch(switches::kTraceToConsole)) {
On 2013/07/03 17:42:28, piman wrote:
> Could we move this to ChildThread, so that it applies to all processes,
> including e.g. GPU or Plugin?
That's a great idea. Done.

[piman, 2013-07-03 19:52:23]

LGTM+nit

https://codereview.chromium.org/18174006/diff/7002/content/browser/renderer_h...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/18174006/diff/7002/content/browser/renderer_h...
content/browser/renderer_host/render_process_host_impl.cc:1008:
switches::kTraceToConsole,
nit: you'll probably also want to add it to BrowserChildProcessHostImpl::Launch

[Ian Vollick, 2013-07-03 20:01:54]

On 2013/07/03 19:52:23, piman wrote:
> LGTM+nit
> 
>
https://codereview.chromium.org/18174006/diff/7002/content/browser/renderer_h...
> File content/browser/renderer_host/render_process_host_impl.cc (right):
> 
>
https://codereview.chromium.org/18174006/diff/7002/content/browser/renderer_h...
> content/browser/renderer_host/render_process_host_impl.cc:1008:
> switches::kTraceToConsole,
> nit: you'll probably also want to add it to
BrowserChildProcessHostImpl::Launch

Ah, right! Thanks. Done.

[commit-bot: I haz the power, 2013-07-03 20:06:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/18174006/16002

[commit-bot: I haz the power, 2013-07-03 21:53:47]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-07-04 00:47:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/18174006/16002

[commit-bot: I haz the power, 2013-07-04 02:04:19]

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

[Ian Vollick, 2013-07-04 10:16:04]

On 2013/07/04 02:04:19, I haz the power (commit-bot) wrote:
> Retried try job too often on linux_rel for step(s) nacl_integration
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

Needed to guard the code in trace_event_impl.cc that accesses the command line
so it won't run on NaCl.

[commit-bot: I haz the power, 2013-07-04 10:16:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/18174006/40001

[commit-bot: I haz the power, 2013-07-04 10:46:38]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[Ian Vollick, 2013-07-04 14:27:40]

On 2013/07/04 10:46:38, I haz the power (commit-bot) wrote:
> Sorry for I got bad news for ya.
> Compile failed with a clobber build on win_x64_rel.
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
> Your code is likely broken or HEAD is junk. Please ensure your
> code is not broken then alert the build sheriffs.
> Look at the try server FAQ for more details.

willchan@, nduca@: I've had to make some more changes to trace_event_impl.cc,
and they're big enough that I'd like to get the go ahead from you before I
attempt to put this in the cq. When you have a moment can you PTAL?

[nduca, 2013-07-11 17:35:15]

still lgtm

[commit-bot: I haz the power, 2013-07-11 18:22:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/18174006/72001

[commit-bot: I haz the power, 2013-07-11 19:04:56]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-07-11 19:36:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/18174006/56294995...

[commit-bot: I haz the power, 2013-07-11 20:03:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vollick@chromium.org/18174006/58252813...

[commit-bot: I haz the power, 2013-07-11 23:10:42]

Change committed as 211245