Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 397967f1 authored by Winson Chung's avatar Winson Chung
Browse files

Ensure that binders are unlinked from death recipients

- Add a callback to assist data requester to notify owners that the request
  is completed so they can clean up accordingly

Bug: 67864419
Test: com.android.server.am.AssistDataRequesterTest
Change-Id: I775aaf78ac7d9632cd9ca0a4cf05f2a87211d5fe
parent 60f493d4
Loading
Loading
Loading
Loading
+24 −9
Original line number Diff line number Diff line
@@ -37,17 +37,12 @@ class AssistDataReceiverProxy implements AssistDataRequesterCallbacks,
    private static final String TAG = TAG_WITH_CLASS_NAME ? "AssistDataReceiverProxy" : TAG_AM;

    private String mCallerPackage;
    private boolean mBinderDied;
    private IAssistDataReceiver mReceiver;

    public AssistDataReceiverProxy(IAssistDataReceiver receiver, String callerPackage) {
        try {
            receiver.asBinder().linkToDeath(this, 0);
        } catch (RemoteException e) {
            Log.w(TAG, "Could not link to client death", e);
        }
        mReceiver = receiver;
        mCallerPackage = callerPackage;
        linkToDeath();
    }

    @Override
@@ -58,7 +53,7 @@ class AssistDataReceiverProxy implements AssistDataRequesterCallbacks,

    @Override
    public void onAssistDataReceivedLocked(Bundle data, int activityIndex, int activityCount) {
        if (!mBinderDied) {
        if (mReceiver != null) {
            try {
                mReceiver.onHandleAssistData(data);
            } catch (RemoteException e) {
@@ -70,7 +65,7 @@ class AssistDataReceiverProxy implements AssistDataRequesterCallbacks,

    @Override
    public void onAssistScreenshotReceivedLocked(Bitmap screenshot) {
        if (!mBinderDied) {
        if (mReceiver != null) {
            try {
                mReceiver.onHandleAssistScreenshot(screenshot);
            } catch (RemoteException e) {
@@ -80,8 +75,28 @@ class AssistDataReceiverProxy implements AssistDataRequesterCallbacks,
        }
    }

    @Override
    public void onAssistRequestCompleted() {
        unlinkToDeath();
    }

    @Override
    public void binderDied() {
        mBinderDied = true;
        unlinkToDeath();
    }

    private void linkToDeath() {
        try {
            mReceiver.asBinder().linkToDeath(this, 0);
        } catch (RemoteException e) {
            Log.w(TAG, "Could not link to client death", e);
        }
    }

    private void unlinkToDeath() {
        if (mReceiver != null) {
            mReceiver.asBinder().unlinkToDeath(this, 0);
        }
        mReceiver = null;
    }
}
 No newline at end of file
+34 −5
Original line number Diff line number Diff line
@@ -94,6 +94,17 @@ public class AssistDataRequester extends IAssistDataReceiver.Stub {
         */
        @GuardedBy("mCallbacksLock")
        void onAssistScreenshotReceivedLocked(Bitmap screenshot);

        /**
         * Called when there is no more pending assist data or screenshots for the last request.
         * If the request was canceled, then this callback will not be made. In addition, the
         * callback will be made with the {@param mCallbacksLock} held, and only if
         * {@link #canHandleReceivedAssistDataLocked()} is true.
         */
        @GuardedBy("mCallbacksLock")
        default void onAssistRequestCompleted() {
            // Do nothing
        }
    }

    /**
@@ -139,12 +150,13 @@ public class AssistDataRequester extends IAssistDataReceiver.Stub {
    public void requestAssistData(List<IBinder> activityTokens, final boolean fetchData,
            final boolean fetchScreenshot, boolean allowFetchData, boolean allowFetchScreenshot,
            int callingUid, String callingPackage) {
        // TODO: Better handle the cancel case if a request can be reused
        // TODO: Known issue, if the assist data is not allowed on the current activity, then no
        //       assist data is requested for any of the other activities
        // TODO(b/34090158): Known issue, if the assist data is not allowed on the current activity,
        //                   then no assist data is requested for any of the other activities

        // Early exit if there are no activity to fetch for
        if (activityTokens.isEmpty()) {
            // No activities, just dispatch request-complete
            tryDispatchRequestComplete();
            return;
        }

@@ -223,6 +235,9 @@ public class AssistDataRequester extends IAssistDataReceiver.Stub {
                }
            }
        }
        // For the cases where we dispatch null data/screenshot due to permissions, just dispatch
        // request-complete after those are made
        tryDispatchRequestComplete();
    }

    /**
@@ -230,6 +245,11 @@ public class AssistDataRequester extends IAssistDataReceiver.Stub {
     * data. The owner is also responsible for locking before calling this method.
     */
    public void processPendingAssistData() {
        flushPendingAssistData();
        tryDispatchRequestComplete();
    }

    private void flushPendingAssistData() {
        final int dataCount = mAssistData.size();
        for (int i = 0; i < dataCount; i++) {
            dispatchAssistDataReceived(mAssistData.get(i));
@@ -273,8 +293,9 @@ public class AssistDataRequester extends IAssistDataReceiver.Stub {

            if (mCallbacks.canHandleReceivedAssistDataLocked()) {
                // Process any pending data and dispatch the new data as well
                processPendingAssistData();
                flushPendingAssistData();
                dispatchAssistDataReceived(data);
                tryDispatchRequestComplete();
            } else {
                // Queue up the data for processing later
                mAssistData.add(data);
@@ -292,8 +313,9 @@ public class AssistDataRequester extends IAssistDataReceiver.Stub {

            if (mCallbacks.canHandleReceivedAssistDataLocked()) {
                // Process any pending data and dispatch the new data as well
                processPendingAssistData();
                flushPendingAssistData();
                dispatchAssistScreenshotReceived(screenshot);
                tryDispatchRequestComplete();
            } else {
                // Queue up the data for processing later
                mAssistScreenshot.add(screenshot);
@@ -317,6 +339,13 @@ public class AssistDataRequester extends IAssistDataReceiver.Stub {
        mCallbacks.onAssistScreenshotReceivedLocked(screenshot);
    }

    private void tryDispatchRequestComplete() {
        if (mPendingDataCount == 0 && mPendingScreenshotCount == 0 &&
                mAssistData.isEmpty() && mAssistScreenshot.isEmpty()) {
            mCallbacks.onAssistRequestCompleted();
        }
    }

    public void dump(String prefix, PrintWriter pw) {
        pw.print(prefix); pw.print("mPendingDataCount="); pw.println(mPendingDataCount);
        pw.print(prefix); pw.print("mAssistData="); pw.println(mAssistData);
+3 −0
Original line number Diff line number Diff line
@@ -149,6 +149,9 @@ class PinnedStackController {
        @Override
        public void binderDied() {
            // Clean up the state if the listener dies
            if (mPinnedStackListener != null) {
                mPinnedStackListener.asBinder().unlinkToDeath(mPinnedStackListenerDeathHandler, 0);
            }
            mPinnedStackListener = null;
        }
    }
+23 −3
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ import static android.app.AppOpsManager.OP_ASSIST_SCREENSHOT;
import static android.app.AppOpsManager.OP_ASSIST_STRUCTURE;
import static android.graphics.Bitmap.Config.ARGB_8888;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
@@ -130,8 +131,7 @@ public class AssistDataRequesterTest extends ActivityTestsBase {
            mHandler.post(() -> {
                try {
                    mGate.await(10, TimeUnit.SECONDS);
                    mDataRequester.onHandleAssistScreenshot(Bitmap.createBitmap(1, 1,
                            ARGB_8888));
                    mDataRequester.onHandleAssistScreenshot(Bitmap.createBitmap(1, 1, ARGB_8888));
                } catch (InterruptedException e) {
                    Log.e(TAG, "Failed to wait", e);
                }
@@ -197,11 +197,16 @@ public class AssistDataRequesterTest extends ActivityTestsBase {
        assertTrue(mDataRequester.getPendingScreenshotCount() == 0);
        assertTrue(mCallbacks.receivedData.isEmpty());
        assertTrue(mCallbacks.receivedScreenshots.isEmpty());
        assertFalse(mCallbacks.requestCompleted);

        mCallbacks.canHandleReceivedData = true;
        mDataRequester.processPendingAssistData();
        // Since we are posting the callback for the request-complete, flush the handler as well
        mGate.countDown();
        waitForIdle(mHandler);
        assertTrue(mCallbacks.receivedData.size() == 5);
        assertTrue(mCallbacks.receivedScreenshots.size() == 1);
        assertTrue(mCallbacks.requestCompleted);

        // Clear the state and ensure that we only process pending data once
        mCallbacks.reset();
@@ -297,6 +302,7 @@ public class AssistDataRequesterTest extends ActivityTestsBase {
        assertTrue("Expected " + numPendingScreenshots + " pending screenshots, got "
                        + mDataRequester.getPendingScreenshotCount(),
                mDataRequester.getPendingScreenshotCount() == numPendingScreenshots);
        assertFalse("Expected request NOT completed", mCallbacks.requestCompleted);
        mGate.countDown();
        waitForIdle(mHandler);
        assertTrue("Expected " + numReceivedData + " data, received "
@@ -305,6 +311,7 @@ public class AssistDataRequesterTest extends ActivityTestsBase {
        assertTrue("Expected " + numReceivedScreenshots + " screenshots, received "
                        + mCallbacks.receivedScreenshots.size(),
                mCallbacks.receivedScreenshots.size() == numReceivedScreenshots);
        assertTrue("Expected request completed", mCallbacks.requestCompleted);
    }

    private List<IBinder> createActivityList(int size) {
@@ -324,9 +331,10 @@ public class AssistDataRequesterTest extends ActivityTestsBase {
        latch.await(2, TimeUnit.SECONDS);
    }

    private static class Callbacks implements AssistDataRequesterCallbacks {
    private class Callbacks implements AssistDataRequesterCallbacks {

        boolean canHandleReceivedData = true;
        boolean requestCompleted = false;
        ArrayList<Bundle> receivedData = new ArrayList<>();
        ArrayList<Bitmap> receivedScreenshots = new ArrayList<>();

@@ -350,5 +358,17 @@ public class AssistDataRequesterTest extends ActivityTestsBase {
        public void onAssistScreenshotReceivedLocked(Bitmap screenshot) {
            receivedScreenshots.add(screenshot);
        }

        @Override
        public void onAssistRequestCompleted() {
            mHandler.post(() -> {
                try {
                    mGate.await(10, TimeUnit.SECONDS);
                    requestCompleted = true;
                } catch (InterruptedException e) {
                    Log.e(TAG, "Failed to wait", e);
                }
            });
        }
    }
}
 No newline at end of file