Fix WebRTC logging so that logs from libpeerconnection make it to the log.

Fix WebRTC logging so that logs from libpeerconnection make it to the log.

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

[Henrik Grunell, 2013-11-13 20:08:57]

Tommi & Joi, please review whoever gets to it first and/or feels up for it.

Ronghua, owner stamp and review if you like.

[Henrik Grunell, 2013-11-13 20:13:53]

The function naming might be a bit rough. "...FunctionFunction" is strange but
technically correct.

[Jói, 2013-11-13 20:28:37]

Looking now.

On Wed, Nov 13, 2013 at 8:13 PM,  <grunell@chromium.org> wrote:
> The function naming might be a bit rough. "...FunctionFunction" is strange
> but
> technically correct.
>
> https://codereview.chromium.org/71353010/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Jói, 2013-11-13 20:40:12]

LGTM with a nit.

https://codereview.chromium.org/71353010/diff/30001/third_party/libjingle/ove...
File third_party/libjingle/overrides/init_webrtc.cc (right):

https://codereview.chromium.org/71353010/diff/30001/third_party/libjingle/ove...
third_party/libjingle/overrides/init_webrtc.cc:99:
InitDiagnosticLoggingDelegateFunctionFunction init_diagnostic_logging;
nit: Would set this to NULL.

[Ronghua Wu (Left Chromium), 2013-11-13 21:57:24]

owner stamp, LGTM if all the other reviewers are happy

[tommi (sloooow) - chröme, 2013-11-14 08:12:18]

lgtm % nits

https://codereview.chromium.org/71353010/diff/30001/third_party/libjingle/ove...
File third_party/libjingle/overrides/init_webrtc.cc (right):

https://codereview.chromium.org/71353010/diff/30001/third_party/libjingle/ove...
third_party/libjingle/overrides/init_webrtc.cc:99:
InitDiagnosticLoggingDelegateFunctionFunction init_diagnostic_logging;
On 2013/11/13 20:40:13, Jói wrote:
> nit: Would set this to NULL.

+1

https://codereview.chromium.org/71353010/diff/30001/third_party/libjingle/ove...
File third_party/libjingle/overrides/init_webrtc.h (right):

https://codereview.chromium.org/71353010/diff/30001/third_party/libjingle/ove...
third_party/libjingle/overrides/init_webrtc.h:33: typedef void
(*InitDiagnosticLoggingDelegateFunctionFunction)(
add a todo to change the name of InitDiagnosticLoggingDelegateFunction to just
InitDiagnosticLoggingDelegate or even InitDiagnosticLogging.  That'll make this
type name a little better.

https://codereview.chromium.org/71353010/diff/30001/third_party/libjingle/ove...
File third_party/libjingle/overrides/talk/base/logging.cc (right):

https://codereview.chromium.org/71353010/diff/30001/third_party/libjingle/ove...
third_party/libjingle/overrides/talk/base/logging.cc:319: void (*function)(void
(*delegate)(const std::string&))) {
nit: it would be more readable to have a typedef for at least the delegate
function.  for both would be better still imho.  I'm fine with a TODO on that if
you agree.

[Henrik Grunell, 2013-11-14 09:04:08]

https://codereview.chromium.org/71353010/diff/30001/third_party/libjingle/ove...
File third_party/libjingle/overrides/init_webrtc.cc (right):

https://codereview.chromium.org/71353010/diff/30001/third_party/libjingle/ove...
third_party/libjingle/overrides/init_webrtc.cc:99:
InitDiagnosticLoggingDelegateFunctionFunction init_diagnostic_logging;
On 2013/11/13 20:40:13, Jói wrote:
> nit: Would set this to NULL.

Done.

https://codereview.chromium.org/71353010/diff/30001/third_party/libjingle/ove...
File third_party/libjingle/overrides/init_webrtc.h (right):

https://codereview.chromium.org/71353010/diff/30001/third_party/libjingle/ove...
third_party/libjingle/overrides/init_webrtc.h:33: typedef void
(*InitDiagnosticLoggingDelegateFunctionFunction)(
On 2013/11/14 08:12:18, tommi wrote:
> add a todo to change the name of InitDiagnosticLoggingDelegateFunction to just
> InitDiagnosticLoggingDelegate or even InitDiagnosticLogging.  That'll make
this
> type name a little better.

Done.

https://codereview.chromium.org/71353010/diff/30001/third_party/libjingle/ove...
File third_party/libjingle/overrides/talk/base/logging.cc (right):

https://codereview.chromium.org/71353010/diff/30001/third_party/libjingle/ove...
third_party/libjingle/overrides/talk/base/logging.cc:319: void (*function)(void
(*delegate)(const std::string&))) {
On 2013/11/14 08:12:18, tommi wrote:
> nit: it would be more readable to have a typedef for at least the delegate
> function.  for both would be better still imho.  I'm fine with a TODO on that
if
> you agree.

Agree, added todo in the header.

[commit-bot: I haz the power, 2013-11-14 09:07:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/grunell@chromium.org/71353010/30007

[commit-bot: I haz the power, 2013-11-14 12:05:55]

Change committed as 235123