From 6c4c7ba0fe5c25647828786563644c8deec302e9 Mon Sep 17 00:00:00 2001 From: Lei Yu Date: Tue, 10 Apr 2018 10:54:33 -0700 Subject: [PATCH 1/2] Add system app check for anomaly detection. In this CL, don't blame system app in anomaly detection and also add log for it. Following CL will update it to also check whether this system app has launch entry. Bug: 77477987 Test: RunSettingsRoboTests Change-Id: I97490b32bc42ec2f8e03ec2d82f7c8bf89f9c66f --- .../settings/fuelgauge/BatteryUtils.java | 38 +++++++++++++++++++ .../AnomalyDetectionJobService.java | 38 ++++++++++--------- .../settings/fuelgauge/BatteryUtilsTest.java | 32 ++++++++++++++-- .../AnomalyDetectionJobServiceTest.java | 25 ++++++++++++ 4 files changed, 113 insertions(+), 20 deletions(-) diff --git a/src/com/android/settings/fuelgauge/BatteryUtils.java b/src/com/android/settings/fuelgauge/BatteryUtils.java index e8668d1a93a..26439717ecf 100644 --- a/src/com/android/settings/fuelgauge/BatteryUtils.java +++ b/src/com/android/settings/fuelgauge/BatteryUtils.java @@ -25,7 +25,9 @@ import android.os.BatteryManager; import android.os.BatteryStats; import android.os.Bundle; import android.os.Build; +import android.os.Process; import android.os.SystemClock; +import android.os.UserHandle; import android.os.UserManager; import android.support.annotation.IntDef; import android.support.annotation.Nullable; @@ -43,6 +45,7 @@ import com.android.settings.R; import com.android.settings.fuelgauge.anomaly.Anomaly; import com.android.settings.overlay.FeatureFactory; +import com.android.settingslib.fuelgauge.PowerWhitelistBackend; import com.android.settingslib.utils.PowerUtil; import java.lang.annotation.Retention; @@ -531,5 +534,40 @@ public class BatteryUtils { return false; } + + /** + * Return {@code true} if we should hide anomaly app represented by {@code uid} + */ + public boolean shouldHideAnomaly(PowerWhitelistBackend powerWhitelistBackend, int uid) { + final String[] packageNames = mPackageManager.getPackagesForUid(uid); + if (ArrayUtils.isEmpty(packageNames)) { + // Don't show it if app has been uninstalled + return true; + } + + return isSystemUid(uid) || isSystemApp(mPackageManager, packageNames) + || powerWhitelistBackend.isSysWhitelistedExceptIdle(packageNames); + } + + private boolean isSystemUid(int uid) { + final int appUid = UserHandle.getAppId(uid); + return appUid >= Process.ROOT_UID && appUid < Process.FIRST_APPLICATION_UID; + } + + private boolean isSystemApp(PackageManager packageManager, String[] packageNames) { + for (String packageName : packageNames) { + try { + final ApplicationInfo info = packageManager.getApplicationInfo(packageName, + 0 /* flags */); + if ((info.flags & ApplicationInfo.FLAG_SYSTEM) != 0) { + return true; + } + } catch (PackageManager.NameNotFoundException e) { + Log.e(TAG, "Package not found: " + packageName, e); + } + } + + return false; + } } diff --git a/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java b/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java index 8928efd3c4a..661cb53fa56 100644 --- a/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java +++ b/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobService.java @@ -30,6 +30,7 @@ import android.content.ComponentName; import android.content.ContentResolver; import android.content.Context; import android.content.Intent; +import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; import android.os.Bundle; import android.os.Process; @@ -145,21 +146,24 @@ public class AnomalyDetectionJobService extends JobService { : Settings.Global.getInt(contentResolver, Settings.Global.APP_AUTO_RESTRICTION_ENABLED, ON) == ON; final String packageName = batteryUtils.getPackageName(uid); - if (uid != UID_NULL && !isSystemUid(uid) - && !powerWhitelistBackend.isSysWhitelistedExceptIdle( - packageManager.getPackagesForUid(uid))) { - boolean anomalyDetected = true; - if (anomalyInfo.anomalyType - == StatsManagerConfig.AnomalyType.EXCESSIVE_BACKGROUND_SERVICE) { - if (!batteryUtils.isPreOApp(packageName) - || !batteryUtils.isAppHeavilyUsed(batteryStatsHelper, userManager, uid, - policy.excessiveBgDrainPercentage)) { - // Don't report if it is not legacy app or haven't used much battery - anomalyDetected = false; - } - } - if (anomalyDetected) { + final boolean anomalyDetected; + if (isExcessiveBackgroundAnomaly(anomalyInfo)) { + anomalyDetected = batteryUtils.isPreOApp(packageName) + && batteryUtils.isAppHeavilyUsed(batteryStatsHelper, userManager, uid, + policy.excessiveBgDrainPercentage); + } else { + anomalyDetected = true; + } + + if (anomalyDetected) { + if (batteryUtils.shouldHideAnomaly(powerWhitelistBackend, uid)) { + metricsFeatureProvider.action(context, + MetricsProto.MetricsEvent.ACTION_ANOMALY_IGNORED, + packageName, + Pair.create(MetricsProto.MetricsEvent.FIELD_CONTEXT, + anomalyInfo.anomalyType)); + } else { if (autoFeatureOn && anomalyInfo.autoRestriction) { // Auto restrict this app batteryUtils.setForceAppStandby(uid, packageName, @@ -215,8 +219,8 @@ public class AnomalyDetectionJobService extends JobService { return UID_NULL; } - private boolean isSystemUid(int uid) { - final int appUid = UserHandle.getAppId(uid); - return appUid >= Process.ROOT_UID && appUid < Process.FIRST_APPLICATION_UID; + private boolean isExcessiveBackgroundAnomaly(AnomalyInfo anomalyInfo) { + return anomalyInfo.anomalyType + == StatsManagerConfig.AnomalyType.EXCESSIVE_BACKGROUND_SERVICE; } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java index e05ff52246f..aba86c61b98 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java @@ -53,6 +53,7 @@ import com.android.settings.R; import com.android.settings.fuelgauge.anomaly.Anomaly; import com.android.settings.testutils.FakeFeatureFactory; import com.android.settings.testutils.SettingsRobolectricTestRunner; +import com.android.settingslib.fuelgauge.PowerWhitelistBackend; import org.junit.Before; import org.junit.Test; @@ -81,7 +82,7 @@ public class BatteryUtilsTest { private static final long TIME_SINCE_LAST_FULL_CHARGE_US = TIME_SINCE_LAST_FULL_CHARGE_MS * 1000; - private static final int UID = 123; + private static final int UID = 12345; private static final long TIME_EXPECTED_FOREGROUND = 1500; private static final long TIME_EXPECTED_BACKGROUND = 6000; private static final long TIME_EXPECTED_ALL = 7500; @@ -141,6 +142,8 @@ public class BatteryUtilsTest { private ApplicationInfo mHighApplicationInfo; @Mock private ApplicationInfo mLowApplicationInfo; + @Mock + private PowerWhitelistBackend mPowerWhitelistBackend; private BatteryUtils mBatteryUtils; private FakeFeatureFactory mFeatureFactory; private PowerUsageFeatureProvider mProvider; @@ -166,9 +169,9 @@ public class BatteryUtilsTest { when(mBatteryStatsHelper.getStats().computeBatteryRealtime(anyLong(), anyInt())).thenReturn( TIME_SINCE_LAST_FULL_CHARGE_US); - when(mPackageManager.getApplicationInfo(HIGH_SDK_PACKAGE, PackageManager.GET_META_DATA)) + when(mPackageManager.getApplicationInfo(eq(HIGH_SDK_PACKAGE), anyInt())) .thenReturn(mHighApplicationInfo); - when(mPackageManager.getApplicationInfo(LOW_SDK_PACKAGE, PackageManager.GET_META_DATA)) + when(mPackageManager.getApplicationInfo(eq(LOW_SDK_PACKAGE), anyInt())) .thenReturn(mLowApplicationInfo); mHighApplicationInfo.targetSdkVersion = Build.VERSION_CODES.O; mLowApplicationInfo.targetSdkVersion = Build.VERSION_CODES.L; @@ -570,4 +573,27 @@ public class BatteryUtilsTest { assertThat(mBatteryUtils.isAppHeavilyUsed(mBatteryStatsHelper, mUserManager, UID, DISCHARGE_AMOUNT /* threshold */ )).isFalse(); } + + @Test + public void testShouldHideAnomaly_systemApp_returnTrue() { + doReturn(new String[]{HIGH_SDK_PACKAGE}).when(mPackageManager).getPackagesForUid(UID); + mHighApplicationInfo.flags = ApplicationInfo.FLAG_SYSTEM; + + assertThat(mBatteryUtils.shouldHideAnomaly(mPowerWhitelistBackend, UID)).isTrue(); + } + + @Test + public void testShouldHideAnomaly_systemUid_returnTrue() { + final int systemUid = Process.ROOT_UID; + doReturn(new String[]{HIGH_SDK_PACKAGE}).when(mPackageManager).getPackagesForUid(systemUid); + + assertThat(mBatteryUtils.shouldHideAnomaly(mPowerWhitelistBackend, systemUid)).isTrue(); + } + + @Test + public void testShouldHideAnomaly_normalApp_returnFalse() { + doReturn(new String[]{HIGH_SDK_PACKAGE}).when(mPackageManager).getPackagesForUid(UID); + + assertThat(mBatteryUtils.shouldHideAnomaly(mPowerWhitelistBackend, UID)).isFalse(); + } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobServiceTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobServiceTest.java index 49567f6e458..16d7c3f6b37 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobServiceTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batterytip/AnomalyDetectionJobServiceTest.java @@ -141,6 +141,31 @@ public class AnomalyDetectionJobServiceTest { anyInt(), anyLong()); } + @Test + public void testSaveAnomalyToDatabase_systemApp_doNotSaveButLog() { + final ArrayList cookies = new ArrayList<>(); + cookies.add(SUBSCRIBER_COOKIES_AUTO_RESTRICTION); + mBundle.putStringArrayList(StatsManager.EXTRA_STATS_BROADCAST_SUBSCRIBER_COOKIES, cookies); + doReturn(SYSTEM_PACKAGE).when(mBatteryUtils).getPackageName(anyInt()); + doReturn(false).when(mPowerWhitelistBackend).isSysWhitelisted(SYSTEM_PACKAGE); + doReturn(Process.FIRST_APPLICATION_UID).when( + mAnomalyDetectionJobService).extractUidFromStatsDimensionsValue(any()); + doReturn(true).when(mBatteryUtils).shouldHideAnomaly(any(), anyInt()); + + mAnomalyDetectionJobService.saveAnomalyToDatabase(mContext, mBatteryStatsHelper, + mUserManager, mBatteryDatabaseManager, mBatteryUtils, mPolicy, + mPowerWhitelistBackend, mContext.getContentResolver(), + mFeatureFactory.powerUsageFeatureProvider, + mFeatureFactory.metricsFeatureProvider, mBundle); + + verify(mBatteryDatabaseManager, never()).insertAnomaly(anyInt(), anyString(), anyInt(), + anyInt(), anyLong()); + verify(mFeatureFactory.metricsFeatureProvider).action(mContext, + MetricsProto.MetricsEvent.ACTION_ANOMALY_IGNORED, + SYSTEM_PACKAGE, + Pair.create(MetricsProto.MetricsEvent.FIELD_CONTEXT, ANOMALY_TYPE)); + } + @Test public void testSaveAnomalyToDatabase_systemUid_doNotSave() { doReturn(Process.SYSTEM_UID).when( From 7d09e2ea6888724969d5c7e5dddd2e86015ed170 Mon Sep 17 00:00:00 2001 From: Lei Yu Date: Tue, 10 Apr 2018 14:38:46 -0700 Subject: [PATCH 2/2] Add method to check if app has launcher activity This method should only used for system apps. Change-Id: Id4109d8e223933269b8dae3aaa91b8a9186c527c Fixes: 77477987 Test: RunSettingsRoboTests --- .../settings/fuelgauge/BatteryUtils.java | 32 +++++++++++++++++-- .../settings/fuelgauge/BatteryUtilsTest.java | 19 ++++++++++- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/src/com/android/settings/fuelgauge/BatteryUtils.java b/src/com/android/settings/fuelgauge/BatteryUtils.java index 26439717ecf..111b279391c 100644 --- a/src/com/android/settings/fuelgauge/BatteryUtils.java +++ b/src/com/android/settings/fuelgauge/BatteryUtils.java @@ -21,7 +21,7 @@ import android.content.Intent; import android.content.IntentFilter; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; -import android.os.BatteryManager; +import android.content.pm.ResolveInfo; import android.os.BatteryStats; import android.os.Bundle; import android.os.Build; @@ -34,6 +34,7 @@ import android.support.annotation.Nullable; import android.support.annotation.StringRes; import android.support.annotation.VisibleForTesting; import android.support.annotation.WorkerThread; +import android.text.TextUtils; import android.text.format.DateUtils; import android.util.Log; import android.util.SparseLongArray; @@ -545,8 +546,8 @@ public class BatteryUtils { return true; } - return isSystemUid(uid) || isSystemApp(mPackageManager, packageNames) - || powerWhitelistBackend.isSysWhitelistedExceptIdle(packageNames); + return isSystemUid(uid) || powerWhitelistBackend.isSysWhitelistedExceptIdle(packageNames) + || (isSystemApp(mPackageManager, packageNames) && !hasLauncherEntry(packageNames)); } private boolean isSystemUid(int uid) { @@ -569,5 +570,30 @@ public class BatteryUtils { return false; } + + private boolean hasLauncherEntry(String[] packageNames) { + final Intent launchIntent = new Intent(Intent.ACTION_MAIN, null); + launchIntent.addCategory(Intent.CATEGORY_LAUNCHER); + + // If we do not specify MATCH_DIRECT_BOOT_AWARE or + // MATCH_DIRECT_BOOT_UNAWARE, system will derive and update the flags + // according to the user's lock state. When the user is locked, + // components + // with ComponentInfo#directBootAware == false will be filtered. We should + // explicitly include both direct boot aware and unaware components here. + final List resolveInfos = mPackageManager.queryIntentActivities(launchIntent, + PackageManager.MATCH_DISABLED_COMPONENTS + | PackageManager.MATCH_DIRECT_BOOT_AWARE + | PackageManager.MATCH_DIRECT_BOOT_UNAWARE + | PackageManager.MATCH_SYSTEM_ONLY); + for (int i = 0, size = resolveInfos.size(); i < size; i++) { + final ResolveInfo resolveInfo = resolveInfos.get(i); + if (ArrayUtils.contains(packageNames, resolveInfo.activityInfo.packageName)) { + return true; + } + } + + return false; + } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java index aba86c61b98..772bb8d6c1a 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryUtilsTest.java @@ -37,8 +37,10 @@ import static org.mockito.Mockito.when; import android.app.AppOpsManager; import android.content.Context; +import android.content.pm.ActivityInfo; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; +import android.content.pm.ResolveInfo; import android.os.BatteryStats; import android.os.Build; import android.os.Bundle; @@ -575,7 +577,22 @@ public class BatteryUtilsTest { } @Test - public void testShouldHideAnomaly_systemApp_returnTrue() { + public void testShouldHideAnomaly_systemAppWithLauncher_returnTrue() { + final List resolveInfos = new ArrayList<>(); + final ResolveInfo resolveInfo = new ResolveInfo(); + resolveInfo.activityInfo = new ActivityInfo(); + resolveInfo.activityInfo.packageName = HIGH_SDK_PACKAGE; + + doReturn(resolveInfos).when(mPackageManager).queryIntentActivities(any(), anyInt()); + doReturn(new String[]{HIGH_SDK_PACKAGE}).when(mPackageManager).getPackagesForUid(UID); + mHighApplicationInfo.flags = ApplicationInfo.FLAG_SYSTEM; + + assertThat(mBatteryUtils.shouldHideAnomaly(mPowerWhitelistBackend, UID)).isTrue(); + } + + @Test + public void testShouldHideAnomaly_systemAppWithoutLauncher_returnTrue() { + doReturn(new ArrayList<>()).when(mPackageManager).queryIntentActivities(any(), anyInt()); doReturn(new String[]{HIGH_SDK_PACKAGE}).when(mPackageManager).getPackagesForUid(UID); mHighApplicationInfo.flags = ApplicationInfo.FLAG_SYSTEM;