Merge "Fix the race condition between reading and writing the brightness values" (72adb375) · Commits · e / os / android_frameworks_base

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

+3 −1

Original line number	Diff line number	Diff line
		@@ -3009,8 +3009,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());

		@@ -2120,7 +2122,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