Bug 1788734 - Geolocation Intermittent Failures r=owlish,geckoview-reviewers

This patch updates how the mock geolocation provider works. The geolocation test provider now mocks the in-built Android GPS and Network providers. It also now includes an option to continually post locations, like a true location provider would.

Differential Revision: https://phabricator.services.mozilla.com/D161793

mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/GeolocationTest.kt

@@ -7,7 +7,10 @@ import android.content.Context
 import android.location.LocationManager
 import android.os.Handler
 import android.os.Looper
-import androidx.lifecycle.*
+import android.util.Log
+import androidx.lifecycle.DefaultLifecycleObserver
+import androidx.lifecycle.LifecycleOwner
+import androidx.lifecycle.ProcessLifecycleOwner
 import androidx.test.ext.junit.rules.ActivityScenarioRule
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import androidx.test.filters.LargeTest
@@ -25,16 +28,18 @@ import org.mozilla.geckoview.Autofill
 import org.mozilla.geckoview.GeckoResult
 import org.mozilla.geckoview.GeckoSession
 import org.mozilla.geckoview.test.rule.GeckoSessionTestRule
-
+import org.mozilla.geckoview.test.rule.GeckoSessionTestRule.MockLocationProvider
 
 @RunWith(AndroidJUnit4::class)
  @LargeTest
  class GeolocationTest : BaseSessionTest() {
+    private val LOGTAG = "GeolocationTest"
     private val activityRule = ActivityScenarioRule(GeckoViewTestActivity::class.java)
-    private val locProvider = "mockTestLocationProvider";
-    private val inaccurateLocProvider = "inaccurateMockTestLocationProvider";
     private val context = InstrumentationRegistry.getInstrumentation().targetContext
     private lateinit var locManager : LocationManager
+    private lateinit var mockGpsProvider : MockLocationProvider
+    private lateinit var mockNetworkProvider : MockLocationProvider
+
     @get:Rule
     override val rules: RuleChain = RuleChain.outerRule(activityRule).around(sessionRule)
 
@@ -45,8 +50,8 @@ import org.mozilla.geckoview.test.rule.GeckoSessionTestRule
             // Prevents using the network provider for these tests
             sessionRule.setPrefsUntilTestEnd(mapOf("geo.provider.testing" to false))
             locManager = activity.getSystemService(Context.LOCATION_SERVICE) as LocationManager
-            sessionRule.addMockLocationProvider(locManager, locProvider)
-            sessionRule.addMockLocationProvider(locManager, inaccurateLocProvider)
+            mockGpsProvider = sessionRule.MockLocationProvider(locManager, LocationManager.GPS_PROVIDER, 0.0, 0.0, true)
+            mockNetworkProvider = sessionRule.MockLocationProvider (locManager, LocationManager.NETWORK_PROVIDER,0.0, 0.0, true)
         }
     }
 
@@ -56,9 +61,12 @@ import org.mozilla.geckoview.test.rule.GeckoSessionTestRule
             activityRule.scenario.onActivity { activity ->
                 activity.view.releaseSession()
             }
+            mockGpsProvider.removeMockLocationProvider()
+            mockNetworkProvider.removeMockLocationProvider()
         } catch (e : Exception){}
     }
 
