Fix the issue of accessing BrightnessSetting inside a lock

Given BrightnessSetting is operating on the display lock, acquiring
power lock while accessing the BrightnessSetting can result in a
deadlock.

Bug: 278692011
Test: Manually verified the behaviour is intact
Test: atest DisplayBrightnessControllerTest
Change-Id: I40c8340895c165da3b96015bc3e8918c259876ac
This commit is contained in:
Rupesh Bansal
2023-04-25 08:46:51 +00:00
parent b4f8ee63e2
commit 24f717a092
2 changed files with 65 additions and 34 deletions

View File

@@ -437,6 +437,7 @@ public final class DisplayBrightnessController {
* persist the nit value, the nit value for the default display will be loaded.
*/
private void loadNitBasedBrightnessSetting() {
float currentBrightnessSetting = Float.NaN;
if (mDisplayId == Display.DEFAULT_DISPLAY && mPersistBrightnessNitsForDefaultDisplay) {
float brightnessNitsForDefaultDisplay =
mBrightnessSetting.getBrightnessNitsForDefaultDisplay();
@@ -445,15 +446,17 @@ public final class DisplayBrightnessController {
brightnessNitsForDefaultDisplay);
if (BrightnessUtils.isValidBrightnessValue(brightnessForDefaultDisplay)) {
mBrightnessSetting.setBrightness(brightnessForDefaultDisplay);
synchronized (mLock) {
mCurrentScreenBrightness = brightnessForDefaultDisplay;
}
return;
currentBrightnessSetting = brightnessForDefaultDisplay;
}
}
}
if (Float.isNaN(currentBrightnessSetting)) {
currentBrightnessSetting = getScreenBrightnessSetting();
}
synchronized (mLock) {
mCurrentScreenBrightness = getScreenBrightnessSetting();
mCurrentScreenBrightness = currentBrightnessSetting;
}
}
}

View File

@@ -19,10 +19,13 @@ package com.android.server.display.brightness;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.when;
import android.content.Context;
@@ -66,25 +69,27 @@ public final class DisplayBrightnessControllerTest {
@Mock
private HandlerExecutor mBrightnessChangeExecutor;
private final DisplayBrightnessController.Injector mInjector = new
DisplayBrightnessController.Injector() {
@Override
DisplayBrightnessStrategySelector getDisplayBrightnessStrategySelector(
Context context, int displayId) {
return mDisplayBrightnessStrategySelector;
}
};
private DisplayBrightnessController mDisplayBrightnessController;
@Before
public void before() {
MockitoAnnotations.initMocks(this);
when(mContext.getResources()).thenReturn(mResources);
DisplayBrightnessController.Injector injector = new DisplayBrightnessController.Injector() {
@Override
DisplayBrightnessStrategySelector getDisplayBrightnessStrategySelector(
Context context, int displayId) {
return mDisplayBrightnessStrategySelector;
}
};
when(mBrightnessSetting.getBrightness()).thenReturn(Float.NaN);
when(mBrightnessSetting.getBrightnessNitsForDefaultDisplay()).thenReturn(-1f);
when(mResources.getBoolean(
com.android.internal.R.bool.config_persistBrightnessNitsForDefaultDisplay))
.thenReturn(true);
mDisplayBrightnessController = new DisplayBrightnessController(mContext, injector,
mDisplayBrightnessController = new DisplayBrightnessController(mContext, mInjector,
DISPLAY_ID, DEFAULT_BRIGHTNESS, mBrightnessSetting, mOnBrightnessChangeRunnable,
mBrightnessChangeExecutor);
}
@@ -256,27 +261,6 @@ public final class DisplayBrightnessControllerTest {
verifyNoMoreInteractions(mBrightnessChangeExecutor, mBrightnessSetting);
}
@Test
public void testBrightnessNitsForDefaultDisplay() {
float brightness = 0.3f;
float nits = 500;
AutomaticBrightnessController automaticBrightnessController =
mock(AutomaticBrightnessController.class);
when(automaticBrightnessController.convertToFloatScale(nits)).thenReturn(brightness);
when(mBrightnessSetting.getBrightnessNitsForDefaultDisplay()).thenReturn(nits);
mDisplayBrightnessController.setAutomaticBrightnessController(
automaticBrightnessController);
assertEquals(brightness, mDisplayBrightnessController.getCurrentBrightness(),
/* delta= */ 0);
float newBrightness = 0.5f;
float newNits = 700;
when(automaticBrightnessController.convertToNits(newBrightness)).thenReturn(newNits);
mDisplayBrightnessController.setBrightness(newBrightness);
verify(mBrightnessSetting).setBrightnessNitsForDefaultDisplay(newNits);
}
@Test
public void testConvertToNits() {
final float brightness = 0.5f;
@@ -330,4 +314,48 @@ public final class DisplayBrightnessControllerTest {
mDisplayBrightnessController.stop();
verify(mBrightnessSetting).unregisterListener(brightnessSettingListener);
}
@Test
public void testLoadNitBasedBrightnessSetting() {
// When the nits value is valid, the brightness is set from the old default display nits
// value
float nits = 200f;
float brightness = 0.3f;
AutomaticBrightnessController automaticBrightnessController =
mock(AutomaticBrightnessController.class);
when(automaticBrightnessController.convertToFloatScale(nits)).thenReturn(brightness);
when(mBrightnessSetting.getBrightnessNitsForDefaultDisplay()).thenReturn(nits);
mDisplayBrightnessController.setAutomaticBrightnessController(
automaticBrightnessController);
verify(mBrightnessSetting).setBrightness(brightness);
assertEquals(brightness, mDisplayBrightnessController.getCurrentBrightness(), 0.01f);
clearInvocations(automaticBrightnessController, mBrightnessSetting);
// When the nits value is invalid, the brightness is resumed from where it was last set
nits = -1;
brightness = 0.4f;
when(automaticBrightnessController.convertToFloatScale(nits)).thenReturn(brightness);
when(mBrightnessSetting.getBrightnessNitsForDefaultDisplay()).thenReturn(nits);
when(mBrightnessSetting.getBrightness()).thenReturn(brightness);
mDisplayBrightnessController.setAutomaticBrightnessController(
automaticBrightnessController);
verify(mBrightnessSetting, never()).setBrightness(brightness);
assertEquals(brightness, mDisplayBrightnessController.getCurrentBrightness(), 0.01f);
clearInvocations(automaticBrightnessController, mBrightnessSetting);
// When the display is a non-default display, the brightness is resumed from where it was
// last set
int nonDefaultDisplayId = 1;
mDisplayBrightnessController = new DisplayBrightnessController(mContext, mInjector,
nonDefaultDisplayId, DEFAULT_BRIGHTNESS, mBrightnessSetting,
mOnBrightnessChangeRunnable, mBrightnessChangeExecutor);
brightness = 0.5f;
when(mBrightnessSetting.getBrightness()).thenReturn(brightness);
mDisplayBrightnessController.setAutomaticBrightnessController(
automaticBrightnessController);
assertEquals(brightness, mDisplayBrightnessController.getCurrentBrightness(), 0.01f);
verifyZeroInteractions(automaticBrightnessController);
verify(mBrightnessSetting, never()).getBrightnessNitsForDefaultDisplay();
verify(mBrightnessSetting, never()).setBrightness(brightness);
}
}