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

Commit cf0103a7 authored by Rupesh Bansal's avatar Rupesh Bansal
Browse files

Fix the race condition between reading and writing the brightness values 

Bug: 267801038
Test: Manual verified that the device is not facing recurrent ANRs
Change-Id: I53a15dc169c1c52ce843c2ea633008bd92cedd9d
parent d7391e70

services/core/java/com/android/server/display/DisplayManagerService.java

+3 −1
Original line number Diff line number Diff line
@@ -2909,8 +2909,10 @@ public final class DisplayManagerService extends SystemService { 
    }



    private void handleBrightnessChange(LogicalDisplay display) {

        synchronized (mSyncRoot) {

            sendDisplayEventLocked(display, DisplayManagerGlobal.EVENT_DISPLAY_BRIGHTNESS_CHANGED);

        }

    }



    private DisplayDevice getDeviceForDisplayLocked(int displayId) {

        final LogicalDisplay display = mLogicalDisplayMapper.getDisplayLocked(displayId);

services/core/java/com/android/server/display/DisplayPowerController2.java

+6 −3
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ import android.hardware.display.DisplayManagerInternal.DisplayPowerRequest; 
import android.metrics.LogMaker;

import android.net.Uri;

import android.os.Handler;

import android.os.HandlerExecutor;

import android.os.IBinder;

import android.os.Looper;

import android.os.Message;
@@ -494,7 +495,8 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal 
        mDisplayBrightnessController =

                new DisplayBrightnessController(context, null,

                        mDisplayId, mLogicalDisplay.getDisplayInfoLocked().brightnessDefault,

                        brightnessSetting, () -> postBrightnessChangeRunnable());

                        brightnessSetting, () -> postBrightnessChangeRunnable(),

                        new HandlerExecutor(mHandler));

        // Seed the cached brightness

        saveBrightnessInfo(getScreenBrightnessSetting());
