From a5f1b5c629503eae52113e8aac5f80ef0ba8658a Mon Sep 17 00:00:00 2001 From: jackqdyulei Date: Mon, 14 Jan 2019 10:55:39 -0800 Subject: [PATCH] Fix crash for UiBlockerController 1. Don't create UiBlockerController if there is no related controller 2. Don't update visibility if UiBlockerController is null 3. Use findPreference() from DashboardFragment, which already has null check. Change-Id: Iee24c64317fb9d5a1cf2076d25728af485d390c5 Fixes: 122807414 Fixes: 122805831 Test: RunSettingsRoboTests --- .../settings/dashboard/DashboardFragment.java | 24 +++++++----- .../dashboard/UiBlockerController.java | 2 +- .../dashboard/DashboardFragmentTest.java | 39 +++++++++++++++++++ 3 files changed, 55 insertions(+), 10 deletions(-) diff --git a/src/com/android/settings/dashboard/DashboardFragment.java b/src/com/android/settings/dashboard/DashboardFragment.java index 11858a79b6c..e2c013d663f 100644 --- a/src/com/android/settings/dashboard/DashboardFragment.java +++ b/src/com/android/settings/dashboard/DashboardFragment.java @@ -68,7 +68,8 @@ public abstract class DashboardFragment extends SettingsPreferenceFragment private DashboardTilePlaceholderPreferenceController mPlaceholderPreferenceController; private boolean mListeningToCategoryChange; private SummaryLoader mSummaryLoader; - private UiBlockerController mBlockerController; + @VisibleForTesting + UiBlockerController mBlockerController; @Override public void onAttach(Context context) { @@ -111,7 +112,8 @@ public abstract class DashboardFragment extends SettingsPreferenceFragment checkUiBlocker(controllers); } - private void checkUiBlocker(List controllers) { + @VisibleForTesting + void checkUiBlocker(List controllers) { final List keys = new ArrayList<>(); controllers .stream() @@ -121,8 +123,10 @@ public abstract class DashboardFragment extends SettingsPreferenceFragment keys.add(controller.getPreferenceKey()); }); - mBlockerController = new UiBlockerController(keys); - mBlockerController.start(()->updatePreferenceVisibility()); + if (!keys.isEmpty()) { + mBlockerController = new UiBlockerController(keys); + mBlockerController.start(()->updatePreferenceVisibility(mPreferenceControllers)); + } } @Override @@ -355,21 +359,23 @@ public abstract class DashboardFragment extends SettingsPreferenceFragment activity.reportFullyDrawn(); } - updatePreferenceVisibility(); + updatePreferenceVisibility(mPreferenceControllers); } - private void updatePreferenceVisibility() { + @VisibleForTesting + void updatePreferenceVisibility( + Map> preferenceControllers) { final PreferenceScreen screen = getPreferenceScreen(); - if (screen == null) { + if (screen == null || preferenceControllers == null || mBlockerController == null) { return; } final boolean visible = mBlockerController.isBlockerFinished(); for (List controllerList : - mPreferenceControllers.values()) { + preferenceControllers.values()) { for (AbstractPreferenceController controller : controllerList) { final String key = controller.getPreferenceKey(); - final Preference preference = screen.findPreference(key); + final Preference preference = findPreference(key); if (preference != null) { preference.setVisible(visible && controller.isAvailable()); } diff --git a/src/com/android/settings/dashboard/UiBlockerController.java b/src/com/android/settings/dashboard/UiBlockerController.java index eeb56e6daef..710175b112b 100644 --- a/src/com/android/settings/dashboard/UiBlockerController.java +++ b/src/com/android/settings/dashboard/UiBlockerController.java @@ -33,7 +33,7 @@ import java.util.concurrent.TimeUnit; * Control ui blocker data and check whether it is finished * * @see BasePreferenceController.UiBlocker - * @see BasePreferenceController.OnUiBlockListener + * @see BasePreferenceController.UiBlockListener */ public class UiBlockerController { private static final String TAG = "UiBlockerController"; diff --git a/tests/robotests/src/com/android/settings/dashboard/DashboardFragmentTest.java b/tests/robotests/src/com/android/settings/dashboard/DashboardFragmentTest.java index afbb658f9ec..438dfc18caf 100644 --- a/tests/robotests/src/com/android/settings/dashboard/DashboardFragmentTest.java +++ b/tests/robotests/src/com/android/settings/dashboard/DashboardFragmentTest.java @@ -38,6 +38,7 @@ import androidx.preference.PreferenceScreen; import com.android.internal.logging.nano.MetricsProto.MetricsEvent; import com.android.settings.core.PreferenceControllerMixin; +import com.android.settings.slices.BlockingSlicePrefController; import com.android.settings.testutils.FakeFeatureFactory; import com.android.settingslib.core.AbstractPreferenceController; import com.android.settingslib.core.instrumentation.MetricsFeatureProvider; @@ -55,7 +56,10 @@ import org.robolectric.RuntimeEnvironment; import org.robolectric.util.ReflectionHelpers; import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; import java.util.List; +import java.util.Map; @RunWith(RobolectricTestRunner.class) public class DashboardFragmentTest { @@ -66,6 +70,7 @@ public class DashboardFragmentTest { private DashboardCategory mDashboardCategory; private Context mContext; private TestFragment mTestFragment; + private List mControllers; @Before public void setUp() { @@ -83,6 +88,7 @@ public class DashboardFragmentTest { .thenReturn(mDashboardCategory); mTestFragment.onAttach(RuntimeEnvironment.application); when(mContext.getPackageName()).thenReturn("TestPackage"); + mControllers = new ArrayList<>(); } @Test @@ -193,6 +199,39 @@ public class DashboardFragmentTest { DASHBOARD_CONTAINER, null, 0); } + @Test + public void updatePreferenceVisibility_prefKeyNull_shouldNotCrash() { + final Map> prefControllers = new HashMap<>(); + final List controllerList = new ArrayList<>(); + controllerList.add(new TestPreferenceController(mContext)); + prefControllers.put(TestPreferenceController.class, controllerList); + mTestFragment.mBlockerController = new UiBlockerController(Arrays.asList("pref_key")); + + // Should not crash + mTestFragment.updatePreferenceVisibility(prefControllers); + } + + @Test + public void checkUiBlocker_noUiBlocker_controllerIsNull() { + mTestFragment.mBlockerController = null; + mControllers.add(new TestPreferenceController(mContext)); + + mTestFragment.checkUiBlocker(mControllers); + + assertThat(mTestFragment.mBlockerController).isNull(); + } + + @Test + public void checkUiBlocker_hasUiBlocker_controllerNotNull() { + mTestFragment.mBlockerController = null; + mControllers.add(new TestPreferenceController(mContext)); + mControllers.add(new BlockingSlicePrefController(mContext, "pref_key")); + + mTestFragment.checkUiBlocker(mControllers); + + assertThat(mTestFragment.mBlockerController).isNotNull(); + } + public static class TestPreferenceController extends AbstractPreferenceController implements PreferenceControllerMixin {