From a3b89868f46da99bd2dee930d4f9f7d415eb00ff Mon Sep 17 00:00:00 2001 From: ykhung Date: Sat, 11 Sep 2021 00:42:56 +0800 Subject: [PATCH 1/2] Fix the icon flash issue cause from the shared drawable icon different time slot may have the same application entry, we should not make them share the same drawable instance to impact the states, we will use newDrawable() method to create a new one for each application entry Bug: 198553245 Test: make SettingsRoboTests Change-Id: I4a321133ba171817fca1ab7ad47b9af8f7450675 --- src/com/android/settings/fuelgauge/BatteryDiffEntry.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/com/android/settings/fuelgauge/BatteryDiffEntry.java b/src/com/android/settings/fuelgauge/BatteryDiffEntry.java index 0074f9351e2..f7d01d9711f 100644 --- a/src/com/android/settings/fuelgauge/BatteryDiffEntry.java +++ b/src/com/android/settings/fuelgauge/BatteryDiffEntry.java @@ -113,7 +113,7 @@ public class BatteryDiffEntry { /** Gets the app icon {@link Drawable} for this entry. */ public Drawable getAppIcon() { loadLabelAndIcon(); - return mAppIcon; + return mAppIcon.getConstantState().newDrawable(); } /** Gets the app icon id for this entry. */ @@ -348,8 +348,8 @@ public class BatteryDiffEntry { private Drawable getBadgeIconForUser(Drawable icon) { final int userId = UserHandle.getUserId((int) mBatteryHistEntry.mUid); - final UserHandle userHandle = new UserHandle(userId); - return mUserManager.getBadgedIconForUser(icon, userHandle); + return userId == UserHandle.USER_OWNER ? icon : + mUserManager.getBadgedIconForUser(icon, new UserHandle(userId)); } private static boolean isSystemUid(int uid) { From 27efa546cf2514ff98d29e8278a217e6b8b2808f Mon Sep 17 00:00:00 2001 From: ykhung Date: Sat, 11 Sep 2021 16:29:36 +0800 Subject: [PATCH 2/2] Fix failed tests since presubmit is ignored in ag/15802168 Bug: 198553245 Test: make SettingsRoboTests Change-Id: I91e715a6bfb64419b457812423996365a9464a93 --- .../settings/fuelgauge/BatteryDiffEntry.java | 4 +- .../BatteryChartPreferenceControllerTest.java | 5 +- .../fuelgauge/BatteryDiffEntryTest.java | 68 ++++++++++++++++--- 3 files changed, 66 insertions(+), 11 deletions(-) diff --git a/src/com/android/settings/fuelgauge/BatteryDiffEntry.java b/src/com/android/settings/fuelgauge/BatteryDiffEntry.java index f7d01d9711f..e524e984d78 100644 --- a/src/com/android/settings/fuelgauge/BatteryDiffEntry.java +++ b/src/com/android/settings/fuelgauge/BatteryDiffEntry.java @@ -113,7 +113,9 @@ public class BatteryDiffEntry { /** Gets the app icon {@link Drawable} for this entry. */ public Drawable getAppIcon() { loadLabelAndIcon(); - return mAppIcon.getConstantState().newDrawable(); + return mAppIcon != null && mAppIcon.getConstantState() != null + ? mAppIcon.getConstantState().newDrawable() + : null; } /** Gets the app icon id for this entry. */ diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryChartPreferenceControllerTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryChartPreferenceControllerTest.java index 32e4000a480..a9a743b2af2 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/BatteryChartPreferenceControllerTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryChartPreferenceControllerTest.java @@ -105,6 +105,9 @@ public final class BatteryChartPreferenceControllerTest { doReturn(new String[] {"com.android.googlequicksearchbox"}) .when(mFeatureFactory.powerUsageFeatureProvider) .getHideApplicationSummary(mContext); + doReturn(new String[] {"com.android.gms.persistent"}) + .when(mFeatureFactory.powerUsageFeatureProvider) + .getHideApplicationEntries(mContext); mBatteryChartPreferenceController = createController(); mBatteryChartPreferenceController.mPrefContext = mContext; mBatteryChartPreferenceController.mAppListPrefGroup = mAppListGroup; @@ -661,7 +664,7 @@ public final class BatteryChartPreferenceControllerTest { // Verifies the items which are defined in the array list. assertThat(mBatteryChartPreferenceController - .isValidToShowEntry("com.google.android.gms.persistent")) + .isValidToShowEntry("com.android.gms.persistent")) .isFalse(); } diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryDiffEntryTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryDiffEntryTest.java index 85ac9413041..b1d8f0d5a6d 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/BatteryDiffEntryTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryDiffEntryTest.java @@ -17,6 +17,7 @@ package com.android.settings.fuelgauge; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Matchers.anyInt; import static org.mockito.Mockito.doReturn; @@ -28,6 +29,7 @@ import android.content.pm.ApplicationInfo; import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.graphics.drawable.Drawable; +import android.graphics.drawable.Drawable.ConstantState; import android.os.BatteryConsumer; import android.os.UserHandle; import android.os.UserManager; @@ -41,6 +43,10 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.robolectric.RobolectricTestRunner; import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; +import org.robolectric.annotation.Implementation; +import org.robolectric.annotation.Implements; +import org.robolectric.annotation.Resetter; import java.util.ArrayList; import java.util.Collections; @@ -48,6 +54,7 @@ import java.util.List; import java.util.Locale; @RunWith(RobolectricTestRunner.class) +@Config(shadows = {BatteryDiffEntryTest.ShadowUserHandle.class}) public final class BatteryDiffEntryTest { private Context mContext; @@ -60,10 +67,12 @@ public final class BatteryDiffEntryTest { @Mock private Drawable mockBadgedDrawable; @Mock private BatteryHistEntry mBatteryHistEntry; @Mock private PackageInfo mockPackageInfo; + @Mock private ConstantState mockConstantState; @Before public void setUp() { MockitoAnnotations.initMocks(this); + ShadowUserHandle.reset(); mContext = spy(RuntimeEnvironment.application); doReturn(mContext).when(mContext).getApplicationContext(); doReturn(mockUserManager).when(mContext).getSystemService(UserManager.class); @@ -229,6 +238,7 @@ public final class BatteryDiffEntryTest { final ContentValues values = getContentValuesWithType( ConvertUtils.CONSUMER_TYPE_SYSTEM_BATTERY); final BatteryHistEntry batteryHistEntry = new BatteryHistEntry(values); + mockConstantState(mockDrawable); final BatteryDiffEntry entry = createBatteryDiffEntry(10, batteryHistEntry); @@ -239,20 +249,32 @@ public final class BatteryDiffEntryTest { } @Test - public void testGetAppIcon_uidConsumerWithNullIcon_returnDefaultActivityIcon() + public void testGetAppIcon_uidConsumerForNonOwner_returnDefaultActivityIconWithBadge() throws Exception { + ShadowUserHandle.setUid(10); final BatteryDiffEntry entry = createBatteryDiffEntry(mockDrawable); - final int userId = UserHandle.getUserId(1001); + mockConstantState(mockDrawable); + mockConstantState(mockBadgedDrawable); doReturn(mockBadgedDrawable).when(mockUserManager) - .getBadgedIconForUser(mockDrawable, new UserHandle(userId)); + .getBadgedIconForUser(eq(mockDrawable), any()); entry.mAppIcon = null; assertThat(entry.getAppIcon()).isEqualTo(mockBadgedDrawable); + } + + @Test + public void testGetAppIcon_uidConsumerWithNullIcon_returnDefaultActivityIcon() + throws Exception { + final BatteryDiffEntry entry = createBatteryDiffEntry(mockDrawable); + mockConstantState(mockDrawable); + + entry.mAppIcon = null; + assertThat(entry.getAppIcon()).isEqualTo(mockDrawable); assertThat(BatteryDiffEntry.sResourceCache).hasSize(1); // Verifies the app label in the cache. final BatteryEntry.NameAndIcon nameAndIcon = BatteryDiffEntry.sResourceCache.get(entry.getKey()); - assertThat(nameAndIcon.icon).isEqualTo(mockBadgedDrawable); + assertThat(nameAndIcon.icon).isEqualTo(mockDrawable); } @Test @@ -272,19 +294,17 @@ public final class BatteryDiffEntryTest { @Test public void testClearCache_switchLocale_clearCacheIconAndLabel() throws Exception { final int userId = UserHandle.getUserId(1001); - doReturn(mockBadgedDrawable).when(mockUserManager) - .getBadgedIconForUser(mockDrawable, new UserHandle(userId)); - doReturn(mockDrawable2).when(mockUserManager) - .getBadgedIconForUser(mockDrawable2, new UserHandle(userId)); Locale.setDefault(new Locale("en_US")); final BatteryDiffEntry entry1 = createBatteryDiffEntry(mockDrawable); - assertThat(entry1.getAppIcon()).isEqualTo(mockBadgedDrawable); + mockConstantState(mockDrawable); + assertThat(entry1.getAppIcon()).isEqualTo(mockDrawable); // Switch the locale into another one. Locale.setDefault(new Locale("zh_TW")); final BatteryDiffEntry entry2 = createBatteryDiffEntry(mockDrawable2); // We should get new drawable without caching. + mockConstantState(mockDrawable2); assertThat(entry2.getAppIcon()).isEqualTo(mockDrawable2); // Verifies the cache is updated into the new drawable. final BatteryEntry.NameAndIcon nameAndIcon = @@ -440,4 +460,34 @@ public final class BatteryDiffEntryTest { .getPackagesForUid(1001); return createBatteryDiffEntry(10, batteryHistEntry); } + + private void mockConstantState(Drawable drawable) { + doReturn(mockConstantState).when(drawable).getConstantState(); + doReturn(drawable).when(mockConstantState).newDrawable(); + } + + @Implements(UserHandle.class) + public static class ShadowUserHandle { + // Sets the default as thte OWNER role. + private static int sUid = 0; + + public static void setUid(int uid) { + sUid = uid; + } + + @Implementation + public static int myUserId() { + return sUid; + } + + @Implementation + public static int getUserId(int userId) { + return sUid; + } + + @Resetter + public static void reset() { + sUid = 0; + } + } }