OSDN Git Service

AM: refactor LockTask whitelist update logic.
authorCharles He <qiurui@google.com>
Sat, 2 Sep 2017 14:27:16 +0000 (15:27 +0100)
committerCharles He <qiurui@google.com>
Mon, 18 Sep 2017 14:05:34 +0000 (15:05 +0100)
Previously, ActivityManagerService owned the LockTask package whitelist,
whereas LockTaskController contained the update logic. In this refactor,
we simplify the structure by moving the whitelist into LockTaskController.
We also add instrumentation tests to verify this behavior.

Test: bit FrameworksServicesTests:com.android.server.am.LockTaskControllerTest
Test: runtest frameworks-services
Test: go/wm-smoke
Test: cts-tradefed run cts-dev -m DevicePolicyManager --test com.android.cts.devicepolicy.DeviceOwnerTest#testLockTask_deviceOwnerUser
Bug: 63909481
Change-Id: I15ef4a446de800336f2bf9cbbd4acf6685e29320

services/core/java/com/android/server/am/ActivityManagerService.java
services/core/java/com/android/server/am/ActivityStackSupervisor.java
services/core/java/com/android/server/am/LockTaskController.java
services/core/java/com/android/server/am/TaskRecord.java
services/tests/servicestests/src/com/android/server/am/LockTaskControllerTest.java

index 4c66c45..61db49b 100644 (file)
@@ -682,11 +682,6 @@ public class ActivityManagerService extends IActivityManager.Stub
     ActivityInfo mLastAddedTaskActivity;
 
     /**
-     * List of packages whitelisted by DevicePolicyManager for locktask. Indexed by userId.
-     */
-    SparseArray<String[]> mLockTaskPackages = new SparseArray<>();
-
-    /**
      * The package name of the DeviceOwner. This package is not permitted to have its data cleared.
      */
     String mDeviceOwnerName;
