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

Commit 60f493d4 authored by Winson Chung's avatar Winson Chung
Browse files

Fix some voice interaction CTS due to regressions 



- There are actually four things that control the assist data request:
  current app allowed, app ops, disabled context/secure settings, and
  requested context. The previous refactor had accidentally removed the
  check for the secure settings (user disabled context).  This CL adds
  back the check for the user disabled context.
- In addition, it tried to simplify the callbacks by merging the requested
  context and the disabled context into a single state, but the owners
  still expected a callback when a context was requested (even if not
  allowed).  This change will always ensure a callback in that case.
- Also fix small error where we didn't clear the pending data/screenshot
  after processing

Bug: 68759932
Bug: 68762521
Test: com.android.server.am.AssistDataRequesterTest
Test: #testCanNotHandleReceivedData_expectNoCallbacks
Test: #testProcessPendingData
Test: CtsVoiceInteractionTestCases
Test: CtsAssistTestCases

Change-Id: I60e049a6bf835fe726bc1133f2f8c9712bec8626
Signed-off-by: default avatarWinson Chung <winsonc@google.com>
parent 4a76a3f3

services/core/java/com/android/server/am/ActivityManagerService.java

+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();

























services/core/java/com/android/server/am/AssistDataRequester.java



+31
−12




























Original line number
Diff line number
Diff line








@@ -131,9 +131,14 @@ 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) {













    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









@@ -150,8 +155,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 +167,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 +182,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 +196,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 +216,30 @@ public class AssistDataRequester extends IAssistDataReceiver.Stub {













                    // Can't happen















                }















            } else {













                if (mCallbacks.canHandleReceivedAssistDataLocked()) {















                    dispatchAssistScreenshotReceived(null);













                } else {













                    mAssistScreenshot.add(null);













                }















            }















        }















    }































    /**















     * 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() {















        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() {

































services/tests/servicestests/src/com/android/server/am/AssistDataRequesterTest.java



+53
−15




























Original line number
Diff line number
Diff line








@@ -74,6 +74,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 = "";










@@ -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();









@@ -200,16 +202,22 @@ public class AssistDataRequesterTest extends ActivityTestsBase {













        mDataRequester.processPendingAssistData();















        assertTrue(mCallbacks.receivedData.size() == 5);















        assertTrue(mCallbacks.receivedScreenshots.size() == 1);



























        // 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 +226,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 +239,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 +250,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 +260,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 "










@@ -298,6 +330,12 @@ public class AssistDataRequesterTest extends ActivityTestsBase {













        ArrayList<Bundle> receivedData = new ArrayList<>();















        ArrayList<Bitmap> receivedScreenshots = new ArrayList<>();





























        void reset() {













            canHandleReceivedData = true;













            receivedData.clear();













            receivedScreenshots.clear();













        }





























        @Override















        public boolean canHandleReceivedAssistDataLocked() {















            return canHandleReceivedData;

































services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionSessionConnection.java



+17
−3




























Original line number
Diff line number
Diff line








@@ -198,7 +198,10 @@ final class VoiceInteractionSessionConnection implements ServiceConnection,













            mShowArgs = args;















            mShowFlags = flags;





























            disabledContext |= getUserDisabledShowContextLocked();















            mAssistDataRequester.requestAssistData(topActivities,













                    (flags & VoiceInteractionSession.SHOW_WITH_ASSIST) != 0,













                    (flags & VoiceInteractionSession.SHOW_WITH_SCREENSHOT) != 0,















                    (disabledContext & VoiceInteractionSession.SHOW_WITH_ASSIST) == 0,















                    (disabledContext & VoiceInteractionSession.SHOW_WITH_SCREENSHOT) == 0,















                    mCallingUid, mSessionComponentName.getPackageName());









@@ -215,6 +218,7 @@ final class VoiceInteractionSessionConnection implements ServiceConnection,













                    mShowFlags = 0;















                } catch (RemoteException e) {















                }













                mAssistDataRequester.processPendingAssistData();















            } else if (showCallback != null) {















                mPendingShowCallbacks.add(showCallback);















            }









@@ -237,11 +241,16 @@ final class VoiceInteractionSessionConnection implements ServiceConnection,





























    @Override















    public void onAssistDataReceivedLocked(Bundle data, int activityIndex, int activityCount) {













        // Return early if we have no session













        if (mSession == null) {













            return;













        }





























        if (data == null) {















            try {















                mSession.handleAssist(null, null, null, 0, 0);















            } catch (RemoteException e) {













                // Can't happen













                // Ignore















            }















        } else {















            final Bundle assistData = data.getBundle(ASSIST_KEY_DATA);









@@ -267,17 +276,22 @@ final class VoiceInteractionSessionConnection implements ServiceConnection,













                mSession.handleAssist(assistData, structure, content, activityIndex,















                        activityCount);















            } catch (RemoteException e) {













                // Can't happen













                // Ignore















            }















        }















    }































    @Override















    public void onAssistScreenshotReceivedLocked(Bitmap screenshot) {













        // Return early if we have no session













        if (mSession == null) {













            return;













        }





























        try {















            mSession.handleScreenshot(screenshot);















        } catch (RemoteException e) {













            // Can't happen













            // Ignore















        }















    }