android: add IceServer.urls field

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

 /*
  *  Copyright 2013 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 81 matching lines @@
      * setRemoteDescription.
      */
     public void onAddTrack(RtpReceiver receiver, MediaStream[] mediaStreams);
   }
 
   /** Java version of PeerConnectionInterface.IceServer. */
   public static class IceServer {
     // 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;
 
     // If the URIs in |urls| only contain IP addresses, this field can be used
     // to indicate the hostname, which may be necessary for TLS (using the SNI
     // extension). If |urls| itself contains the hostname, this isn't
     // necessary.
     public final String hostname;
 
@@ skipping 12 matching lines @@
     }
 
     @Deprecated
     public IceServer(String uri, String username, String password, TlsCertPolicy tlsCertPolicy) {
       this(uri, username, password, tlsCertPolicy, "");
     }
 
     @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;
       this.hostname = hostname;
       this.tlsAlpnProtocols = tlsAlpnProtocols;
     }
 
     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) {
         this.username = username;
         return this;
       }
 
       public Builder setPassword(String password) {
         this.password = password;
         return this;
       }
 
       public Builder setTlsCertPolicy(TlsCertPolicy tlsCertPolicy) {
         this.tlsCertPolicy = tlsCertPolicy;
         return this;
       }
 
       public Builder setHostname(String hostname) {
         this.hostname = hostname;
         return this;
       }
 
       public Builder setTlsAlpnProtocols(List<String> tlsAlpnProtocols) {
         this.tlsAlpnProtocols = tlsAlpnProtocols;
         return this;
       }
 
       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.

       }
     }
   }
 
   /** Java version of PeerConnectionInterface.IceTransportsType */
   public enum IceTransportsType { NONE, RELAY, NOHOST, ALL }
 
   /** Java version of PeerConnectionInterface.BundlePolicy */
   public enum BundlePolicy { BALANCED, MAXBUNDLE, MAXCOMPAT }
 
@@ skipping 282 matching lines @@
   private native RtpSender nativeCreateSender(String kind, String stream_id);
 
   private native List<RtpSender> nativeGetSenders();
 
   private native List<RtpReceiver> nativeGetReceivers();
 
   private native boolean nativeStartRtcEventLog(int file_descriptor, int max_size_bytes);
 
   private native void nativeStopRtcEventLog();
 }