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

Commit b30739dd authored by Winson Chung's avatar Winson Chung Committed by Android (Google) Code Review
Browse files

Merge changes I775aaf78,I60e049a6

* changes:
  Ensure that binders are unlinked from death recipients
  Fix some voice interaction CTS due to regressions
parents 8095edc1 397967f1
Loading
Loading
Loading
Loading
+3 −1
Original line number Diff line number Diff line
@@ -4741,7 +4741,9 @@ public class ActivityManagerService extends IActivityManager.Stub
                            mWindowManager, appOpsManager, proxy, this,
                            OP_ASSIST_STRUCTURE, OP_NONE);
                    requester.requestAssistData(mStackSupervisor.getTopVisibleActivities(),
                            true, false /* fetchScreenshots */, recentsUid, recentsPackage);
                            true /* fetchData */, false /* fetchScreenshots */,
                            true /* allowFetchData */, false /* alloweFetchScreenshots */,
                            recentsUid, recentsPackage);
                }
                final Intent intent = new Intent();
+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
+65 −17
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
        }
    }

    /**
@@ -131,15 +142,21 @@ public class AssistDataRequester extends IAssistDataReceiver.Stub {
     * @param fetchScreenshot whether or not to fetch the screenshot, only applies if fetchData is
     *     true, the caller is allowed to fetch the assist data, and the current activity allows
     *     assist data to be fetched from it
     * @param allowFetchData to be joined with other checks, determines whether or not the requester
     *     is allowed to fetch the assist data
     * @param allowFetchScreenshot to be joined with other checks, determines whether or not the
     *     requester is allowed to fetch the assist screenshot
     */
    public void requestAssistData(List<IBinder> activityTokens, boolean fetchData,
            boolean fetchScreenshot, 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
    public void requestAssistData(List<IBinder> activityTokens, final boolean fetchData,
            final boolean fetchScreenshot, boolean allowFetchData, boolean allowFetchScreenshot,
            int callingUid, String callingPackage) {
        // 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;
        }

@@ -150,8 +167,8 @@ public class AssistDataRequester extends IAssistDataReceiver.Stub {
        } catch (RemoteException e) {
            // Should never happen
        }
        fetchData &= isAssistDataAllowed;
        fetchScreenshot &= fetchData && isAssistDataAllowed
        allowFetchData &= isAssistDataAllowed;
        allowFetchScreenshot &= fetchData && isAssistDataAllowed
                && (mRequestScreenshotAppOps != OP_NONE);

        mCanceled = false;
@@ -162,7 +179,7 @@ public class AssistDataRequester extends IAssistDataReceiver.Stub {

        if (fetchData) {
            if (mAppOpsManager.checkOpNoThrow(mRequestStructureAppOps, callingUid, callingPackage)
                    == MODE_ALLOWED) {
                    == MODE_ALLOWED && allowFetchData) {
                final int numActivities = activityTokens.size();
                for (int i = 0; i < numActivities; i++) {
                    IBinder topActivity = activityTokens.get(i);
@@ -177,8 +194,12 @@ public class AssistDataRequester extends IAssistDataReceiver.Stub {
                            mPendingDataCount++;
                        } else if (i == 0) {
                            // Wasn't allowed... given that, let's not do the screenshot either.
                            if (mCallbacks.canHandleReceivedAssistDataLocked()) {
                                dispatchAssistDataReceived(null);
                            fetchScreenshot = false;
                            } else {
                                mAssistData.add(null);
                            }
                            allowFetchScreenshot = false;
                            break;
                        }
                    } catch (RemoteException e) {
@@ -187,14 +208,18 @@ public class AssistDataRequester extends IAssistDataReceiver.Stub {
                }
            } else {
                // Wasn't allowed... given that, let's not do the screenshot either.
                if (mCallbacks.canHandleReceivedAssistDataLocked()) {
                    dispatchAssistDataReceived(null);
                fetchScreenshot = false;
                } else {
                    mAssistData.add(null);
                }
                allowFetchScreenshot = false;
            }
        }

        if (fetchScreenshot) {
            if (mAppOpsManager.checkOpNoThrow(mRequestScreenshotAppOps, callingUid, callingPackage)
                    == MODE_ALLOWED) {
                    == MODE_ALLOWED && allowFetchScreenshot) {
                try {
                    MetricsLogger.count(mContext, "assist_with_screen", 1);
                    mPendingScreenshotCount++;
@@ -203,24 +228,38 @@ public class AssistDataRequester extends IAssistDataReceiver.Stub {
                    // Can't happen
                }
            } else {
                if (mCallbacks.canHandleReceivedAssistDataLocked()) {
                    dispatchAssistScreenshotReceived(null);
                } else {
                    mAssistScreenshot.add(null);
                }
            }
        }
        // For the cases where we dispatch null data/screenshot due to permissions, just dispatch
        // request-complete after those are made
        tryDispatchRequestComplete();
    }

    /**
     * This call should only be made when the callbacks are capable of handling the received assist
     * data.
     * 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));
        }
        mAssistData.clear();
        final int screenshotsCount = mAssistScreenshot.size();
        for (int i = 0; i < screenshotsCount; i++) {
            dispatchAssistScreenshotReceived(mAssistScreenshot.get(i));
        }
        mAssistScreenshot.clear();
    }

    public int getPendingDataCount() {
@@ -254,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);
@@ -273,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);
@@ -298,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;
        }
    }
+76 −18
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;
@@ -74,6 +75,8 @@ public class AssistDataRequesterTest extends ActivityTestsBase {
    private static final boolean CALLER_ASSIST_SCREENSHOT_ALLOWED = true;
    private static final boolean FETCH_DATA = true;
    private static final boolean FETCH_SCREENSHOTS = true;
    private static final boolean ALLOW_FETCH_DATA = true;
    private static final boolean ALLOW_FETCH_SCREENSHOTS = true;

    private static final int TEST_UID = 0;
    private static final String TEST_PACKAGE = "";
@@ -128,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);
                }
@@ -153,7 +155,7 @@ public class AssistDataRequesterTest extends ActivityTestsBase {
                CALLER_ASSIST_SCREENSHOT_ALLOWED);

        mDataRequester.requestAssistData(createActivityList(5), FETCH_DATA, FETCH_SCREENSHOTS,
                TEST_UID, TEST_PACKAGE);
                ALLOW_FETCH_DATA, ALLOW_FETCH_SCREENSHOTS, TEST_UID, TEST_PACKAGE);
        assertReceivedDataCount(5, 5, 1, 1);
    }

@@ -163,18 +165,18 @@ public class AssistDataRequesterTest extends ActivityTestsBase {
                CALLER_ASSIST_SCREENSHOT_ALLOWED);

        mDataRequester.requestAssistData(createActivityList(0), FETCH_DATA, FETCH_SCREENSHOTS,
                TEST_UID, TEST_PACKAGE);
                ALLOW_FETCH_DATA, ALLOW_FETCH_SCREENSHOTS, TEST_UID, TEST_PACKAGE);
        assertReceivedDataCount(0, 0, 0, 0);
    }

    @Test
    public void testCurrentAppDisallow_expectNoCallbacks() throws Exception {
    public void testCurrentAppDisallow_expectNullCallbacks() throws Exception {
        setupMocks(!CURRENT_ACTIVITY_ASSIST_ALLOWED, CALLER_ASSIST_STRUCTURE_ALLOWED,
                CALLER_ASSIST_SCREENSHOT_ALLOWED);

        mDataRequester.requestAssistData(createActivityList(5), FETCH_DATA, FETCH_SCREENSHOTS,
                TEST_UID, TEST_PACKAGE);
        assertReceivedDataCount(0, 0, 0, 0);
                ALLOW_FETCH_DATA, ALLOW_FETCH_SCREENSHOTS, TEST_UID, TEST_PACKAGE);
        assertReceivedDataCount(0, 1, 0, 1);
    }

    @Test
@@ -184,7 +186,7 @@ public class AssistDataRequesterTest extends ActivityTestsBase {

        mCallbacks.canHandleReceivedData = false;
        mDataRequester.requestAssistData(createActivityList(5), FETCH_DATA, FETCH_SCREENSHOTS,
                TEST_UID, TEST_PACKAGE);
                ALLOW_FETCH_DATA, ALLOW_FETCH_SCREENSHOTS, TEST_UID, TEST_PACKAGE);
        assertTrue(mDataRequester.getPendingDataCount() == 5);
        assertTrue(mDataRequester.getPendingScreenshotCount() == 1);
        mGate.countDown();
@@ -195,21 +197,32 @@ 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();
        mDataRequester.processPendingAssistData();
        assertTrue(mCallbacks.receivedData.isEmpty());
        assertTrue(mCallbacks.receivedScreenshots.isEmpty());
    }

    @Test
    public void testNoFetchData_expectNoCallbacks() throws Exception {
    public void testNoFetchData_expectNoDataCallbacks() throws Exception {
        setupMocks(CURRENT_ACTIVITY_ASSIST_ALLOWED, CALLER_ASSIST_STRUCTURE_ALLOWED,
                CALLER_ASSIST_SCREENSHOT_ALLOWED);

        mDataRequester.requestAssistData(createActivityList(5), !FETCH_DATA, FETCH_SCREENSHOTS,
                TEST_UID, TEST_PACKAGE);
        assertReceivedDataCount(0, 0, 0, 0);
                ALLOW_FETCH_DATA, ALLOW_FETCH_SCREENSHOTS, TEST_UID, TEST_PACKAGE);
        assertReceivedDataCount(0, 0, 0, 1);
    }

    @Test
@@ -218,9 +231,9 @@ public class AssistDataRequesterTest extends ActivityTestsBase {
                CALLER_ASSIST_SCREENSHOT_ALLOWED);

        mDataRequester.requestAssistData(createActivityList(5), FETCH_DATA, FETCH_SCREENSHOTS,
                TEST_UID, TEST_PACKAGE);
                ALLOW_FETCH_DATA, ALLOW_FETCH_SCREENSHOTS, TEST_UID, TEST_PACKAGE);
        // Expect a single null data when the appops is denied
        assertReceivedDataCount(0, 1, 0, 0);
        assertReceivedDataCount(0, 1, 0, 1);
    }

    @Test
@@ -231,9 +244,9 @@ public class AssistDataRequesterTest extends ActivityTestsBase {
                anyBoolean(), anyBoolean());

        mDataRequester.requestAssistData(createActivityList(5), FETCH_DATA, FETCH_SCREENSHOTS,
                TEST_UID, TEST_PACKAGE);
                ALLOW_FETCH_DATA, ALLOW_FETCH_SCREENSHOTS, TEST_UID, TEST_PACKAGE);
        // Expect a single null data when requestAssistContextExtras() fails
        assertReceivedDataCount(0, 1, 0, 0);
        assertReceivedDataCount(0, 1, 0, 1);
    }

    @Test
@@ -242,7 +255,7 @@ public class AssistDataRequesterTest extends ActivityTestsBase {
                CALLER_ASSIST_SCREENSHOT_ALLOWED);

        mDataRequester.requestAssistData(createActivityList(5), FETCH_DATA, !FETCH_SCREENSHOTS,
                TEST_UID, TEST_PACKAGE);
                ALLOW_FETCH_DATA, ALLOW_FETCH_SCREENSHOTS, TEST_UID, TEST_PACKAGE);
        assertReceivedDataCount(5, 5, 0, 0);
    }

@@ -252,11 +265,35 @@ public class AssistDataRequesterTest extends ActivityTestsBase {
                !CALLER_ASSIST_SCREENSHOT_ALLOWED);

        mDataRequester.requestAssistData(createActivityList(5), FETCH_DATA, FETCH_SCREENSHOTS,
                TEST_UID, TEST_PACKAGE);
                ALLOW_FETCH_DATA, ALLOW_FETCH_SCREENSHOTS, TEST_UID, TEST_PACKAGE);
        // Expect a single null screenshot when the appops is denied
        assertReceivedDataCount(5, 5, 0, 1);
    }

    @Test
    public void testCanNotHandleReceivedData_expectNoCallbacks() throws Exception {
        setupMocks(CURRENT_ACTIVITY_ASSIST_ALLOWED, !CALLER_ASSIST_STRUCTURE_ALLOWED,
                !CALLER_ASSIST_SCREENSHOT_ALLOWED);

        mCallbacks.canHandleReceivedData = false;
        mDataRequester.requestAssistData(createActivityList(5), FETCH_DATA, FETCH_SCREENSHOTS,
                ALLOW_FETCH_DATA, ALLOW_FETCH_SCREENSHOTS, TEST_UID, TEST_PACKAGE);
        mGate.countDown();
        waitForIdle(mHandler);
        assertTrue(mCallbacks.receivedData.isEmpty());
        assertTrue(mCallbacks.receivedScreenshots.isEmpty());
    }

    @Test
    public void testRequestDataNoneAllowed_expectNullCallbacks() throws Exception {
        setupMocks(CURRENT_ACTIVITY_ASSIST_ALLOWED, CALLER_ASSIST_STRUCTURE_ALLOWED,
                CALLER_ASSIST_SCREENSHOT_ALLOWED);

        mDataRequester.requestAssistData(createActivityList(5), FETCH_DATA, FETCH_SCREENSHOTS,
                !ALLOW_FETCH_DATA, !ALLOW_FETCH_SCREENSHOTS, TEST_UID, TEST_PACKAGE);
        assertReceivedDataCount(0, 1, 0, 1);
    }

    private void assertReceivedDataCount(int numPendingData, int numReceivedData,
            int numPendingScreenshots, int numReceivedScreenshots) throws Exception {
        assertTrue("Expected " + numPendingData + " pending data, got "
@@ -265,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 "
@@ -273,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) {
@@ -292,12 +331,19 @@ 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<>();

        void reset() {
            canHandleReceivedData = true;
            receivedData.clear();
            receivedScreenshots.clear();
        }

        @Override
        public boolean canHandleReceivedAssistDataLocked() {
            return canHandleReceivedData;
@@ -312,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
Loading