Add the key extraction method in PowerUsageSummary

This cl is to fix the preference flicker issue. The main reason is
that we cannot find cached preference because of the inconsistent
search key. The search key is calculated without considering
uidObj, but the key in preference(aka changed preference) maybe
changed later based on the existence of uidObj. So when we search
the changed preference using the wrong key, we won't get it even
though it exists in the cache.

In my cl, I group two parts(search key calc + considering uidObj)
together in a method to extract the key.

Bug: 33663705
Test: make RunSettingsRoboTests
Change-Id: I2ca0c558ffc0d49ed9576321ecbc02542b568d65

diff --git a/src/com/android/settings/fuelgauge/PowerUsageSummary.java b/src/com/android/settings/fuelgauge/PowerUsageSummary.java
index 14e6693..c4b97d4 100644 (file)
--- a/src/com/android/settings/fuelgauge/PowerUsageSummary.java
+++ b/src/com/android/settings/fuelgauge/PowerUsageSummary.java
@@ -30,6 +30,7 @@ import android.support.annotation.VisibleForTesting;
 import android.support.v7.preference.Preference;
 import android.support.v7.preference.PreferenceGroup;
 import android.text.TextUtils;
+import android.util.Log;
 import android.util.SparseArray;
 import android.util.TypedValue;
 import android.view.Menu;
@@ -359,10 +360,8 @@ public class PowerUsageSummary extends PowerUsageBase {
                         userHandle);
                 final CharSequence contentDescription = mUm.getBadgedLabelForUser(entry.getLabel(),
                         userHandle);
-                final String key = sipper.drainType == DrainType.APP ? sipper.getPackages() != null
-                        ? TextUtils.concat(sipper.getPackages()).toString()
-                        : String.valueOf(sipper.getUid())
-                        : sipper.drainType.toString();
+
+                final String key = extractKeyFromSipper(sipper);
                 PowerGaugePreference pref = (PowerGaugePreference) getCachedPreference(key);
                 if (pref == null) {
                     pref = new PowerGaugePreference(getPrefContext(), badgedIcon,
@@ -376,9 +375,6 @@ public class PowerUsageSummary extends PowerUsageBase {
                 pref.setTitle(entry.getLabel());
                 pref.setOrder(i + 1);
                 pref.setPercent(percentOfMax, percentOfTotal);
-                if (sipper.uidObj != null) {
-                    pref.setKey(Integer.toString(sipper.uidObj.getUid()));
-                }
                 if ((sipper.drainType != DrainType.APP || sipper.uidObj.getUid() == 0)
                          && sipper.drainType != DrainType.USER) {
                     pref.setTint(colorControl);
@@ -399,6 +395,20 @@ public class PowerUsageSummary extends PowerUsageBase {
         BatteryEntry.startRequestQueue();
     }
 
+    @VisibleForTesting
+    String extractKeyFromSipper(BatterySipper sipper) {
+        if (sipper.uidObj != null) {
+            return Integer.toString(sipper.getUid());
+        } else if (sipper.drainType != DrainType.APP) {
+            return sipper.drainType.toString();
+        } else if (sipper.getPackages() != null) {
+            return TextUtils.concat(sipper.getPackages()).toString();
+        } else {
+            Log.w(TAG, "Inappropriate BatterySipper without uid and package names: " + sipper);
+            return "-1";
+        }
+    }
+
     private static List<BatterySipper> getFakeStats() {
         ArrayList<BatterySipper> stats = new ArrayList<>();
         float use = 5;

diff --git a/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageSummaryTest.java b/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageSummaryTest.java
index ff84741..1cf83f1 100644 (file)
--- a/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageSummaryTest.java
+++ b/tests/robotests/src/com/android/settings/fuelgauge/PowerUsageSummaryTest.java
@@ -1,10 +1,28 @@
+/*
+ * Copyright (C) 2016 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.settings.fuelgauge;
 
 import android.content.Context;
 import android.content.Intent;
+import android.text.TextUtils;
 import android.view.Menu;
 import android.view.MenuInflater;
 import android.view.MenuItem;
+import com.android.internal.os.BatterySipper;
+import com.android.internal.os.BatteryStatsImpl;
 import com.android.settings.R;
 import com.android.settings.TestConfig;
 import com.android.settings.testutils.FakeFeatureFactory;
@@ -32,6 +50,8 @@ import static org.mockito.Mockito.when;
 @RunWith(RobolectricTestRunner.class)
 @Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION)
 public class PowerUsageSummaryTest {
+    private static final String[] PACKAGE_NAMES = {"com.app1", "com.app2"};
+    private static final int UID = 123;
 
     private static final Intent ADDITIONAL_BATTERY_INFO_INTENT =
             new Intent("com.example.app.ADDITIONAL_BATTERY_INFO");
@@ -46,13 +66,17 @@ public class PowerUsageSummaryTest {
     private MenuItem mAdditionalBatteryInfoMenu;
     @Mock
     private MenuInflater mMenuInflater;
+    @Mock
+    private BatterySipper mBatterySipper;
 
     private TestFragment mFragment;
     private FakeFeatureFactory mFeatureFactory;
+    private PowerUsageSummary mPowerUsageSummary;
 
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
+
         FakeFeatureFactory.setupForTest(mContext);
         mFeatureFactory = (FakeFeatureFactory) FakeFeatureFactory.getFactory(mContext);
 
@@ -67,6 +91,11 @@ public class PowerUsageSummaryTest {
                 .thenReturn(MENU_ADDITIONAL_BATTERY_INFO);
         when(mFeatureFactory.powerUsageFeatureProvider.getAdditionalBatteryInfoIntent())
                 .thenReturn(ADDITIONAL_BATTERY_INFO_INTENT);
+
+        mPowerUsageSummary = new PowerUsageSummary();
+
+        when(mBatterySipper.getPackages()).thenReturn(PACKAGE_NAMES);
+        when(mBatterySipper.getUid()).thenReturn(UID);
     }
 
     @Test
@@ -96,6 +125,33 @@ public class PowerUsageSummaryTest {
                 Menu.NONE, R.string.additional_battery_info);
     }
 
+    @Test
+    public void testExtractKeyFromSipper_TypeAPPUidObjectNull_ReturnPackageNames() {
+        mBatterySipper.uidObj = null;
+        mBatterySipper.drainType = BatterySipper.DrainType.APP;
+
+        final String key = mPowerUsageSummary.extractKeyFromSipper(mBatterySipper);
+        assertThat(key).isEqualTo(TextUtils.concat(mBatterySipper.getPackages()).toString());
+    }
+
+    @Test
+    public void testExtractKeyFromSipper_TypeOther_ReturnDrainType() {
+        mBatterySipper.uidObj = null;
+        mBatterySipper.drainType = BatterySipper.DrainType.BLUETOOTH;
+
+        final String key = mPowerUsageSummary.extractKeyFromSipper(mBatterySipper);
+        assertThat(key).isEqualTo(mBatterySipper.drainType.toString());
+    }
+
+    @Test
+    public void testExtractKeyFromSipper_TypeAPPUidObjectNotNull_ReturnUid() {
+        mBatterySipper.uidObj = new BatteryStatsImpl.Uid(new BatteryStatsImpl(), UID);
+        mBatterySipper.drainType = BatterySipper.DrainType.APP;
+
+        final String key = mPowerUsageSummary.extractKeyFromSipper(mBatterySipper);
+        assertThat(key).isEqualTo(Integer.toString(mBatterySipper.getUid()));
+    }
+
     public static class TestFragment extends PowerUsageSummary {
 
         private Context mContext;