webrtc-internals: show addIceCandidate when it fails

content/renderer/media/rtc_peer_connection_handler.cc

Index: content/renderer/media/rtc_peer_connection_handler.cc
diff --git a/content/renderer/media/rtc_peer_connection_handler.cc b/content/renderer/media/rtc_peer_connection_handler.cc
index bf7b89e94a01f0714c249f38de5accc15a489041..1ddbca2b08c4a8882c4c3118a356051dd1f1c176 100644
--- a/content/renderer/media/rtc_peer_connection_handler.cc
+++ b/content/renderer/media/rtc_peer_connection_handler.cc
@@ -1432,6 +1432,11 @@ bool RTCPeerConnectionHandler::addICECandidate(
   }
 
   if (peer_connection_tracker_) {
+    if (!return_value) {
+      // Ensure that addIceCandidate shows up even if there is an error.

[tommi (sloooow) - chröme, 2017/01/09 14:41:17]

reporting this twice intentionally?

[fippo, 2017/01/09 15:09:32]

Yes. The first one (with the true argument) generates the addIceCandidate when
there is an error, the second call (with the actual result) generates the
addIceCandidateOnFailure (when return_value is false).

This sounds rather weird but handling succeeded=false in
https://cs.chromium.org/chromium/src/content/renderer/media/peer_connection_t...
would be equally messy :-/

[hta - Chromium, 2017/01/16 09:12:22]

The original logic seemed to be that you logged either success or error. Why is
it a good thing to log two lines, given that you include all the information
both times?

An alternative rendering would be to change
https://cs.chromium.org/chromium/src/content/renderer/media/peer_connection_t...
from "addIceCandidate" to "addIceCandidate(success)" to indicate that the
log-line is happening after the call has completed.

+      peer_connection_tracker_->TrackAddIceCandidate(
+          this, candidate, PeerConnectionTracker::SOURCE_REMOTE, true);
+    }
     peer_connection_tracker_->TrackAddIceCandidate(
         this, candidate, PeerConnectionTracker::SOURCE_REMOTE, return_value);
   }