Merge "Use cancellation signals for timeouts and cancel-by-CCService operations"

6 files changed, 60 insertions, 25 deletions

diff --git a/core/java/android/service/contentcapture/ContentCaptureService.java b/core/java/android/service/contentcapture/ContentCaptureService.java
index 36e2d1f6b251..ac2532dcea7d 100644
--- a/core/java/android/service/contentcapture/ContentCaptureService.java
+++ b/core/java/android/service/contentcapture/ContentCaptureService.java
@@ -544,11 +544,14 @@ public abstract class ContentCaptureService extends Service {
                 Preconditions.checkNotNull(adapter);
                 Preconditions.checkNotNull(executor);
 
-                DataShareReadAdapterDelegate delegate =
-                        new DataShareReadAdapterDelegate(executor, adapter);
+                ICancellationSignal cancellationSignalTransport =
+                        CancellationSignal.createTransport();
+
+                DataShareReadAdapterDelegate delegate = new DataShareReadAdapterDelegate(
+                        executor, cancellationSignalTransport, adapter);
 
                 try {
-                    callback.accept(delegate);
+                    callback.accept(cancellationSignalTransport, delegate);
                 } catch (RemoteException e) {
                     Slog.e(TAG, "Failed to accept data sharing", e);
                 }
@@ -655,12 +658,16 @@ public abstract class ContentCaptureService extends Service {
         private final Object mLock = new Object();
         private final WeakReference<DataShareReadAdapter> mAdapterReference;
         private final WeakReference<Executor> mExecutorReference;
+        private final WeakReference<ICancellationSignal> mCancellationSignalReference;
 
-        DataShareReadAdapterDelegate(Executor executor, DataShareReadAdapter adapter) {
+        DataShareReadAdapterDelegate(Executor executor,
+                ICancellationSignal cancellationSignalTransport, DataShareReadAdapter adapter) {
             Preconditions.checkNotNull(executor);
+            Preconditions.checkNotNull(cancellationSignalTransport);
             Preconditions.checkNotNull(adapter);
 
             mExecutorReference = new WeakReference<>(executor);
+            mCancellationSignalReference = new WeakReference<>(cancellationSignalTransport);
             mAdapterReference = new WeakReference<>(adapter);
         }
 
@@ -668,7 +675,17 @@ public abstract class ContentCaptureService extends Service {
         public void start(ParcelFileDescriptor fd, ICancellationSignal remoteCancellationSignal)
                 throws RemoteException {
             synchronized (mLock) {
-                CancellationSignal cancellationSignal = new CancellationSignal();
+                ICancellationSignal serverControlledCancellationSignal =
+                        mCancellationSignalReference.get();
+
+                if (serverControlledCancellationSignal == null) {
+                    Slog.w(TAG, "Can't execute onStart(), reference to cancellation signal has "
+                            + "been GC'ed");
+                    return;
+                }
+
+                CancellationSignal cancellationSignal =
+                        CancellationSignal.fromTransport(serverControlledCancellationSignal);
                 cancellationSignal.setRemote(remoteCancellationSignal);
 
                 executeAdapterMethodLocked(
diff --git a/core/java/android/service/contentcapture/DataShareReadAdapter.java b/core/java/android/service/contentcapture/DataShareReadAdapter.java
index d9350ba5d774..ca6820110ea9 100644
--- a/core/java/android/service/contentcapture/DataShareReadAdapter.java
+++ b/core/java/android/service/contentcapture/DataShareReadAdapter.java
@@ -40,8 +40,7 @@ public interface DataShareReadAdapter {
     void onStart(@NonNull ParcelFileDescriptor fd, @NonNull CancellationSignal cancellationSignal);
 
     /**
-     * Signals that the session failed to start or terminated unsuccessfully (e.g. due to a
-     * timeout).
+     * Signals that the session failed to start or terminated unsuccessfully.
      **/
     void onError(int errorCode);
 }
diff --git a/core/java/android/service/contentcapture/IDataShareCallback.aidl b/core/java/android/service/contentcapture/IDataShareCallback.aidl
index c1aa1bb7dcb5..d972adadb53c 100644
--- a/core/java/android/service/contentcapture/IDataShareCallback.aidl
+++ b/core/java/android/service/contentcapture/IDataShareCallback.aidl
@@ -16,10 +16,11 @@
 
 package android.service.contentcapture;
 
+import android.os.ICancellationSignal;
 import android.service.contentcapture.IDataShareReadAdapter;
 
 /** @hide */
 oneway interface IDataShareCallback {
-    void accept(in IDataShareReadAdapter adapter);
+    void accept(in ICancellationSignal cancellationSignal, in IDataShareReadAdapter adapter);
     void reject();
 }
diff --git a/core/java/android/view/contentcapture/ContentCaptureManager.java b/core/java/android/view/contentcapture/ContentCaptureManager.java
index 81c83834098c..cede3b5cf9fe 100644
--- a/core/java/android/view/contentcapture/ContentCaptureManager.java
+++ b/core/java/android/view/contentcapture/ContentCaptureManager.java
@@ -35,6 +35,7 @@ import android.os.Binder;
 import android.os.CancellationSignal;
 import android.os.Handler;
 import android.os.IBinder;
+import android.os.ICancellationSignal;
 import android.os.Looper;
 import android.os.ParcelFileDescriptor;
 import android.os.RemoteException;
@@ -655,9 +656,12 @@ public final class ContentCaptureManager {
         Preconditions.checkNotNull(dataShareWriteAdapter);
         Preconditions.checkNotNull(executor);
 
+        ICancellationSignal cancellationSignalTransport = CancellationSignal.createTransport();
+
         try {
-            mService.shareData(request,
-                    new DataShareAdapterDelegate(executor, dataShareWriteAdapter));
+            mService.shareData(request, cancellationSignalTransport,
+                    new DataShareAdapterDelegate(executor,
+                            cancellationSignalTransport, dataShareWriteAdapter));
         } catch (RemoteException e) {
             e.rethrowFromSystemServer();
         }
@@ -715,20 +719,30 @@ public final class ContentCaptureManager {
 
         private final WeakReference<DataShareWriteAdapter> mAdapterReference;
         private final WeakReference<Executor> mExecutorReference;
+        private final WeakReference<ICancellationSignal> mCancellationSignal;
 
-        private DataShareAdapterDelegate(Executor executor, DataShareWriteAdapter adapter) {
+        private DataShareAdapterDelegate(Executor executor,
+                ICancellationSignal cancellationSignalTransport, DataShareWriteAdapter adapter) {
             Preconditions.checkNotNull(executor);
+            Preconditions.checkNotNull(cancellationSignalTransport);
             Preconditions.checkNotNull(adapter);
 
             mExecutorReference = new WeakReference<>(executor);
             mAdapterReference = new WeakReference<>(adapter);
+            mCancellationSignal = new WeakReference<>(cancellationSignalTransport);
         }
 
         @Override
         public void write(ParcelFileDescriptor destination)
                 throws RemoteException {
-            // TODO(b/148264965): implement this.
-            CancellationSignal cancellationSignal = new CancellationSignal();
+            ICancellationSignal cancellationSignalTransport = mCancellationSignal.get();
+            if (cancellationSignalTransport == null) {
+                Slog.w(TAG, "Can't execute write(), reference to cancellation signal has been "
+                        + "GC'ed");
+            }
+            CancellationSignal cancellationSignal =
+                    CancellationSignal.fromTransport(cancellationSignalTransport);
+
             executeAdapterMethodLocked(adapter -> adapter.onWrite(destination, cancellationSignal),
                     "onWrite");
         }
diff --git a/core/java/android/view/contentcapture/IContentCaptureManager.aidl b/core/java/android/view/contentcapture/IContentCaptureManager.aidl
index e8d85ac69907..5217e68eac50 100644
--- a/core/java/android/view/contentcapture/IContentCaptureManager.aidl
+++ b/core/java/android/view/contentcapture/IContentCaptureManager.aidl
@@ -23,6 +23,7 @@ import android.view.contentcapture.DataRemovalRequest;
 import android.view.contentcapture.DataShareRequest;
 import android.view.contentcapture.IDataShareWriteAdapter;
 import android.os.IBinder;
+import android.os.ICancellationSignal;
 
 import com.android.internal.os.IResultReceiver;
 
@@ -68,7 +69,8 @@ oneway interface IContentCaptureManager {
     /**
     * Requests sharing of a binary data with the content capture service.
     */
-    void shareData(in DataShareRequest request, in IDataShareWriteAdapter adapter);
+    void shareData(in DataShareRequest request, in ICancellationSignal cancellationSignal,
+                   in IDataShareWriteAdapter adapter);
 
     /**
      * Returns whether the content capture feature is enabled for the calling user.
diff --git a/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java b/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java
index 9245a1da43b2..efa127be706b 100644
--- a/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java
+++ b/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java
@@ -646,6 +646,7 @@ public final class ContentCaptureManagerService extends
 
         @Override
         public void shareData(@NonNull DataShareRequest request,
+                @NonNull ICancellationSignal clientCancellationSignal,
                 @NonNull IDataShareWriteAdapter clientAdapter) {
             Preconditions.checkNotNull(request);
             Preconditions.checkNotNull(clientAdapter);
@@ -667,7 +668,8 @@ public final class ContentCaptureManagerService extends
                 }
 
                 service.onDataSharedLocked(request,
-                        new DataShareCallbackDelegate(request, clientAdapter));
+                        new DataShareCallbackDelegate(request, clientCancellationSignal,
+                                clientAdapter));
             }
         }
 
@@ -920,17 +922,20 @@ public final class ContentCaptureManagerService extends
     private class DataShareCallbackDelegate extends IDataShareCallback.Stub {
 
         @NonNull private final DataShareRequest mDataShareRequest;
+        @NonNull private final ICancellationSignal mClientCancellationSignal;
         @NonNull private final IDataShareWriteAdapter mClientAdapter;
 
         DataShareCallbackDelegate(@NonNull DataShareRequest dataShareRequest,
+                @NonNull ICancellationSignal clientCancellationSignal,
                 @NonNull IDataShareWriteAdapter clientAdapter) {
             mDataShareRequest = dataShareRequest;
+            mClientCancellationSignal = clientCancellationSignal;
             mClientAdapter = clientAdapter;
         }
 
         @Override
-        public void accept(IDataShareReadAdapter serviceAdapter)
-                            throws RemoteException {
+        public void accept(ICancellationSignal serviceCancellationSignal,
+                IDataShareReadAdapter serviceAdapter) throws RemoteException {
             Slog.i(mTag, "Data share request accepted by Content Capture service");
 
             Pair<ParcelFileDescriptor, ParcelFileDescriptor> clientPipe = createPipe();
@@ -962,15 +967,12 @@ public final class ContentCaptureManagerService extends
             mClientAdapter.write(source_in);
             serviceAdapter.start(sink_out, cancellationSignalTransport);
 
-            // TODO(b/148264965): use cancellation signals for timeouts and cancelling
             CancellationSignal cancellationSignal =
                     CancellationSignal.fromTransport(cancellationSignalTransport);
 
             cancellationSignal.setOnCancelListener(() -> {
                 try {
-                    // TODO(b/148264965): this should propagate with the cancellation signal to the
-                    // client
-                    mClientAdapter.error(DataShareWriteAdapter.ERROR_UNKNOWN);
+                    mClientCancellationSignal.cancel();
                 } catch (RemoteException e) {
                     Slog.e(mTag, "Failed to propagate cancel operation to the caller", e);
                 }
@@ -1029,14 +1031,14 @@ public final class ContentCaptureManagerService extends
 
                     if (!finishedSuccessfully) {
                         try {
-                            mClientAdapter.error(DataShareWriteAdapter.ERROR_UNKNOWN);
+                            mClientCancellationSignal.cancel();
                         } catch (RemoteException e) {
-                            Slog.e(mTag, "Failed to call error() to client", e);
+                            Slog.e(mTag, "Failed to cancel() the client operation", e);
                         }
                         try {
-                            serviceAdapter.error(DataShareWriteAdapter.ERROR_UNKNOWN);
+                            serviceCancellationSignal.cancel();
                         } catch (RemoteException e) {
-                            Slog.e(mTag, "Failed to call error() to service", e);
+                            Slog.e(mTag, "Failed to cancel() the service operation", e);
                         }
                     }
                 }