Create the API that returns the RTCConfiguration of the PeerConnection.

third_party/WebKit/Source/modules/peerconnection/RTCPeerConnection.cpp

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 280 matching lines @@
         UseCounter::count(context, UseCounter::RTCIceServerURL);
         urlStrings.push_back(iceServer.url());
       } else {
         exceptionState.throwTypeError("Malformed RTCIceServer");
         return WebRTCConfiguration();
       }
 
       String username = iceServer.username();
       String credential = iceServer.credential();
 
+      Vector<WebURL> urls;
       for (const String& urlString : urlStrings) {
         KURL url(KURL(), urlString);
         if (!url.isValid()) {
           exceptionState.throwDOMException(
               SyntaxError, "'" + urlString + "' is not a valid URL.");
           return WebRTCConfiguration();
         }
         if (!(url.protocolIs("turn") || url.protocolIs("turns") ||
               url.protocolIs("stun"))) {
           exceptionState.throwDOMException(
               SyntaxError, "'" + url.protocol() +
                                "' is not one of the supported URL schemes "
                                "'stun', 'turn' or 'turns'.");
           return WebRTCConfiguration();
         }
         if ((url.protocolIs("turn") || url.protocolIs("turns")) &&
             (username.isNull() || credential.isNull())) {
           exceptionState.throwDOMException(InvalidAccessError,
                                            "Both username and credential are "
                                            "required when the URL scheme is "
                                            "\"turn\" or \"turns\".");
         }
-        iceServers.push_back(WebRTCIceServer{url, username, credential});
+        urls.push_back(url);
       }
+      iceServers.push_back(WebRTCIceServer{urls, username, credential});

[dcheng, 2017/03/09 21:36:54]

Nit: () rather than {}. We only use uniform initialization syntax when the
alternatives don't work well:
https://www.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Variab...

[zhihuang1, 2017/03/10 03:20:43]

Thanks for the link. Good to learn that.

[dcheng, 2017/03/10 19:54:00]

Ah... I missed that this was a struct. It's OK to inline it into push back and
just write {urls, username, credential} then

[Taylor_Brandstetter, 2017/03/10 20:06:03]

Actually, isn't emplace_back preferable then?

iceServers.emplace_back(urls, username, credential);

     }
     webConfiguration.iceServers = iceServers;
   }
 
   if (configuration.hasCertificates()) {
     const HeapVector<Member<RTCCertificate>>& certificates =
         configuration.certificates();
     WebVector<std::unique_ptr<WebRTCCertificate>> certificatesCopy(
         certificates.size());
     for (size_t i = 0; i < certificates.size(); ++i) {
       certificatesCopy[i] = certificates[i]->certificateShallowCopy();
     }
     webConfiguration.certificates = std::move(certificatesCopy);
   }
 
   return webConfiguration;
 }
 
