Adding visible networks to the Geolocation Header.

chrome/android/java/src/org/chromium/chrome/browser/omnibox/geo/GeolocationHeader.java

Index: chrome/android/java/src/org/chromium/chrome/browser/omnibox/geo/GeolocationHeader.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/omnibox/geo/GeolocationHeader.java b/chrome/android/java/src/org/chromium/chrome/browser/omnibox/geo/GeolocationHeader.java
index 6691246d77dd159b3d002b38c374c9fa5fc29bdf..4a7abbc1341b8a9a8a2d1b561e443054b92e8467 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/omnibox/geo/GeolocationHeader.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/omnibox/geo/GeolocationHeader.java
@@ -21,11 +21,14 @@ import com.google.protobuf.nano.MessageNano;
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
+import org.chromium.base.VisibleForTesting;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.base.metrics.RecordHistogram;
 import org.chromium.chrome.browser.ChromeFeatureList;
 import org.chromium.chrome.browser.UrlConstants;
+import org.chromium.chrome.browser.omnibox.geo.VisibleNetworks.VisibleCell;
+import org.chromium.chrome.browser.omnibox.geo.VisibleNetworks.VisibleWifi;
 import org.chromium.chrome.browser.preferences.website.ContentSetting;
 import org.chromium.chrome.browser.preferences.website.GeolocationInfo;
 import org.chromium.chrome.browser.preferences.website.WebsitePreferenceBridge;
@@ -34,9 +37,14 @@ import org.chromium.chrome.browser.util.UrlUtilities;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
+import java.util.Arrays;
+import java.util.HashSet;
 import java.util.Locale;
+import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
+import javax.annotation.Nullable;
+
 /**
  * Provides methods for building the X-Geo HTTP header, which provides device location to a server
  * when making an HTTP request.
@@ -148,10 +156,14 @@ public class GeolocationHeader {
             UMA_PERM_NOT_HTTPS})
     public @interface UmaPermission {}
 
-    private static final int LOCATION_SOURCE_HIGH_ACCURACY = 0;
-    private static final int LOCATION_SOURCE_BATTERY_SAVING = 1;
-    private static final int LOCATION_SOURCE_GPS_ONLY = 2;
-    private static final int LOCATION_SOURCE_MASTER_OFF = 3;
+    @VisibleForTesting
+    static final int LOCATION_SOURCE_HIGH_ACCURACY = 0;
+    @VisibleForTesting
+    static final int LOCATION_SOURCE_BATTERY_SAVING = 1;
+    @VisibleForTesting
+    static final int LOCATION_SOURCE_GPS_ONLY = 2;
+    @VisibleForTesting
+    static final int LOCATION_SOURCE_MASTER_OFF = 3;
     @Retention(RetentionPolicy.SOURCE)
     @IntDef({LOCATION_SOURCE_HIGH_ACCURACY, LOCATION_SOURCE_BATTERY_SAVING,
             LOCATION_SOURCE_GPS_ONLY, LOCATION_SOURCE_MASTER_OFF})
@@ -191,12 +203,27 @@ public class GeolocationHeader {
     /** The location descriptor prefix used in the X-Geo header to specify a proto wire encoding */
     private static final String LOCATION_PROTO_PREFIX = "w ";
 
+    /** The location descriptor separator used in the X-Geo header to upload multiple descriptors */
+    private static final String LOCATION_PROTO_SEPARATOR = " ";
+
     /** The location descriptor prefix used in the X-Geo header to specify an ASCII encoding */
     private static final String LOCATION_ASCII_PREFIX = "a ";
 
     /** The time of the first location refresh. Contains Long.MAX_VALUE if not set. */
     private static long sFirstLocationTime = Long.MAX_VALUE;
 