@@ -2116,7 +2118,8 @@ final class DisplayPowerController2 implements AutomaticBrightnessController.Cal 
        mPendingAutoBrightnessAdjustment = getAutoBrightnessAdjustmentSetting();

        if (userSwitch) {

            // Don't treat user switches as user initiated change.

            mDisplayBrightnessController.setCurrentScreenBrightness(mDisplayBrightnessController

            mDisplayBrightnessController

                    .setAndNotifyCurrentScreenBrightness(mDisplayBrightnessController

                            .getPendingScreenBrightness());

            updateAutoBrightnessAdjustment();

            if (mAutomaticBrightnessController != null) {

services/core/java/com/android/server/display/brightness/DisplayBrightnessController.java

+36 −15
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ package com.android.server.display.brightness; 


import android.content.Context;

import android.hardware.display.DisplayManagerInternal;

import android.os.HandlerExecutor;

import android.os.PowerManager;

import android.util.IndentingPrintWriter;
@@ -79,12 +80,16 @@ public final class DisplayBrightnessController { 
    @GuardedBy("mLock")

    private DisplayBrightnessStrategy mDisplayBrightnessStrategy;



    // The executor on which the mOnBrightnessChangeRunnable is executed. This ensures that the

    // callback is not executed in sync and is not blocking the thread from which it is called.

    private final HandlerExecutor mBrightnessChangeExecutor;



    /**

     * The constructor of DisplayBrightnessController.

     */

    public DisplayBrightnessController(Context context, Injector injector, int displayId,

            float defaultScreenBrightness, BrightnessSetting brightnessSetting,

            Runnable onBrightnessChangeRunnable) {

            Runnable onBrightnessChangeRunnable, HandlerExecutor brightnessChangeExecutor) {

        if (injector == null) {

            injector = new Injector();

        }
@@ -97,6 +102,7 @@ public final class DisplayBrightnessController { 
        mOnBrightnessChangeRunnable = onBrightnessChangeRunnable;

        mDisplayBrightnessStrategySelector = injector.getDisplayBrightnessStrategySelector(context,

                displayId);

        mBrightnessChangeExecutor = brightnessChangeExecutor;

    }



    /**
@@ -152,12 +158,14 @@ public final class DisplayBrightnessController { 
     * Sets the current screen brightness to the supplied value, and notifies all the listeners

     * requesting for change events on brightness change.

     */

    public void setCurrentScreenBrightness(float brightnessValue) {

    public void setAndNotifyCurrentScreenBrightness(float brightnessValue) {

        final boolean hasBrightnessChanged;

        synchronized (mLock) {

            if (brightnessValue != mCurrentScreenBrightness) {

                mCurrentScreenBrightness = brightnessValue;

                mOnBrightnessChangeRunnable.run();

            hasBrightnessChanged = (brightnessValue != mCurrentScreenBrightness);

            setCurrentScreenBrightnessLocked(brightnessValue);

        }

        if (hasBrightnessChanged) {

            notifyCurrentScreenBrightness();

        }

    }
@@ -205,12 +213,14 @@ public final class DisplayBrightnessController { 
                setTemporaryBrightnessLocked(PowerManager.BRIGHTNESS_INVALID_FLOAT);

                return false;

            }

            setCurrentScreenBrightness(mPendingScreenBrightness);

            setCurrentScreenBrightnessLocked(mPendingScreenBrightness);

            mLastUserSetScreenBrightness = mPendingScreenBrightness;

            mPendingScreenBrightness = PowerManager.BRIGHTNESS_INVALID_FLOAT;

            setTemporaryBrightnessLocked(PowerManager.BRIGHTNESS_INVALID_FLOAT);

            return true;

        }

        notifyCurrentScreenBrightness();

        return true;



    }



    /**
@@ -264,9 +274,10 @@ public final class DisplayBrightnessController { 
                    || brightnessValue == mCurrentScreenBrightness) {

                return;

            }

            setCurrentScreenBrightness(brightnessValue);

            setBrightness(brightnessValue);

            setCurrentScreenBrightnessLocked(brightnessValue);

        }

        notifyCurrentScreenBrightness();

        setBrightness(brightnessValue);

    }



    /**
@@ -312,7 +323,7 @@ public final class DisplayBrightnessController { 
    }



    @VisibleForTesting

    BrightnessSetting.BrightnessSettingListener getBrightnessSettingListenerLocked() {

    BrightnessSetting.BrightnessSettingListener getBrightnessSettingListener() {

        return mBrightnessSettingListener;

    }
@@ -320,16 +331,26 @@ public final class DisplayBrightnessController { 
     * Returns the current selected DisplayBrightnessStrategy

     */

    @VisibleForTesting

    DisplayBrightnessStrategy getCurrentDisplayBrightnessStrategyLocked() {

    DisplayBrightnessStrategy getCurrentDisplayBrightnessStrategy() {

        synchronized (mLock) {

            return mDisplayBrightnessStrategy;

        }

    }



    @GuardedBy("mLock")

    private void setTemporaryBrightnessLocked(float temporaryBrightness) {

        synchronized (mLock) {

        mDisplayBrightnessStrategySelector.getTemporaryDisplayBrightnessStrategy()

                .setTemporaryScreenBrightness(temporaryBrightness);

    }



    @GuardedBy("mLock")

    private void setCurrentScreenBrightnessLocked(float brightnessValue) {

        if (brightnessValue != mCurrentScreenBrightness) {

            mCurrentScreenBrightness = brightnessValue;

        }

    }



    private void notifyCurrentScreenBrightness() {

        mBrightnessChangeExecutor.execute(mOnBrightnessChangeRunnable);

    }

}

services/tests/servicestests/src/com/android/server/display/brightness/DisplayBrightnessControllerTest.java

+22 −15
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ import static org.mockito.Mockito.when; 


import android.content.Context;

import android.hardware.display.DisplayManagerInternal.DisplayPowerRequest;

import android.os.HandlerExecutor;

import android.os.PowerManager;

import android.view.Display;
@@ -58,6 +59,9 @@ public final class DisplayBrightnessControllerTest { 
    @Mock

    private Runnable mOnBrightnessChangeRunnable;



    @Mock

    private HandlerExecutor mBrightnessChangeExecutor;



    private DisplayBrightnessController mDisplayBrightnessController;



    @Before
@@ -72,7 +76,8 @@ public final class DisplayBrightnessControllerTest { 
        };

        when(mBrightnessSetting.getBrightness()).thenReturn(Float.NaN);

        mDisplayBrightnessController = new DisplayBrightnessController(mContext, injector,

                DISPLAY_ID, DEFAULT_BRIGHTNESS, mBrightnessSetting, mOnBrightnessChangeRunnable);

                DISPLAY_ID, DEFAULT_BRIGHTNESS, mBrightnessSetting, mOnBrightnessChangeRunnable,

                mBrightnessChangeExecutor);

    }



    @Test
@@ -90,7 +95,7 @@ public final class DisplayBrightnessControllerTest { 
                targetDisplayState)).thenReturn(displayBrightnessStrategy);

        mDisplayBrightnessController.updateBrightness(displayPowerRequest, targetDisplayState);

        verify(displayBrightnessStrategy).updateBrightness(displayPowerRequest);

        assertEquals(mDisplayBrightnessController.getCurrentDisplayBrightnessStrategyLocked(),

        assertEquals(mDisplayBrightnessController.getCurrentDisplayBrightnessStrategy(),

                displayBrightnessStrategy);

    }
@@ -116,14 +121,14 @@ public final class DisplayBrightnessControllerTest { 
        // Current Screen brightness is set as expected when a different value than the current

        // is set

        float currentScreenBrightness = 0.4f;

        mDisplayBrightnessController.setCurrentScreenBrightness(currentScreenBrightness);

        mDisplayBrightnessController.setAndNotifyCurrentScreenBrightness(currentScreenBrightness);

        assertEquals(mDisplayBrightnessController.getCurrentBrightness(),

                currentScreenBrightness, 0.0f);

        verify(mOnBrightnessChangeRunnable).run();

        verify(mBrightnessChangeExecutor).execute(mOnBrightnessChangeRunnable);



        // No change to the current screen brightness is same as the existing one

        mDisplayBrightnessController.setCurrentScreenBrightness(currentScreenBrightness);

        verifyNoMoreInteractions(mOnBrightnessChangeRunnable);

        mDisplayBrightnessController.setAndNotifyCurrentScreenBrightness(currentScreenBrightness);

        verifyNoMoreInteractions(mBrightnessChangeExecutor);

    }



    @Test
@@ -146,7 +151,7 @@ public final class DisplayBrightnessControllerTest { 
                TemporaryBrightnessStrategy.class);

        when(mDisplayBrightnessStrategySelector.getTemporaryDisplayBrightnessStrategy()).thenReturn(

                temporaryBrightnessStrategy);

        mDisplayBrightnessController.setCurrentScreenBrightness(currentBrightness);

        mDisplayBrightnessController.setAndNotifyCurrentScreenBrightness(currentBrightness);

        mDisplayBrightnessController.setPendingScreenBrightness(currentBrightness);

        mDisplayBrightnessController.setTemporaryBrightness(currentBrightness);

        assertFalse(mDisplayBrightnessController.updateUserSetScreenBrightness());
@@ -159,7 +164,7 @@ public final class DisplayBrightnessControllerTest { 
        currentBrightness = 0.4f;

        float pendingScreenBrightness = 0.3f;

        float temporaryScreenBrightness = 0.2f;

        mDisplayBrightnessController.setCurrentScreenBrightness(currentBrightness);

        mDisplayBrightnessController.setAndNotifyCurrentScreenBrightness(currentBrightness);

        mDisplayBrightnessController.setPendingScreenBrightness(pendingScreenBrightness);

        mDisplayBrightnessController.setTemporaryBrightness(temporaryScreenBrightness);

        assertTrue(mDisplayBrightnessController.updateUserSetScreenBrightness());
@@ -167,7 +172,8 @@ public final class DisplayBrightnessControllerTest { 
                pendingScreenBrightness, 0.0f);

        assertEquals(mDisplayBrightnessController.getLastUserSetScreenBrightness(),

                pendingScreenBrightness, 0.0f);

        verify(mOnBrightnessChangeRunnable, times(2)).run();

        verify(mBrightnessChangeExecutor, times(2))

                .execute(mOnBrightnessChangeRunnable);

        verify(temporaryBrightnessStrategy, times(2))

                .setTemporaryScreenBrightness(PowerManager.BRIGHTNESS_INVALID_FLOAT);

        assertEquals(mDisplayBrightnessController.getPendingScreenBrightness(),
@@ -181,7 +187,7 @@ public final class DisplayBrightnessControllerTest { 
        mDisplayBrightnessController.registerBrightnessSettingChangeListener(

                brightnessSettingListener);

        verify(mBrightnessSetting).registerListener(brightnessSettingListener);

        assertEquals(mDisplayBrightnessController.getBrightnessSettingListenerLocked(),

        assertEquals(mDisplayBrightnessController.getBrightnessSettingListener(),

                brightnessSettingListener);

    }
@@ -225,19 +231,20 @@ public final class DisplayBrightnessControllerTest { 
        mDisplayBrightnessController.updateScreenBrightnessSetting(brightnessValue);

        assertEquals(mDisplayBrightnessController.getCurrentBrightness(), brightnessValue,

                0.0f);

        verify(mOnBrightnessChangeRunnable).run();

        verify(mBrightnessChangeExecutor).execute(mOnBrightnessChangeRunnable);

        verify(mBrightnessSetting).setBrightness(brightnessValue);



        // Does nothing if the value is invalid

        mDisplayBrightnessController.updateScreenBrightnessSetting(Float.NaN);

        verifyNoMoreInteractions(mOnBrightnessChangeRunnable, mBrightnessSetting);

        verifyNoMoreInteractions(mBrightnessChangeExecutor, mBrightnessSetting);



        // Does nothing if the value is same as the current brightness

        brightnessValue = 0.2f;

        mDisplayBrightnessController.setCurrentScreenBrightness(brightnessValue);

        verify(mOnBrightnessChangeRunnable, times(2)).run();

        mDisplayBrightnessController.setAndNotifyCurrentScreenBrightness(brightnessValue);

        verify(mBrightnessChangeExecutor, times(2))

                .execute(mOnBrightnessChangeRunnable);

        mDisplayBrightnessController.updateScreenBrightnessSetting(brightnessValue);

        verifyNoMoreInteractions(mOnBrightnessChangeRunnable, mBrightnessSetting);

        verifyNoMoreInteractions(mBrightnessChangeExecutor, mBrightnessSetting);

    }



    @Test