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

Commit 44b0e105 authored by Ahmad Khalil's avatar Ahmad Khalil
Browse files

Fix binderProxy crashes due to excessive vibration params requests

To prevent redundant requests for vibration params while a request with the same usage is already in progress, we'll simply return the existing CompletableFuture associated with the ongoing request.

Fix: 355320860
Flag: android.os.vibrator.throttle_vibration_params_requests
Test: atest VibratorControlServiceTest
Change-Id: I03b283cb6513e99640a75cf2c57d06a0935c32fd
parent 3a34c3a6
Loading
Loading
Loading
Loading
+10 −0
Original line number Diff line number Diff line
@@ -64,3 +64,13 @@ flag {
        purpose: PURPOSE_FEATURE
    }
}

flag {
    namespace: "haptics"
    name: "throttle_vibration_params_requests"
    description: "Control the frequency of vibration params requests to prevent overloading the vendor service"
    bug: "355320860"
    metadata {
      purpose: PURPOSE_BUGFIX
    }
}
+11 −2
Original line number Diff line number Diff line
@@ -41,6 +41,7 @@ import android.os.RemoteException;
import android.os.SystemClock;
import android.os.VibrationAttributes;
import android.os.VibrationEffect;
import android.os.vibrator.Flags;
import android.util.IndentingPrintWriter;
import android.util.IntArray;
import android.util.Slog;
@@ -265,9 +266,15 @@ final class VibratorControlService extends IVibratorControlService.Stub {
                return null;
            }

            if (Flags.throttleVibrationParamsRequests() && mVibrationParamRequest != null
                    && mVibrationParamRequest.usage == usage) {
                // Reuse existing future for ongoing request with same usage.
                return mVibrationParamRequest.future;
            }

            try {
                endOngoingRequestVibrationParamsLocked(/* wasCancelled= */ true);
                mVibrationParamRequest = new VibrationParamRequest(uid);
                mVibrationParamRequest = new VibrationParamRequest(uid, usage);
                vibratorController.requestVibrationParams(vibrationType, timeoutInMillis,
                        mVibrationParamRequest.token);
                return mVibrationParamRequest.future;
@@ -533,10 +540,12 @@ final class VibratorControlService extends IVibratorControlService.Stub {
        public final CompletableFuture<Void> future = new CompletableFuture<>();
        public final IBinder token = new Binder();
        public final int uid;
        public final @VibrationAttributes.Usage int usage;
        public final long uptimeMs;

        VibrationParamRequest(int uid) {
        VibrationParamRequest(int uid, @VibrationAttributes.Usage int usage) {
            this.uid = uid;
            this.usage = usage;
            uptimeMs = SystemClock.uptimeMillis();
        }

+52 −5
Original line number Diff line number Diff line
@@ -45,6 +45,11 @@ import android.os.Handler;
import android.os.IBinder;
import android.os.Process;
import android.os.test.TestLooper;
import android.os.vibrator.Flags;
import android.platform.test.annotations.RequiresFlagsDisabled;
import android.platform.test.annotations.RequiresFlagsEnabled;
import android.platform.test.flag.junit.CheckFlagsRule;
import android.platform.test.flag.junit.DeviceFlagsValueProvider;
import android.util.SparseArray;

import androidx.test.InstrumentationRegistry;
@@ -63,7 +68,6 @@ import org.mockito.junit.MockitoRule;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;

public class VibratorControlServiceTest {

@@ -71,6 +75,8 @@ public class VibratorControlServiceTest {

    @Rule
    public MockitoRule rule = MockitoJUnit.rule();
    @Rule
    public final CheckFlagsRule mCheckFlagsRule = DeviceFlagsValueProvider.createCheckFlagsRule();

    @Mock private VibrationScaler mMockVibrationScaler;
    @Mock private PackageManagerInternal mPackageManagerInternalMock;
@@ -98,6 +104,7 @@ public class VibratorControlServiceTest {
        mVibratorControlService = new VibratorControlService(
                InstrumentationRegistry.getContext(), new VibratorControllerHolder(),
                mMockVibrationScaler, mVibrationSettings, mStatsLoggerMock, mLock);
        mFakeVibratorController.setVibratorControlService(mVibratorControlService);
    }

    @Test
@@ -280,10 +287,10 @@ public class VibratorControlServiceTest {
        CompletableFuture<Void> future =
                mVibratorControlService.triggerVibrationParamsRequest(UID, USAGE_RINGTONE,
                        timeoutInMillis);
        try {
            future.orTimeout(timeoutInMillis, TimeUnit.MILLISECONDS).get();
        } catch (Throwable ignored) {
        }
        mTestLooper.dispatchAll();

        assertThat(future).isNotNull();
        assertThat(future.isDone()).isTrue();
        assertThat(mFakeVibratorController.didRequestVibrationParams).isTrue();
        assertThat(mFakeVibratorController.requestVibrationType).isEqualTo(
                ScaleParam.TYPE_RINGTONE);
@@ -315,6 +322,46 @@ public class VibratorControlServiceTest {
        }
    }

    @Test
    @RequiresFlagsEnabled(Flags.FLAG_THROTTLE_VIBRATION_PARAMS_REQUESTS)
    public void testRequestVibrationParams_withOngoingRequestAndSameUsage_returnOngoingFuture() {
        int timeoutInMillis = 10;
        mVibratorControlService.registerVibratorController(mFakeVibratorController);
        CompletableFuture<Void> future =
                mVibratorControlService.triggerVibrationParamsRequest(UID, USAGE_RINGTONE,
                        timeoutInMillis);
        CompletableFuture<Void> future2 =
                mVibratorControlService.triggerVibrationParamsRequest(UID, USAGE_RINGTONE,
                        timeoutInMillis);
        mTestLooper.dispatchAll();

        assertThat(future).isNotNull();
        assertThat(future).isEqualTo(future2);
        assertThat(future.isDone()).isTrue();
        assertThat(mFakeVibratorController.requestVibrationParamsCounter).isEqualTo(1);
    }

    @Test
    @RequiresFlagsDisabled(Flags.FLAG_THROTTLE_VIBRATION_PARAMS_REQUESTS)
    public void testRequestVibrationParams_withOngoingRequestAndSameUsage_returnNewFuture() {
        int timeoutInMillis = 10;
        mVibratorControlService.registerVibratorController(mFakeVibratorController);
        CompletableFuture<Void> future =
                mVibratorControlService.triggerVibrationParamsRequest(UID, USAGE_RINGTONE,
                        timeoutInMillis);
        CompletableFuture<Void> future2 =
                mVibratorControlService.triggerVibrationParamsRequest(UID, USAGE_RINGTONE,
                        timeoutInMillis);
        mTestLooper.dispatchAll();

        assertThat(future).isNotNull();
        assertThat(future2).isNotNull();
        assertThat(future).isNotEqualTo(future2);
        assertThat(future.isDone()).isTrue();
        assertThat(future2.isDone()).isTrue();
        assertThat(mFakeVibratorController.requestVibrationParamsCounter).isEqualTo(2);
    }

    private static int buildVibrationTypesMask(int... types) {
        int typesMask = 0;
        for (int type : types) {
+2 −0
Original line number Diff line number Diff line
@@ -39,6 +39,7 @@ public final class FakeVibratorController extends IVibratorController.Stub {
    public boolean didRequestVibrationParams = false;
    public int requestVibrationType = VibrationAttributes.USAGE_UNKNOWN;
    public long requestTimeoutInMillis = 0;
    public int requestVibrationParamsCounter = 0;

    public FakeVibratorController(Looper looper) {
        mHandler = new Handler(looper);
@@ -58,6 +59,7 @@ public final class FakeVibratorController extends IVibratorController.Stub {
        didRequestVibrationParams = true;
        requestVibrationType = vibrationType;
        requestTimeoutInMillis = timeoutInMillis;
        requestVibrationParamsCounter++;
        mHandler.post(() -> {
            if (mVibratorControlService != null) {
                mVibratorControlService.onRequestVibrationParamsComplete(token, mRequestResult);