android: add IceServer.urls field

webrtc/sdk/android/api/org/webrtc/PeerConnection.java

Index: webrtc/sdk/android/api/org/webrtc/PeerConnection.java
diff --git a/webrtc/sdk/android/api/org/webrtc/PeerConnection.java b/webrtc/sdk/android/api/org/webrtc/PeerConnection.java
index 832241097406e06e0c1f97c65a86fabf7f7f23c5..02f9a8989ede26faa2d1a4cd3cba8d198cc2021f 100644
--- a/webrtc/sdk/android/api/org/webrtc/PeerConnection.java
+++ b/webrtc/sdk/android/api/org/webrtc/PeerConnection.java
@@ -99,7 +99,8 @@ public class PeerConnection {
     // List of URIs associated with this server. Valid formats are described
     // in RFC7064 and RFC7065, and more may be added in the future. The "host"
     // part of the URI may contain either an IP address or a hostname.
-    public final String uri;
+    @Deprecated public final String uri;
+    public final List<String> urls;
     public final String username;
     public final String password;
     public final TlsCertPolicy tlsCertPolicy;
@@ -132,12 +133,16 @@ public class PeerConnection {
     @Deprecated
     public IceServer(String uri, String username, String password, TlsCertPolicy tlsCertPolicy,
         String hostname) {
-      this(uri, username, password, tlsCertPolicy, hostname, null);
+      this(uri, null, username, password, tlsCertPolicy, hostname, null);

[sakal, 2017/09/07 07:46:36]

Collections.singletonList(uri) instead of null

[korniltsev, 2017/09/07 20:33:46]

Done.

     }
 
-    private IceServer(String uri, String username, String password, TlsCertPolicy tlsCertPolicy,
-        String hostname, List<String> tlsAlpnProtocols) {
+    private IceServer(String uri, List<String> urls, String username, String password,
+        TlsCertPolicy tlsCertPolicy, String hostname, List<String> tlsAlpnProtocols) {
+      if (uri == null && urls == null) {

[sakal, 2017/09/07 07:46:36]

I don't think either of them should be null, use ||. Also add the check for
username, password and hostname.

[korniltsev, 2017/09/07 20:33:46]

Done. Also added urls.isEmpty() check. Also checked urls items are not null.

+        throw new NullPointerException("uri == null && urls == null");

[sakal, 2017/09/07 07:46:36]

IllegalArgumentException instead of NullPointerException

[korniltsev, 2017/09/07 20:33:46]

Done.

+      }
       this.uri = uri;
+      this.urls = urls;
       this.username = username;
       this.password = password;
       this.tlsCertPolicy = tlsCertPolicy;
@@ -146,24 +151,29 @@ public class PeerConnection {
     }
 
     public String toString() {
-      return uri + " [" + username + ":" + password + "] [" + tlsCertPolicy + "] [" + hostname
-          + "] [" + tlsAlpnProtocols + "]";
+      return uri != null ? uri

[sakal, 2017/09/07 07:46:36]

Ignore uri here and just print urls. Ensure that List.toString returns a
sensible presentation of the urls.

[korniltsev, 2017/09/07 20:33:46]

Not quite sure about the second part. I know ArrayList and LinkedList return 
proper representation, but there are probably other implementations which
don't and I don't understand how can I ensure it. 

[sakal, 2017/09/08 08:22:54]

It's enough as long as this works in practice with implementation that we use.

+                         : urls + " [" + username + ":" + password + "] [" + tlsCertPolicy + "] ["
+              + hostname + "] [" + tlsAlpnProtocols + "]";
     }
 
     public static Builder builder(String uri) {
-      return new Builder(uri);
+      return new Builder(Collections.singletonList(uri));
+    }
+
+    public static Builder builder(List<String> urls) {
+      return new Builder(urls);

[sakal, 2017/09/07 07:46:36]

Throw IllegalArgumentException if urls doesn't contain at least one url or it is
null.

[korniltsev, 2017/09/07 20:33:46]

The check is inside IceServer's constructor. Should I duplicate it so it fails
faster?

[sakal, 2017/09/08 08:22:54]

urls.isEmpty will throw a null pointer exception now if urls is null. We need to
check it here because otherwise urls.get(0) will throw. Can you add a check for
urls == null || urls.isEmpty()

     }
 
     public static class Builder {
-      private String uri;
+      private final List<String> urls;
       private String username = "";
       private String password = "";
       private TlsCertPolicy tlsCertPolicy = TlsCertPolicy.TLS_CERT_POLICY_SECURE;
       private String hostname = "";
       private List<String> tlsAlpnProtocols;
 
-      private Builder(String uri) {
-        this.uri = uri;
+      private Builder(List<String> urls) {
+        this.urls = urls;
       }
 
       public Builder setUsername(String username) {
@@ -192,7 +202,8 @@ public class PeerConnection {
       }
 
       public IceServer createIceServer() {
-        return new IceServer(uri, username, password, tlsCertPolicy, hostname, tlsAlpnProtocols);
+        return new IceServer(
+            null, urls, username, password, tlsCertPolicy, hostname, tlsAlpnProtocols);

[sakal, 2017/09/07 07:46:36]

Pass in urls.get(0) instead of null.

[korniltsev, 2017/09/07 20:33:46]

Done.

       }
     }
   }