GeoLocation: add support for GmsCore location provider

device/geolocation/android/java/src/org/chromium/device/geolocation/LocationProviderFactory.java

Index: device/geolocation/android/java/src/org/chromium/device/geolocation/LocationProviderFactory.java
diff --git a/device/geolocation/android/java/src/org/chromium/device/geolocation/LocationProviderFactory.java b/device/geolocation/android/java/src/org/chromium/device/geolocation/LocationProviderFactory.java
index 3ffc36af9dbcdfa4e6d41a423772a568009bad44..bf80cae4e4af2536dabe7c992cafa0eafe56799f 100644
--- a/device/geolocation/android/java/src/org/chromium/device/geolocation/LocationProviderFactory.java
+++ b/device/geolocation/android/java/src/org/chromium/device/geolocation/LocationProviderFactory.java
@@ -9,8 +9,7 @@ import android.content.Context;
 import org.chromium.base.VisibleForTesting;
 
 /**
- * Factory to create a LocationProvider to allow us to inject
- * a mock for tests.
+ * Factory to create a LocationProvider to allow us to inject a mock for tests.
  */
 public class LocationProviderFactory {
     private static LocationProviderFactory.LocationProvider sProviderImpl;
@@ -20,8 +19,9 @@ public class LocationProviderFactory {
      */
     public interface LocationProvider {
         /**
-         * Start listening for location updates.
-         * @param enableHighAccuracy Whether or not to enable high accuracy location providers.
+         * Start listening for location updates. Calling several times before stop() is interpreted
+         * as restart.
+         * @param enableHighAccuracy Whether or not to enable high accuracy location.
          */
         public void start(boolean enableHighAccuracy);
 
@@ -40,11 +40,13 @@ public class LocationProviderFactory {
 
     @VisibleForTesting
     public static void setLocationProviderImpl(LocationProviderFactory.LocationProvider provider) {

[Reilly Grant (use Gerrit), 2017/04/20 00:53:32]

I'm confused that we have both this and
LocationProviderAndroid.setLocationManagerForTesting().

[mcasas, 2017/04/20 15:05:16]

There's several levels of injection for testing.  The 
LocationProviderFactory can get a LocationProvider injected, which 
is this method.  

LocationProviderAndroid, which is one of the potential 
LocationProvider implementations, uses an internal 
android.location.LocationManager, which we also want to be able to
inject for test purposes. 

The other meaningful LocationProvider, LocationProviderGmsCore,
has a constructor where we inject the
com.google.android.gms.common.api.GoogleApiClient and
com.google.android.gms.location.FusedLocationProviderApi for
the purpose of this test.

-        assert sProviderImpl == null;
         sProviderImpl = provider;
     }
 
     public static LocationProvider create(Context context) {
+        if (sProviderImpl != null) return sProviderImpl;
+
+        sProviderImpl = new LocationProviderGmsCore(context);
         if (sProviderImpl == null) {

[agrieve, 2017/04/20 15:22:23]

This will never be true since it's assigned on the previous line.

[mcasas, 2017/04/20 18:59:32]

Hmm yeah, that's not what I meant.  
Added a static method to check for GmsCore and used
it here to see which LocationProvider should be instantiated.

             sProviderImpl = new LocationProviderAndroid(context);
         }