@@ -10882,7 +10877,6 @@ public class ActivityManagerService extends IActivityManager.Stub
 
     @Override
     public void updateLockTaskPackages(int userId, String[] packages) {
-        // TODO: move this into LockTaskController
         final int callingUid = Binder.getCallingUid();
         if (callingUid != 0 && callingUid != SYSTEM_UID) {
             enforceCallingPermission(android.Manifest.permission.UPDATE_LOCK_TASK_PACKAGES,
@@ -10891,8 +10885,7 @@ public class ActivityManagerService extends IActivityManager.Stub
         synchronized (this) {
             if (DEBUG_LOCKTASK) Slog.w(TAG_LOCKTASK, "Whitelisting " + userId + ":" +
                     Arrays.toString(packages));
-            mLockTaskPackages.put(userId, packages);
-            mLockTaskController.onLockTaskPackagesUpdated();
+            mLockTaskController.updateLockTaskPackages(userId, packages);
         }
     }
 
index e4a2273..9b8a663 100644 (file)
@@ -164,7 +164,6 @@ import java.io.PrintWriter;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
@@ -3545,15 +3544,6 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D
         pw.println("mCurTaskIdForUser=" + mCurTaskIdForUser);
         pw.print(prefix); pw.println("mUserStackInFront=" + mUserStackInFront);
         pw.print(prefix); pw.println("mStacks=" + mStacks);
-        // TODO: move this to LockTaskController
-        final SparseArray<String[]> packages = mService.mLockTaskPackages;
-        if (packages.size() > 0) {
-            pw.print(prefix); pw.println("mLockTaskPackages (userId:packages)=");
-            for (int i = 0; i < packages.size(); ++i) {
-                pw.print(prefix); pw.print(prefix); pw.print(packages.keyAt(i));
-                pw.print(":"); pw.println(Arrays.toString(packages.valueAt(i)));
-            }
-        }
         if (!mWaitingForActivityVisible.isEmpty()) {
             pw.print(prefix); pw.println("mWaitingForActivityVisible=");
             for (int i = 0; i < mWaitingForActivityVisible.size(); ++i) {
index d8706bc..241e583 100644 (file)
@@ -31,6 +31,7 @@ import static android.os.UserHandle.USER_ALL;
 import static android.os.UserHandle.USER_CURRENT;
 import static android.provider.Settings.Secure.LOCK_TO_APP_EXIT_LOCKED;
 import static android.view.Display.DEFAULT_DISPLAY;
+
 import static com.android.server.am.ActivityManagerDebugConfig.DEBUG_LOCKTASK;
 import static com.android.server.am.ActivityManagerDebugConfig.POSTFIX_LOCKTASK;
 import static com.android.server.am.ActivityManagerDebugConfig.TAG_AM;
@@ -55,7 +56,9 @@ import android.os.RemoteException;
 import android.os.ServiceManager;
 import android.provider.Settings;
 import android.util.Slog;
+import android.util.SparseArray;
 
+import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.statusbar.IStatusBarService;
 import com.android.internal.widget.LockPatternUtils;
@@ -65,6 +68,7 @@ import com.android.server.wm.WindowManagerService;
 
 import java.io.PrintWriter;
 import java.util.ArrayList;
+import java.util.Arrays;
 
 /**
  * Helper class that deals with all things related to task locking. This includes the screen pinning
@@ -123,6 +127,11 @@ public class LockTaskController {
     private final ArrayList<TaskRecord> mLockTaskModeTasks = new ArrayList<>();
 
     /**
+     * Packages that are allowed to be launched into the lock task mode for each user.
+     */
+    private final SparseArray<String[]> mLockTaskPackages = new SparseArray<>();
+
+    /**
      * Store the current lock task mode. Possible values:
      * {@link ActivityManager#LOCK_TASK_MODE_NONE}, {@link ActivityManager#LOCK_TASK_MODE_LOCKED},
      * {@link ActivityManager#LOCK_TASK_MODE_PINNED}
@@ -159,8 +168,8 @@ public class LockTaskController {
     }
 
     /**
-     * @return whether the given task can be moved to the back of the stack. Locked tasks cannot be
-     * moved to the back of the stack.
+     * @return whether the given task is locked at the moment. Locked tasks cannot be moved to the
+     * back of the stack.
      */
     boolean checkLockedTask(TaskRecord task) {
         if (mLockTaskModeTasks.contains(task)) {
@@ -452,29 +461,35 @@ public class LockTaskController {
     }
 
     /**
-     * Called when the list of packages whitelisted for lock task mode is changed. Any currently
-     * locked tasks that got removed from the whitelist will be finished.
+     * Update packages that are allowed to be launched in lock task mode.
+     * @param userId Which user this whitelist is associated with
+     * @param packages The whitelist of packages allowed in lock task mode
+     * @see #mLockTaskPackages
      */
-    // TODO: Missing unit tests
-    void onLockTaskPackagesUpdated() {
-        boolean didSomething = false;
+    void updateLockTaskPackages(int userId, String[] packages) {
+        mLockTaskPackages.put(userId, packages);
+
+        boolean taskChanged = false;
         for (int taskNdx = mLockTaskModeTasks.size() - 1; taskNdx >= 0; --taskNdx) {
             final TaskRecord lockedTask = mLockTaskModeTasks.get(taskNdx);
-            final boolean wasWhitelisted =
-                    (lockedTask.mLockTaskAuth == LOCK_TASK_AUTH_LAUNCHABLE) ||
-                            (lockedTask.mLockTaskAuth == LOCK_TASK_AUTH_WHITELISTED);
+            final boolean wasWhitelisted = lockedTask.mLockTaskAuth == LOCK_TASK_AUTH_LAUNCHABLE
+                    || lockedTask.mLockTaskAuth == LOCK_TASK_AUTH_WHITELISTED;
             lockedTask.setLockTaskAuth();
-            final boolean isWhitelisted =
-                    (lockedTask.mLockTaskAuth == LOCK_TASK_AUTH_LAUNCHABLE) ||
-                            (lockedTask.mLockTaskAuth == LOCK_TASK_AUTH_WHITELISTED);
-            if (wasWhitelisted && !isWhitelisted) {
-                // Lost whitelisting authorization. End it now.
-                if (DEBUG_LOCKTASK) Slog.d(TAG_LOCKTASK, "onLockTaskPackagesUpdated: removing " +
-                        lockedTask + " mLockTaskAuth()=" + lockedTask.lockTaskAuthToString());
-                removeLockedTask(lockedTask);
-                lockedTask.performClearTaskLocked();
-                didSomething = true;
+            final boolean isWhitelisted = lockedTask.mLockTaskAuth == LOCK_TASK_AUTH_LAUNCHABLE
+                    || lockedTask.mLockTaskAuth == LOCK_TASK_AUTH_WHITELISTED;
+
+            if (mLockTaskModeState != LOCK_TASK_MODE_LOCKED
+                    || lockedTask.userId != userId
+                    || !wasWhitelisted || isWhitelisted) {
+                continue;
             }
+
+            // Terminate locked tasks that have recently lost whitelist authorization.
+            if (DEBUG_LOCKTASK) Slog.d(TAG_LOCKTASK, "onLockTaskPackagesUpdated: removing " +
+                    lockedTask + " mLockTaskAuth()=" + lockedTask.lockTaskAuthToString());
+            removeLockedTask(lockedTask);
+            lockedTask.performClearTaskLocked();
+            taskChanged = true;
         }
 
         for (int displayNdx = mSupervisor.getChildCount() - 1; displayNdx >= 0; --displayNdx) {
@@ -484,22 +499,40 @@ public class LockTaskController {
                 stack.onLockTaskPackagesUpdatedLocked();
             }
         }
+
         final ActivityRecord r = mSupervisor.topRunningActivityLocked();
-        final TaskRecord task = r != null ? r.getTask() : null;
-        if (mLockTaskModeTasks.isEmpty() && task != null
+        final TaskRecord task = (r != null) ? r.getTask() : null;
+        if (mLockTaskModeTasks.isEmpty() && task!= null
                 && task.mLockTaskAuth == LOCK_TASK_AUTH_LAUNCHABLE) {
             // This task must have just been authorized.
             if (DEBUG_LOCKTASK) Slog.d(TAG_LOCKTASK,
                     "onLockTaskPackagesUpdated: starting new locktask task=" + task);
-            setLockTaskMode(task, LOCK_TASK_MODE_LOCKED, "package updated",
-                    false);
-            didSomething = true;
+            setLockTaskMode(task, LOCK_TASK_MODE_LOCKED, "package updated", false);
+            taskChanged = true;
         }
-        if (didSomething) {
+
+        if (taskChanged) {
             mSupervisor.resumeFocusedStackTopActivityLocked();
         }
     }
 
+    boolean isPackageWhitelisted(int userId, String pkg) {
+        if (pkg == null) {
+            return false;
+        }
+        String[] whitelist;
+        whitelist = mLockTaskPackages.get(userId);
+        if (whitelist == null) {
+            return false;
+        }
+        for (String whitelistedPkg : whitelist) {
+            if (pkg.equals(whitelistedPkg)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     /**
      * @return the topmost locked task
      */
@@ -556,8 +589,18 @@ public class LockTaskController {
     }
 
     public void dump(PrintWriter pw, String prefix) {
-        pw.print(prefix); pw.print("mLockTaskModeState=" + lockTaskModeToString());
-        pw.println(" mLockTaskModeTasks" + mLockTaskModeTasks);
+        pw.println(prefix + "LockTaskController");
+        prefix = prefix + "  ";
+        pw.println(prefix + "mLockTaskModeState=" + lockTaskModeToString());
+        pw.println(prefix + "mLockTaskModeTasks=");
+        for (int i = 0; i < mLockTaskModeTasks.size(); ++i) {
+            pw.println(prefix + "  #" + i + " " + mLockTaskModeTasks.get(i));
+        }
+        pw.println(prefix + "mLockTaskPackages (userId:packages)=");
+        for (int i = 0; i < mLockTaskPackages.size(); ++i) {
+            pw.println(prefix + "  u" + mLockTaskPackages.keyAt(i)
+                    + ":" + Arrays.toString(mLockTaskPackages.valueAt(i)));
+        }
     }
 
     private String lockTaskModeToString() {
index 62afcd2..c78dc40 100644 (file)
@@ -78,7 +78,6 @@ import android.app.ActivityManager.TaskSnapshot;
 import android.app.ActivityOptions;
 import android.app.AppGlobals;
 import android.app.IActivityManager;
-import android.app.WindowConfiguration;
 import android.content.ComponentName;
 import android.content.Intent;
 import android.content.pm.ActivityInfo;
@@ -857,8 +856,13 @@ class TaskRecord extends ConfigurationContainer implements TaskWindowContainerLi
         }
         mResizeMode = info.resizeMode;
         mSupportsPictureInPicture = info.supportsPictureInPicture();
-        mLockTaskMode = info.lockTaskLaunchMode;
         mPrivileged = (info.applicationInfo.privateFlags & PRIVATE_FLAG_PRIVILEGED) != 0;
+        mLockTaskMode = info.lockTaskLaunchMode;
+        if (!mPrivileged && (mLockTaskMode == LOCK_TASK_LAUNCH_MODE_ALWAYS
+                || mLockTaskMode == LOCK_TASK_LAUNCH_MODE_NEVER)) {
+            // Non-priv apps are not allowed to use always or never, fall back to default
+            mLockTaskMode = LOCK_TASK_LAUNCH_MODE_DEFAULT;
+        }
         setLockTaskAuth();
     }
 
@@ -1409,16 +1413,11 @@ class TaskRecord extends ConfigurationContainer implements TaskWindowContainerLi
     }
 
     void setLockTaskAuth() {
-        if (!mPrivileged &&
-                (mLockTaskMode == LOCK_TASK_LAUNCH_MODE_ALWAYS ||
-                        mLockTaskMode == LOCK_TASK_LAUNCH_MODE_NEVER)) {
-            // Non-priv apps are not allowed to use always or never, fall back to default
-            mLockTaskMode = LOCK_TASK_LAUNCH_MODE_DEFAULT;
-        }
+        final String pkg = (realActivity != null) ? realActivity.getPackageName() : null;
         switch (mLockTaskMode) {
             case LOCK_TASK_LAUNCH_MODE_DEFAULT:
-                mLockTaskAuth = isLockTaskWhitelistedLocked() ?
-                    LOCK_TASK_AUTH_WHITELISTED : LOCK_TASK_AUTH_PINNABLE;
+                mLockTaskAuth = mService.mLockTaskController.isPackageWhitelisted(userId, pkg)
+                        ? LOCK_TASK_AUTH_WHITELISTED : LOCK_TASK_AUTH_PINNABLE;
                 break;
 
             case LOCK_TASK_LAUNCH_MODE_NEVER:
@@ -1430,31 +1429,14 @@ class TaskRecord extends ConfigurationContainer implements TaskWindowContainerLi
                 break;
 
             case LOCK_TASK_LAUNCH_MODE_IF_WHITELISTED:
-                mLockTaskAuth = isLockTaskWhitelistedLocked() ?
-                        LOCK_TASK_AUTH_LAUNCHABLE : LOCK_TASK_AUTH_PINNABLE;
+                mLockTaskAuth = mService.mLockTaskController.isPackageWhitelisted(userId, pkg)
+                        LOCK_TASK_AUTH_LAUNCHABLE : LOCK_TASK_AUTH_PINNABLE;
                 break;
         }
         if (DEBUG_LOCKTASK) Slog.d(TAG_LOCKTASK, "setLockTaskAuth: task=" + this +
                 " mLockTaskAuth=" + lockTaskAuthToString());
     }
 
-    private boolean isLockTaskWhitelistedLocked() {
-        String pkg = (realActivity != null) ? realActivity.getPackageName() : null;
-        if (pkg == null) {
-            return false;
-        }
-        String[] packages = mService.mLockTaskPackages.get(userId);
-        if (packages == null) {
-            return false;
-        }
-        for (int i = packages.length - 1; i >= 0; --i) {
-            if (pkg.equals(packages[i])) {
-                return true;
-            }
-        }
-        return false;
-    }
-
     boolean isOverHomeStack() {
         return mTaskToReturnTo == ACTIVITY_TYPE_HOME;
     }
index e98e5bf..f9d7f9d 100644 (file)
@@ -20,19 +20,13 @@ import static android.app.ActivityManager.LOCK_TASK_MODE_LOCKED;
 import static android.app.ActivityManager.LOCK_TASK_MODE_NONE;
 import static android.app.ActivityManager.LOCK_TASK_MODE_PINNED;
 import static android.os.Process.SYSTEM_UID;
+
 import static com.android.server.am.LockTaskController.STATUS_BAR_MASK_LOCKED;
 import static com.android.server.am.LockTaskController.STATUS_BAR_MASK_PINNED;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyInt;
-import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
+
+import static org.junit.Assert.*;
+import static org.mockito.ArgumentMatchers.*;
+import static org.mockito.Mockito.*;
 
 import android.app.StatusBarManager;
 import android.app.admin.IDevicePolicyManager;
@@ -72,7 +66,8 @@ import org.mockito.verification.VerificationMode;
 @SmallTest
 public class LockTaskControllerTest {
     private static final String TEST_PACKAGE_NAME = "com.test.package";
-    private static final String TEST_CLASS_NAME = TEST_PACKAGE_NAME + ".TestClass";
+    private static final String TEST_PACKAGE_NAME_2 = "com.test.package2";
+    private static final String TEST_CLASS_NAME = ".TestClass";
     private static final int TEST_USER_ID = 123;
     private static final int TEST_UID = 10467;
 
@@ -309,15 +304,111 @@ public class LockTaskControllerTest {
         verify(mLockPatternUtils).requireCredentialEntry(UserHandle.USER_ALL);
     }
 
+    @Test
+    public void testUpdateLockTaskPackages() throws Exception {
+        String[] whitelist1 = {TEST_PACKAGE_NAME, TEST_PACKAGE_NAME_2};
+        String[] whitelist2 = {TEST_PACKAGE_NAME};
+
+        // No package is whitelisted initially
+        for (String pkg : whitelist1) {
+            assertFalse("Package shouldn't be whitelisted: " + pkg,
+                    mLockTaskController.isPackageWhitelisted(TEST_USER_ID, pkg));
+            assertFalse("Package shouldn't be whitelisted for user 0: " + pkg,
+                    mLockTaskController.isPackageWhitelisted(0, pkg));
+        }
+
+        // Apply whitelist
+        mLockTaskController.updateLockTaskPackages(TEST_USER_ID, whitelist1);
+
+        // Assert the whitelist is applied to the correct user
+        for (String pkg : whitelist1) {
+            assertTrue("Package should be whitelisted: " + pkg,
+                    mLockTaskController.isPackageWhitelisted(TEST_USER_ID, pkg));
+            assertFalse("Package shouldn't be whitelisted for user 0: " + pkg,
+                    mLockTaskController.isPackageWhitelisted(0, pkg));
+        }
+
+        // Update whitelist
+        mLockTaskController.updateLockTaskPackages(TEST_USER_ID, whitelist2);
+
+        // Assert the new whitelist is applied
+        assertTrue("Package should remain whitelisted: " + TEST_PACKAGE_NAME,
+                mLockTaskController.isPackageWhitelisted(TEST_USER_ID, TEST_PACKAGE_NAME));
+        assertFalse("Package should no longer be whitelisted: " + TEST_PACKAGE_NAME_2,
+                mLockTaskController.isPackageWhitelisted(TEST_USER_ID, TEST_PACKAGE_NAME_2));
+    }
+
+    @Test
+    public void testUpdateLockTaskPackages_taskRemoved() throws Exception {
+        // GIVEN two tasks which are whitelisted initially
+        TaskRecord tr1 = getTaskRecordForUpdate(TEST_PACKAGE_NAME, true);
+        TaskRecord tr2 = getTaskRecordForUpdate(TEST_PACKAGE_NAME_2, false);
+        String[] whitelist = {TEST_PACKAGE_NAME, TEST_PACKAGE_NAME_2};
+        mLockTaskController.updateLockTaskPackages(TEST_USER_ID, whitelist);
+
+        // GIVEN the tasks are launched into LockTask mode
+        mLockTaskController.startLockTaskMode(tr1, false, TEST_UID);
+        mLockTaskController.startLockTaskMode(tr2, false, TEST_UID);
+        assertEquals(LOCK_TASK_MODE_LOCKED, mLockTaskController.getLockTaskModeState());
+        assertTrue(mLockTaskController.checkLockedTask(tr1));
+        assertTrue(mLockTaskController.checkLockedTask(tr2));
+        verifyLockTaskStarted(STATUS_BAR_MASK_LOCKED);
+
+        // WHEN removing one package from whitelist
+        whitelist = new String[] {TEST_PACKAGE_NAME};
+        mLockTaskController.updateLockTaskPackages(TEST_USER_ID, whitelist);
+
+        // THEN the task running that package should be stopped
+        verify(tr2).performClearTaskLocked();
+        assertFalse(mLockTaskController.checkLockedTask(tr2));
+        // THEN the other task should remain locked
+        assertEquals(LOCK_TASK_MODE_LOCKED, mLockTaskController.getLockTaskModeState());
+        assertTrue(mLockTaskController.checkLockedTask(tr1));
+        verifyLockTaskStarted(STATUS_BAR_MASK_LOCKED);
+
+        // WHEN removing the last package from whitelist
+        whitelist = new String[] {};
+        mLockTaskController.updateLockTaskPackages(TEST_USER_ID, whitelist);
+
+        // THEN the last task should be cleared, and the system should quit LockTask mode
+        verify(tr1).performClearTaskLocked();
+        assertFalse(mLockTaskController.checkLockedTask(tr1));
+        assertEquals(LOCK_TASK_MODE_NONE, mLockTaskController.getLockTaskModeState());
+        verifyLockTaskStopped(times(1));
+    }
+
     private TaskRecord getTaskRecord(int lockTaskAuth) {
+        return getTaskRecord(TEST_PACKAGE_NAME, lockTaskAuth);
+    }
+
+    private TaskRecord getTaskRecord(String pkg, int lockTaskAuth) {
         TaskRecord tr = mock(TaskRecord.class);
         tr.mLockTaskAuth = lockTaskAuth;
         tr.intent = new Intent()
-                .setComponent(new ComponentName(TEST_PACKAGE_NAME, TEST_CLASS_NAME));
+                .setComponent(ComponentName.createRelative(pkg, TEST_CLASS_NAME));
         tr.userId = TEST_USER_ID;
         return tr;
     }
 
+    /**
+     * @param isAppAware {@code true} if the app has marked if_whitelisted in its manifest
+     */
+    private TaskRecord getTaskRecordForUpdate(String pkg, boolean isAppAware) {
+        final int authIfWhitelisted = isAppAware
+                ? TaskRecord.LOCK_TASK_AUTH_LAUNCHABLE
+                : TaskRecord.LOCK_TASK_AUTH_WHITELISTED;
+        TaskRecord tr = getTaskRecord(pkg, authIfWhitelisted);
+        doAnswer((invocation) -> {
+            boolean isWhitelisted =
+                    mLockTaskController.isPackageWhitelisted(TEST_USER_ID, pkg);
+            tr.mLockTaskAuth = isWhitelisted
+                    ? authIfWhitelisted
+                    : TaskRecord.LOCK_TASK_AUTH_PINNABLE;
+            return null;
+        }).when(tr).setLockTaskAuth();
+        return tr;
+    }
+
     private void verifyLockTaskStarted(int statusBarMask) throws Exception {
         // THEN the keyguard should have been disabled
         verify(mWindowManager).disableKeyguard(any(IBinder.class), anyString());