Bug 1018383 - Added request-level caching in NetworkGeolocationProvider.js. r=jdm

This is a cleanup of the patch used in the Tarako bug. I made it clearer that this is request-level caching, NOT position caching.
 This is specifically for preventing unnecessary requests to the server. It is unrelated to the W3C PositionOptions and their effect on position caching.
 Hopefully this detail is clearer in this patch.

Updated mochitests that were relying on old logic to succeed

The failing mochitests were relying on the provider to have a specific behaviour -behaviours that are incidental to the test case. I updated these to reflect the behaviour of the current system.

dom/system/NetworkGeolocationProvider.js

@@ -20,7 +20,7 @@ let gLoggingEnabled = false;
    from the Wifi subsystem.  If this timer fires, we believe the Wifi scan has
    had a problem and we no longer can use Wifi to position the user this time
    around (we will continue to be hopeful that Wifi will recover).
- 
+
    This timeout value is also used when Wifi scanning is disabled (see
    gWifiScanningEnabled).  In this case, we use this timer to collect cell/ip
    data and xhr it to the location server.
@@ -39,6 +39,172 @@ function LOG(aMsg) {
   }
 }
 
+function CachedRequest(loc, cellInfo, wifiList) {
+  this.location = loc;
+
+  let wifis = new Set();
+  if (wifiList) {
+    for (let i = 0; i < wifiList.length; i++) {
+      wifis.add(wifiList[i].macAddress);
+    }
+  }
+
+  // Use only these values for equality
+  // (the JSON will contain additional values in future)
+  function makeCellKey(cell) {
+    return "" + cell.radio + ":" + cell.mobileCountryCode + ":" +
+    cell.mobileNetworkCode + ":" + cell.locationAreaCode + ":" +
+    cell.cellId;
+  }
+
+  let cells = new Set();
+  if (cellInfo) {
+    for (let i = 0; i < cellInfo.length; i++) {
+      cells.add(makeCellKey(cellInfo[i]));
+    }
+  }
+
+  this.hasCells = () => cells.size > 0;
+
+  this.hasWifis = () => wifis.size > 0;
+
+  // if fields match
+  this.isCellEqual = function(cellInfo) {
+    if (!this.hasCells()) {
+      return false;
+    }
+
+    let len1 = cells.size;
+    let len2 = cellInfo.length;
+
+    if (len1 != len2) {
+      LOG("cells not equal len");
+      return false;
+    }
+
+    for (let i = 0; i < len2; i++) {
+      if (!cells.has(makeCellKey(cellInfo[i]))) {
+        return false;
+      }
+    }
+    return true;
+  };
+
+  // if 50% of the SSIDS match
+  this.isWifiApproxEqual = function(wifiList) {
+    if (!this.hasWifis()) {
+      return false;
+    }
+
+    // if either list is a 50% subset of the other, they are equal
+    let common = 0;
+    for (let i = 0; i < wifiList.length; i++) {
+      if (wifis.has(wifiList[i].macAddress)) {
+        common++;
+      }
+    }
+    let kPercentMatch = 0.5;
+    return common >= (Math.max(wifis.size, wifiList.length) * kPercentMatch);
+  };
+
+  this.isGeoip = function() {
+    return !this.hasCells() && !this.hasWifis();
+  };
+
+  this.isCellAndWifi = function() {
+    return this.hasCells() && this.hasWifis();
+  };
+
+  this.isCellOnly = function() {
+    return this.hasCells() && !this.hasWifis();
+  };
+
+  this.isWifiOnly = function() {
+    return this.hasWifis() && !this.hasCells();
+  };
+ }
+
+let gCachedRequest = null;
+let gDebugCacheReasoning = ""; // for logging the caching logic
+
+// This function serves two purposes:
+// 1) do we have a cached request
+// 2) is the cached request better than what newCell and newWifiList will obtain
+// If the cached request exists, and we know it to have greater accuracy
+// by the nature of its origin (wifi/cell/geoip), use its cached location.
+//
+// If there is more source info than the cached request had, return false
+// In other cases, MLS is known to produce better/worse accuracy based on the
+// inputs, so base the decision on that.
+function isCachedRequestMoreAccurateThanServerRequest(newCell, newWifiList)
+{
+  gDebugCacheReasoning = "";
+  let isNetworkRequestCacheEnabled = true;
+  try {
+    // Mochitest needs this pref to simulate request failure
+    isNetworkRequestCacheEnabled = Services.prefs.getBoolPref("geo.wifi.debug.requestCache.enabled");
+    if (!isNetworkRequestCacheEnabled) {
+      gCachedRequest = null;
+    }
+  } catch (e) {}
+
+  if (!gCachedRequest || !isNetworkRequestCacheEnabled) {
+    gDebugCacheReasoning = "No cached data";
+    return false;
+  }
+
+  if (!newCell && !newWifiList) {
+    gDebugCacheReasoning = "New req. is GeoIP.";
+    return true;
+  }
+
+  if (newCell && newWifiList && (gCachedRequest.isCellOnly() || gCachedRequest.isWifiOnly())) {
+    gDebugCacheReasoning = "New req. is cell+wifi, cache only cell or wifi.";
+    return false;
+  }
+
+  if (newCell && gCachedRequest.isWifiOnly()) {
+    // In order to know if a cell-only request should trump a wifi-only request
+    // need to know if wifi is low accuracy. >5km would be VERY low accuracy,
+    // it is worth trying the cell
+    var isHighAccuracyWifi = gCachedRequest.location.coords.accuracy < 5000;
+    gDebugCacheReasoning = "Req. is cell, cache is wifi, isHigh:" + isHighAccuracyWifi;
+    return isHighAccuracyWifi;
+  }
+
+  let hasEqualCells = false;
+  if (newCell) {
+    hasEqualCells = gCachedRequest.isCellEqual(newCell);
+  }
+
+  let hasEqualWifis = false;
+  if (newWifiList) {
+    hasEqualWifis = gCachedRequest.isWifiApproxEqual(newWifiList);
+  }
+
+  gDebugCacheReasoning = "EqualCells:" + hasEqualCells + " EqualWifis:" + hasEqualWifis;
+
+  if (gCachedRequest.isCellOnly()) {
+    gDebugCacheReasoning += ", Cell only.";
+    if (hasEqualCells) {
+      return true;
+    }
+  } else if (gCachedRequest.isWifiOnly() && hasEqualWifis) {
+    gDebugCacheReasoning +=", Wifi only."
+    return true;
+  } else if (gCachedRequest.isCellAndWifi()) {
+     gDebugCacheReasoning += ", Cache has Cell+Wifi.";
+    if ((hasEqualCells && hasEqualWifis) ||
+        (!newWifiList && hasEqualCells) ||
+        (!newCell && hasEqualWifis))
+    {
+     return true;
+    }
+  }
+
+  return false;
+}
+
 function WifiGeoCoordsObject(lat, lon, acc, alt, altacc) {
   this.latitude = lat;
   this.longitude = lon;
@@ -140,7 +306,7 @@ WifiGeoPositionProvider.prototype = {
           }
         }
       },
