From 21147c02f8ee508b9cb0fba8e578f2e2e9be6ed7 Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Mon, 21 Dec 2020 14:18:46 -0500 Subject: [PATCH] Show LargeIcon as wide for apps targeting S. * This does not inhibit the grouping behavior which demotes this image and shows it in a smaller square area on the left. * This also converts the Notification class to calculate margins and sizes in DIP instead of PX, as that is more resilient. Fixes: 175409684 Test: atest NotificationTemplateTest Change-Id: I35d28c8df341dbbac2774026c6ca749e296c0482 --- core/java/android/app/Notification.java | 108 ++++++++++++------ .../android/view/NotificationHeaderView.java | 15 ++- .../statusbar/NotificationGroupingUtil.java | 8 +- .../devicepolicy/DevicePolicyManagerTest.java | 12 ++ .../server/devicepolicy/DpmMockContext.java | 7 ++ .../NetworkNotificationManagerTest.java | 24 +++- 6 files changed, 131 insertions(+), 43 deletions(-) diff --git a/core/java/android/app/Notification.java b/core/java/android/app/Notification.java index 61fdc86d12fb6..9a247edf5ac72 100644 --- a/core/java/android/app/Notification.java +++ b/core/java/android/app/Notification.java @@ -30,7 +30,6 @@ import android.annotation.IdRes; import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; -import android.annotation.Px; import android.annotation.RequiresPermission; import android.annotation.SdkConstant; import android.annotation.SdkConstant.SdkConstantType; @@ -218,6 +217,11 @@ public class Notification implements Parcelable */ private static final int MAX_REPLY_HISTORY = 5; + /** + * Maximum aspect ratio of the large icon. 16:9 + */ + private static final float MAX_LARGE_ICON_ASPECT_RATIO = 16f / 9f; + /** * Maximum number of (generic) action buttons in a notification (contextual action buttons are * handled separately). @@ -4229,9 +4233,9 @@ public class Notification implements Parcelable /** * Add a large icon to the notification content view. * - * In the platform template, this image will be shown on the left of the notification view - * in place of the {@link #setSmallIcon(Icon) small icon} (which will be placed in a small - * badge atop the large icon). + * In the platform template, this image will be shown either on the right of the + * notification, with an aspect ratio of up to 16:9, or (when the notification is grouped) + * on the left in place of the {@link #setSmallIcon(Icon) small icon}. */ @NonNull public Builder setLargeIcon(Bitmap b) { @@ -4241,9 +4245,9 @@ public class Notification implements Parcelable /** * Add a large icon to the notification content view. * - * In the platform template, this image will be shown on the left of the notification view - * in place of the {@link #setSmallIcon(Icon) small icon} (which will be placed in a small - * badge atop the large icon). + * In the platform template, this image will be shown either on the right of the + * notification, with an aspect ratio of up to 16:9, or (when the notification is grouped) + * on the left in place of the {@link #setSmallIcon(Icon) small icon}. */ @NonNull public Builder setLargeIcon(Icon icon) { @@ -5111,8 +5115,7 @@ public class Notification implements Parcelable if (result == null) { result = new TemplateBindResult(); } - final boolean largeIconShown = bindLargeIcon(contentView, p); - calculateLargeIconMarginEnd(largeIconShown, result); + bindLargeIcon(contentView, p, result); if (p.mHeaderless) { // views in the headerless (collapsed) state result.mHeadingExtraMarginSet.applyToView(contentView, @@ -5124,28 +5127,54 @@ public class Notification implements Parcelable } } - private void calculateLargeIconMarginEnd(boolean largeIconShown, + // This code is executed on behalf of other apps' notifications, sometimes even by 3p apps, + // a use case that is not supported by the Compat Framework library. Workarounds to resolve + // the change's state in NotificationManagerService were very complex. These behavior + // changes are entirely visual, and should otherwise be undetectable by apps. + @SuppressWarnings("AndroidFrameworkCompatChange") + private void calculateLargeIconDimens(boolean largeIconShown, @NonNull TemplateBindResult result) { final Resources resources = mContext.getResources(); - final int contentMargin = resources.getDimensionPixelOffset( - R.dimen.notification_content_margin_end); - final int expanderSize = resources.getDimensionPixelSize( - R.dimen.notification_header_expand_icon_size) - contentMargin; - final int extraMarginEndIfVisible = resources.getDimensionPixelSize( - R.dimen.notification_right_icon_size) + contentMargin; - result.setRightIconState(largeIconShown, extraMarginEndIfVisible, expanderSize); + final float density = resources.getDisplayMetrics().density; + final float contentMarginDp = resources.getDimension( + R.dimen.notification_content_margin_end) / density; + final float expanderSizeDp = resources.getDimension( + R.dimen.notification_header_expand_icon_size) / density - contentMarginDp; + final float viewHeightDp = resources.getDimension( + R.dimen.notification_right_icon_size) / density; + float viewWidthDp = viewHeightDp; // icons are 1:1 by default + if (largeIconShown && ( + mContext.getApplicationInfo().targetSdkVersion >= Build.VERSION_CODES.S + || DevFlags.shouldBackportSNotifRules(mContext.getContentResolver()))) { + Drawable drawable = mN.mLargeIcon.loadDrawable(mContext); + if (drawable != null) { + int iconWidth = drawable.getIntrinsicWidth(); + int iconHeight = drawable.getIntrinsicHeight(); + if (iconWidth > iconHeight && iconHeight > 0) { + final float maxViewWidthDp = viewHeightDp * MAX_LARGE_ICON_ASPECT_RATIO; + viewWidthDp = Math.min(viewHeightDp * iconWidth / iconHeight, + maxViewWidthDp); + } + } + } + final float extraMarginEndDpIfVisible = viewWidthDp + contentMarginDp; + result.setRightIconState(largeIconShown, viewWidthDp, + extraMarginEndDpIfVisible, expanderSizeDp); } /** * Bind the large icon. - * @return if the largeIcon is visible */ - private boolean bindLargeIcon(RemoteViews contentView, StandardTemplateParams p) { + private void bindLargeIcon(RemoteViews contentView, @NonNull StandardTemplateParams p, + @NonNull TemplateBindResult result) { if (mN.mLargeIcon == null && mN.largeIcon != null) { mN.mLargeIcon = Icon.createWithBitmap(mN.largeIcon); } boolean showLargeIcon = mN.mLargeIcon != null && !p.hideLargeIcon; + calculateLargeIconDimens(showLargeIcon, result); if (showLargeIcon) { + contentView.setViewLayoutWidth(R.id.right_icon, + result.mRightIconWidthDp, TypedValue.COMPLEX_UNIT_DIP); contentView.setViewVisibility(R.id.right_icon, View.VISIBLE); contentView.setImageViewIcon(R.id.right_icon, mN.mLargeIcon); processLargeLegacyIcon(mN.mLargeIcon, contentView, p); @@ -5155,7 +5184,6 @@ public class Notification implements Parcelable // visibility) is used by NotificationGroupingUtil to set the visibility. contentView.setImageViewIcon(R.id.right_icon, null); } - return showLargeIcon; } private void bindNotificationHeader(RemoteViews contentView, StandardTemplateParams p) { @@ -7793,7 +7821,8 @@ public class Notification implements Parcelable // NOTE: This template doesn't support moving this icon to the left, so we don't // need to fully apply the MarginSet contentView.setViewLayoutMargin(R.id.notification_messaging, RemoteViews.MARGIN_END, - bindResult.mHeadingExtraMarginSet.getValue(), TypedValue.COMPLEX_UNIT_PX); + bindResult.mHeadingExtraMarginSet.getDpValue(), + TypedValue.COMPLEX_UNIT_DIP); } contentView.setInt(R.id.status_bar_latest_event_content, "setLayoutColor", mBuilder.isColorized(p) @@ -8792,7 +8821,8 @@ public class Notification implements Parcelable // also update the end margin to account for the large icon or expander Resources resources = mBuilder.mContext.getResources(); result.mTitleMarginSet.applyToView(remoteViews, R.id.notification_main_column, - resources.getDimensionPixelOffset(R.dimen.notification_content_margin_end)); + resources.getDimension(R.dimen.notification_content_margin_end) + / resources.getDisplayMetrics().density); } } @@ -11030,6 +11060,7 @@ public class Notification implements Parcelable */ private static class TemplateBindResult { boolean mRightIconVisible; + float mRightIconWidthDp; /** * The margin end that needs to be added to the heading so that it won't overlap @@ -11054,11 +11085,13 @@ public class Notification implements Parcelable */ public final MarginSet mTitleMarginSet = new MarginSet(); - public void setRightIconState(boolean visible, int marginEndIfVisible, int expanderSize) { + public void setRightIconState(boolean visible, float widthDp, + float marginEndDpIfVisible, float expanderSizeDp) { mRightIconVisible = visible; - mHeadingExtraMarginSet.setValues(0, marginEndIfVisible); - mHeadingFullMarginSet.setValues(expanderSize, marginEndIfVisible + expanderSize); - mTitleMarginSet.setValues(0, marginEndIfVisible + expanderSize); + mRightIconWidthDp = widthDp; + mHeadingExtraMarginSet.setValues(0, marginEndDpIfVisible); + mHeadingFullMarginSet.setValues(expanderSizeDp, marginEndDpIfVisible + expanderSizeDp); + mTitleMarginSet.setValues(0, marginEndDpIfVisible + expanderSizeDp); } /** @@ -11067,10 +11100,10 @@ public class Notification implements Parcelable * left_icon and adjust the margins, and to undo that change as well. */ private class MarginSet { - private int mValueIfGone; - private int mValueIfVisible; + private float mValueIfGone; + private float mValueIfVisible; - public void setValues(int valueIfGone, int valueIfVisible) { + public void setValues(float valueIfGone, float valueIfVisible) { mValueIfGone = valueIfGone; mValueIfVisible = valueIfVisible; } @@ -11080,23 +11113,26 @@ public class Notification implements Parcelable } public void applyToView(@NonNull RemoteViews views, @IdRes int viewId, - @Px int extraMargin) { - final int marginEnd = getValue() + extraMargin; + float extraMarginDp) { + final float marginEndDp = getDpValue() + extraMarginDp; if (viewId == R.id.notification_header) { - views.setInt(R.id.notification_header, "setTopLineExtraMarginEnd", marginEnd); + views.setFloat(R.id.notification_header, + "setTopLineExtraMarginEndDp", marginEndDp); } else { views.setViewLayoutMargin(viewId, RemoteViews.MARGIN_END, - marginEnd, TypedValue.COMPLEX_UNIT_PX); + marginEndDp, TypedValue.COMPLEX_UNIT_DIP); } if (mRightIconVisible) { views.setIntTag(viewId, R.id.tag_margin_end_when_icon_visible, - mValueIfVisible + extraMargin); + TypedValue.createComplexDimension( + mValueIfVisible + extraMarginDp, TypedValue.COMPLEX_UNIT_DIP)); views.setIntTag(viewId, R.id.tag_margin_end_when_icon_gone, - mValueIfGone + extraMargin); + TypedValue.createComplexDimension( + mValueIfGone + extraMarginDp, TypedValue.COMPLEX_UNIT_DIP)); } } - public int getValue() { + public float getDpValue() { return mRightIconVisible ? mValueIfVisible : mValueIfGone; } } diff --git a/core/java/android/view/NotificationHeaderView.java b/core/java/android/view/NotificationHeaderView.java index 673ed0d8b95db..5ce4c50cc85cc 100644 --- a/core/java/android/view/NotificationHeaderView.java +++ b/core/java/android/view/NotificationHeaderView.java @@ -156,13 +156,24 @@ public class NotificationHeaderView extends FrameLayout { * Sets the extra margin at the end of the top line of left-aligned text + icons. * This value will have the margin required to accommodate the expand button added to it. * - * @param extraMarginEnd extra margin + * @param extraMarginEnd extra margin in px */ - @RemotableViewMethod public void setTopLineExtraMarginEnd(int extraMarginEnd) { mTopLineView.setHeaderTextMarginEnd(extraMarginEnd + mHeadingEndMargin); } + /** + * Sets the extra margin at the end of the top line of left-aligned text + icons. + * This value will have the margin required to accommodate the expand button added to it. + * + * @param extraMarginEndDp extra margin in dp + */ + @RemotableViewMethod + public void setTopLineExtraMarginEndDp(float extraMarginEndDp) { + setTopLineExtraMarginEnd( + (int) (extraMarginEndDp * getResources().getDisplayMetrics().density)); + } + /** * Get the current margin end value for the header text. * Add this to {@link #getTopLineBaseMarginEnd()} to get the total margin of the top line. diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationGroupingUtil.java b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationGroupingUtil.java index dbee0ee8f5d56..9ef304d7e83c5 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/NotificationGroupingUtil.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/NotificationGroupingUtil.java @@ -20,6 +20,8 @@ import android.app.Notification; import android.graphics.drawable.Drawable; import android.graphics.drawable.Icon; import android.text.TextUtils; +import android.util.DisplayMetrics; +import android.util.TypedValue; import android.view.NotificationHeaderView; import android.view.View; import android.view.ViewGroup; @@ -456,12 +458,14 @@ public class NotificationGroupingUtil { if (target == null) { return; } - Integer value = (Integer) target.getTag(iconVisible + final Integer data = (Integer) target.getTag(iconVisible ? com.android.internal.R.id.tag_margin_end_when_icon_visible : com.android.internal.R.id.tag_margin_end_when_icon_gone); - if (value == null) { + if (data == null) { return; } + final DisplayMetrics metrics = target.getResources().getDisplayMetrics(); + final int value = TypedValue.complexToDimensionPixelOffset(data, metrics); if (target instanceof NotificationHeaderView) { ((NotificationHeaderView) target).setTopLineExtraMarginEnd(value); } else { diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java index 1c7da3b989306..c1b1133dbb228 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -116,7 +116,9 @@ import org.hamcrest.BaseMatcher; import org.hamcrest.Description; import org.hamcrest.Matcher; import org.junit.After; +import org.junit.AfterClass; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Test; import org.mockito.Mockito; import org.mockito.internal.util.collections.Sets; @@ -206,6 +208,16 @@ public class DevicePolicyManagerTest extends DpmTestBase { private static final String PROFILE_OFF_SUSPENSION_TEXT = "suspension_text"; private static final String PROFILE_OFF_SUSPENSION_SOON_TEXT = "suspension_tomorrow_text"; + @BeforeClass + public static void setUpClass() { + Notification.DevFlags.sForceDefaults = true; + } + + @AfterClass + public static void tearDownClass() { + Notification.DevFlags.sForceDefaults = false; + } + @Before public void setUp() throws Exception { diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DpmMockContext.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DpmMockContext.java index 1d2dcaecf978e..6068fdf9b5b5a 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DpmMockContext.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DpmMockContext.java @@ -17,6 +17,7 @@ package com.android.server.devicepolicy; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import android.annotation.Nullable; import android.app.AppOpsManager; @@ -33,6 +34,7 @@ import android.os.Handler; import android.os.UserHandle; import android.test.mock.MockContext; import android.util.ArrayMap; +import android.util.DisplayMetrics; import android.util.ExceptionUtils; import androidx.annotation.NonNull; @@ -174,6 +176,11 @@ public class DpmMockContext extends MockContext { binder = new MockBinder(); resources = mock(Resources.class); spiedContext = mock(Context.class); + + // Set up density for notification building + DisplayMetrics displayMetrics = mock(DisplayMetrics.class); + displayMetrics.density = 2.25f; + when(resources.getDisplayMetrics()).thenReturn(displayMetrics); } @Override diff --git a/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java b/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java index b47be97ed0029..cd4cfcf188045 100644 --- a/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java +++ b/tests/net/java/com/android/server/connectivity/NetworkNotificationManagerTest.java @@ -20,6 +20,7 @@ import static com.android.server.connectivity.NetworkNotificationManager.Notific import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyInt; +import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; @@ -39,6 +40,7 @@ import android.net.NetworkCapabilities; import android.net.NetworkInfo; import android.os.UserHandle; import android.telephony.TelephonyManager; +import android.util.DisplayMetrics; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; @@ -46,7 +48,9 @@ import androidx.test.runner.AndroidJUnit4; import com.android.internal.R; import com.android.server.connectivity.NetworkNotificationManager.NotificationType; +import org.junit.AfterClass; import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.AdditionalAnswers; @@ -82,6 +86,7 @@ public class NetworkNotificationManagerTest { @Mock Context mCtx; @Mock Resources mResources; + @Mock DisplayMetrics mDisplayMetrics; @Mock PackageManager mPm; @Mock TelephonyManager mTelephonyManager; @Mock NotificationManager mNotificationManager; @@ -93,6 +98,17 @@ public class NetworkNotificationManagerTest { NetworkNotificationManager mManager; + + @BeforeClass + public static void setUpClass() { + Notification.DevFlags.sForceDefaults = true; + } + + @AfterClass + public static void tearDownClass() { + Notification.DevFlags.sForceDefaults = false; + } + @Before public void setUp() { MockitoAnnotations.initMocks(this); @@ -103,6 +119,7 @@ public class NetworkNotificationManagerTest { mCellNai.networkInfo = mNetworkInfo; mVpnNai.networkCapabilities = VPN_CAPABILITIES; mVpnNai.networkInfo = mNetworkInfo; + mDisplayMetrics.density = 2.275f; doReturn(true).when(mVpnNai).isVPN(); when(mCtx.getResources()).thenReturn(mResources); when(mCtx.getPackageManager()).thenReturn(mPm); @@ -114,6 +131,7 @@ public class NetworkNotificationManagerTest { .thenReturn(mNotificationManager); when(mNetworkInfo.getExtraInfo()).thenReturn("extra"); when(mResources.getColor(anyInt(), any())).thenReturn(0xFF607D8B); + when(mResources.getDisplayMetrics()).thenReturn(mDisplayMetrics); mManager = new NetworkNotificationManager(mCtx, mTelephonyManager); } @@ -136,15 +154,15 @@ public class NetworkNotificationManagerTest { public void testTitleOfPrivateDnsBroken() { // Test the title of mobile data. verifyTitleByNetwork(100, mCellNai, R.string.mobile_no_internet); - reset(mResources); + clearInvocations(mResources); // Test the title of wifi. verifyTitleByNetwork(101, mWifiNai, R.string.wifi_no_internet); - reset(mResources); + clearInvocations(mResources); // Test the title of other networks. verifyTitleByNetwork(102, mVpnNai, R.string.other_networks_no_internet); - reset(mResources); + clearInvocations(mResources); } @Test