Remove org.openqa.selenium.net.INetAddress, an unnecessary abstraction

around java.net.InetAddress. Not only is INetAddress just not necessary,
its constructor triggers a DNS lookup.

These DNS lookups cannot be avoided when creating a FirefoxDriver instance,
as the code is referenced through a static initializer in
org.openqa.selenium.firefox.internal.NewProfileExtensionConnection.

By removing INetAddress, not only is the code simpler, but the time to
start the FirefoxDriver for the first time on Windows is reduced by up to
30s (depending on the machine's network config)

Author: jleyba

java/client/src/org/openqa/selenium/net/INetAddress.java

@@ -16,25 +16,26 @@
  */
 package org.openqa.selenium.net;
 
+import static java.util.Collections.list;
+
+import com.google.common.collect.Iterables;
+
+import java.net.Inet6Address;
 import java.net.InetAddress;
 import java.net.SocketException;
-import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
-import java.util.Enumeration;
 import java.util.Iterator;
-import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
 public class NetworkInterface {
 
   private final String name;
-  private final Iterable<INetAddress> inetAddresses;
+  private final Iterable<InetAddress> inetAddresses;
   private boolean isLoopback;
 
   public NetworkInterface(java.net.NetworkInterface networkInterface) {
-    this(networkInterface.getName(), asIterableAddr(networkInterface.getInetAddresses()));
+    this(networkInterface.getName(), list(networkInterface.getInetAddresses()));
     try {
       // Issue 1181 : determine whether this NetworkInterface instance is loopback
       // from java.net.NetworkInterface API
@@ -44,18 +45,18 @@ public NetworkInterface(java.net.NetworkInterface networkInterface) {
       // If an SocketException is caught, determine whether this NetworkInterface
       // instance is loopback from computation from its inetAddresses
       this.isLoopback =
-          isLoopBackFromINetAddresses(asIterableAddr(networkInterface.getInetAddresses()));
+          isLoopBackFromINetAddresses(list(networkInterface.getInetAddresses()));
     }
   }
 
-  NetworkInterface(String name, Iterable<INetAddress> inetAddresses) {
+  NetworkInterface(String name, Iterable<InetAddress> inetAddresses) {
     this.name = name;
-    this.inetAddresses = inetAddresses;
+    this.inetAddresses = Iterables.unmodifiableIterable(inetAddresses);
   }
 
-  NetworkInterface(String name, INetAddress... inetAddresses) {
+  NetworkInterface(String name, InetAddress... inetAddresses) {
     this(name, Arrays.asList(inetAddresses));
-    this.isLoopback = isLoopBackFromINetAddresses(Arrays.asList(inetAddresses));
+    this.isLoopback = isLoopBackFromINetAddresses(this.inetAddresses);
   }
 
   public boolean isIp4AddressBindingOnly() {
@@ -66,63 +67,59 @@ public boolean isLoopBack() {
     return isLoopback;
   }
 
-  public final boolean isLoopBackFromINetAddresses(Iterable<INetAddress> inetAddresses) {
+  private boolean isLoopBackFromINetAddresses(Iterable<InetAddress> inetAddresses) {
     // Let's hope there's no such thing as network interfaces with mixed addresses ;)
-    Iterator<INetAddress> iterator = inetAddresses.iterator();
+    Iterator<InetAddress> iterator = inetAddresses.iterator();
     return iterator.hasNext() && iterator.next().isLoopbackAddress();
   }
 
-  public INetAddress getIp4LoopbackOnly() {
+  public InetAddress getIp4LoopbackOnly() {
     // Goes by the wildly unscientific assumption that if there are more than one set of
     // loopback addresses, firefox will bind to the last one we get.
     // An alternate theory if this fails is that firefox prefers 127.0.0.1
     // Most "normal" boxes don't have multiple addresses so we'll just refine this
     // algorithm until it works.
     // See NetworkUtilsTest#testOpenSuseBoxIssue1181
-    INetAddress lastFound = null;
+    InetAddress lastFound = null;
     // Issue 1181
     if (!isLoopback) {
       return lastFound;
     }
-    for (INetAddress inetAddress : inetAddresses) {
-      if (inetAddress.isLoopbackAddress() && inetAddress.isIPv4Address()) {
+    for (InetAddress inetAddress : inetAddresses) {
+      if (inetAddress.isLoopbackAddress() && !isIpv6(inetAddress)) {
         lastFound = inetAddress;
       }
     }
     return lastFound;
   }
 
-  public INetAddress getIp4NonLoopBackOnly() {
-    for (INetAddress inetAddress : inetAddresses) {
-      if (!inetAddress.isLoopbackAddress() && inetAddress.isIPv4Address()) {
+  static boolean isIpv6(InetAddress address) {
+    return address instanceof Inet6Address;
+  }
+
+  public InetAddress getIp4NonLoopBackOnly() {
+    for (InetAddress inetAddress : inetAddresses) {
+      if (!inetAddress.isLoopbackAddress() && !isIpv6(inetAddress)) {
         return inetAddress;
       }
     }
     return null;
   }
 
-  public INetAddress getIp6Address() {
-    for (INetAddress inetAddress : inetAddresses) {
-      if (inetAddress.isIPv6Address()) {
+  public InetAddress getIp6Address() {
+    for (InetAddress inetAddress : inetAddresses) {
+      if (isIpv6(inetAddress)) {
         return inetAddress;
       }
     }
     return null;
   }
 
-  public Iterable<INetAddress> getInetAddresses() {
+  public Iterable<InetAddress> getInetAddresses() {
     return inetAddresses;
   }
 
   public String getName() {
     return name;
   }
-
-  static Iterable<INetAddress> asIterableAddr(Enumeration<InetAddress> tEnumeration) {
-    List<INetAddress> result = new ArrayList<INetAddress>();
-    while (tEnumeration.hasMoreElements()) {
-      result.add(new INetAddress(tEnumeration.nextElement()));
-    }
-    return Collections.unmodifiableList(result);
-  }
 }

java/client/src/org/openqa/selenium/net/NetworkInterface.java

@@ -15,9 +15,13 @@
  */
 package org.openqa.selenium.net;
 
+import static org.openqa.selenium.net.NetworkInterface.isIpv6;
+
 import org.openqa.selenium.Platform;
 import org.openqa.selenium.WebDriverException;
 
+import java.net.Inet6Address;
+import java.net.InetAddress;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
@@ -35,7 +39,7 @@ public NetworkUtils() {
   }
 
   public String getPrivateLocalAddress() {
-    List<INetAddress> addresses = getLocalInterfaceAddress();
+    List<InetAddress> addresses = getLocalInterfaceAddress();
     if (addresses.isEmpty()) {
       return "127.0.0.1";
     }
@@ -47,7 +51,7 @@ public String getPrivateLocalAddress() {
    * Used by the mobile emulators that refuse to access localhost or 127.0.0.1 The IP4/IP6
    * requirements of this method are as-of-yet unspecified, but we return the string that is
    * associated with the IP4 interface
-   * 
+   *
    * @return A String representing the host name or non-loopback IP4 address of this machine.
    */
   public String getNonLoopbackAddressOfThisMachine() {
@@ -56,12 +60,12 @@ public String getNonLoopbackAddressOfThisMachine() {
 
   /**
    * Returns a non-loopback IP4 hostname of the local host.
-   * 
+   *
    * @return A string hostName
    */
-  public INetAddress getIp4NonLoopbackAddressOfThisMachine() {
+  public InetAddress getIp4NonLoopbackAddressOfThisMachine() {
     for (NetworkInterface iface : networkInterfaceProvider.getNetworkInterfaces()) {
-      final INetAddress ip4NonLoopback = iface.getIp4NonLoopBackOnly();
+      final InetAddress ip4NonLoopback = iface.getIp4NonLoopBackOnly();
       if (ip4NonLoopback != null) {
         return ip4NonLoopback;
       }
@@ -73,7 +77,7 @@ public INetAddress getIp4NonLoopbackAddressOfThisMachine() {
    * Returns a single address that is guaranteed to resolve to an ipv4 representation of localhost
    * This may either be a hostname or an ip address, dependending if we can guarantee what that the
    * hostname will resolve to ip4.
-   * 
+   *
    * @return The address part og such an address
    */
   public String obtainLoopbackIp4Address() {
@@ -90,7 +94,7 @@ public String obtainLoopbackIp4Address() {
     if (Platform.getCurrent().is(Platform.UNIX)) {
       NetworkInterface linuxLoopback = networkInterfaceProvider.getLoInterface();
       if (linuxLoopback != null) {
-        final INetAddress netAddress = linuxLoopback.getIp4LoopbackOnly();
+        final InetAddress netAddress = linuxLoopback.getIp4LoopbackOnly();
         if (netAddress != null) {
           return netAddress.getHostAddress();
         }
@@ -104,10 +108,10 @@ public String obtainLoopbackIp4Address() {
   }
 
 
-  private INetAddress grabFirstNetworkAddress() {
+  private InetAddress grabFirstNetworkAddress() {
     NetworkInterface firstInterface =
         networkInterfaceProvider.getNetworkInterfaces().iterator().next();
-    INetAddress firstAddress = null;
+    InetAddress firstAddress = null;
     if (firstInterface != null) {
       firstAddress = firstInterface.getInetAddresses().iterator().next();
     }
@@ -121,7 +125,7 @@ private INetAddress grabFirstNetworkAddress() {
 
   public String getIpOfLoopBackIp4() {
     for (NetworkInterface iface : networkInterfaceProvider.getNetworkInterfaces()) {
-      final INetAddress netAddress = iface.getIp4LoopbackOnly();
+      final InetAddress netAddress = iface.getIp4LoopbackOnly();
       if (netAddress != null) {
         return netAddress.getHostAddress();
       }
@@ -138,13 +142,13 @@ private NetworkInterface getLoopBackAndIp4Only() {
     return null;
   }
 
-  private List<INetAddress> getLocalInterfaceAddress() {
-    List<INetAddress> localAddresses = new ArrayList<INetAddress>();
+  private List<InetAddress> getLocalInterfaceAddress() {
+    List<InetAddress> localAddresses = new ArrayList<InetAddress>();
 
     for (NetworkInterface iface : networkInterfaceProvider.getNetworkInterfaces()) {
-      for (INetAddress addr : iface.getInetAddresses()) {
+      for (InetAddress addr : iface.getInetAddresses()) {
         // filter out Inet6 Addr Entries
-        if (addr.isLoopbackAddress() && !addr.isIPv6Address()) {
+        if (addr.isLoopbackAddress() && !isIpv6(addr))  {
           localAddresses.add(addr);
         }
       }
@@ -156,8 +160,8 @@ private List<INetAddress> getLocalInterfaceAddress() {
     if (Platform.getCurrent().is(Platform.UNIX)) {
       NetworkInterface linuxLoopback = networkInterfaceProvider.getLoInterface();
       if (linuxLoopback != null) {
-        for (INetAddress inetAddress : linuxLoopback.getInetAddresses()) {
-          if (!inetAddress.isIPv6Address()) {
+        for (InetAddress inetAddress : linuxLoopback.getInetAddresses()) {
+          if (!isIpv6(inetAddress)) {
             localAddresses.add(inetAddress);
           }
         }
@@ -197,8 +201,8 @@ private static void dumpToConsole(StringBuilder result, NetworkInterface inNetwo
     dumpAddresses(result, inNetworkInterface.getInetAddresses());
   }
 
-  private static void dumpAddresses(StringBuilder result, Iterable<INetAddress> inetAddresses) {
-    for (INetAddress address : inetAddresses) {
+  private static void dumpAddresses(StringBuilder result, Iterable<InetAddress> inetAddresses) {
+    for (InetAddress address : inetAddresses) {
       result.append("   address.getHostName() = ");
       result.append(address.getHostName());
       result.append("\n");

java/client/src/org/openqa/selenium/net/NetworkUtils.java

@@ -28,6 +28,7 @@
 import org.openqa.selenium.io.ZipTest;
 import org.openqa.selenium.logging.LoggingTest;
 import org.openqa.selenium.logging.PerformanceLoggingMockTest;
+import org.openqa.selenium.net.LinuxEphemeralPortRangeDetectorTest;
 import org.openqa.selenium.net.NetworkUtilsTest;
 import org.openqa.selenium.os.CommandLineTest;
 import org.openqa.selenium.os.WindowsUtilsUnitTest;
@@ -46,6 +47,7 @@
     IgnoreComparitorUnitTest.class,
     IndividualKeyboardActionsTest.class,
     IndividualMouseActionsTest.class,
+    LinuxEphemeralPortRangeDetectorTest.class,
     LoggingTest.class,
     NetworkUtilsTest.class,
     OutputTypeTest.class,

java/client/test/org/openqa/selenium/SmallTests.java

@@ -15,29 +15,38 @@
 limitations under the License.
  */
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.openqa.selenium.Platform.LINUX;
+
 import org.junit.Assume;
+import org.junit.BeforeClass;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
 import org.openqa.selenium.Platform;
 
 import java.io.StringReader;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-import static org.openqa.selenium.Platform.LINUX;
-
+@RunWith(JUnit4.class)
 public class LinuxEphemeralPortRangeDetectorTest {
 
+  @BeforeClass
+  public static void requiresLinux() {
+    Assume.assumeTrue(Platform.getCurrent().is(LINUX));
+  }
+
   @Test
   public void decodeEphemeralPorts() throws Exception {
     String range ="1234 65533";
-    EphemeralPortRangeDetector ephemeralEphemeralPortDetector = new LinuxEphemeralPortRangeDetector(new StringReader(range));
+    EphemeralPortRangeDetector ephemeralEphemeralPortDetector =
+        new LinuxEphemeralPortRangeDetector(new StringReader(range));
     assertEquals( 1234, ephemeralEphemeralPortDetector.getLowestEphemeralPort());
     assertEquals( 65533, ephemeralEphemeralPortDetector.getHighestEphemeralPort());
   }
 
   @Test
   public void currentValues(){
-    Assume.assumeTrue(Platform.getCurrent().is(LINUX));
     LinuxEphemeralPortRangeDetector detector = LinuxEphemeralPortRangeDetector.getInstance();
     assertTrue( detector.getLowestEphemeralPort() > 1024);
     assertTrue( detector.getHighestEphemeralPort() < 65536);