Replace use of system_wrappers/include/logging.h by base/logging.h.

Replace use of system_wrappers/include/logging.h by base/logging.h.

BUG=webrtc:5118
Review-Url: https://codereview.webrtc.org/2781343002
Cr-Commit-Position: refs/heads/master@{#17539}
Committed: https://chromium.googlesource.com/external/webrtc/+/368f5cf27e079e55c6a87297ab50fb8ca028c8b7

[nisse-webrtc, 2017-03-30 08:24:08]

nisse@webrtc.org changed reviewers:
+ solenberg@webrtc.org

[nisse-webrtc, 2017-03-30 08:24:09]

PTAL.

Do you know if there's any changes to the behavior of LOG(LS_INFO) and friends
when switching from system_wrappers' implementation to base/logging.h?

Actually deleting logging code from system_wrappers could be done in this cl, or
as a follow up.

[the sun, 2017-03-30 09:00:33]

lgtm

[nisse-webrtc, 2017-03-30 10:29:59]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2017-03-30 10:30:07]

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-03-30 10:35:17]

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

[commit-bot: I haz the power, 2017-03-30 10:35:19]

Try jobs failed on following builders:
  presubmit on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/15604)

[nisse-webrtc, 2017-03-30 10:45:57]

nisse@webrtc.org changed reviewers:
+ tommi@webrtc.org

[nisse-webrtc, 2017-03-30 10:45:58]

Tommi, do you know how this might affect Chrome logging? It seems chromium has
some override magic for webrtc/base/logging.h, but it's blissfully unaware of
systemwrappers/include/logging.h.

So this might change how log messages from the affected files will end up in
chrome.

[nisse-webrtc, 2017-03-31 11:56:53]

nisse@webrtc.org changed reviewers:
+ henrikg@webrtc.org

[nisse-webrtc, 2017-03-31 11:56:54]

I've made a separate exploratory cl to instead of switching to base/logging.h,
just delete all logging which is depends on system_wrappers/include/logging.h.

See https://codereview.webrtc.org/2787263003/. That diff should highlight all
the messages which are moved over to base/logging.h by this cl. As far as I can
tell, the messages don't include any personal info, and they're mainly for
exceptional conditions so they shouldn't bloat the log either.

Ok?

[Henrik Grunell WebRTC, 2017-04-03 11:16:13]

On 2017/03/30 10:45:58, nisse-webrtc wrote:
> Tommi, do you know how this might affect Chrome logging? It seems chromium has
> some override magic for webrtc/base/logging.h, but it's blissfully unaware of
> systemwrappers/include/logging.h.
> 
> So this might change how log messages from the affected files will end up in
> chrome.

Currently, during the lifetime of WebRtcVoiceEngine the system_wrapper LOG()
messages are forwarded to logging.h LOG(), via the Trace callback mechanism.
https://cs.chromium.org/chromium/src/third_party/webrtc/media/engine/webrtcvo...

If there's any logging outside of that lifetime, it will start showing up in the
Chrome log and/or the special WebRTC log in Chrome after this change.

[Henrik Grunell WebRTC, 2017-04-03 11:16:53]

On 2017/03/31 11:56:54, nisse-webrtc wrote:
> I've made a separate exploratory cl to instead of switching to base/logging.h,
> just delete all logging which is depends on system_wrappers/include/logging.h.
> 
> See https://codereview.webrtc.org/2787263003/. That diff should highlight all
> the messages which are moved over to base/logging.h by this cl. As far as I
can
> tell, the messages don't include any personal info, and they're mainly for
> exceptional conditions so they shouldn't bloat the log either.
> 
> Ok?

Great with that CL. Looks good.

[Henrik Grunell WebRTC, 2017-04-03 11:18:59]