+
     private fun setEnableLocationPermissions(){
         sessionRule.delegateDuringNextWait(object : GeckoSession.PermissionDelegate {
             override fun onContentPermissionRequest(
@@ -104,15 +112,17 @@ import org.mozilla.geckoview.test.rule.GeckoSessionTestRule
     @Test fun jsContentRequestForLocation() {
         val mockLat = 1.1111
         val mockLon = 2.2222
-        sessionRule.setMockLocation(locManager, locProvider, mockLat, mockLon)
+        mockGpsProvider.setMockLocation(mockLat, mockLon)
+        mockGpsProvider.setDoContinuallyPost(true)
+        mockGpsProvider.postLocation()
         mainSession.loadTestPath(HELLO_HTML_PATH)
         mainSession.waitForPageStop()
         setEnableLocationPermissions()
 
         val position = getCurrentPositionJS()
+        mockGpsProvider.stopPostingLocation()
         assertThat("Mocked latitude matches.", position["latitude"] as Number, equalTo(mockLat))
         assertThat("Mocked longitude matches.", position["longitude"] as Number, equalTo(mockLon))
-
     }
 
     @GeckoSessionTestRule.NullDelegate(Autofill.Delegate::class)
@@ -132,19 +142,25 @@ import org.mozilla.geckoview.test.rule.GeckoSessionTestRule
         setEnableLocationPermissions()
 
         // Test when lower accuracy is more recent
-        sessionRule.setMockLocation(locManager, locProvider, highMockLat, highMockLon, highAccuracy)
+        mockGpsProvider.setMockLocation(highMockLat, highMockLon, highAccuracy)
+        mockGpsProvider.setDoContinuallyPost(false)
+        mockGpsProvider.postLocation()
+
         // Sleep ensures the mocked locations have different clock times
         Thread.sleep(10)
         // Set inaccurate second, so that it is the most recent location
-        sessionRule.setMockLocation(locManager, inaccurateLocProvider, lowMockLat, lowMockLon, lowAccuracy)
+        mockNetworkProvider.setMockLocation(lowMockLat, lowMockLon, lowAccuracy)
+        mockNetworkProvider.setDoContinuallyPost(false)
+        mockNetworkProvider.postLocation()
+
         val position = getCurrentPositionJS(0, 3000, false);
         assertThat("Higher accuracy latitude is expected.", position["latitude"] as Number, equalTo(highMockLat))
         assertThat("Higher accuracy longitude is expected.", position["longitude"] as Number, equalTo(highMockLon))
 
         // Test that higher accuracy becomes stale after 6 seconds
-        sessionRule.setMockLocation(locManager, locProvider, highMockLat, highMockLon, highAccuracy)
+        mockGpsProvider.postLocation()
         Thread.sleep(6001)
-        sessionRule.setMockLocation(locManager, inaccurateLocProvider, lowMockLat, lowMockLon, lowAccuracy)
+        mockNetworkProvider.postLocation()
         val inaccuratePosition = getCurrentPositionJS(0, 3000, false);
         assertThat("Lower accuracy latitude is expected.", inaccuratePosition["latitude"] as Number, equalTo(lowMockLat))
         assertThat("Lower accuracy longitude is expected.", inaccuratePosition["longitude"] as Number, equalTo(lowMockLon))
@@ -153,21 +169,33 @@ import org.mozilla.geckoview.test.rule.GeckoSessionTestRule
     @GeckoSessionTestRule.NullDelegate(Autofill.Delegate::class)
     // Testing that high accuracy requests a fresh location
     @Test fun highAccuracyTest() {
-        val highAccuracy = .000001f
-        val latitude = 1.1111
-        val longitude = 2.2222
+        val accuracyMed = 4f
+        val accuracyHigh = .000001f
+        val latMedAcc = 1.1111
+        val lonMedAcc = 2.2222
+        val latHighAcc = 3.3333
+        val lonHighAcc = 4.4444
 
         // High accuracy usage requires HTTPS
         mainSession.loadUri("https://example.com/")
         mainSession.waitForPageStop()
         setEnableLocationPermissions()
 
-        sessionRule.setMockLocation(locManager, locProvider, latitude, longitude, highAccuracy)
+        // Have two location providers posting locations
+        mockNetworkProvider.setMockLocation(latMedAcc, lonMedAcc, accuracyMed)
+        mockNetworkProvider.setDoContinuallyPost(true)
+        mockNetworkProvider.postLocation()
 
-        val highAccuracyPosition = getCurrentPositionJS(0, 3000, true);
-        // JS enableHighAccuracy requires it asks the device for a new location and not used a cached location
-        assertThat("New device latitude is expected.", highAccuracyPosition["latitude"] as Number, not(equalTo(latitude)))
-        assertThat("New device longitude is expected.", highAccuracyPosition["longitude"] as Number, not(equalTo(longitude)))
+        mockGpsProvider.setMockLocation(latHighAcc, lonHighAcc, accuracyHigh)
+        mockGpsProvider.setDoContinuallyPost(true)
+        mockGpsProvider.postLocation()
+
+        val highAccuracyPosition = getCurrentPositionJS(0, 6001, true);
+        mockGpsProvider.stopPostingLocation()
+        mockNetworkProvider.stopPostingLocation()
+
+        assertThat("High accuracy latitude is expected.", highAccuracyPosition["latitude"] as Number, equalTo(latHighAcc))
+        assertThat("High accuracy longitude is expected.", highAccuracyPosition["longitude"] as Number, equalTo(lonHighAcc))
     }
 
     @GeckoSessionTestRule.NullDelegate(Autofill.Delegate::class)
@@ -177,6 +205,7 @@ import org.mozilla.geckoview.test.rule.GeckoSessionTestRule
         val beforePauseLon = 2.2222
         val afterPauseLat = 3.3333
         val afterPauseLon = 4.4444
+        mockGpsProvider.setDoContinuallyPost(true)
 
         mainSession.loadTestPath(HELLO_HTML_PATH)
         mainSession.waitForPageStop()
@@ -188,12 +217,15 @@ import org.mozilla.geckoview.test.rule.GeckoSessionTestRule
         // Monitor lifecycle changes
         ProcessLifecycleOwner.get().lifecycle.addObserver(object: DefaultLifecycleObserver {
             override fun onResume(owner: LifecycleOwner) {
+                Log.i(LOGTAG, "onResume Event")
                 actualResumeCount++;
                 super.onResume(owner)
                 try {
                     mainSession.setActive(true)
                     // onResume is also called when starting too
                     if(actualResumeCount > 1) {
+                        // Ensures the location has had time to post
+                        Thread.sleep(3001)
                         val onResumeFromPausePosition = getCurrentPositionJS()
                         assertThat("Latitude after onPause matches.", onResumeFromPausePosition["latitude"] as Number, equalTo(afterPauseLat))
                         assertThat("Longitude after onPause matches.", onResumeFromPausePosition["longitude"] as Number, equalTo(afterPauseLon))
@@ -203,31 +235,29 @@ import org.mozilla.geckoview.test.rule.GeckoSessionTestRule
                     assertThat("onResume count matches.", actualResumeCount, equalTo(2))
                     assertThat("onPause count matches.", actualPauseCount, equalTo(1))
                     try {
-                        sessionRule.removeMockLocationProvider(locManager, locProvider)
+                        mockGpsProvider.removeMockLocationProvider()
                     } catch (e: Exception) {
                         // Cleanup could have already occurred
                     }
                 }
             }
             override fun onPause(owner: LifecycleOwner) {
+                Log.i(LOGTAG, "onPause Event")
                 actualPauseCount ++;
                 super.onPause(owner)
                 try {
-                    sessionRule.setMockLocation(locManager, locProvider, afterPauseLat, afterPauseLon)
-                    // Ensures the location does not go stale before onResume occurs
-                    Handler(Looper.getMainLooper()).postDelayed({
-                        try {
-                            sessionRule.setMockLocation(locManager, locProvider, afterPauseLat, afterPauseLon)
-                        } catch (e: Exception) { }
-                    }, 5000)
+                    mockGpsProvider.setMockLocation(afterPauseLat, afterPauseLon)
+                    mockGpsProvider.postLocation()
                 } catch (e: Exception) {
+                    Log.w(LOGTAG, "onPause was called too late.")
                     // Potential situation where onPause is called too late
                 }
             }
         })
 
         // Before onPause Event
-        sessionRule.setMockLocation(locManager, locProvider, beforePauseLat, beforePauseLon)
+        mockGpsProvider.setMockLocation(beforePauseLat, beforePauseLon)
+        mockGpsProvider.postLocation()
         val beforeOnPausePosition = getCurrentPositionJS()
         assertThat("Latitude before onPause matches.", beforeOnPausePosition["latitude"] as Number, equalTo(beforePauseLat))
         assertThat("Longitude before onPause matches.", beforeOnPausePosition["longitude"] as Number, equalTo(beforePauseLon))
@@ -242,7 +272,8 @@ import org.mozilla.geckoview.test.rule.GeckoSessionTestRule
 
         // After/During onPause Event
         val whilePausingPosition = getCurrentPositionJSWithWait()
-        assertThat("Longitude after/during onPause matches.", whilePausingPosition["latitude"] as Number, equalTo(afterPauseLat))
+        mockGpsProvider.stopPostingLocation()
+        assertThat("Latitude after/during onPause matches.", whilePausingPosition["latitude"] as Number, equalTo(afterPauseLat))
         assertThat("Longitude after/during onPause matches.", whilePausingPosition["longitude"] as Number, equalTo(afterPauseLon))
 
         assertThat("onResume count matches.", actualResumeCount, equalTo(2))

mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/PermissionDelegateTest.kt

@@ -174,9 +174,9 @@ class PermissionDelegateTest : BaseSessionTest() {
         sessionRule.setPrefsUntilTestEnd(mapOf("geo.provider.testing" to false))
         var context = InstrumentationRegistry.getInstrumentation().targetContext
         var locManager = context.getSystemService(Context.LOCATION_SERVICE) as LocationManager
-        var locProvider = "permissionsLocationProvider";
-        sessionRule.addMockLocationProvider(locManager, locProvider)
-        sessionRule.setMockLocation(locManager, locProvider, 1.1111, 2.2222)
+        var locProvider = sessionRule.MockLocationProvider(locManager, "permissionsLocationProvider",
+                1.1111, 2.2222, false)
+        locProvider.postLocation()
 
         mainSession.delegateDuringNextWait(object : PermissionDelegate {
             // Ensure the content permission is asked first, before the Android permission.
@@ -242,7 +242,7 @@ class PermissionDelegateTest : BaseSessionTest() {
         })
         mainSession.reload()
         mainSession.waitForPageStop()
-        sessionRule.removeMockLocationProvider(locManager, locProvider)
+        locProvider.removeMockLocationProvider()
     }
 
     @Test fun geolocation_reject() {

mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/rule/GeckoSessionTestRule.java

@@ -46,6 +46,9 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
@@ -2074,83 +2077,6 @@ public class GeckoSessionTestRule implements TestRule {
     session.getPanZoomController().onTouchEvent(moveEvent);
   }
 
-  /**
-   * Adds a mock location provider that can have locations manually set. NB: Likely also need to set
-   * geo.provider.testing to false to prevent network geolocation from interfering.
-   *
-   * @param locationManager location manager to accept the locations
-   * @param mockproviderName unique name of the location provider
-   */
-  public void addMockLocationProvider(LocationManager locationManager, String mockproviderName) {
-    // Ensures that only one location provider with this name exists
-    removeMockLocationProvider(locationManager, mockproviderName);
-    locationManager.addTestProvider(
-        mockproviderName,
-        false,
-        false,
-        false,
-        false,
-        false,
-        false,
-        false,
-        Criteria.POWER_LOW,
-        Criteria.ACCURACY_FINE);
-    locationManager.setTestProviderEnabled(mockproviderName, true);
-  }
-
-  /**
-   * Removes the location provider.
-   *
-   * @param locationManager location manager to accept the locations
-   * @param mockproviderName unique name of the location provider to remove
-   */
-  public void removeMockLocationProvider(LocationManager locationManager, String mockproviderName) {
-    try {
-      locationManager.removeTestProvider(mockproviderName);
-    } catch (Exception e) {
-      // Throws an exception if there is no provider with that name
-    }
-  }
-
-  /**
-   * Sets the mock location on a given location provider. NB: The system may still prioritize other
-   * location providers, accuracy determines preference.
-   *
-   * @param locationManager location manager to accept the locations
-   * @param mockProviderName location provider that will use this location
-   * @param latitude latitude in degrees to mock
-   * @param longitude longitude in degrees to mock
-   */
-  public void setMockLocation(
-      LocationManager locationManager, String mockProviderName, double latitude, double longitude) {
-    // Closer accuracy helps ensure the mock location provider is prioritized
-    setMockLocation(locationManager, mockProviderName, latitude, longitude, .000001f);
-  }
-
-  /**
-   * Sets the mock location on a given location provider. Use when accuracy needs to be specified.
-   * NB: The system may still prioritize other location providers, accuracy determines preference.
-   *
-   * @param locationManager location manager to accept the locations
-   * @param mockProviderName location provider that will use this location
-   * @param latitude latitude in degrees to mock
-   * @param longitude longitude in degrees to mock
-   * @param accuracy horizontal accuracy in meters to mock
-   */
-  public void setMockLocation(
-      LocationManager locationManager,
-      String mockProviderName,
-      double latitude,
-      double longitude,
-      float accuracy) {
-    Location location = new Location(mockProviderName);
-    location.setAccuracy(accuracy);
-    location.setLatitude(latitude);
-    location.setLongitude(longitude);
-    location.setElapsedRealtimeNanos(SystemClock.elapsedRealtimeNanos());
-    location.setTime(System.currentTimeMillis());
-    locationManager.setTestProviderLocation(mockProviderName, location);
-  }
   /**
    * Simulates a press to the Home button, causing the application to go to onPause. NB: Some time
    * must elapse for the event to fully occur.
@@ -2179,6 +2105,189 @@ public class GeckoSessionTestRule implements TestRule {
     context.startActivity(notificationIntent);
   }
 
+  /**
+   * Mock Location Provider can be used in testing for creating mock locations. NB: Likely also need
+   * to set test setting geo.provider.testing to false to prevent network geolocation from
+   * interfering when using.
+   */
+  public class MockLocationProvider {
+
+    private final LocationManager locationManager;
+    private final String mockProviderName;
+    private boolean isActiveTestProvider = false;
+    private double mockLatitude;
+    private double mockLongitude;
+    private float mockAccuracy = .000001f;
+    private boolean doContinuallyPost;
+
+    @Nullable private ScheduledExecutorService executor;
+
+    /**
+     * Mock Location Provider adds a test provider to the location manager and controls sending mock
+     * locations. Use @{@link #postLocation()} to post the location to the location manager.
+     * Use @{@link #removeMockLocationProvider()} to remove the location provider to clean-up the
+     * test harness. Default accuracy is .000001f.
+     *
+     * @param locationManager location manager to accept the locations
+     * @param mockProviderName location provider that will use this location
+     * @param mockLatitude initial latitude in degrees that @{@link #postLocation()} will use
+     * @param mockLongitude initial longitude in degrees that @{@link #postLocation()} will use
+     * @param doContinuallyPost when posting a location, continue to post every 3s to keep location
+     *     current
+     */
+    public MockLocationProvider(
+        LocationManager locationManager,
+        String mockProviderName,
+        double mockLatitude,
+        double mockLongitude,
+        boolean doContinuallyPost) {
+      this.locationManager = locationManager;
+      this.mockProviderName = mockProviderName;
+      this.mockLatitude = mockLatitude;
+      this.mockLongitude = mockLongitude;
+      this.doContinuallyPost = doContinuallyPost;
+      addMockLocationProvider();
+    }
+
+    /** Adds a mock location provider that can have locations manually set. */
+    private void addMockLocationProvider() {
+      // Ensures that only one location provider with this name exists
+      removeMockLocationProvider();
+      locationManager.addTestProvider(
+          mockProviderName,
+          false,
+          false,
+          false,
+          false,
+          false,
+          false,
+          false,
+          Criteria.POWER_LOW,
+          Criteria.ACCURACY_FINE);
+      locationManager.setTestProviderEnabled(mockProviderName, true);
+      isActiveTestProvider = true;
+    }
+
+    /**
+     * Removes the location provider. Recommend calling when ending test to prevent the mock
+     * provider remaining as a test provider.
+     */
+    public void removeMockLocationProvider() {
+      stopPostingLocation();
+      try {
+        locationManager.removeTestProvider(mockProviderName);
+      } catch (Exception e) {
+        // Throws an exception if there is no provider with that name
+      }
+      isActiveTestProvider = false;
+    }
+
+    /**
+     * Sets the mock location on MockLocationProvider, that will be used by @{@link #postLocation()}
+     *
+     * @param latitude latitude in degrees to mock
+     * @param longitude longitude in degrees to mock
+     */
+    public void setMockLocation(double latitude, double longitude) {
+      mockLatitude = latitude;
+      mockLongitude = longitude;
+    }
+
+    /**
+     * Sets the mock location on a MockLocationProvider, that will be used by @{@link
+     * #postLocation()} . Note, changing the accuracy can affect the importance of the mock provider
+     * compared to other location providers.
+     *
+     * @param latitude latitude in degrees to mock
+     * @param longitude longitude in degrees to mock
+     * @param accuracy horizontal accuracy in meters to mock
+     */
+    public void setMockLocation(double latitude, double longitude, float accuracy) {
+      mockLatitude = latitude;
+      mockLongitude = longitude;
+      mockAccuracy = accuracy;
+    }
+
+    /**
+     * When doContinuallyPost is set to true, @{@link #postLocation()} will post the location to the
+     * location manager every 3s. When set to false, @{@link #postLocation()} will only post the
+     * location once. Purpose is to prevent the location from becoming stale.
+     *
+     * @param doContinuallyPost setting for continually posting the location after calling @{@link
+     *     #postLocation()}
+     */
+    public void setDoContinuallyPost(boolean doContinuallyPost) {
+      this.doContinuallyPost = doContinuallyPost;
+    }
+
+    /**
+     * Shutsdown and removes the executor created by @{@link #postLocation()} when @{@link
+     * #doContinuallyPost is true} to stop posting the location.
+     */
+    public void stopPostingLocation() {
+      if (executor != null) {
+        executor.shutdown();
+        executor = null;
+      }
+    }
+
+    /**
+     * Posts the set location to the system location manager. If @{@link #doContinuallyPost} is
+     * true, the location will be posted every 3s by an executor, otherwise will post once.
+     */
+    public void postLocation() {
+      if (!isActiveTestProvider) {
+        throw new IllegalStateException("The mock test provider is not active.");
+      }
+
+      // Ensure the thread that was posting a location (if applicable) is stopped.
+      stopPostingLocation();
+
+      // Set Location
+      Location location = new Location(mockProviderName);
+      location.setAccuracy(mockAccuracy);
+      location.setLatitude(mockLatitude);
+      location.setLongitude(mockLongitude);
+      location.setElapsedRealtimeNanos(SystemClock.elapsedRealtimeNanos());
+      location.setTime(System.currentTimeMillis());
+      locationManager.setTestProviderLocation(mockProviderName, location);
+      Log.i(
+          LOGTAG,
+          mockProviderName
+              + " is posting location, lat: "
+              + mockLatitude
+              + " lon: "
+              + mockLongitude
+              + " acc: "
+              + mockAccuracy);
+      // Continually post location
+      if (doContinuallyPost) {
+        executor = Executors.newScheduledThreadPool(1);
+        executor.scheduleAtFixedRate(
+            new Runnable() {
+              @Override
+              public void run() {
+                location.setElapsedRealtimeNanos(SystemClock.elapsedRealtimeNanos());
+                location.setTime(System.currentTimeMillis());
+                locationManager.setTestProviderLocation(mockProviderName, location);
+                Log.i(
+                    LOGTAG,
+                    mockProviderName
+                        + " is posting location, lat: "
+                        + mockLatitude
+                        + " lon: "
+                        + mockLongitude
+                        + " acc: "
+                        + mockAccuracy);
+              }
+            },
+            0,
+            3,
+            TimeUnit.SECONDS);
+      }
+    }
+  }
+
   Map<GeckoSession, WebExtension.Port> mPorts = new HashMap<>();
 
   private class MessageDelegate implements WebExtension.MessageDelegate, WebExtension.PortDelegate {