Merge "Address Robin's code review comments in initial FlpHal submission." into klp-dev

3 files changed, 54 insertions, 27 deletions

diff --git a/services/java/com/android/server/LocationManagerService.java b/services/java/com/android/server/LocationManagerService.java
index 49746ff..a32699a 100644
--- a/services/java/com/android/server/LocationManagerService.java
+++ b/services/java/com/android/server/LocationManagerService.java
@@ -437,7 +437,10 @@ public class LocationManagerService extends ILocationManager.Stub {
         FusedProxy fusedProxy = FusedProxy.createAndBind(
                 mContext,
                 mLocationHandler,
-                flpHardwareProvider.getLocationHardware());
+                flpHardwareProvider.getLocationHardware(),
+                com.android.internal.R.bool.config_enableFusedLocationOverlay,
+                com.android.internal.R.string.config_fusedLocationProviderPackageName,
+                com.android.internal.R.array.config_locationProviderPackageNames);
 
         if(fusedProxy == null) {
             Slog.e(TAG, "No FusedProvider found.");
diff --git a/services/java/com/android/server/location/FlpHardwareProvider.java b/services/java/com/android/server/location/FlpHardwareProvider.java
index 469f7ce..226c18c 100644
--- a/services/java/com/android/server/location/FlpHardwareProvider.java
+++ b/services/java/com/android/server/location/FlpHardwareProvider.java
@@ -28,11 +28,10 @@ import android.location.LocationManager;
 
 import android.content.Context;
 import android.os.Bundle;
-import android.os.Handler;
+import android.os.Looper;
 import android.os.RemoteException;
 import android.os.SystemClock;
 import android.util.Log;
-import android.util.Slog;
 
 /**
  * This class is an interop layer for JVM types and the JNI code that interacts
@@ -48,6 +47,7 @@ public class FlpHardwareProvider {
 
     private final static String TAG = "FlpHardwareProvider";
     private final Context mContext;
+    private final Object mLocationSinkLock = new Object();
 
     public static FlpHardwareProvider getInstance(Context context) {
         if (sSingletonInstance == null) {
@@ -61,7 +61,6 @@ public class FlpHardwareProvider {
         mContext = context;
 
         // register for listening for passive provider data
-        Handler handler = new Handler();
         LocationManager manager = (LocationManager) mContext.getSystemService(
                 Context.LOCATION_SERVICE);
         manager.requestLocationUpdates(
@@ -69,7 +68,7 @@ public class FlpHardwareProvider {
                 0 /* minTime */,
                 0 /* minDistance */,
                 new NetworkLocationListener(),
-                handler.getLooper());
+                Looper.myLooper());
     }
 
     public static boolean isSupported() {
@@ -87,9 +86,13 @@ public class FlpHardwareProvider {
             location.setElapsedRealtimeNanos(SystemClock.elapsedRealtimeNanos());
         }
 
+        IFusedLocationHardwareSink sink;
+        synchronized (mLocationSinkLock) {
+            sink = mLocationSink;
+        }
         try {
-            if (mLocationSink != null) {
-                mLocationSink.onLocationAvailable(locations);
+            if (sink != null) {
+                sink.onLocationAvailable(locations);
             }
         } catch (RemoteException e) {
             Log.e(TAG, "RemoteException calling onLocationAvailable");
@@ -98,9 +101,13 @@ public class FlpHardwareProvider {
 
     // FlpDiagnosticCallbacks members
     private void onDataReport(String data) {
+        IFusedLocationHardwareSink sink;
+        synchronized (mLocationSinkLock) {
+            sink = mLocationSink;
+        }
         try {
             if (mLocationSink != null) {
-                mLocationSink.onDiagnosticDataAvailable(data);
+                sink.onDiagnosticDataAvailable(data);
             }
         } catch (RemoteException e) {
             Log.e(TAG, "RemoteException calling onDiagnosticDataAvailable");
@@ -199,19 +206,24 @@ public class FlpHardwareProvider {
     private final IFusedLocationHardware mLocationHardware = new IFusedLocationHardware.Stub() {
         @Override
         public void registerSink(IFusedLocationHardwareSink eventSink) {
-            // only one sink is allowed at the moment
-            if (mLocationSink != null) {
-                throw new RuntimeException("IFusedLocationHardware does not support multiple sinks");
+            synchronized (mLocationSinkLock) {
+                // only one sink is allowed at the moment
+                if (mLocationSink != null) {
+                    throw new RuntimeException(
+                            "IFusedLocationHardware does not support multiple sinks");
+                }
+
+                mLocationSink = eventSink;
             }
-
-            mLocationSink = eventSink;
         }
 
         @Override
         public void unregisterSink(IFusedLocationHardwareSink eventSink) {
-            // don't throw if the sink is not registered, simply make it a no-op
-            if (mLocationSink == eventSink) {
-                mLocationSink = null;
+            synchronized (mLocationSinkLock) {
+                // don't throw if the sink is not registered, simply make it a no-op
+                if (mLocationSink == eventSink) {
+                    mLocationSink = null;
+                }
             }
         }
 
diff --git a/services/java/com/android/server/location/FusedProxy.java b/services/java/com/android/server/location/FusedProxy.java
index 8278b96..f7fac77 100644
--- a/services/java/com/android/server/location/FusedProxy.java
+++ b/services/java/com/android/server/location/FusedProxy.java
@@ -33,10 +33,8 @@ import android.util.Log;
  */
 public final class FusedProxy {
     private final String TAG = "FusedProxy";
-
-    private ServiceWatcher mServiceWatcher;
-
-    private FusedLocationHardwareSecure mLocationHardware;
+    private final ServiceWatcher mServiceWatcher;
+    private final FusedLocationHardwareSecure mLocationHardware;
 
     /**
      * Constructor of the class.
@@ -46,7 +44,13 @@ public final class FusedProxy {
      * @param handler           The handler needed for construction.
      * @param locationHardware  The instance of the Fused location hardware assigned to the proxy.
      */
-    private FusedProxy(Context context, Handler handler, IFusedLocationHardware locationHardware) {
+    private FusedProxy(
+            Context context,
+            Handler handler,
+            IFusedLocationHardware locationHardware,
+            int overlaySwitchResId,
+            int defaultServicePackageNameResId,
+            int initialPackageNameResId) {
         mLocationHardware = new FusedLocationHardwareSecure(
                 locationHardware,
                 context,
@@ -63,9 +67,9 @@ public final class FusedProxy {
                 context,
                 TAG,
                 "com.android.location.service.FusedProvider",
-                com.android.internal.R.bool.config_enableFusedLocationOverlay,
-                com.android.internal.R.string.config_fusedLocationProviderPackageName,
-                com.android.internal.R.array.config_locationProviderPackageNames,
+                overlaySwitchResId,
+                defaultServicePackageNameResId,
+                initialPackageNameResId,
                 newServiceWork,
                 handler);
     }
@@ -82,9 +86,17 @@ public final class FusedProxy {
     public static FusedProxy createAndBind(
             Context context,
             Handler handler,
-            IFusedLocationHardware locationHardware
-            ) {
-        FusedProxy fusedProxy = new FusedProxy(context, handler, locationHardware);
+            IFusedLocationHardware locationHardware,
+            int overlaySwitchResId,
+            int defaultServicePackageNameResId,
+            int initialPackageNameResId) {
+        FusedProxy fusedProxy = new FusedProxy(
+                context,
+                handler,
+                locationHardware,
+                overlaySwitchResId,
+                defaultServicePackageNameResId,
+                initialPackageNameResId);
 
         // try to bind the Fused provider
         if (!fusedProxy.mServiceWatcher.start()) {