+RTCConfiguration getWebKitRtcConfiguration(WebRTCConfiguration& blinkConfig) {

[dcheng, 2017/03/09 21:36:54]

Pass argument by const ref.

The naming is also a bit confusing here, IMO (it's not a getter, it's a
conversion helper). We're also converting between two WebKit types so my earlier
naming suggestion doesn't really work here either.

[zhihuang1, 2017/03/10 03:20:43]

It is not const because the ownership of blinkConfig.certificates will be
changed.
What about just merging this with the getConfiguration() since this is only used
once.

+  RTCConfiguration rtcConfig;
+
+  switch (blinkConfig.iceTransportPolicy) {
+    case blink::WebRTCIceTransportPolicy::kNone:
+      rtcConfig.setIceTransportPolicy("none");
+      break;
+    case blink::WebRTCIceTransportPolicy::kRelay:
+      rtcConfig.setIceTransportPolicy("relay");
+      break;
+    case blink::WebRTCIceTransportPolicy::kAll:
+      rtcConfig.setIceTransportPolicy("all");
+      break;
+  }
+
+  switch (blinkConfig.bundlePolicy) {
+    case blink::WebRTCBundlePolicy::kBalanced:
+      rtcConfig.setBundlePolicy("balanced");
+      break;
+    case blink::WebRTCBundlePolicy::kMaxBundle:
+      rtcConfig.setBundlePolicy("max-bundle");
+      break;
+    case blink::WebRTCBundlePolicy::kMaxCompat:
+      rtcConfig.setBundlePolicy("max-compat");
+      break;
+  }
+
+  switch (blinkConfig.rtcpMuxPolicy) {
+    case blink::WebRTCRtcpMuxPolicy::kNegotiate:
+      rtcConfig.setRtcpMuxPolicy("negotiate");
+      break;
+    case blink::WebRTCRtcpMuxPolicy::kRequire:
+      rtcConfig.setRtcpMuxPolicy("require");
+      break;
+  }
+
+  HeapVector<RTCIceServer> servers;
+  for (WebRTCIceServer blink_server : blinkConfig.iceServers) {

[tommi (sloooow) - chröme, 2017/03/09 19:37:08]

nit: const WebRTCIceServer&
as is, you're creating a copy. WebRTCIceServer might under the hood be
implemented to do a shallow copy, but that feels like an implementation detail
that you don't need to rely on here.

[zhihuang1, 2017/03/10 03:20:43]

Done.

+    RTCIceServer server;
+    Vector<String> urlStrings;
+    StringOrStringSequence urls;
+    for (WebURL blink_url : blink_server.urls) {

[tommi (sloooow) - chröme, 2017/03/09 19:37:08]

const WebURL&

[zhihuang1, 2017/03/10 03:20:43]

Done.

+      String urlString(blink_url.string().utf8().c_str());
+      urlStrings.push_back(urlString);
+    }
+    urls.setStringSequence(urlStrings);
+    server.setURLs(urls);
+    String username(blink_server.username.utf8().c_str());
+    server.setUsername(blink_server.username);
+    String credential(blink_server.credential.utf8().c_str());
+    server.setCredential(credential);
+    servers.push_back(server);
+  }
+  rtcConfig.setIceServers(servers);
+
+  HeapVector<Member<RTCCertificate>> certificates;
+  for (std::unique_ptr<blink::WebRTCCertificate>& blink_certificate :

[dcheng, 2017/03/09 21:36:54]

auto& for conciseness

[zhihuang1, 2017/03/10 03:20:43]

Done.

+       blinkConfig.certificates) {
+    certificates.push_back(new RTCCertificate(std::move(blink_certificate)));
+  }
+  rtcConfig.setCertificates(certificates);
+
+  return rtcConfig;
+}
+
 RTCOfferOptionsPlatform* parseOfferOptions(const Dictionary& options,
                                            ExceptionState& exceptionState) {
   if (options.isUndefinedOrNull())
     return nullptr;
 
   const Vector<String>& propertyNames =
       options.getPropertyNames(exceptionState);
   if (exceptionState.hadException())
     return nullptr;
 
@@ skipping 435 matching lines @@
 
 RTCSessionDescription* RTCPeerConnection::remoteDescription() {
   WebRTCSessionDescription webSessionDescription =
       m_peerHandler->remoteDescription();
   if (webSessionDescription.isNull())
     return nullptr;
 
   return RTCSessionDescription::create(webSessionDescription);
 }
 
+void RTCPeerConnection::getConfiguration(RTCConfiguration& rtcConfig) {
+  blink::WebRTCConfiguration blinkConfig = m_peerHandler->getConfiguration();
+  rtcConfig = getWebKitRtcConfiguration(blinkConfig);

[dcheng, 2017/03/09 21:36:54]

Nit: reads a bit more naturally to just return the value directly, rather than
assigning to a mutable ref (as this doesn't have any other return values)

[zhihuang1, 2017/03/10 03:20:43]

I agree that the direct return is more nature but it seems that I cannot do
that.
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/scrip...
The RTCConfiguration is a dictionary so the generated file in V8 will expect an
output parameter.

[dcheng, 2017/03/10 19:54:00]

Acknowledged.

+}
+
 void RTCPeerConnection::setConfiguration(
     ScriptState* scriptState,
     const RTCConfiguration& rtcConfiguration,
     ExceptionState& exceptionState) {
   if (throwExceptionIfSignalingStateClosed(m_signalingState, exceptionState))
     return;
 
   WebRTCConfiguration configuration = parseConfiguration(
       scriptState->getExecutionContext(), rtcConfiguration, exceptionState);
 
@@ skipping 691 matching lines @@
 DEFINE_TRACE(RTCPeerConnection) {
   visitor->trace(m_localStreams);
   visitor->trace(m_remoteStreams);
   visitor->trace(m_dispatchScheduledEventRunner);
   visitor->trace(m_scheduledEvents);
   EventTargetWithInlineData::trace(visitor);
   SuspendableObject::trace(visitor);
 }
 
 }  // namespace blink