From 764d513d00122e9402364e370bed634df7323d81 Mon Sep 17 00:00:00 2001 From: ykhung Date: Sun, 10 Jul 2022 23:47:42 +0800 Subject: [PATCH] Support multi-user privacy for battery usage chart When there are multiple accounts in the devices, the battery usage list is shared in the current design. We will aggregate other users usage data into a single item to support multi-user privacy requirements Screenshot: https://screenshot.googleplex.com/AkFTUtNvnoxcuGR Bug: 202119550 Test: make RunSettingsRoboTests Change-Id: I6cb55f0d50a4caca83212a0a54410530a032c089 --- res/values/strings.xml | 2 + .../settings/fuelgauge/BatteryUtils.java | 2 + .../batteryusage/BatteryDiffEntry.java | 14 ++++ .../fuelgauge/batteryusage/ConvertUtils.java | 61 ++++++++++++++- .../fuelgauge/BatteryBackupHelperTest.java | 11 ++- .../batteryusage/BatteryDiffEntryTest.java | 41 ++++++---- .../batteryusage/ConvertUtilsTest.java | 76 +++++++++++++++++++ 7 files changed, 191 insertions(+), 16 deletions(-) diff --git a/res/values/strings.xml b/res/values/strings.xml index b7007c4b69c..fde4f262b59 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -6646,6 +6646,8 @@ No usage for past 24 hr + + Other users Battery left estimate is based on your device usage diff --git a/src/com/android/settings/fuelgauge/BatteryUtils.java b/src/com/android/settings/fuelgauge/BatteryUtils.java index 3f6c1657165..a6c48a48d6b 100644 --- a/src/com/android/settings/fuelgauge/BatteryUtils.java +++ b/src/com/android/settings/fuelgauge/BatteryUtils.java @@ -69,6 +69,8 @@ public class BatteryUtils { public static final int UID_REMOVED_APPS = -4; /** Special UID value for data usage by tethering. */ public static final int UID_TETHERING = -5; + /** Special UID for aggregated other users. */ + public static final long UID_OTHER_USERS = Long.MIN_VALUE; @Retention(RetentionPolicy.SOURCE) @IntDef({StatusType.SCREEN_USAGE, diff --git a/src/com/android/settings/fuelgauge/batteryusage/BatteryDiffEntry.java b/src/com/android/settings/fuelgauge/batteryusage/BatteryDiffEntry.java index 8b995c859f9..d4d7a00fdc2 100644 --- a/src/com/android/settings/fuelgauge/batteryusage/BatteryDiffEntry.java +++ b/src/com/android/settings/fuelgauge/batteryusage/BatteryDiffEntry.java @@ -113,6 +113,9 @@ public class BatteryDiffEntry { /** Gets the app label name for this entry. */ public String getAppLabel() { + if (isOtherUsers()) { + return mContext.getString(R.string.battery_usage_other_users); + } loadLabelAndIcon(); // Returns default applicationn label if we cannot find it. return mAppLabel == null || mAppLabel.length() == 0 @@ -122,6 +125,9 @@ public class BatteryDiffEntry { /** Gets the app icon {@link Drawable} for this entry. */ public Drawable getAppIcon() { + if (isOtherUsers()) { + return mContext.getDrawable(R.drawable.ic_power_system); + } loadLabelAndIcon(); return mAppIcon != null && mAppIcon.getConstantState() != null ? mAppIcon.getConstantState().newDrawable() @@ -156,6 +162,9 @@ public class BatteryDiffEntry { /** Whether the current BatteryDiffEntry is system component or not. */ public boolean isSystemEntry() { + if (isOtherUsers()) { + return true; + } switch (mBatteryHistEntry.mConsumerType) { case ConvertUtils.CONSUMER_TYPE_USER_BATTERY: case ConvertUtils.CONSUMER_TYPE_SYSTEM_BATTERY: @@ -175,6 +184,11 @@ public class BatteryDiffEntry { return false; } + private boolean isOtherUsers() { + return mBatteryHistEntry.mConsumerType == ConvertUtils.CONSUMER_TYPE_UID_BATTERY + && mBatteryHistEntry.mUid == BatteryUtils.UID_OTHER_USERS; + } + void loadLabelAndIcon() { if (mIsLoaded) { return; diff --git a/src/com/android/settings/fuelgauge/batteryusage/ConvertUtils.java b/src/com/android/settings/fuelgauge/batteryusage/ConvertUtils.java index 76f9419ac41..168fe0f5383 100644 --- a/src/com/android/settings/fuelgauge/batteryusage/ConvertUtils.java +++ b/src/com/android/settings/fuelgauge/batteryusage/ConvertUtils.java @@ -21,6 +21,7 @@ import android.content.Context; import android.os.BatteryUsageStats; import android.os.LocaleList; import android.os.UserHandle; +import android.os.UserManager; import android.text.format.DateFormat; import android.text.format.DateUtils; import android.util.ArraySet; @@ -28,6 +29,8 @@ import android.util.Log; import androidx.annotation.VisibleForTesting; +import com.android.settings.Utils; +import com.android.settings.fuelgauge.BatteryUtils; import com.android.settings.overlay.FeatureFactory; import java.lang.annotation.Retention; @@ -265,17 +268,55 @@ public final class ConvertUtils { } } insert24HoursData(BatteryChartView.SELECTED_INDEX_ALL, resultMap); + resolveMultiUsersData(context, resultMap); if (purgeLowPercentageAndFakeData) { purgeLowPercentageAndFakeData(context, resultMap); } return resultMap; } + @VisibleForTesting + static void resolveMultiUsersData( + final Context context, + final Map> indexedUsageMap) { + final int currentUserId = context.getUserId(); + final UserHandle userHandle = + Utils.getManagedProfile(context.getSystemService(UserManager.class)); + final int workProfileUserId = + userHandle != null ? userHandle.getIdentifier() : Integer.MIN_VALUE; + // Loops for all BatteryDiffEntry in the different slots. + for (List entryList : indexedUsageMap.values()) { + double consumePowerFromOtherUsers = 0f; + double consumePercentageFromOtherUsers = 0f; + final Iterator iterator = entryList.iterator(); + while (iterator.hasNext()) { + final BatteryDiffEntry entry = iterator.next(); + final BatteryHistEntry batteryHistEntry = entry.mBatteryHistEntry; + if (batteryHistEntry.mConsumerType != CONSUMER_TYPE_UID_BATTERY) { + continue; + } + // Whether the BatteryHistEntry represents the current user data? + if (batteryHistEntry.mUserId == currentUserId + || batteryHistEntry.mUserId == workProfileUserId) { + continue; + } + // Removes and aggregates non-current users data from the list. + iterator.remove(); + consumePowerFromOtherUsers += entry.mConsumePower; + consumePercentageFromOtherUsers += entry.getPercentOfTotal(); + } + if (consumePercentageFromOtherUsers != 0) { + entryList.add(createOtherUsersEntry(context, consumePowerFromOtherUsers, + consumePercentageFromOtherUsers)); + } + } + } + private static void insert24HoursData( final int desiredIndex, final Map> indexedUsageMap) { final Map resultMap = new HashMap<>(); - double totalConsumePower = 0.0; + double totalConsumePower = 0f; // Loops for all BatteryDiffEntry and aggregate them together. for (List entryList : indexedUsageMap.values()) { for (BatteryDiffEntry entry : entryList) { @@ -361,4 +402,22 @@ public final class ConvertUtils { return locales != null && !locales.isEmpty() ? locales.get(0) : Locale.getDefault(); } + + private static BatteryDiffEntry createOtherUsersEntry( + Context context, double consumePower, double consumePercentage) { + final ContentValues values = new ContentValues(); + values.put(BatteryHistEntry.KEY_UID, BatteryUtils.UID_OTHER_USERS); + values.put(BatteryHistEntry.KEY_USER_ID, BatteryUtils.UID_OTHER_USERS); + values.put(BatteryHistEntry.KEY_CONSUMER_TYPE, CONSUMER_TYPE_UID_BATTERY); + // We will show the percentage for the "other users" item only, the aggregated + // running time information is useless for users to identify individual apps. + final BatteryDiffEntry batteryDiffEntry = new BatteryDiffEntry( + context, + /*foregroundUsageTimeInMs=*/ 0, + /*backgroundUsageTimeInMs=*/ 0, + consumePower, + new BatteryHistEntry(values)); + batteryDiffEntry.setTotalConsumePower(100 * consumePower / consumePercentage); + return batteryDiffEntry; + } } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryBackupHelperTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryBackupHelperTest.java index 14bbeea7187..283df7bcc43 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/BatteryBackupHelperTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryBackupHelperTest.java @@ -70,6 +70,7 @@ import org.robolectric.annotation.Resetter; import java.util.Arrays; import java.util.List; +import java.util.Set; import java.util.concurrent.TimeUnit; @RunWith(RobolectricTestRunner.class) @@ -343,9 +344,17 @@ public final class BatteryBackupHelperTest { private void verifyBackupData(String expectedResult) throws Exception { final byte[] expectedBytes = expectedResult.getBytes(); + final ArgumentCaptor captor = ArgumentCaptor.forClass(byte[].class); + final Set expectedResultSet = + Set.of(expectedResult.split(BatteryBackupHelper.DELIMITER)); + verify(mBackupDataOutput).writeEntityHeader( BatteryBackupHelper.KEY_OPTIMIZATION_LIST, expectedBytes.length); - verify(mBackupDataOutput).writeEntityData(expectedBytes, expectedBytes.length); + verify(mBackupDataOutput).writeEntityData(captor.capture(), eq(expectedBytes.length)); + final String actualResult = new String(captor.getValue()); + final Set actualResultSet = + Set.of(actualResult.split(BatteryBackupHelper.DELIMITER)); + assertThat(actualResultSet).isEqualTo(expectedResultSet); } private void createTestingData( diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryDiffEntryTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryDiffEntryTest.java index bb19b5a53dc..683f0fd2df4 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryDiffEntryTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/BatteryDiffEntryTest.java @@ -138,7 +138,7 @@ public final class BatteryDiffEntryTest { // Generates fake testing data. final ContentValues values = getContentValuesWithType( ConvertUtils.CONSUMER_TYPE_SYSTEM_BATTERY); - values.put("drainType", + values.put(BatteryHistEntry.KEY_DRAIN_TYPE, Integer.valueOf(BatteryConsumer.POWER_COMPONENT_AMBIENT_DISPLAY)); final BatteryHistEntry batteryHistEntry = new BatteryHistEntry(values); @@ -164,7 +164,7 @@ public final class BatteryDiffEntryTest { // Generates fake testing data. final ContentValues values = getContentValuesWithType( ConvertUtils.CONSUMER_TYPE_USER_BATTERY); - values.put("userId", Integer.valueOf(1001)); + values.put(BatteryHistEntry.KEY_USER_ID, Integer.valueOf(1001)); final BatteryHistEntry batteryHistEntry = new BatteryHistEntry(values); final BatteryDiffEntry entry = createBatteryDiffEntry(10, batteryHistEntry); @@ -189,8 +189,8 @@ public final class BatteryDiffEntryTest { final String fakePackageName = "com.fake.google.com"; final ContentValues values = getContentValuesWithType( ConvertUtils.CONSUMER_TYPE_UID_BATTERY); - values.put("uid", /*invalid uid*/ 10001); - values.put("packageName", fakePackageName); + values.put(BatteryHistEntry.KEY_UID, /*invalid uid*/ 10001); + values.put(BatteryHistEntry.KEY_PACKAGE_NAME, fakePackageName); doReturn(mMockAppInfo).when(mMockPackageManager) .getApplicationInfo(fakePackageName, 0); doReturn(expectedAppLabel).when(mMockPackageManager) @@ -233,7 +233,7 @@ public final class BatteryDiffEntryTest { final String expectedAppLabel = "fake app label"; final ContentValues values = getContentValuesWithType( ConvertUtils.CONSUMER_TYPE_UID_BATTERY); - values.put("appLabel", expectedAppLabel); + values.put(BatteryHistEntry.KEY_APP_LABEL, expectedAppLabel); final BatteryHistEntry batteryHistEntry = new BatteryHistEntry(values); final BatteryDiffEntry entry = createBatteryDiffEntry(10, batteryHistEntry); @@ -391,8 +391,8 @@ public final class BatteryDiffEntryTest { final String fakePackageName = "com.fake.google.com"; final ContentValues values = getContentValuesWithType( ConvertUtils.CONSUMER_TYPE_UID_BATTERY); - values.put("uid", /*invalid uid*/ 10001); - values.put("packageName", fakePackageName); + values.put(BatteryHistEntry.KEY_UID, /*invalid uid*/ 10001); + values.put(BatteryHistEntry.KEY_PACKAGE_NAME, fakePackageName); final BatteryDiffEntry entry = createBatteryDiffEntry(10, new BatteryHistEntry(values)); @@ -424,7 +424,7 @@ public final class BatteryDiffEntryTest { final String expectedPackageName = "com.fake.google.com"; final ContentValues values = getContentValuesWithType( ConvertUtils.CONSUMER_TYPE_UID_BATTERY); - values.put("packageName", expectedPackageName); + values.put(BatteryHistEntry.KEY_PACKAGE_NAME, expectedPackageName); final BatteryDiffEntry entry = createBatteryDiffEntry(10, new BatteryHistEntry(values)); @@ -437,7 +437,7 @@ public final class BatteryDiffEntryTest { final ContentValues values = getContentValuesWithType( ConvertUtils.CONSUMER_TYPE_UID_BATTERY); values.put( - "packageName", + BatteryHistEntry.KEY_PACKAGE_NAME, expectedPackageName + ":privileged_process0"); final BatteryDiffEntry entry = createBatteryDiffEntry(10, new BatteryHistEntry(values)); @@ -445,11 +445,24 @@ public final class BatteryDiffEntryTest { assertThat(entry.getPackageName()).isEqualTo(expectedPackageName); } + @Test + public void getAppLabel_withOtherUsersUid_returnExpectedLabel() { + final ContentValues values = getContentValuesWithType( + ConvertUtils.CONSUMER_TYPE_UID_BATTERY); + values.put(BatteryHistEntry.KEY_UID, BatteryUtils.UID_OTHER_USERS); + + final BatteryDiffEntry batteryDiffEntry = createBatteryDiffEntry( + /*consumePower=*/ 0, new BatteryHistEntry(values)); + + assertThat(batteryDiffEntry.getAppLabel()) + .isEqualTo(mContext.getString(R.string.battery_usage_other_users)); + } + private BatteryDiffEntry createBatteryDiffEntry( int consumerType, long uid, boolean isHidden) { final ContentValues values = getContentValuesWithType(consumerType); - values.put("isHidden", isHidden); - values.put("uid", uid); + values.put(BatteryHistEntry.KEY_IS_HIDDEN, isHidden); + values.put(BatteryHistEntry.KEY_UID, uid); return new BatteryDiffEntry( mContext, /*foregroundUsageTimeInMs=*/ 0, @@ -472,15 +485,15 @@ public final class BatteryDiffEntryTest { private static ContentValues getContentValuesWithType(int consumerType) { final ContentValues values = new ContentValues(); - values.put("consumerType", Integer.valueOf(consumerType)); + values.put(BatteryHistEntry.KEY_CONSUMER_TYPE, Integer.valueOf(consumerType)); return values; } private BatteryDiffEntry createBatteryDiffEntry(Drawable drawable) throws Exception { final ContentValues values = getContentValuesWithType( ConvertUtils.CONSUMER_TYPE_UID_BATTERY); - values.put("uid", 1001); - values.put("packageName", "com.a.b.c"); + values.put(BatteryHistEntry.KEY_UID, 1001); + values.put(BatteryHistEntry.KEY_PACKAGE_NAME, "com.a.b.c"); final BatteryHistEntry batteryHistEntry = new BatteryHistEntry(values); doReturn(drawable).when(mMockPackageManager).getDefaultActivityIcon(); doReturn(null).when(mMockPackageManager).getApplicationInfo("com.a.b.c", 0); diff --git a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/ConvertUtilsTest.java b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/ConvertUtilsTest.java index 0b8a98a742d..c1f981539c1 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/ConvertUtilsTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/batteryusage/ConvertUtilsTest.java @@ -27,6 +27,7 @@ import android.os.BatteryUsageStats; import android.os.LocaleList; import android.os.UserHandle; +import com.android.settings.fuelgauge.BatteryUtils; import com.android.settings.fuelgauge.PowerUsageFeatureProvider; import com.android.settings.testutils.FakeFeatureFactory; @@ -39,6 +40,7 @@ import org.robolectric.RobolectricTestRunner; import org.robolectric.RuntimeEnvironment; import java.util.Arrays; +import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -371,6 +373,71 @@ public final class ConvertUtilsTest { assertThat(ConvertUtils.getLocale(mContext)).isEqualTo(Locale.getDefault()); } + @Test + public void resolveMultiUsersData_replaceOtherUsersItemWithExpectedEntry() { + final int currentUserId = mContext.getUserId(); + final Map> entryMap = new HashMap<>(); + // Without other users time slot. + entryMap.put(0, Arrays.asList( + createBatteryDiffEntry( + currentUserId, + ConvertUtils.CONSUMER_TYPE_UID_BATTERY, + /*consumePercentage=*/ 50))); + // With other users time slot. + final List withOtherUsersList = new ArrayList<>(); + entryMap.put(1, withOtherUsersList); + withOtherUsersList.add( + createBatteryDiffEntry( + currentUserId + 1, + ConvertUtils.CONSUMER_TYPE_SYSTEM_BATTERY, + /*consumePercentage=*/ 20)); + withOtherUsersList.add( + createBatteryDiffEntry( + currentUserId + 2, + ConvertUtils.CONSUMER_TYPE_UID_BATTERY, + /*consumePercentage=*/ 30)); + withOtherUsersList.add( + createBatteryDiffEntry( + currentUserId + 3, + ConvertUtils.CONSUMER_TYPE_UID_BATTERY, + /*consumePercentage=*/ 40)); + + ConvertUtils.resolveMultiUsersData(mContext, entryMap); + + assertThat(entryMap.get(0).get(0).getPercentOfTotal()).isEqualTo(50); + // Asserts with other users items. + final List entryList = entryMap.get(1); + assertThat(entryList).hasSize(2); + assertBatteryDiffEntry( + entryList.get(0), + currentUserId + 1, + /*uid=*/ 0, + ConvertUtils.CONSUMER_TYPE_SYSTEM_BATTERY, + /*consumePercentage=*/ 20); + assertBatteryDiffEntry( + entryList.get(1), + BatteryUtils.UID_OTHER_USERS, + BatteryUtils.UID_OTHER_USERS, + ConvertUtils.CONSUMER_TYPE_UID_BATTERY, + /*consumePercentage=*/ 70); + } + + private BatteryDiffEntry createBatteryDiffEntry( + long userId, int counsumerType, double consumePercentage) { + final ContentValues values = new ContentValues(); + values.put(BatteryHistEntry.KEY_USER_ID, userId); + values.put(BatteryHistEntry.KEY_CONSUMER_TYPE, counsumerType); + final BatteryDiffEntry batteryDiffEntry = + new BatteryDiffEntry( + mContext, + /*foregroundUsageTimeInMs=*/ 0, + /*backgroundUsageTimeInMs=*/ 0, + /*consumePower=*/ consumePercentage, + new BatteryHistEntry(values)); + batteryDiffEntry.setTotalConsumePower(100f); + return batteryDiffEntry; + } + private static BatteryHistEntry createBatteryHistEntry( String packageName, String appLabel, double consumePower, long uid, long foregroundUsageTimeInMs, long backgroundUsageTimeInMs) { @@ -389,6 +456,15 @@ public final class ConvertUtilsTest { return new BatteryHistEntry(values); } + private static void assertBatteryDiffEntry( + BatteryDiffEntry entry, long userId, long uid, int counsumerType, + double consumePercentage) { + assertThat(entry.mBatteryHistEntry.mUid).isEqualTo(uid); + assertThat(entry.mBatteryHistEntry.mUserId).isEqualTo(userId); + assertThat(entry.mBatteryHistEntry.mConsumerType).isEqualTo(counsumerType); + assertThat(entry.getPercentOfTotal()).isEqualTo(consumePercentage); + } + private static void assertBatteryDiffEntry( BatteryDiffEntry entry, int percentOfTotal, long foregroundUsageTimeInMs, long backgroundUsageTimeInMs) {