-      
+
       handleError: function(message) {
         gLoggingEnabled = false;
         LOG("settings callback threw an exception, dropping");
@@ -178,6 +344,10 @@ WifiGeoPositionProvider.prototype = {
       return;
     }
 
+    // Without clearing this, we could end up using the cache almost indefinitely
+    // TODO: add logic for cache lifespan, for now just be safe and clear it
+    gCachedRequest = null;
+
     if (this.timer) {
       this.timer.cancel();
       this.timer = null;
@@ -232,7 +402,7 @@ WifiGeoPositionProvider.prototype = {
   },
 
   getMobileInfo: function() {
-    LOG("updateMobileInfo called");
+    LOG("getMobileInfo called");
     try {
       let radioService = Cc["@mozilla.org/ril;1"]
                     .getService(Ci.nsIRadioInterfaceLayer);
@@ -269,6 +439,30 @@ WifiGeoPositionProvider.prototype = {
   },
 
   sendLocationRequest: function (wifiData) {
+    let data = {};
+    if (wifiData) {
+      data.wifiAccessPoints = wifiData;
+    }
+
+    if (gCellScanningEnabled) {
+      let cellData = this.getMobileInfo();
+      if (cellData && cellData.length > 0) {
+        data.cellTowers = cellData;
+      }
+    }
+
+    let useCached = isCachedRequestMoreAccurateThanServerRequest(data.cellTowers,
+                                                                 data.wifiAccessPoints);
+
+    LOG("Use request cache:" + useCached + " reason:" + gDebugCacheReasoning);
+
+    if (useCached) {
+      gCachedRequest.location.timestamp = Date.now();
+      this.listener.update(gCachedRequest.location);
+      return;
+    }
+
+    // From here on, do a network geolocation request //
     let url = Services.urlFormatter.formatURLPref("geo.wifi.uri");
     let listener = this.listener;
     LOG("Sending request: " + url + "\n");
@@ -304,23 +498,12 @@ WifiGeoPositionProvider.prototype = {
                                                   xhr.response.accuracy);
 
       listener.update(newLocation);
+      gCachedRequest = new CachedRequest(newLocation, data.cellTowers, data.wifiAccessPoints);
     };
 
-    let data = {};
-    if (wifiData) {
-      data.wifiAccessPoints = wifiData;
-    }
-
-    if (gCellScanningEnabled) {
-      let cellData = this.getMobileInfo();
-      if (cellData) {
-        data.cellTowers = cellData;
-      }
-    }
-
-    data = JSON.stringify(data);
-    LOG("sending " + data);
-    xhr.send(data);
+    var requestData = JSON.stringify(data);
+    LOG("sending " + requestData);
+    xhr.send(requestData);
   },
 };
 

dom/tests/mochitest/geolocation/geolocation_common.js

@@ -1,3 +1,4 @@
+var BASE_URL = "http://mochi.test:8888/tests/dom/tests/mochitest/geolocation/network_geolocation.sjs";
 
 function sleep(delay)
 {
@@ -11,7 +12,7 @@ function force_prompt(allow, callback) {
 
 function start_sending_garbage(callback)
 {
-  SpecialPowers.pushPrefEnv({"set": [["geo.wifi.uri", "http://mochi.test:8888/tests/dom/tests/mochitest/geolocation/network_geolocation.sjs?action=respond-garbage"]]}, function() {
+  SpecialPowers.pushPrefEnv({"set": [["geo.wifi.uri", BASE_URL + "?action=respond-garbage"]]}, function() {
     // we need to be sure that all location data has been purged/set.
     sleep(1000);
     callback.call();
@@ -20,7 +21,7 @@ function start_sending_garbage(callback)
 
 function stop_sending_garbage(callback)
 {
-  SpecialPowers.pushPrefEnv({"set": [["geo.wifi.uri", "http://mochi.test:8888/tests/dom/tests/mochitest/geolocation/network_geolocation.sjs"]]}, function() {
+  SpecialPowers.pushPrefEnv({"set": [["geo.wifi.uri", BASE_URL + ""]]}, function() {
     // we need to be sure that all location data has been purged/set.
     sleep(1000);
     callback.call();
@@ -29,31 +30,37 @@ function stop_sending_garbage(callback)
 
 function stop_geolocationProvider(callback)
 {
-  SpecialPowers.pushPrefEnv({"set": [["geo.wifi.uri", "http://mochi.test:8888/tests/dom/tests/mochitest/geolocation/network_geolocation.sjs?action=stop-responding"]]}, function() {
+  SpecialPowers.pushPrefEnv({"set": [["geo.wifi.uri", BASE_URL + "?action=stop-responding"]]}, function() {
     // we need to be sure that all location data has been purged/set.
     sleep(1000);
     callback.call();
   });
 }
 
+function set_network_request_cache_enabled(enabled, callback)
+{
+  SpecialPowers.pushPrefEnv({"set": [["geo.wifi.debug.requestCache.enabled", enabled]]}, callback);
+}
+
 function worse_geolocationProvider(callback)
 {
-  SpecialPowers.pushPrefEnv({"set": [["geo.wifi.uri", "http://mochi.test:8888/tests/dom/tests/mochitest/geolocation/network_geolocation.sjs?action=worse-accuracy"]]}, callback);
+  SpecialPowers.pushPrefEnv({"set": [["geo.wifi.uri", BASE_URL + "?action=worse-accuracy"]]}, callback);
 }
 
 function resume_geolocationProvider(callback)
 {
-  SpecialPowers.pushPrefEnv({"set": [["geo.wifi.uri", "http://mochi.test:8888/tests/dom/tests/mochitest/geolocation/network_geolocation.sjs"]]}, callback);
+  SpecialPowers.pushPrefEnv({"set": [["geo.wifi.uri", BASE_URL + ""]]}, callback);
 }
 
 function delay_geolocationProvider(delay, callback)
 {
-  SpecialPowers.pushPrefEnv({"set": [["geo.wifi.uri", "http://mochi.test:8888/tests/dom/tests/mochitest/geolocation/network_geolocation.sjs?delay=" + delay]]}, callback);
+  SpecialPowers.pushPrefEnv({"set": [["geo.wifi.uri", BASE_URL + "?delay=" + delay]]}, callback);
 }
 
 function send404_geolocationProvider(callback)
 {
-  SpecialPowers.pushPrefEnv({"set": [["geo.wifi.uri", "http://mochi.test:8888/tests/dom/tests/mochitest/geolocation/network_geolocation.sjs?action=send404"]]}, callback);
+  set_network_request_cache_enabled(false, function() {
+    SpecialPowers.pushPrefEnv({"set": [["geo.wifi.uri", BASE_URL + "?action=send404"]]}, callback);});
 }
 
 function check_geolocation(location) {
@@ -76,8 +83,8 @@ function check_geolocation(location) {
   // optional ok("heading" in coords, "Check to see if there is a heading");
   // optional ok("speed" in coords, "Check to see if there is a speed");
 
-  ok (location.coords.latitude  == 37.41857, "lat matches known value");
-  ok (location.coords.longitude == -122.08769, "lon matches known value");
+  ok (Math.abs(location.coords.latitude - 37.41857) < 0.001, "lat matches known value");
+  ok (Math.abs(location.coords.longitude + 122.08769) < 0.001, "lon matches known value");
   // optional  ok(location.coords.altitude == 42, "alt matches known value");
   // optional  ok(location.coords.altitudeAccuracy == 42, "alt acc matches known value");
 }

dom/tests/mochitest/geolocation/test_cachedPosition.html

@@ -22,12 +22,18 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=850442
 SimpleTest.waitForExplicitFinish();
 
 resume_geolocationProvider(function() {
-  force_prompt(true, test1);
+    force_prompt(true, function () {
+      set_network_request_cache_enabled(false, function() {
+        test1();
+      });
+    });
 });
 
 function done() {
-  resume_geolocationProvider(function() {
-    SimpleTest.finish();
+  set_network_request_cache_enabled(true, function() {
+    resume_geolocationProvider(function() {
+      SimpleTest.finish();
+    });
   });
 }
 

dom/tests/mochitest/geolocation/test_timerRestartWatch.html

@@ -37,15 +37,18 @@ function errorCallback(err) {
   if (times >= 3) {
     navigator.geolocation.clearWatch(watchID);
     resume_geolocationProvider(function() {
-      SimpleTest.finish();
+      set_network_request_cache_enabled(true,
+        function() { SimpleTest.finish(); } );
     });
   }
 }
 
 function successCallback(position) {
+  ok(1, "on success");
   // Now that we got a success callback, lets try to ensure
   // that we get a timeout error.
-  stop_geolocationProvider(function() {});
+  // The network cache is already off, now stop the sjs from reponding to requests
+  stop_geolocationProvider(function(){});
 }
 
 var options = {
@@ -54,7 +57,10 @@ var options = {
 };
 
 function test1() {
-  watchID = navigator.geolocation.watchPosition(successCallback, errorCallback, options);
+ set_network_request_cache_enabled(false,
+      function() {
+        watchID = navigator.geolocation.watchPosition(successCallback, errorCallback, options);
+      });
 }
 </script>
 </pre>