On 2017/04/03 11:16:13, Henrik Grunell WebRTC wrote:
> On 2017/03/30 10:45:58, nisse-webrtc wrote:
> > Tommi, do you know how this might affect Chrome logging? It seems chromium
has
> > some override magic for webrtc/base/logging.h, but it's blissfully unaware
of
> > systemwrappers/include/logging.h.
> > 
> > So this might change how log messages from the affected files will end up in
> > chrome.
> 
> Currently, during the lifetime of WebRtcVoiceEngine the system_wrapper LOG()
> messages are forwarded to logging.h LOG(), via the Trace callback mechanism.
>
https://cs.chromium.org/chromium/src/third_party/webrtc/media/engine/webrtcvo...
> 
> If there's any logging outside of that lifetime, it will start showing up in
the
> Chrome log and/or the special WebRTC log in Chrome after this change.

I'm supposing that those are rare (you could check that), so lgtm.

[Henrik Grunell WebRTC, 2017-04-03 11:21:27]

On 2017/04/03 11:18:59, Henrik Grunell WebRTC wrote:
> On 2017/04/03 11:16:13, Henrik Grunell WebRTC wrote:
> > On 2017/03/30 10:45:58, nisse-webrtc wrote:
> > > Tommi, do you know how this might affect Chrome logging? It seems chromium
> has
> > > some override magic for webrtc/base/logging.h, but it's blissfully unaware
> of
> > > systemwrappers/include/logging.h.
> > > 
> > > So this might change how log messages from the affected files will end up
in
> > > chrome.
> > 
> > Currently, during the lifetime of WebRtcVoiceEngine the system_wrapper LOG()
> > messages are forwarded to logging.h LOG(), via the Trace callback mechanism.
> >
>
https://cs.chromium.org/chromium/src/third_party/webrtc/media/engine/webrtcvo...
> > 
> > If there's any logging outside of that lifetime, it will start showing up in
> the
> > Chrome log and/or the special WebRTC log in Chrome after this change.
> 
> I'm supposing that those are rare (you could check that), so lgtm.

Tommi also has a good point in a comment I don't see here.

[nisse-webrtc, 2017-04-04 13:27:43]

nisse@webrtc.org changed reviewers:
+ kwiberg@webrtc.org

[nisse-webrtc, 2017-04-04 13:27:44]

Karl, can you approve this? Tommi is OOO.

His "other comment" was that some of the logging listed in my exploratory cl
https://codereview.webrtc.org/2787263003/ could be replaced by DCHECKs.

[kwiberg-webrtc, 2017-04-05 09:43:40]

lgtm!

Don't forget the follow-up CL that deletes the system_wrappers implementation!

[nisse-webrtc, 2017-04-05 11:26:46]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2017-04-05 11:26:54]

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-04-05 11:28:23]

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

[commit-bot: I haz the power, 2017-04-05 11:28:24]

Try jobs failed on following builders:
  ios64_sim_ios10_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_ios10_dbg/bui...)
  ios_arm64_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/19225)
  ios_arm64_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/19022)
  ios_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/24526)
  ios_rel on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/23356)

[nisse-webrtc, 2017-04-05 11:39:09]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2017-04-05 11:39:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
solenberg@webrtc.org, kwiberg@webrtc.org, henrikg@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2781343002/#ps60001 (title:
"Rebased.")

[commit-bot: I haz the power, 2017-04-05 11:39:17]

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-04-05 12:00:36]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1491392349918960,
"parent_rev": "2299b0ab976770158347b8376e4693990e0c8eeb", "commit_rev":
"368f5cf27e079e55c6a87297ab50fb8ca028c8b7"}

[commit-bot: I haz the power, 2017-04-05 12:00:40]

Description was changed from

==========
Replace use of system_wrappers/include/logging.h by base/logging.h.

BUG=webrtc:5118
==========

to

==========
Replace use of system_wrappers/include/logging.h by base/logging.h.

BUG=webrtc:5118

Review-Url: https://codereview.webrtc.org/2781343002
Cr-Commit-Position: refs/heads/master@{#17539}
Committed:
https://chromium.googlesource.com/external/webrtc/+/368f5cf27e079e55c6a87297a...
==========

[commit-bot: I haz the power, 2017-04-05 12:00:42]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/external/webrtc/+/368f5cf27e079e55c6a87297a...