Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6875f99a authored by Yuri Lin's avatar Yuri Lin
Browse files

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
parent ef74fc97

core/java/android/app/NotificationChannel.java

+46 −21
Original line number Diff line number Diff line
@@ -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;

core/java/android/app/notification.aconfig

+10 −0
Original line number Diff line number Diff line
@@ -153,6 +153,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"

core/tests/coretests/src/android/app/NotificationChannelTest.java

+58 −4
Original line number Diff line number Diff line
@@ -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
@@ -281,6 +282,59 @@ public class NotificationChannelTest { 
                .isEqualTo(result.getVibrationPattern());

    }



    @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");