GeoLocation: add support for GmsCore location provider

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

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.device.geolocation;
+
+import android.content.Context;
+import android.location.Location;
+import android.os.Bundle;
+
+import com.google.android.gms.common.ConnectionResult;
+import com.google.android.gms.common.api.GoogleApiClient;
+import com.google.android.gms.common.api.GoogleApiClient.ConnectionCallbacks;
+import com.google.android.gms.common.api.GoogleApiClient.OnConnectionFailedListener;
+import com.google.android.gms.location.FusedLocationProviderApi;
+import com.google.android.gms.location.LocationListener;
+import com.google.android.gms.location.LocationRequest;
+import com.google.android.gms.location.LocationServices;
+
+import org.chromium.base.Log;
+import org.chromium.base.ThreadUtils;
+
+/**
+ * This is a LocationProvider using Google Play Services.
+ *
+ * https://developers.google.com/android/reference/com/google/android/gms/location/package-summary
+ */
+public class LocationProviderGmsCore implements ConnectionCallbacks, OnConnectionFailedListener,
+                                                LocationListener,
+                                                LocationProviderFactory.LocationProvider {
+    private static final String TAG = "cr_LocationProvider";
+
+    private final GoogleApiClient mGoogleApiClient;
+    private FusedLocationProviderApi mLocationProviderApi = LocationServices.FusedLocationApi;
+
+    private boolean mEnablehighAccuracy = false;

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

nit: don't explicitly initialize fields to default values. It results in more
bytecode.

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

Ouch! Done.

+    private LocationRequest mLocationRequest;
+
+    LocationProviderGmsCore(Context context) {
+        mGoogleApiClient = new GoogleApiClient.Builder(context)
+                                   .addApi(LocationServices.API)
+                                   .addConnectionCallbacks(this)
+                                   .addOnConnectionFailedListener(this)
+                                   .build();
+        assert mGoogleApiClient != null;
+    }
+
+    LocationProviderGmsCore(GoogleApiClient client, FusedLocationProviderApi locationApi) {
+        mGoogleApiClient = client;
+        mLocationProviderApi = locationApi;
+    }
+
+    // ConnectionCallbacks implementation
+    @Override
+    public void onConnected(Bundle connectionHint) {
+        ThreadUtils.assertOnUiThread();
+
+        mLocationRequest = LocationRequest.create();
+        if (mEnablehighAccuracy) {
+            mLocationRequest.setPriority(LocationRequest.PRIORITY_HIGH_ACCURACY);
+        }
+
+        final Location location = mLocationProviderApi.getLastLocation(mGoogleApiClient);
+        if (location != null) {
+            LocationProviderAdapter.onNewLocationAvailable(location);
+        }
+
+        try {
+            // Request updates on UI Thread replicating LocationProviderAndroid's behaviour.
+            mLocationProviderApi.requestLocationUpdates(
+                    mGoogleApiClient, mLocationRequest, this, ThreadUtils.getUiThreadLooper());
+        } catch (IllegalStateException e) {
+            Log.e(TAG, "Caught IllegalStateException registering for location updates.");

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

When does this happen? Would be good to have a comment about it.

Also - it's not super helpful to swallow the stack trace, better would be:
Log.e(TAG, "Unexpected exception", e)

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

Done.

+            assert false;
+        }
+    }
+
+    @Override
+    public void onConnectionSuspended(int cause) {}
+
+    // OnConnectionFailedListener implementation
+    @Override
+    public void onConnectionFailed(ConnectionResult result) {
+        Log.e(TAG, "onConnectionFailed " + result.toString());

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

Use ("onConnectionFailed %s", result). Although... Maybe it would make more
sense to Log from within newErrorAvailable rather than constructing two separate
messages?

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

Moved to newErrorAvailable() and logged there.

+        LocationProviderAdapter.newErrorAvailable(
+                "Failed to connect to Google Play Services :" + result.toString());
+    }
+
+    // LocationProviderFactory.LocationProvider implementation
+    @Override
+    public void start(boolean enableHighAccuracy) {
+        ThreadUtils.assertOnUiThread();
+        if (mGoogleApiClient.isConnected()) mGoogleApiClient.disconnect();
+
+        mEnablehighAccuracy = enableHighAccuracy;
+        mGoogleApiClient.connect(); // Should return via onConnected().
+    }
+
+    @Override
+    public void stop() {
+        ThreadUtils.assertOnUiThread();
+        if (!mGoogleApiClient.isConnected()) return;
+
+        mLocationProviderApi.removeLocationUpdates(mGoogleApiClient, this);
+
+        mGoogleApiClient.disconnect();
+    }
+
+    @Override
+    public boolean isRunning() {
+        assert ThreadUtils.runningOnUiThread();
+        if (mGoogleApiClient == null) return false;
+        return mGoogleApiClient.isConnecting() || mGoogleApiClient.isConnected();
+    }
+
+    // LocationListener implementation
+    @Override
+    public void onLocationChanged(Location location) {
+        Log.i(TAG, "onLocationChanged: " + location.toString());

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

Might be PII here? Don't want to log user's location.

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

Correct. This was for development only anyway. Removed.

+        LocationProviderAdapter.onNewLocationAvailable(location);
+    }
+}