From d1f7f96a5803c0d7e11ece57919aae1958b65438 Mon Sep 17 00:00:00 2001 From: Pinyao Ting Date: Fri, 7 Aug 2020 15:52:29 -0700 Subject: [PATCH] clean-up unrequired slice access from default home app this CL reverts ag/12226048 since it's no longer needed Bug: 12335685 Test: atest SliceManagerServiceTest Change-Id: Ib5e62649f21fda515b222e490b90ef57aa99e27b --- core/java/android/app/usage/UsageEvents.java | 2 +- .../server/slice/SliceManagerService.java | 179 +----------------- .../slice/PackageMatchingCacheTest.java | 76 -------- .../server/slice/SliceManagerServiceTest.java | 4 - 4 files changed, 9 insertions(+), 252 deletions(-) delete mode 100644 services/tests/uiservicestests/src/com/android/server/slice/PackageMatchingCacheTest.java diff --git a/core/java/android/app/usage/UsageEvents.java b/core/java/android/app/usage/UsageEvents.java index 3522b1b8aff57..0e6a0637d8016 100644 --- a/core/java/android/app/usage/UsageEvents.java +++ b/core/java/android/app/usage/UsageEvents.java @@ -199,7 +199,7 @@ public final class UsageEvents implements Parcelable { public static final int NOTIFICATION_INTERRUPTION = 12; /** - * A Slice was pinned by the default launcher or the default assistant. + * A Slice was pinned by the default assistant. * @hide */ @SystemApi diff --git a/services/core/java/com/android/server/slice/SliceManagerService.java b/services/core/java/com/android/server/slice/SliceManagerService.java index 2a74b3d238290..5aedfc19028b9 100644 --- a/services/core/java/com/android/server/slice/SliceManagerService.java +++ b/services/core/java/com/android/server/slice/SliceManagerService.java @@ -28,8 +28,6 @@ import static android.os.Process.SYSTEM_UID; import android.Manifest.permission; import android.annotation.NonNull; import android.app.AppOpsManager; -import android.app.role.OnRoleHoldersChangedListener; -import android.app.role.RoleManager; import android.app.slice.ISliceManager; import android.app.slice.SliceSpec; import android.app.usage.UsageStatsManagerInternal; @@ -41,9 +39,7 @@ import android.content.Context; import android.content.Intent; import android.content.IntentFilter; import android.content.pm.PackageManager; -import android.content.pm.PackageManagerInternal; import android.content.pm.ProviderInfo; -import android.content.pm.ResolveInfo; import android.net.Uri; import android.os.Binder; import android.os.Handler; @@ -65,7 +61,6 @@ import com.android.internal.app.AssistUtils; import com.android.server.LocalServices; import com.android.server.ServiceThread; import com.android.server.SystemService; -import com.android.server.SystemService.TargetUser; import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlPullParserException; @@ -77,10 +72,7 @@ import java.io.ByteArrayOutputStream; import java.io.FileDescriptor; import java.io.IOException; import java.util.ArrayList; -import java.util.List; import java.util.Objects; -import java.util.concurrent.Executor; -import java.util.function.Supplier; public class SliceManagerService extends ISliceManager.Stub { @@ -88,16 +80,13 @@ public class SliceManagerService extends ISliceManager.Stub { private final Object mLock = new Object(); private final Context mContext; - private final PackageManagerInternal mPackageManagerInternal; private final AppOpsManager mAppOps; private final AssistUtils mAssistUtils; @GuardedBy("mLock") private final ArrayMap mPinnedSlicesByUri = new ArrayMap<>(); @GuardedBy("mLock") - private final SparseArray mAssistantLookup = new SparseArray<>(); - @GuardedBy("mLock") - private final SparseArray mHomeLookup = new SparseArray<>(); + private final SparseArray mLastAssistantPackage = new SparseArray<>(); private final Handler mHandler; private final SlicePermissionManager mPermissions; @@ -110,8 +99,6 @@ public class SliceManagerService extends ISliceManager.Stub { @VisibleForTesting SliceManagerService(Context context, Looper looper) { mContext = context; - mPackageManagerInternal = Objects.requireNonNull( - LocalServices.getService(PackageManagerInternal.class)); mAppOps = context.getSystemService(AppOpsManager.class); mAssistUtils = new AssistUtils(context); mHandler = new Handler(looper); @@ -124,7 +111,6 @@ public class SliceManagerService extends ISliceManager.Stub { filter.addAction(Intent.ACTION_PACKAGE_DATA_CLEARED); filter.addAction(Intent.ACTION_PACKAGE_REMOVED); filter.addDataScheme("package"); - mRoleObserver = new RoleObserver(); mContext.registerReceiverAsUser(mReceiver, UserHandle.ALL, filter, null, mHandler); } @@ -174,8 +160,7 @@ public class SliceManagerService extends ISliceManager.Stub { mHandler.post(() -> { if (slicePkg != null && !Objects.equals(pkg, slicePkg)) { mAppUsageStats.reportEvent(slicePkg, user, - isAssistant(pkg, user) || isDefaultHomeApp(pkg, user) - ? SLICE_PINNED_PRIV : SLICE_PINNED); + isAssistant(pkg, user) ? SLICE_PINNED_PRIV : SLICE_PINNED); } }); } @@ -440,38 +425,19 @@ public class SliceManagerService extends ISliceManager.Stub { private boolean hasFullSliceAccess(String pkg, int userId) { long ident = Binder.clearCallingIdentity(); try { - boolean ret = isDefaultHomeApp(pkg, userId) || isAssistant(pkg, userId) - || isGrantedFullAccess(pkg, userId); - return ret; + return isAssistant(pkg, userId) || isGrantedFullAccess(pkg, userId); } finally { Binder.restoreCallingIdentity(ident); } } private boolean isAssistant(String pkg, int userId) { - return getAssistantMatcher(userId).matches(pkg); - } - - private boolean isDefaultHomeApp(String pkg, int userId) { - return getHomeMatcher(userId).matches(pkg); - } - - private PackageMatchingCache getAssistantMatcher(int userId) { - PackageMatchingCache matcher = mAssistantLookup.get(userId); - if (matcher == null) { - matcher = new PackageMatchingCache(() -> getAssistant(userId)); - mAssistantLookup.put(userId, matcher); + if (pkg == null) return false; + if (!pkg.equals(mLastAssistantPackage.get(userId))) { + // Failed on cached value, try updating. + mLastAssistantPackage.put(userId, getAssistant(userId)); } - return matcher; - } - - private PackageMatchingCache getHomeMatcher(int userId) { - PackageMatchingCache matcher = mHomeLookup.get(userId); - if (matcher == null) { - matcher = new PackageMatchingCache(() -> getDefaultHome(userId)); - mHomeLookup.put(userId, matcher); - } - return matcher; + return pkg.equals(mLastAssistantPackage.get(userId)); } private String getAssistant(int userId) { @@ -482,111 +448,6 @@ public class SliceManagerService extends ISliceManager.Stub { return cn.getPackageName(); } - /** - * A cached value of the default home app - */ - private String mCachedDefaultHome = null; - - // Based on getDefaultHome in ShortcutService. - // TODO: Unify if possible - @VisibleForTesting - protected String getDefaultHome(int userId) { - - // Set VERIFY to true to run the cache in "shadow" mode for cache - // testing. Do not commit set to true; - final boolean VERIFY = false; - - if (mCachedDefaultHome != null) { - if (!VERIFY) { - return mCachedDefaultHome; - } - } - - final long token = Binder.clearCallingIdentity(); - try { - final List allHomeCandidates = new ArrayList<>(); - - // Default launcher from package manager. - final ComponentName defaultLauncher = mPackageManagerInternal - .getHomeActivitiesAsUser(allHomeCandidates, userId); - - ComponentName detected = defaultLauncher; - - // Cache the default launcher. It is not a problem if the - // launcher is null - eventually, the default launcher will be - // set to something non-null. - mCachedDefaultHome = ((detected != null) ? detected.getPackageName() : null); - - if (detected == null) { - // If we reach here, that means it's the first check since the user was created, - // and there's already multiple launchers and there's no default set. - // Find the system one with the highest priority. - // (We need to check the priority too because of FallbackHome in Settings.) - // If there's no system launcher yet, then no one can access slices, until - // the user explicitly sets one. - final int size = allHomeCandidates.size(); - - int lastPriority = Integer.MIN_VALUE; - for (int i = 0; i < size; i++) { - final ResolveInfo ri = allHomeCandidates.get(i); - if (!ri.activityInfo.applicationInfo.isSystemApp()) { - continue; - } - if (ri.priority < lastPriority) { - continue; - } - detected = ri.activityInfo.getComponentName(); - lastPriority = ri.priority; - } - } - final String ret = ((detected != null) ? detected.getPackageName() : null); - if (VERIFY) { - if (mCachedDefaultHome != null && !mCachedDefaultHome.equals(ret)) { - Slog.e(TAG, "getDefaultHome() cache failure, is " + - mCachedDefaultHome + " should be " + ret); - } - } - return ret; - } finally { - Binder.restoreCallingIdentity(token); - } - } - - public void invalidateCachedDefaultHome() { - mCachedDefaultHome = null; - } - - /** - * Listen for changes in the roles, and invalidate the cached default - * home as necessary. - */ - private RoleObserver mRoleObserver; - - class RoleObserver implements OnRoleHoldersChangedListener { - private RoleManager mRm; - private final Executor mExecutor; - - RoleObserver() { - mExecutor = mContext.getMainExecutor(); - register(); - } - - public void register() { - mRm = mContext.getSystemService(RoleManager.class); - if (mRm != null) { - mRm.addOnRoleHoldersChangedListenerAsUser(mExecutor, this, UserHandle.ALL); - invalidateCachedDefaultHome(); - } - } - - @Override - public void onRoleHoldersChanged(@NonNull String roleName, @NonNull UserHandle user) { - if (RoleManager.ROLE_HOME.equals(roleName)) { - invalidateCachedDefaultHome(); - } - } - } - private boolean isGrantedFullAccess(String pkg, int userId) { return mPermissions.hasFullAccess(pkg, userId); } @@ -635,30 +496,6 @@ public class SliceManagerService extends ISliceManager.Stub { return mPermissions.getAllPackagesGranted(pkg); } - /** - * Holder that caches a package that has access to a slice. - */ - static class PackageMatchingCache { - - private final Supplier mPkgSource; - private String mCurrentPkg; - - public PackageMatchingCache(Supplier pkgSource) { - mPkgSource = pkgSource; - } - - public boolean matches(String pkgCandidate) { - if (pkgCandidate == null) return false; - - if (Objects.equals(pkgCandidate, mCurrentPkg)) { - return true; - } - // Failed on cached value, try updating. - mCurrentPkg = mPkgSource.get(); - return Objects.equals(pkgCandidate, mCurrentPkg); - } - } - public static class Lifecycle extends SystemService { private SliceManagerService mService; diff --git a/services/tests/uiservicestests/src/com/android/server/slice/PackageMatchingCacheTest.java b/services/tests/uiservicestests/src/com/android/server/slice/PackageMatchingCacheTest.java deleted file mode 100644 index f6c854e23494e..0000000000000 --- a/services/tests/uiservicestests/src/com/android/server/slice/PackageMatchingCacheTest.java +++ /dev/null @@ -1,76 +0,0 @@ -/* - * Copyright (C) 2018 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file - * except in compliance with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the - * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the specific language governing - * permissions and limitations under the License. - */ - -package com.android.server.slice; - -import static org.junit.Assert.assertTrue; -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.mockito.Mockito.when; - -import android.testing.AndroidTestingRunner; -import android.testing.TestableLooper.RunWithLooper; - -import androidx.test.filters.SmallTest; - -import com.android.server.UiServiceTestCase; -import com.android.server.slice.SliceManagerService.PackageMatchingCache; - -import org.junit.Test; -import org.junit.runner.RunWith; - -import java.util.function.Supplier; - -@SmallTest -@RunWith(AndroidTestingRunner.class) -@RunWithLooper -public class PackageMatchingCacheTest extends UiServiceTestCase { - - private final Supplier supplier = mock(Supplier.class); - private final PackageMatchingCache cache = new PackageMatchingCache(supplier); - - @Test - public void testNulls() { - // Doesn't get for a null input - cache.matches(null); - verify(supplier, never()).get(); - - // Gets once valid input in sent. - cache.matches(""); - verify(supplier).get(); - } - - @Test - public void testCaching() { - when(supplier.get()).thenReturn("ret.pkg"); - - assertTrue(cache.matches("ret.pkg")); - assertTrue(cache.matches("ret.pkg")); - assertTrue(cache.matches("ret.pkg")); - - verify(supplier, times(1)).get(); - } - - @Test - public void testGetOnFailure() { - when(supplier.get()).thenReturn("ret.pkg"); - assertTrue(cache.matches("ret.pkg")); - - when(supplier.get()).thenReturn("other.pkg"); - assertTrue(cache.matches("other.pkg")); - verify(supplier, times(2)).get(); - } -} diff --git a/services/tests/uiservicestests/src/com/android/server/slice/SliceManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/slice/SliceManagerServiceTest.java index a4436951f48b0..cf1c36c0d243e 100644 --- a/services/tests/uiservicestests/src/com/android/server/slice/SliceManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/slice/SliceManagerServiceTest.java @@ -90,8 +90,6 @@ public class SliceManagerServiceTest extends UiServiceTestCase { @Test public void testAddPinCreatesPinned() throws RemoteException { - doReturn("pkg").when(mService).getDefaultHome(anyInt()); - mService.pinSlice("pkg", TEST_URI, EMPTY_SPECS, mToken); mService.pinSlice("pkg", TEST_URI, EMPTY_SPECS, mToken); verify(mService, times(1)).createPinnedSlice(eq(maybeAddUserId(TEST_URI, 0)), anyString()); @@ -99,8 +97,6 @@ public class SliceManagerServiceTest extends UiServiceTestCase { @Test public void testRemovePinDestroysPinned() throws RemoteException { - doReturn("pkg").when(mService).getDefaultHome(anyInt()); - mService.pinSlice("pkg", TEST_URI, EMPTY_SPECS, mToken); when(mCreatedSliceState.unpin(eq("pkg"), eq(mToken))).thenReturn(false);