[tracing] Do not check enabled for filtering in TRACE_EVENT_CATEGORY_GROUP_ENABLED

[tracing] Do not check enabled for filtering in TRACE_EVENT_CATEGORY_GROUP_ENABLED

The TRACE_EVENT_CATEGORY_GROUP_ENABLED sets enabled to true even when
filtering mode was turned on. This should be set only for recording and
etw modes.

BUG=669611, 670013
Committed: https://crrev.com/1ce3cd84139f6f60cb6a7458f0fbc95315241fd9
Cr-Commit-Position: refs/heads/master@{#437686}

[ssid, 2016-12-02 22:49:19]

Description was changed from

==========
[tracing] Do not check enabled for filtering in
TRACE_EVENT_CATEGORY_GROUP_ENABLED

The TRACE_EVENT_CATEGORY_GROUP_ENABLED sets enabled to true even when
filtering mode was turned on. This should be set only for recording and
etw modes.

BUG=669611
==========

to

==========
[tracing] Do not check enabled for filtering in
TRACE_EVENT_CATEGORY_GROUP_ENABLED

The TRACE_EVENT_CATEGORY_GROUP_ENABLED sets enabled to true even when
filtering mode was turned on. This should be set only for recording and
etw modes.

BUG=669611, 670013
==========

[ssid, 2016-12-02 22:53:46]

ssid@chromium.org changed reviewers:
+ oysteine@chromium.org, primiano@chromium.org

[ssid, 2016-12-02 22:53:47]

See https://bugs.chromium.org/p/chromium/issues/detail?id=669611 for context.

This should fix the issue of some code doing:
TRACE_EVENT_CATEGORY_GROUP_ENABLED("disabled.xx")
if(enabled)
  do_lot_of_heavy_work();

But, we still have an issue of code doing:
TRACE_EVENT1(
    "disabled.xx", "", "snapshot", do_something_heavy());

I cannot think of a way to avoid the second one since we are doing the filtering
much later after this check in AddTraceEvent. But, I am not sure if some code
actually does this.

[ssid, 2016-12-02 22:59:31]

Patchset #2 (id:10001) has been deleted

[oystein (OOO til 10th of July), 2016-12-06 22:20:01]

On 2016/12/02 22:53:47, ssid wrote:
> See https://bugs.chromium.org/p/chromium/issues/detail?id=669611 for context.
> 
> This should fix the issue of some code doing:
> TRACE_EVENT_CATEGORY_GROUP_ENABLED("disabled.xx")
> if(enabled)
>   do_lot_of_heavy_work();
> 
> But, we still have an issue of code doing:
> TRACE_EVENT1(
>     "disabled.xx", "", "snapshot", do_something_heavy());
> 
> I cannot think of a way to avoid the second one since we are doing the
filtering
> much later after this check in AddTraceEvent. But, I am not sure if some code
> actually does this.

lgtm and thanks :)

Agreed that we can't avoid that second issue, but I also don't recall seeing
that pattern anywhere; pretty sure expensive tracing is constricted to
convertibles.

[Primiano Tucci (use gerrit), 2016-12-08 15:01:43]

LGTM thanks for fixing this.

On 2016/12/06 22:20:01, oystein wrote:
> > But, we still have an issue of code doing:
> > TRACE_EVENT1(
> >     "disabled.xx", "", "snapshot", do_something_heavy());
> > 
> > I cannot think of a way to avoid the second one since we are doing the
> filtering
> > much later after this check in AddTraceEvent. But, I am not sure if some
code
> > actually does this.
> 
> lgtm and thanks :)
> 
> Agreed that we can't avoid that second issue, but I also don't recall seeing
> that pattern anywhere; pretty sure expensive tracing is constricted to
> convertibles.

Yeah, don't worry about this.

[Primiano Tucci (use gerrit), 2016-12-08 15:01:49]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2016-12-08 15:02:11]

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-12-08 17:06:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-08 17:06:40]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[ssid, 2016-12-08 18:01:12]

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

[commit-bot: I haz the power, 2016-12-08 18:02:07]

Dry run: 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-12-08 20:20:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-08 20:20:15]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[ssid, 2016-12-09 19:05:50]

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

[commit-bot: I haz the power, 2016-12-09 19:06:31]

Dry run: 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-12-09 21:12:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-09 21:12:40]

Dry run: This issue passed the CQ dry run.

[ssid, 2016-12-09 22:08:13]

The CQ bit was checked by ssid@chromium.org

[commit-bot: I haz the power, 2016-12-09 22:08:48]

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-12-09 23:24:25]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1481321293749840, "parent_rev":
"961841f9eac9130db361598b1bfa4900f73237f0", "commit_rev":
"3083387f86148ae3ee30ab2f98a88158462daca4"}

[commit-bot: I haz the power, 2016-12-09 23:24:51]

Description was changed from

==========
[tracing] Do not check enabled for filtering in
TRACE_EVENT_CATEGORY_GROUP_ENABLED

The TRACE_EVENT_CATEGORY_GROUP_ENABLED sets enabled to true even when
filtering mode was turned on. This should be set only for recording and
etw modes.

BUG=669611, 670013
==========

to

==========
[tracing] Do not check enabled for filtering in
TRACE_EVENT_CATEGORY_GROUP_ENABLED

The TRACE_EVENT_CATEGORY_GROUP_ENABLED sets enabled to true even when
filtering mode was turned on. This should be set only for recording and
etw modes.

BUG=669611, 670013

Review-Url: https://codereview.chromium.org/2547193002
==========

[commit-bot: I haz the power, 2016-12-09 23:24:52]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-12-12 14:58:11]

Description was changed from

==========
[tracing] Do not check enabled for filtering in
TRACE_EVENT_CATEGORY_GROUP_ENABLED

The TRACE_EVENT_CATEGORY_GROUP_ENABLED sets enabled to true even when
filtering mode was turned on. This should be set only for recording and
etw modes.

BUG=669611, 670013

Review-Url: https://codereview.chromium.org/2547193002
==========

to

==========
[tracing] Do not check enabled for filtering in
TRACE_EVENT_CATEGORY_GROUP_ENABLED

The TRACE_EVENT_CATEGORY_GROUP_ENABLED sets enabled to true even when
filtering mode was turned on. This should be set only for recording and
etw modes.

BUG=669611, 670013

Committed: https://crrev.com/1ce3cd84139f6f60cb6a7458f0fbc95315241fd9
Cr-Commit-Position: refs/heads/master@{#437686}
==========

[commit-bot: I haz the power, 2016-12-12 14:58:11]

Patchset 1 (id:??) landed as
https://crrev.com/1ce3cd84139f6f60cb6a7458f0fbc95315241fd9
Cr-Commit-Position: refs/heads/master@{#437686}