From 883ff1a65daf54d925f21daabcf5a0830f650796 Mon Sep 17 00:00:00 2001
From: "Philip P. Moltmann" <moltmann@google.com>
Date: Mon, 7 Jan 2019 14:34:47 -0800
Subject: [PATCH] Automatically unbind for AbstractRemoteService

No need for services to think about unbinding anymore. Now the
AbstractRemoteService counts how many requests are not yet finished and
once the number of unfinished requests drops to zero, schedules an
unbind.

Bug: 117779333
Test: - Started settings which makes many calls to
        PermissionControllerManager. Saw the automatic unbind to eventually
        happen.
      - atest CtsContentCaptureServiceTestCases (with content capture
        enabled and with content capture disabled)
      - atest CtsAutoFillServiceTestCases (4 tests out of WebViewActivityTest fail without and with this change)
Change-Id: Ief2f3512df5a1c55694c0c6b449079a49089bcde
---
 .../internal/infra/AbstractRemoteService.java | 38 +++++++++++++++++--
 .../server/autofill/RemoteFillService.java    |  3 --
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/core/java/com/android/internal/infra/AbstractRemoteService.java b/core/java/com/android/internal/infra/AbstractRemoteService.java
index c94c64a879b31..e8ac2239c3fde 100644
--- a/core/java/com/android/internal/infra/AbstractRemoteService.java
+++ b/core/java/com/android/internal/infra/AbstractRemoteService.java
@@ -39,6 +39,7 @@ import com.android.internal.annotations.GuardedBy;
 
 import java.io.PrintWriter;
 import java.lang.ref.WeakReference;
+import java.util.ArrayList;
 
 /**
  * Base class representing a remote service.
@@ -93,6 +94,9 @@ public abstract class AbstractRemoteService<S extends AbstractRemoteService<S, I
     // Used just for debugging purposes (on dump)
     private long mNextUnbind;
 
+    /** Requests that have been scheduled, but that are not finished yet */
+    private final ArrayList<PendingRequest<S, I>> mUnfinishedRequests = new ArrayList<>();
+
     /**
      * Callback called when the service dies.
      *
@@ -229,6 +233,8 @@ public abstract class AbstractRemoteService<S extends AbstractRemoteService<S, I
                 .append(mComponentName.flattenToString()).println();
         pw.append(prefix).append(tab).append("destroyed=")
                 .append(String.valueOf(mDestroyed)).println();
+        pw.append(prefix).append(tab).append("numUnfinishedRequests=")
+                .append(String.valueOf(mUnfinishedRequests.size()));
         final boolean bound = handleIsBound();
         pw.append(prefix).append(tab).append("bound=")
                 .append(String.valueOf(bound));
@@ -257,15 +263,30 @@ public abstract class AbstractRemoteService<S extends AbstractRemoteService<S, I
      * <p>This request must be responded by the service somehow (typically using a callback),
      * othewise it will trigger a {@link PendingRequest#onTimeout(AbstractRemoteService)} if the
      * service doesn't respond.
-     *
-     * <p><b>NOTE: </b>this request is responsible for calling {@link #scheduleUnbind()}.
      */
     protected void scheduleRequest(@NonNull PendingRequest<S, I> pendingRequest) {
-        cancelScheduledUnbind();
         mHandler.sendMessage(obtainMessage(
                 AbstractRemoteService::handlePendingRequest, this, pendingRequest));
     }
 
+    /**
+     * Marks a pendingRequest as finished.
+     *
+     * @param finshedRequest The request that is finished
+     */
+    void finishRequest(@NonNull PendingRequest<S, I> finshedRequest) {
+        mHandler.sendMessage(
+                obtainMessage(AbstractRemoteService::handleFinishRequest, this, finshedRequest));
+    }
+
+    private void handleFinishRequest(@NonNull PendingRequest<S, I> finshedRequest) {
+        mUnfinishedRequests.remove(finshedRequest);
+
+        if (mUnfinishedRequests.isEmpty()) {
+            scheduleUnbind();
+        }
+    }
+
     /**
      * Schedules an async request.
      *
@@ -273,7 +294,6 @@ public abstract class AbstractRemoteService<S extends AbstractRemoteService<S, I
      * a simple {@link Runnable}.
      */
     protected void scheduleAsyncRequest(@NonNull AsyncRequest<I> request) {
-        scheduleUnbind();
         // TODO(b/117779333): fix generics below
         @SuppressWarnings({"unchecked", "rawtypes"})
         final MyAsyncPendingRequest<S, I> asyncRequest = new MyAsyncPendingRequest(this, request);
@@ -341,6 +361,10 @@ public abstract class AbstractRemoteService<S extends AbstractRemoteService<S, I
             handleEnsureBound();
         } else {
             if (mVerbose) Slog.v(mTag, "handlePendingRequest(): " + pendingRequest);
+
+            mUnfinishedRequests.add(pendingRequest);
+            cancelScheduledUnbind();
+
             pendingRequest.run();
             if (pendingRequest.isFinal()) {
                 mCompleted = true;
@@ -504,6 +528,12 @@ public abstract class AbstractRemoteService<S extends AbstractRemoteService<S, I
                 }
                 mCompleted = true;
             }
+
+            S service = mWeakService.get();
+            if (service != null) {
+                service.finishRequest(this);
+            }
+
             mServiceHandler.removeCallbacks(mTimeoutTrigger);
             return true;
         }
diff --git a/services/autofill/java/com/android/server/autofill/RemoteFillService.java b/services/autofill/java/com/android/server/autofill/RemoteFillService.java
index 34fe5d9163564..e8a52b4a12a92 100644
--- a/services/autofill/java/com/android/server/autofill/RemoteFillService.java
+++ b/services/autofill/java/com/android/server/autofill/RemoteFillService.java
@@ -139,9 +139,6 @@ final class RemoteFillService
         if (mPendingRequest == pendingRequest) {
             mPendingRequest = null;
         }
-        if (mPendingRequest == null) {
-            scheduleUnbind();
-        }
         return true;
     }