diff options
| author | 2025-02-03 17:17:32 -0500 | |
|---|---|---|
| committer | 2025-02-04 14:59:03 -0500 | |
| commit | 6875f99a68d797ff70a4b224638c67283dfe7ff3 (patch) | |
| tree | 1002532aaf58707f0a6a3d2afc93057452334c51 | |
| parent | ef74fc9767570d7d3d89bded8216392c8114acb4 (diff) | |
Estimate vibration effect size using available parcel data
While we want to avoid reading in vibration effects that are too large (which can cause the XML serialization to fail), serializing directly in the constructor to check the length causes too much heap churn. This change uses the remaining data available in the parcel to estimate the size of the vibration effect, after moving it to the end of the parcel.
Flag: android.app.notif_channel_estimate_effect_size
Bug: 391908451
Test: heapdump, NotificationChannelTest
Change-Id: Ic58addd01721c8a0f358deb8629302a3d57e7673
| -rw-r--r-- | core/java/android/app/NotificationChannel.java | 67 | ||||
| -rw-r--r-- | core/java/android/app/notification.aconfig | 10 | ||||
| -rw-r--r-- | core/tests/coretests/src/android/app/NotificationChannelTest.java | 62 |
3 files changed, 114 insertions, 25 deletions
diff --git a/core/java/android/app/NotificationChannel.java b/core/java/android/app/NotificationChannel.java index c6f008c46f99..d88395331656 100644 --- a/core/java/android/app/NotificationChannel.java +++ b/core/java/android/app/NotificationChannel.java @@ -380,17 +380,6 @@ public final class NotificationChannel implements Parcelable { mSound = null; } mLights = in.readByte() != 0; - mVibrationPattern = in.createLongArray(); - if (mVibrationPattern != null && mVibrationPattern.length > MAX_VIBRATION_LENGTH) { - mVibrationPattern = Arrays.copyOf(mVibrationPattern, MAX_VIBRATION_LENGTH); - } - if (Flags.notificationChannelVibrationEffectApi()) { - mVibrationEffect = - in.readInt() != 0 ? VibrationEffect.CREATOR.createFromParcel(in) : null; - if (Flags.notifChannelCropVibrationEffects() && mVibrationEffect != null) { - mVibrationEffect = getTrimmedVibrationEffect(mVibrationEffect); - } - } mUserLockedFields = in.readInt(); mUserVisibleTaskShown = in.readByte() != 0; mVibrationEnabled = in.readByte() != 0; @@ -412,6 +401,38 @@ public final class NotificationChannel implements Parcelable { mImportantConvo = in.readBoolean(); mDeletedTime = in.readLong(); mImportanceLockedDefaultApp = in.readBoolean(); + + // Add new fields above this line and not after vibration effect! When + // notif_channel_estimate_effect_size is true, we use parcel size to detect whether the + // vibration effect might be too large to handle, so this must remain at the end lest any + // following fields cause the data to get incorrectly dropped. + mVibrationPattern = in.createLongArray(); + if (mVibrationPattern != null && mVibrationPattern.length > MAX_VIBRATION_LENGTH) { + mVibrationPattern = Arrays.copyOf(mVibrationPattern, MAX_VIBRATION_LENGTH); + } + boolean largeEffect = false; + if (Flags.notifChannelEstimateEffectSize()) { + // Note that we must check the length of remaining data in the parcel before reading in + // the data. + largeEffect = (in.dataAvail() > MAX_SERIALIZED_VIBRATION_LENGTH); + } + if (Flags.notificationChannelVibrationEffectApi()) { + mVibrationEffect = + in.readInt() != 0 ? VibrationEffect.CREATOR.createFromParcel(in) : null; + if (Flags.notifChannelCropVibrationEffects() && mVibrationEffect != null) { + if (Flags.notifChannelEstimateEffectSize()) { + // Try trimming the effect if the remaining parcel size is large. If trimming is + // not applicable for the effect, rather than serializing to XML (expensive) to + // check the exact serialized length, we just reject the effect. + if (largeEffect) { + mVibrationEffect = mVibrationEffect.cropToLengthOrNull( + MAX_VIBRATION_LENGTH); + } + } else { + mVibrationEffect = getTrimmedVibrationEffect(mVibrationEffect); + } + } + } } @Override @@ -444,15 +465,6 @@ public final class NotificationChannel implements Parcelable { dest.writeByte((byte) 0); } dest.writeByte(mLights ? (byte) 1 : (byte) 0); - dest.writeLongArray(mVibrationPattern); - if (Flags.notificationChannelVibrationEffectApi()) { - if (mVibrationEffect != null) { - dest.writeInt(1); - mVibrationEffect.writeToParcel(dest, /* flags= */ 0); - } else { - dest.writeInt(0); - } - } dest.writeInt(mUserLockedFields); dest.writeByte(mUserVisibleTaskShown ? (byte) 1 : (byte) 0); dest.writeByte(mVibrationEnabled ? (byte) 1 : (byte) 0); @@ -480,6 +492,17 @@ public final class NotificationChannel implements Parcelable { dest.writeBoolean(mImportantConvo); dest.writeLong(mDeletedTime); dest.writeBoolean(mImportanceLockedDefaultApp); + + // Add new fields above this line; vibration effect must remain the last entry. + dest.writeLongArray(mVibrationPattern); + if (Flags.notificationChannelVibrationEffectApi()) { + if (mVibrationEffect != null) { + dest.writeInt(1); + mVibrationEffect.writeToParcel(dest, /* flags= */ 0); + } else { + dest.writeInt(0); + } + } } /** @hide */ @@ -605,7 +628,9 @@ public final class NotificationChannel implements Parcelable { return input; } - // Returns trimmed vibration effect or null if not trimmable. + // Returns trimmed vibration effect or null if not trimmable and the serialized string is too + // long. Note that this method involves serializing the full VibrationEffect, which may be + // expensive. private VibrationEffect getTrimmedVibrationEffect(VibrationEffect effect) { if (effect == null) { return null; diff --git a/core/java/android/app/notification.aconfig b/core/java/android/app/notification.aconfig index 733a348aa825..a10b6ff39a37 100644 --- a/core/java/android/app/notification.aconfig +++ b/core/java/android/app/notification.aconfig @@ -154,6 +154,16 @@ flag { } flag { + name: "notif_channel_estimate_effect_size" + namespace: "systemui" + description: "When reading vibration effects from parcel, estimate size instead of unnecessarily serializing to XML" + bug: "391908451" + metadata { + purpose: PURPOSE_BUGFIX + } +} + +flag { name: "evenly_divided_call_style_action_layout" namespace: "systemui" description: "Evenly divides horizontal space for action buttons in CallStyle notifications." diff --git a/core/tests/coretests/src/android/app/NotificationChannelTest.java b/core/tests/coretests/src/android/app/NotificationChannelTest.java index e4b54071e892..b1d995a2eb9d 100644 --- a/core/tests/coretests/src/android/app/NotificationChannelTest.java +++ b/core/tests/coretests/src/android/app/NotificationChannelTest.java @@ -69,6 +69,9 @@ import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; +import platform.test.runner.parameterized.ParameterizedAndroidJunit4; +import platform.test.runner.parameterized.Parameters; + import java.io.BufferedInputStream; import java.io.BufferedOutputStream; import java.io.ByteArrayInputStream; @@ -78,9 +81,6 @@ import java.util.Arrays; import java.util.List; import java.util.function.Consumer; -import platform.test.runner.parameterized.ParameterizedAndroidJunit4; -import platform.test.runner.parameterized.Parameters; - @RunWith(ParameterizedAndroidJunit4.class) @UsesFlags(android.app.Flags.class) @SmallTest @@ -92,7 +92,8 @@ public class NotificationChannelTest { @Parameters(name = "{0}") public static List<FlagsParameterization> getParams() { return FlagsParameterization.allCombinationsOf( - Flags.FLAG_NOTIF_CHANNEL_CROP_VIBRATION_EFFECTS); + Flags.FLAG_NOTIF_CHANNEL_CROP_VIBRATION_EFFECTS, + Flags.FLAG_NOTIF_CHANNEL_ESTIMATE_EFFECT_SIZE); } @Rule @@ -282,6 +283,59 @@ public class NotificationChannelTest { } @Test + @EnableFlags({Flags.FLAG_NOTIFICATION_CHANNEL_VIBRATION_EFFECT_API, + Flags.FLAG_NOTIF_CHANNEL_CROP_VIBRATION_EFFECTS, + Flags.FLAG_NOTIF_CHANNEL_ESTIMATE_EFFECT_SIZE}) + public void testVibrationEffect_droppedIfTooLargeAndNotTrimmable() { + NotificationChannel channel = new NotificationChannel("id", "name", 3); + // populate pattern with contents + long[] pattern = new long[65550 / 2]; + for (int i = 0; i < pattern.length; i++) { + pattern[i] = 100; + } + // repeating effects cannot be trimmed + VibrationEffect effect = VibrationEffect.createWaveform(pattern, 1); + channel.setVibrationEffect(effect); + + NotificationChannel result = writeToAndReadFromParcel(channel); + assertThat(result.getVibrationEffect()).isNull(); + } + + @Test + @EnableFlags({Flags.FLAG_NOTIFICATION_CHANNEL_VIBRATION_EFFECT_API, + Flags.FLAG_NOTIF_CHANNEL_CROP_VIBRATION_EFFECTS, + Flags.FLAG_NOTIF_CHANNEL_ESTIMATE_EFFECT_SIZE}) + public void testVibrationEffect_trimmedIfLargeAndTrimmable() { + NotificationChannel channel = new NotificationChannel("id", "name", 3); + // populate pattern with contents + long[] pattern = new long[65550 / 2]; + for (int i = 0; i < pattern.length; i++) { + pattern[i] = 100; + } + // Effect is equivalent to the pattern + VibrationEffect effect = VibrationEffect.createWaveform(pattern, -1); + channel.setVibrationEffect(effect); + + NotificationChannel result = writeToAndReadFromParcel(channel); + assertThat(result.getVibrationEffect()).isNotNull(); + assertThat(result.getVibrationEffect().computeCreateWaveformOffOnTimingsOrNull()).hasLength( + NotificationChannel.MAX_VIBRATION_LENGTH); + } + + @Test + @EnableFlags({Flags.FLAG_NOTIFICATION_CHANNEL_VIBRATION_EFFECT_API, + Flags.FLAG_NOTIF_CHANNEL_CROP_VIBRATION_EFFECTS, + Flags.FLAG_NOTIF_CHANNEL_ESTIMATE_EFFECT_SIZE}) + public void testVibrationEffect_keptIfSmall() { + NotificationChannel channel = new NotificationChannel("id", "name", 3); + VibrationEffect effect = VibrationEffect.createOneShot(1, 100); + channel.setVibrationEffect(effect); + + NotificationChannel result = writeToAndReadFromParcel(channel); + assertThat(result.getVibrationEffect()).isEqualTo(effect); + } + + @Test public void testRestoreSoundUri_customLookup() throws Exception { Uri uriToBeRestoredUncanonicalized = Uri.parse("content://media/1"); Uri uriToBeRestoredCanonicalized = Uri.parse("content://media/1?title=Song&canonical=1"); |