+    /** Present in WiFi SSID that should not be mapped */
+    private static final String SSID_NOMAP = "_nomap";
+
+    /** Present in WiFi SSID that opted out */
+    private static final String SSID_OPTOUT = "_optout";
+
+    private static int sLocationSourceForTesting;
+    private static boolean sUseLocationSourceForTesting;
+
+    private static boolean sAppPermissionGrantedForTesting;
+    private static boolean sUseAppPermissionGrantedForTesting;
+
     /**
      * Requests a location refresh so that a valid location will be available for constructing
      * an X-Geo header in the near future (i.e. within 5 minutes).
@@ -209,6 +236,12 @@ public class GeolocationHeader {
         }
         GeolocationTracker.refreshLastKnownLocation(
                 ContextUtils.getApplicationContext(), REFRESH_LOCATION_AGE);
+
+        // Only refresh visible networks if enabled.
+        if (ChromeFeatureList.isEnabled(ChromeFeatureList.XGEO_VISIBLE_NETWORKS)) {
+            VisibleNetworksTracker.refreshLastKnownVisibleNetworks(
+                    ContextUtils.getApplicationContext(), REFRESH_LOCATION_AGE);
+        }
     }
 
     /**
@@ -261,8 +294,8 @@ public class GeolocationHeader {
      */
     public static String getGeoHeader(String url, Tab tab) {

[dougt, 2017/05/18 02:33:15]

This method is getting a bit unwieldy.  We should follow up and attempt to
refactor.

[lbargu, 2017/05/18 15:14:09]

Agree. Already added all new functionality to private methods, but added a TODO.
Will fix in a follow up CL.

         boolean isIncognito = tab.isIncognito();
-        boolean locationAttached = true;
-        Location location = null;
+        Location locationToAttach = null;
+        VisibleNetworks visibleNetworksToAttach = null;
         long locationAge = Long.MAX_VALUE;
         @HeaderState int headerState = geoHeaderStateForUrl(url, isIncognito, true);
         boolean isXGeoVisibleNetworksEnabled =
@@ -271,20 +304,37 @@ public class GeolocationHeader {
             // Only send X-Geo header if there's a fresh location available.
             // Use flag controlling visible network changes to decide whether GPS location should be
             // included as a fallback.
-            location = GeolocationTracker.getLastKnownLocation(
+            locationToAttach = GeolocationTracker.getLastKnownLocation(

[dougt, 2017/05/18 02:33:15]

Since this is on the hot path, I think we should collect how long this call
takes.

[lbargu, 2017/05/18 15:14:08]

Added TODO. Will add metrics in follow up.
For now, I guess we should be able to rely on overall performance metrics.

                     ContextUtils.getApplicationContext(), isXGeoVisibleNetworksEnabled);
-            if (location == null) {
+            if (locationToAttach == null) {
                 recordHistogram(UMA_LOCATION_NOT_AVAILABLE);
-                locationAttached = false;
             } else {
-                locationAge = GeolocationTracker.getLocationAge(location);
+                locationAge = GeolocationTracker.getLocationAge(locationToAttach);
                 if (locationAge > MAX_LOCATION_AGE) {
+                    // Do not attach the location
                     recordHistogram(UMA_LOCATION_STALE);
-                    locationAttached = false;
+                    locationToAttach = null;
+                } else {
+                    recordHistogram(UMA_HEADER_SENT);
+                }
+            }
+
+            // The header state is enabled, so this means we have app permissions, and the url is
+            // allowed to receive location. Before attempting to attach visible networks, check if
+            // the device location settigs are in High Accuracy or Battery Saving, meaning the
+            // device is allowed to use network-based location.
+            if (isXGeoVisibleNetworksEnabled
+                    && (getLocationSource() == LOCATION_SOURCE_HIGH_ACCURACY
+                               || getLocationSource() == LOCATION_SOURCE_BATTERY_SAVING)) {
+                if (locationToAttach == null
+                        || GeolocationTracker.getLocationAge(locationToAttach)
+                                > REFRESH_LOCATION_AGE) {

[dougt, 2017/05/18 02:33:15]

I would have turned this logic around a bit and just have a longer if stm.


if (isXGeoVisibleNetworksEnabled && locationToAttach != null &&
(getLocationSource() == LOCATION_SOURCE_HIGH_ACCURACY || getLocationSource() ==
LOCATION_SOURCE_BATTERY_SAVING) &&
GeolocationTracker.getLocationAge(locationToAttach) > REFRESH_LOCATION_AGE {
....
}

I would probably also just factor this out into a private mthod that returns a
VisibleNetworks object (or null) in a follow up?

[lbargu, 2017/05/18 15:14:09]

Moved checks to private methods. Should be easier to read now.

+                    // We should attach visible networks in this case, since permissions are good
+                    // but we didn't attach location or it's older than the refresh time.

[dougt, 2017/05/18 02:33:15]

no need for this comment.

[lbargu, 2017/05/18 15:14:08]

Done.

+                    visibleNetworksToAttach = VisibleNetworksTracker.getLastKnownVisibleNetworks(
+                            ContextUtils.getApplicationContext());
                 }
             }
-        } else {
-            locationAttached = false;
         }
 
         @LocationSource int locationSource = getLocationSource();
@@ -292,8 +342,8 @@ public class GeolocationHeader {
         @Permission int domainPermission = getDomainPermission(url, isIncognito);
 
         // Record the permission state with a histogram.
-        recordPermissionHistogram(
-                locationSource, appPermission, domainPermission, locationAttached, headerState);
+        recordPermissionHistogram(locationSource, appPermission, domainPermission,
+                locationToAttach != null, headerState);
 
         if (locationSource != LOCATION_SOURCE_MASTER_OFF && appPermission != PERMISSION_BLOCKED
                 && domainPermission != PERMISSION_BLOCKED && !isIncognito) {
@@ -303,60 +353,54 @@ public class GeolocationHeader {
                     ? 0
                     : SystemClock.elapsedRealtime() - sFirstLocationTime;
             // Record the Time Listening with a histogram.
-            recordTimeListeningHistogram(locationSource, locationAttached, duration);
+            recordTimeListeningHistogram(locationSource, locationToAttach != null, duration);
         }
 
-        // Note that strictly speaking "location == null" is not needed here as the
-        // logic above prevents location being null when locationAttached is true.
-        // It is here to prevent problems if the logic above is changed.
-        if (!locationAttached || location == null) return null;
-
-        recordHistogram(UMA_HEADER_SENT);
-
-        // Timestamp in microseconds since the UNIX epoch.
-        long timestamp = location.getTime() * 1000;
-        // Latitude times 1e7.
-        int latitudeE7 = (int) (location.getLatitude() * 10000000);
-        // Longitude times 1e7.
-        int longitudeE7 = (int) (location.getLongitude() * 10000000);
-        // Radius of 68% accuracy in mm.
-        int radius = (int) (location.getAccuracy() * 1000);
-
-        // Encode location using ascii protobuf format followed by base64 encoding.
-        // https://goto.google.com/partner_location_proto
-        String locationAscii = String.format(Locale.US,
-                "role:1 producer:12 timestamp:%d latlng{latitude_e7:%d longitude_e7:%d} radius:%d",
-                timestamp, latitudeE7, longitudeE7, radius);
-        String locationAsciiEncoding =
-                new String(Base64.encode(locationAscii.getBytes(), Base64.NO_WRAP));
 
         if (!isXGeoVisibleNetworksEnabled) {
-            return XGEO_HEADER_PREFIX + LOCATION_ASCII_PREFIX + locationAsciiEncoding;
+            if (locationToAttach != null) {

[dougt, 2017/05/18 02:33:15]

nit: avoid the block, and just return early:

if (locationToAttach == null) return null;

[lbargu, 2017/05/18 15:14:09]

Done.

+                // Timestamp in microseconds since the UNIX epoch.
+                long timestamp = locationToAttach.getTime() * 1000;
+                // Latitude times 1e7.
+                int latitudeE7 = (int) (locationToAttach.getLatitude() * 10000000);
+                // Longitude times 1e7.
+                int longitudeE7 = (int) (locationToAttach.getLongitude() * 10000000);
+                // Radius of 68% accuracy in mm.
+                int radius = (int) (locationToAttach.getAccuracy() * 1000);
+
+                // Encode location using ascii protobuf format followed by base64 encoding.
+                // https://goto.google.com/partner_location_proto
+                String locationAscii = String.format(Locale.US,
+                        "role:1 producer:12 timestamp:%d latlng{latitude_e7:%d longitude_e7:%d}"
+                                + " radius:%d",
+                        timestamp, latitudeE7, longitudeE7, radius);
+                String locationAsciiEncoding =
+                        new String(Base64.encode(locationAscii.getBytes(), Base64.NO_WRAP));
+                return XGEO_HEADER_PREFIX + LOCATION_ASCII_PREFIX + locationAsciiEncoding;
+            }
+            return null;
         }
 
-        // Create a LatLng for the coordinates.
-        PartnerLocationDescriptor.LatLng latlng = new PartnerLocationDescriptor.LatLng();
-        latlng.latitudeE7 = latitudeE7;
-        latlng.longitudeE7 = longitudeE7;
+        // Proto encoding
+        String locationProtoEncoding = encodeProtoLocation(locationToAttach);
+        String visibleNetworksProtoEncoding = encodeProtoVisibleNetworks(visibleNetworksToAttach);
 
-        // Populate a LocationDescriptor with the LatLng.
-        PartnerLocationDescriptor.LocationDescriptor locationDescriptor =
-                new PartnerLocationDescriptor.LocationDescriptor();
-        locationDescriptor.latlng = latlng;
-        // Include role, producer, timestamp and radius.
-        locationDescriptor.role = PartnerLocationDescriptor.CURRENT_LOCATION;
-        locationDescriptor.producer = PartnerLocationDescriptor.DEVICE_LOCATION;
-        locationDescriptor.timestamp = timestamp;
-        locationDescriptor.radius = (float) radius;
-
-        String locationProtoEncoding = Base64.encodeToString(
-                MessageNano.toByteArray(locationDescriptor), Base64.NO_WRAP | Base64.URL_SAFE);
-
-        return XGEO_HEADER_PREFIX + LOCATION_PROTO_PREFIX + locationProtoEncoding;
+        if (locationProtoEncoding != null) {
+            if (visibleNetworksProtoEncoding != null) {
+                return XGEO_HEADER_PREFIX + LOCATION_PROTO_PREFIX + locationProtoEncoding
+                        + LOCATION_PROTO_SEPARATOR + visibleNetworksProtoEncoding;
+            }
+            return XGEO_HEADER_PREFIX + LOCATION_PROTO_PREFIX + locationProtoEncoding;
+        }

[dougt, 2017/05/18 02:33:15]

Would something like the following work?

if (locationProtoEncoding != null) {
  String result = XGEO_HEADER_PREFIX + LOCATION_PROTO_PREFIX +
locationProtoEncoding;
  if (visibleNetworksProtoEncoding != null)
    result += LOCATION_PROTO_SEPARATOR + visibleNetworksProtoEncoding;
  return result;
}

[Ted C, 2017/05/18 14:43:03]

We should use a StringBuilder if we go with this approach.

If we dropped the trailing space from the proto prefix, could we just do:
if (locaitonProtoEncoding == null && visibleNetworksProtoEncoding == null)
return null;

StringBuilder header = new StringBuilder(XGEO_HEADER_PREFIX +
LOCATION_PROTO_PREFIX);
if (locationProtoEncoding != null) {
    header.append(LOCATION_PROTO_SEPARATOR).append(locationProtoEncoding);
}
if (visibleNetworksProtoEncoding != null) {
   
header.append(LOCATION_PROTO_SEPARATOR).append(visibleNetworksProtoEncoding);
}

If we can't drop the trailing space, then it gets slightly more annoying where
we will need to check locaitonProtoEncoding != null in the second if and only
add the separator in that case.

[lbargu, 2017/05/18 15:14:08]

Done.

[lbargu, 2017/05/18 15:14:09]

Done with builder.

+        if (visibleNetworksProtoEncoding != null) {
+            return XGEO_HEADER_PREFIX + LOCATION_PROTO_PREFIX + visibleNetworksProtoEncoding;
+        }
+        return null;
     }
 
     @CalledByNative
     static boolean hasGeolocationPermission() {
+        if (sUseAppPermissionGrantedForTesting) return sAppPermissionGrantedForTesting;

[Ted C, 2017/05/18 14:43:03]

can we use
http://robolectric.org/javadoc/latest/org/robolectric/shadows/ShadowPackageMa...

instead of introducing this static?

[lbargu, 2017/05/18 15:14:08]

It's not available in chromium source code?

[Ted C, 2017/05/18 15:25:29]

Indeed it is not...sadness.

Looking at what is in the codebase:
http://robolectric.org/javadoc/2.4/org/robolectric/res/builder/RobolectricPac...

It doesn't look like what we'll need.  The only other option would be to build
our entire own mock context and set that on ContextUtils.  I have no idea the
implications that would have on the rest of the change.  It is one of those
things that I think would be worth 5 min of investigation an no more.

[lbargu, 2017/05/18 15:56:28]

Tried, but a time drainer, no success :(

         int pid = Process.myPid();
         int uid = Process.myUid();
         if (ApiCompatibilityUtils.checkPermission(ContextUtils.getApplicationContext(),
@@ -384,6 +428,9 @@ public class GeolocationHeader {
      */
     @Permission
     static int getGeolocationPermission(Tab tab) {
+        if (sUseAppPermissionGrantedForTesting) {
+            return sAppPermissionGrantedForTesting ? PERMISSION_GRANTED : PERMISSION_BLOCKED;
+        }
         if (hasGeolocationPermission()) return PERMISSION_GRANTED;
         return tab.getWindowAndroid().canRequestPermission(
                        Manifest.permission.ACCESS_COARSE_LOCATION)
@@ -416,6 +463,18 @@ public class GeolocationHeader {
         return locationPermission;
     }
 
+    @VisibleForTesting
+    static void setLocationSourceForTesting(int locationSourceForTesting) {
+        sLocationSourceForTesting = locationSourceForTesting;
+        sUseLocationSourceForTesting = true;
+    }
+
+    @VisibleForTesting
+    static void setAppPermissionGrantedForTesting(boolean appPermissionGrantedForTesting) {
+        sAppPermissionGrantedForTesting = appPermissionGrantedForTesting;
+        sUseAppPermissionGrantedForTesting = true;
+    }
+
     /** Records a data point for the Geolocation.HeaderSentOrNot histogram. */
     private static void recordHistogram(int result) {
         RecordHistogram.recordEnumeratedHistogram("Geolocation.HeaderSentOrNot", result, UMA_MAX);
@@ -426,6 +485,8 @@ public class GeolocationHeader {
     // We should replace our usage of LOCATION_PROVIDERS_ALLOWED when the min API is 19.
     @SuppressWarnings("deprecation")
     private static int getLocationSource() {
+        if (sUseLocationSourceForTesting) return sLocationSourceForTesting;
+
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) {
             int locationMode;
             try {
@@ -675,4 +736,187 @@ public class GeolocationHeader {
         RecordHistogram.recordCustomCountHistogram(
                 name, duration, 1, LOCATION_AGE_HISTOGRAM_MAX_SECONDS, 50);
     }
+
+    /**
+     * Encodes location into proto encoding.
+     */
+    @Nullable
+    @VisibleForTesting
+    static String encodeProtoLocation(@Nullable Location location) {
+        if (location == null) {
+            // No data to encode.
+            return null;
+        }
+        // Timestamp in microseconds since the UNIX epoch.
+        long timestamp = location.getTime() * 1000;
+        // Latitude times 1e7.
+        int latitudeE7 = (int) (location.getLatitude() * 10000000);
+        // Longitude times 1e7.
+        int longitudeE7 = (int) (location.getLongitude() * 10000000);
+        // Radius of 68% accuracy in mm.
+        int radius = (int) (location.getAccuracy() * 1000);
+
+        // Create a LatLng for the coordinates.
+        PartnerLocationDescriptor.LatLng latlng = new PartnerLocationDescriptor.LatLng();
+        latlng.latitudeE7 = latitudeE7;
+        latlng.longitudeE7 = longitudeE7;
+
+        // Populate a LocationDescriptor with the LatLng.
+        PartnerLocationDescriptor.LocationDescriptor locationDescriptor =
+                new PartnerLocationDescriptor.LocationDescriptor();
+        locationDescriptor.latlng = latlng;
+        // Include role, producer, timestamp and radius.
+        locationDescriptor.role = PartnerLocationDescriptor.CURRENT_LOCATION;
+        locationDescriptor.producer = PartnerLocationDescriptor.DEVICE_LOCATION;
+        locationDescriptor.timestamp = timestamp;
+        locationDescriptor.radius = (float) radius;
+        return encodeLocationDescriptor(locationDescriptor);
+    }
+
+    /**
+     * Encodes the given proto location descriptor into a BASE64 URL_SAFE encoding.
+     */
+    private static String encodeLocationDescriptor(
+            PartnerLocationDescriptor.LocationDescriptor locationDescriptor) {
+        return Base64.encodeToString(
+                MessageNano.toByteArray(locationDescriptor), Base64.NO_WRAP | Base64.URL_SAFE);
+    }
+
+    /**
+     * Encodes visible networks in proto encoding.
+     */
+    @Nullable
+    @VisibleForTesting
+    static String encodeProtoVisibleNetworks(@Nullable VisibleNetworks visibleNetworks) {
+        VisibleNetworks visibleNetworksToEncode = trimVisibleNetworks(visibleNetworks);
+        if (visibleNetworksToEncode == null) {
+            // No data to encode.
+            return null;
+        }
+        VisibleWifi connectedWifi = visibleNetworksToEncode.connectedWifi();
+        VisibleCell connectedCell = visibleNetworksToEncode.connectedCell();
+        Set<VisibleWifi> visibleWifis = visibleNetworksToEncode.allVisibleWifis();
+        Set<VisibleCell> visibleCells = visibleNetworksToEncode.allVisibleCells();
+
+        int numVisibleNetworks = (connectedWifi != null ? 1 : 0)
+                + (visibleWifis != null ? visibleWifis.size() : 0) + (connectedCell != null ? 1 : 0)
+                + (visibleCells != null ? visibleCells.size() : 0);
+        if (numVisibleNetworks == 0) {
+            // No data to encode.
+            return null;
+        }
+
+        int i = 0;
+        PartnerLocationDescriptor.VisibleNetwork[] protoNetworks =
+                new PartnerLocationDescriptor.VisibleNetwork[numVisibleNetworks];
+        if (connectedWifi != null) {
+            protoNetworks[i++] = connectedWifi.toProto(true);
+        }
+        if (visibleWifis != null) {
+            for (VisibleWifi visibleWifi : visibleWifis) {
+                protoNetworks[i++] = visibleWifi.toProto(false);
+            }
+        }
+        if (connectedCell != null) {
+            protoNetworks[i++] = connectedCell.toProto(true);
+        }
+        if (visibleCells != null) {
+            for (VisibleCell visibleCell : visibleCells) {
+                protoNetworks[i++] = visibleCell.toProto(false);
+            }
+        }
+
+        PartnerLocationDescriptor.LocationDescriptor locationDescriptor =
+                new PartnerLocationDescriptor.LocationDescriptor();
+        locationDescriptor.role = PartnerLocationDescriptor.CURRENT_LOCATION;
+        locationDescriptor.producer = PartnerLocationDescriptor.DEVICE_LOCATION;
+        locationDescriptor.visibleNetwork = protoNetworks;
+
+        return encodeLocationDescriptor(locationDescriptor);
+    }
+
+    @Nullable
+    @VisibleForTesting
+    static VisibleNetworks trimVisibleNetworks(@Nullable VisibleNetworks visibleNetworks) {
+        if (visibleNetworks == null || visibleNetworks.isEmpty()) {
+            return null;
+        }
+        // Trim visible networks to only include a limited number of visible not-conntected networks
+        // based on flag.
+        VisibleCell connectedCell = visibleNetworks.connectedCell();
+        VisibleWifi connectedWifi = visibleNetworks.connectedWifi();
+        Set<VisibleCell> visibleCells = visibleNetworks.allVisibleCells();
+        Set<VisibleWifi> visibleWifis = visibleNetworks.allVisibleWifis();
+        VisibleCell extraVisibleCell = null;
+        VisibleWifi extraVisibleWifi = null;
+        if (shouldExcludeVisibleWifi(connectedWifi)) {
+            // Trim the connected wifi.
+            connectedWifi = null;
+        }
+        // Select the extra visible cell.
+        if (visibleCells != null) {
+            for (VisibleCell candidateCell : visibleCells) {
+                if (ApiCompatibilityUtils.objectEquals(connectedCell, candidateCell)) {
+                    // Do not include this candidate cell, since its already the connected one.
+                    continue;
+                }
+                // Add it and since we only want one, stop iterating over other cells.
+                extraVisibleCell = candidateCell;
+                break;
+            }
+        }
+        // Select the extra visible wifi.
+        if (visibleWifis != null) {
+            for (VisibleWifi candidateWifi : visibleWifis) {
+                if (shouldExcludeVisibleWifi(candidateWifi)) {
+                    // Do not include this candidate wifi.
+                    continue;
+                }
+                if (ApiCompatibilityUtils.objectEquals(connectedWifi, candidateWifi)) {
+                    // Replace the connected, since the candidate will have level. This is because
+                    // the android APIs exposing connected WIFI do not expose level, while the ones
+                    // exposing visible wifis expose level.
+                    connectedWifi = candidateWifi;
+                    // Do not include this candidate wifi, since its already the connected one.
+                    continue;
+                }
+                // Keep the one with stronger level (since it's negative, this is the smaller value)
+                if (extraVisibleWifi == null || extraVisibleWifi.level() > candidateWifi.level()) {
+                    extraVisibleWifi = candidateWifi;
+                }
+            }
+        }
+
+        if (connectedCell == null && connectedWifi == null && extraVisibleCell == null
+                && extraVisibleWifi == null) {
+            return null;
+        }
+
+        return VisibleNetworks.create(connectedWifi, connectedCell,
+                extraVisibleWifi != null ? new HashSet<>(Arrays.asList(extraVisibleWifi)) : null,

[Ted C, 2017/05/18 14:43:03]

I would use newHashSet here

https://cs.chromium.org/chromium/src/base/android/java/src/org/chromium/base/...

[lbargu, 2017/05/18 15:14:09]

Done.

+                extraVisibleCell != null ? new HashSet<>(Arrays.asList(extraVisibleCell)) : null);
+    }
+
+    /**
+     * Returns whether the provided {@link VisibleWifi} should be excluded. This can happen if the
+     * network is opted out (ssid contains "_nomap" or "_optout").
+     */
+    private static boolean shouldExcludeVisibleWifi(@Nullable VisibleWifi visibleWifi) {
+        if (visibleWifi == null) {
+            return true;
+        }
+        String ssid = visibleWifi.ssid();
+        String bssid = visibleWifi.bssid();
+        if (ssid == null) {
+            // No ssid, so the networks is not opted out and should not be excluded.
+            return false;
+        }
+        // Optimization to avoid costly toLowerCase() in most cases.
+        if (ssid.indexOf('_') < 0) {
+            // No "_nomap
+            return false;
+        }
+        String ssidLowerCase = ssid.toLowerCase(Locale.ENGLISH);
+        return ssidLowerCase.contains(SSID_NOMAP) || ssidLowerCase.contains(SSID_OPTOUT);
+    }
 }