Merge "Revert "Reduce allocations on location delivery"" into sc-v2-dev

services/core/java/com/android/server/location/provider/LocationProviderManager.java

@@ -177,7 +177,7 @@ public class LocationProviderManager extends
     protected interface LocationTransport {
 
         void deliverOnLocationChanged(LocationResult locationResult,
-                @Nullable IRemoteCallback onCompleteCallback) throws Exception;
+                @Nullable Runnable onCompleteCallback) throws Exception;
         void deliverOnFlushComplete(int requestCode) throws Exception;
     }
 
@@ -197,8 +197,9 @@ public class LocationProviderManager extends
 
         @Override
         public void deliverOnLocationChanged(LocationResult locationResult,
-                @Nullable IRemoteCallback onCompleteCallback) throws RemoteException {
+                @Nullable Runnable onCompleteCallback) throws RemoteException {
-            mListener.onLocationChanged(locationResult.asList(), onCompleteCallback);
+            mListener.onLocationChanged(locationResult.asList(),
+                    SingleUseCallback.wrap(onCompleteCallback));
         }
 
         @Override
@@ -226,7 +227,7 @@ public class LocationProviderManager extends
 
         @Override
         public void deliverOnLocationChanged(LocationResult locationResult,
-                @Nullable IRemoteCallback onCompleteCallback)
+                @Nullable Runnable onCompleteCallback)
                 throws PendingIntent.CanceledException {
             BroadcastOptions options = BroadcastOptions.makeBasic();
             options.setDontSendToRestrictedApps(true);
@@ -242,34 +243,20 @@ public class LocationProviderManager extends
                 intent.putExtra(KEY_LOCATIONS, locationResult.asList().toArray(new Location[0]));
             }
 
-            PendingIntent.OnFinished onFinished = null;
-
             // send() SHOULD only run the completion callback if it completes successfully. however,
-            // b/201299281 (which could not be fixed in the S timeframe) means that it's possible
+            // b/199464864 (which could not be fixed in the S timeframe) means that it's possible
             // for send() to throw an exception AND run the completion callback. if this happens, we
             // would over-release the wakelock... we take matters into our own hands to ensure that
             // the completion callback can only be run if send() completes successfully. this means
             // the completion callback may be run inline - but as we've never specified what thread
             // the callback is run on, this is fine.
-            GatedCallback gatedCallback;
+            GatedCallback gatedCallback = new GatedCallback(onCompleteCallback);
-            if (onCompleteCallback != null) {
-                gatedCallback = new GatedCallback(() -> {
-                    try {
-                        onCompleteCallback.sendResult(null);
-                    } catch (RemoteException e) {
-                        throw e.rethrowFromSystemServer();
-                    }
-                });
-                onFinished = (pI, i, rC, rD, rE) -> gatedCallback.run();
-            } else {
-                gatedCallback = new GatedCallback(null);
-            }
 
             mPendingIntent.send(
                     mContext,
                     0,
                     intent,
-                    onFinished,
+                    (pI, i, rC, rD, rE) -> gatedCallback.run(),
                     null,
                     null,
                     options.toBundle());
@@ -306,7 +293,7 @@ public class LocationProviderManager extends
 
         @Override
         public void deliverOnLocationChanged(@Nullable LocationResult locationResult,
-                @Nullable IRemoteCallback onCompleteCallback)
+                @Nullable Runnable onCompleteCallback)
                 throws RemoteException {
             // ILocationCallback doesn't currently support completion callbacks
             Preconditions.checkState(onCompleteCallback == null);
@@ -727,13 +714,6 @@ public class LocationProviderManager extends
 
         final PowerManager.WakeLock mWakeLock;
 
-        // b/206340085 - if we allocate a new wakelock releaser object for every delivery we
-        // increase the risk of resource starvation. if a client stops processing deliveries the
-        // system server binder allocation pool will be starved as we continue to queue up
-        // deliveries, each with a new allocation. in order to mitigate this, we use a single
-        // releaser object per registration rather than per delivery.
-        final ExternalWakeLockReleaser mWakeLockReleaser;
-
         private volatile ProviderTransport mProviderTransport;
         private int mNumLocationsDelivered = 0;
         private long mExpirationRealtimeMs = Long.MAX_VALUE;
@@ -747,7 +727,6 @@ public class LocationProviderManager extends
                     .newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, WAKELOCK_TAG);
             mWakeLock.setReferenceCounted(true);
             mWakeLock.setWorkSource(request.getWorkSource());
-            mWakeLockReleaser = new ExternalWakeLockReleaser(identity, mWakeLock);
         }
 
         @Override
@@ -964,7 +943,7 @@ public class LocationProviderManager extends
                     }
 
                     listener.deliverOnLocationChanged(deliverLocationResult,
-                            mUseWakeLock ? mWakeLockReleaser : null);
+                            mUseWakeLock ? mWakeLock::release : null);
                     EVENT_LOG.logProviderDeliveredLocations(mName, locationResult.size(),
                             getIdentity());
                 }
@@ -2782,7 +2761,7 @@ public class LocationProviderManager extends
         @GuardedBy("this")
         private boolean mRun;
 
-        GatedCallback(@Nullable Runnable callback) {
+        GatedCallback(Runnable callback) {
             mCallback = callback;
         }
 
@@ -2817,24 +2796,4 @@ public class LocationProviderManager extends
             }
         }
     }
-
-    private static class ExternalWakeLockReleaser extends IRemoteCallback.Stub {
-
-        private final CallerIdentity mIdentity;
-        private final PowerManager.WakeLock mWakeLock;
-
-        ExternalWakeLockReleaser(CallerIdentity identity, PowerManager.WakeLock wakeLock) {
-            mIdentity = identity;
-            mWakeLock = Objects.requireNonNull(wakeLock);
-        }
-
-        @Override
-        public void sendResult(Bundle data) {
-            try {
-                mWakeLock.release();
-            } catch (RuntimeException e) {
-                Log.e(TAG, "wakelock over-released by " + mIdentity, e);
-            }
-        }
-    }
 }