Trace loggging: Check for g_event_logger is not null before calling it.

Trace loggging: Check for g_event_logger is not null before calling it.

BUG=webrtc:7868
Review-Url: https://codereview.webrtc.org/2961663002
Cr-Commit-Position: refs/heads/master@{#18933}
Committed: https://chromium.googlesource.com/external/webrtc/+/7480da4118b72b3a64f007cac6556dc64d5682ca

[jtt_webrtc, 2017-06-26 18:49:01]

Description was changed from

==========
Trace loggging: Check for g_event_logger is not null before calling it.

BUG=webrtc::7868
==========

to

==========
Trace loggging: Check for g_event_logger is not null before calling it.

BUG=webrtc:7868
==========

[jtt_webrtc, 2017-06-26 18:49:27]

jtteh@webrtc.org changed reviewers:
+ tkchin@webrtc.org

[tkchin_webrtc, 2017-06-27 22:56:23]

tkchin@webrtc.org changed reviewers:
+ haysc@webrtc.org

[tkchin_webrtc, 2017-06-27 22:56:40]

https://codereview.webrtc.org/2961663002/diff/1/webrtc/base/event_tracer.cc
File webrtc/base/event_tracer.cc (right):

https://codereview.webrtc.org/2961663002/diff/1/webrtc/base/event_tracer.cc#n...
webrtc/base/event_tracer.cc:377: if (g_event_logger) {
I'm not sure how thread-safe this check is. Ask owner of the file?

https://codereview.webrtc.org/2961663002/diff/1/webrtc/sdk/objc/Framework/Uni...
File webrtc/sdk/objc/Framework/UnitTests/RTCPeerConnectionTest.mm (right):

https://codereview.webrtc.org/2961663002/diff/1/webrtc/sdk/objc/Framework/Uni...
webrtc/sdk/objc/Framework/UnitTests/RTCPeerConnectionTest.mm:28: -
(void)tracingTestNoInitialization;
This shouldn't belong in RTCPeerConnectionTest.
Write a new UT class/file for RTCTracing instead.

[jtt_webrtc, 2017-06-28 17:32:20]

jtteh@webrtc.org changed reviewers:
+ juberti@webrtc.org

[jtt_webrtc, 2017-06-28 17:44:19]

jtteh@webrtc.org changed reviewers:
+ kjellander@webrtc.org, magjed@webrtc.org, tommi@webrtc.org
- juberti@webrtc.org

[jtt_webrtc, 2017-06-28 17:44:26]

https://codereview.webrtc.org/2961663002/diff/1/webrtc/base/event_tracer.cc
File webrtc/base/event_tracer.cc (right):

https://codereview.webrtc.org/2961663002/diff/1/webrtc/base/event_tracer.cc#n...
webrtc/base/event_tracer.cc:377: if (g_event_logger) {
On 2017/06/27 22:56:40, tkchin_webrtc wrote:
> I'm not sure how thread-safe this check is. Ask owner of the file?

The only time g_event_logger is changed is when SetupInternalTracer or
ShutdownInternalTracer is called.
Since we're just checking if g_event_logger is not null, and if it is null, it
is okay if the logging is not called.

Adding owner to reviewers.

[jtt_webrtc, 2017-07-06 20:23:27]

bump.

[tommi, 2017-07-07 08:53:51]

https://codereview.webrtc.org/2961663002/diff/20001/webrtc/base/event_tracer.cc
File webrtc/base/event_tracer.cc (right):

https://codereview.webrtc.org/2961663002/diff/20001/webrtc/base/event_tracer....
webrtc/base/event_tracer.cc:383: if (g_event_logger) {
can you turn this around and do:

if (!g_event_logger)
  return false;

<code here as before>

https://codereview.webrtc.org/2961663002/diff/20001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/UnitTests/RTCPeerConnectionTest.mm (right):

https://codereview.webrtc.org/2961663002/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/UnitTests/RTCPeerConnectionTest.mm:92: 
can you revert this file? (doesn't look like there's any change besides just
removing an empty line)

https://codereview.webrtc.org/2961663002/diff/20001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/UnitTests/RTCTracingTest.mm (right):

https://codereview.webrtc.org/2961663002/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/UnitTests/RTCTracingTest.mm:2: *  Copyright 2015 The
WebRTC project authors. All Rights Reserved.
the year should be 2017

[jtt_webrtc, 2017-07-07 16:22:40]

https://codereview.webrtc.org/2961663002/diff/20001/webrtc/base/event_tracer.cc
File webrtc/base/event_tracer.cc (right):

https://codereview.webrtc.org/2961663002/diff/20001/webrtc/base/event_tracer....
webrtc/base/event_tracer.cc:383: if (g_event_logger) {
On 2017/07/07 08:53:50, tommi (wëbrtc) wrote:
> can you turn this around and do:
> 
> if (!g_event_logger)
>   return false;
> 
> <code here as before>

Done.

https://codereview.webrtc.org/2961663002/diff/20001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/UnitTests/RTCPeerConnectionTest.mm (right):

https://codereview.webrtc.org/2961663002/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/UnitTests/RTCPeerConnectionTest.mm:92: 
On 2017/07/07 08:53:50, tommi (wëbrtc) wrote:
> can you revert this file? (doesn't look like there's any change besides just
> removing an empty line)

Done.

https://codereview.webrtc.org/2961663002/diff/20001/webrtc/sdk/objc/Framework...
File webrtc/sdk/objc/Framework/UnitTests/RTCTracingTest.mm (right):

https://codereview.webrtc.org/2961663002/diff/20001/webrtc/sdk/objc/Framework...
webrtc/sdk/objc/Framework/UnitTests/RTCTracingTest.mm:2: *  Copyright 2015 The
WebRTC project authors. All Rights Reserved.
On 2017/07/07 08:53:50, tommi (wëbrtc) wrote:
> the year should be 2017

Done.

[tommi, 2017-07-07 17:39:16]

The CQ bit was checked by tommi@webrtc.org

[tommi, 2017-07-07 17:39:16]

lgtm

[commit-bot: I haz the power, 2017-07-07 17:39:24]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-07-07 18:02:15]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1499449156070580,
"parent_rev": "fb660ae633ba4bc4bd7f7607d1aeb8e501570b09", "commit_rev":
"7480da4118b72b3a64f007cac6556dc64d5682ca"}

[commit-bot: I haz the power, 2017-07-07 18:02:22]

Description was changed from

==========
Trace loggging: Check for g_event_logger is not null before calling it.

BUG=webrtc:7868
==========

to

==========
Trace loggging: Check for g_event_logger is not null before calling it.

BUG=webrtc:7868

Review-Url: https://codereview.webrtc.org/2961663002
Cr-Commit-Position: refs/heads/master@{#18933}
Committed:
https://chromium.googlesource.com/external/webrtc/+/7480da4118b72b3a64f007cac...
==========

[commit-bot: I haz the power, 2017-07-07 18:02:23]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/external/webrtc/+/7480da4118b72b3a64f007cac...