From c38f7e1d0a5d6a1d776e290f93207bfcf42b6bc8 Mon Sep 17 00:00:00 2001 From: Matthew Fritze Date: Fri, 13 Jul 2018 14:05:38 -0700 Subject: [PATCH] Add a back-up icon resource to icon-less Slices Currently, when a Settings Slice dosen't have an icon, we add an IconCompat object with the resource 0x0 - which gives an empty icon. This is from the UX direction that we should only have icons for Settings Slices when the corresponding Settings have icons in the Settings UI. However, this causes an issue with a recent change to SliceView, which crashes the UI when a Slice is rendered without an icon. Previously, the icon code path was only exercised when the Slice Shortcut view was being used, but after the change, the icon path is always used and thus crashes when trying to fetch a resource with id 0x0 from Settings or another provider. About 2/3rds of Settings Slices do not have icons. This change adds the Settings App icon as the back-up icon for any Slice which would otherwise not have an icon. The impact of missing this change is: - Settings Slices cannot be shown in launcher spaces until a post-P update comes from the support library. - If Settings launches with the bug, Slices cannot patch the API which would require all Slices to have a non empty icon (b/111438616) Bug: 111082093 Test: Robotests, Settings Search UI testing, Slice browser testing Change-Id: I6f326b6b41bf59011a211c6340dd639f68e754e1 --- .../settings/slices/SliceBuilderUtils.java | 17 +++- .../slices/SliceBuilderUtilsTest.java | 78 +++++++++++++++++-- 2 files changed, 87 insertions(+), 8 deletions(-) diff --git a/src/com/android/settings/slices/SliceBuilderUtils.java b/src/com/android/settings/slices/SliceBuilderUtils.java index a56a29067e6..c1c3b8e8229 100644 --- a/src/com/android/settings/slices/SliceBuilderUtils.java +++ b/src/com/android/settings/slices/SliceBuilderUtils.java @@ -49,6 +49,7 @@ import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; +import androidx.annotation.VisibleForTesting; import androidx.core.graphics.drawable.IconCompat; import androidx.slice.Slice; import androidx.slice.builders.ListBuilder; @@ -242,7 +243,7 @@ public class SliceBuilderUtils { private static Slice buildToggleSlice(Context context, SliceData sliceData, BasePreferenceController controller) { final PendingIntent contentIntent = getContentPendingIntent(context, sliceData); - final IconCompat icon = IconCompat.createWithResource(context, sliceData.getIconResource()); + final IconCompat icon = getSafeIcon(context, sliceData); final CharSequence subtitleText = getSubtitleText(context, controller, sliceData); @ColorInt final int color = Utils.getColorAccentDefaultColor(context); final TogglePreferenceController toggleController = @@ -266,7 +267,7 @@ public class SliceBuilderUtils { private static Slice buildIntentSlice(Context context, SliceData sliceData, BasePreferenceController controller) { final PendingIntent contentIntent = getContentPendingIntent(context, sliceData); - final IconCompat icon = IconCompat.createWithResource(context, sliceData.getIconResource()); + final IconCompat icon = getSafeIcon(context, sliceData); final CharSequence subtitleText = getSubtitleText(context, controller, sliceData); @ColorInt final int color = Utils.getColorAccentDefaultColor(context); final List keywords = buildSliceKeywords(sliceData); @@ -287,7 +288,7 @@ public class SliceBuilderUtils { final SliderPreferenceController sliderController = (SliderPreferenceController) controller; final PendingIntent actionIntent = getSliderAction(context, sliceData); final PendingIntent contentIntent = getContentPendingIntent(context, sliceData); - final IconCompat icon = IconCompat.createWithResource(context, sliceData.getIconResource()); + final IconCompat icon = getSafeIcon(context, sliceData); @ColorInt final int color = Utils.getColorAccentDefaultColor(context); final CharSequence subtitleText = getSubtitleText(context, controller, sliceData); final SliceAction primaryAction = new SliceAction(contentIntent, icon, @@ -382,4 +383,14 @@ public class SliceBuilderUtils { .setKeywords(keywords) .build(); } + + @VisibleForTesting + static IconCompat getSafeIcon(Context context, SliceData data) { + int iconResource = data.getIconResource(); + + if (iconResource == 0) { + iconResource = R.drawable.ic_settings; + } + return IconCompat.createWithResource(context, iconResource); + } } diff --git a/tests/robotests/src/com/android/settings/slices/SliceBuilderUtilsTest.java b/tests/robotests/src/com/android/settings/slices/SliceBuilderUtilsTest.java index ed33a80c94e..b96c12886d7 100644 --- a/tests/robotests/src/com/android/settings/slices/SliceBuilderUtilsTest.java +++ b/tests/robotests/src/com/android/settings/slices/SliceBuilderUtilsTest.java @@ -48,8 +48,11 @@ import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.robolectric.RuntimeEnvironment; +import androidx.core.graphics.drawable.IconCompat; import androidx.slice.Slice; +import androidx.slice.SliceMetadata; import androidx.slice.SliceProvider; +import androidx.slice.core.SliceAction; import androidx.slice.widget.SliceLiveData; @RunWith(SettingsRobolectricTestRunner.class) @@ -406,27 +409,92 @@ public class SliceBuilderUtilsTest { assertThat(intentData).isEqualTo(expectedUri); } + @Test + public void buildIntentSlice_noIconPassed_returnsSliceWithIcon() { + final int expectedIconResource = IconCompat.createWithResource(mContext, + R.drawable.ic_settings).toIcon().getResId(); + final SliceData sliceData = getDummyData(CONTEXT_CONTROLLER, SliceData.SliceType.INTENT, + 0x0); + + final Slice slice = SliceBuilderUtils.buildSlice(mContext, sliceData); + + final SliceMetadata metadata = SliceMetadata.from(mContext, slice); + final SliceAction primaryAction = metadata.getPrimaryAction(); + final int actualIconResource = primaryAction.getIcon().toIcon().getResId(); + assertThat(actualIconResource).isEqualTo(expectedIconResource); + } + + @Test + public void buildToggleSlice_noIconPassed_returnsSliceWithIcon() { + final int expectedIconResource = IconCompat.createWithResource(mContext, + R.drawable.ic_settings).toIcon().getResId(); + final SliceData dummyData = getDummyData(TOGGLE_CONTROLLER, SliceData.SliceType.SWITCH, + 0x0); + + final Slice slice = SliceBuilderUtils.buildSlice(mContext, dummyData); + + final SliceMetadata metadata = SliceMetadata.from(mContext, slice); + final SliceAction primaryAction = metadata.getPrimaryAction(); + final int actualIconResource = primaryAction.getIcon().toIcon().getResId(); + assertThat(actualIconResource).isEqualTo(expectedIconResource); + } + + @Test + public void buildSliderSlice_noIconPassed_returnsSliceWithIcon() { + final int expectedIconResource = IconCompat.createWithResource(mContext, + R.drawable.ic_settings).toIcon().getResId(); + final SliceData data = getDummyData(SLIDER_CONTROLLER, SliceData.SliceType.SLIDER, 0x0); + + final Slice slice = SliceBuilderUtils.buildSlice(mContext, data); + + final SliceMetadata metadata = SliceMetadata.from(mContext, slice); + final SliceAction primaryAction = metadata.getPrimaryAction(); + final int actualIconResource = primaryAction.getIcon().toIcon().getResId(); + assertThat(actualIconResource).isEqualTo(expectedIconResource); + } + + @Test + public void getSafeIcon_replacesEmptyIconWithSettingsIcon() { + final int settingsIcon = R.drawable.ic_settings; + final int zeroIcon = 0x0; + final SliceData data = getDummyData(TOGGLE_CONTROLLER, SliceData.SliceType.SWITCH, + zeroIcon); + + final IconCompat actualIcon = SliceBuilderUtils.getSafeIcon(mContext, data); + + final int actualIconResource = actualIcon.toIcon().getResId(); + assertThat(actualIconResource).isNotEqualTo(zeroIcon); + assertThat(actualIconResource).isEqualTo(settingsIcon); + } + private SliceData getDummyData() { - return getDummyData(TOGGLE_CONTROLLER, SUMMARY, SliceData.SliceType.SWITCH, SCREEN_TITLE); + return getDummyData(TOGGLE_CONTROLLER, SUMMARY, SliceData.SliceType.SWITCH, SCREEN_TITLE, + ICON); + } + + private SliceData getDummyData(Class prefController, int sliceType, int icon) { + return getDummyData(TOGGLE_CONTROLLER, SUMMARY, SliceData.SliceType.SWITCH, SCREEN_TITLE, + icon); } private SliceData getDummyData(String summary, String screenTitle) { - return getDummyData(TOGGLE_CONTROLLER, summary, SliceData.SliceType.SWITCH, screenTitle); + return getDummyData(TOGGLE_CONTROLLER, summary, SliceData.SliceType.SWITCH, screenTitle, + ICON); } private SliceData getDummyData(Class prefController, int sliceType) { - return getDummyData(prefController, SUMMARY, sliceType, SCREEN_TITLE); + return getDummyData(prefController, SUMMARY, sliceType, SCREEN_TITLE, ICON); } private SliceData getDummyData(Class prefController, String summary, int sliceType, - String screenTitle) { + String screenTitle, int icon) { return new SliceData.Builder() .setKey(KEY) .setTitle(TITLE) .setSummary(summary) .setScreenTitle(screenTitle) .setKeywords(KEYWORDS) - .setIcon(ICON) + .setIcon(icon) .setFragmentName(FRAGMENT_NAME) .setUri(URI) .setPreferenceControllerClassName(prefController.getName())