From 0ee31107edadae1842adc9ec1c15492bfe9ea52f Mon Sep 17 00:00:00 2001
From: Ady Abraham <adyabr@google.com>
Date: Thu, 26 Aug 2021 21:52:44 -0700
Subject: [PATCH] DisplayManagerGlobal: fix native refresh rate callback
 registration

When AChoreographer is the only callback registered with
DisplayManagerGlobal and there are no other display listeners,
DisplayManagerGlobal incorrectly skips callback registration with DMS.

Bug: 193945763
Test: atest DisplayManagerGlobalTest
Change-Id: I4b737fa0a91541fe331edcf3454724200e2db971
---
 .../display/DisplayManagerGlobal.java         | 29 +++++++++----
 .../display/DisplayManagerGlobalTest.java     | 41 +++++++++++++++++++
 2 files changed, 63 insertions(+), 7 deletions(-)

diff --git a/core/java/android/hardware/display/DisplayManagerGlobal.java b/core/java/android/hardware/display/DisplayManagerGlobal.java
index a9b95fce87778..6c3936569c28e 100644
--- a/core/java/android/hardware/display/DisplayManagerGlobal.java
+++ b/core/java/android/hardware/display/DisplayManagerGlobal.java
@@ -361,6 +361,11 @@ public final class DisplayManagerGlobal {
         for (int i = 0; i < numListeners; i++) {
             mask |= mDisplayListeners.get(i).mEventsMask;
         }
+        if (mDispatchNativeCallbacks) {
+            mask |= DisplayManager.EVENT_FLAG_DISPLAY_ADDED
+                    | DisplayManager.EVENT_FLAG_DISPLAY_CHANGED
+                    | DisplayManager.EVENT_FLAG_DISPLAY_REMOVED;
+        }
         return mask;
     }
 
@@ -1047,12 +1052,17 @@ public final class DisplayManagerGlobal {
 
     private static native void nSignalNativeCallbacks(float refreshRate);
 
-    // Called from AChoreographer via JNI.
-    // Registers AChoreographer so that refresh rate callbacks can be dispatched from DMS.
-    private void registerNativeChoreographerForRefreshRateCallbacks() {
+    /**
+     * Called from AChoreographer via JNI.
+     * Registers AChoreographer so that refresh rate callbacks can be dispatched from DMS.
+     * Public for unit testing to be able to call this method.
+     */
+    @VisibleForTesting
+    public void registerNativeChoreographerForRefreshRateCallbacks() {
         synchronized (mLock) {
-            registerCallbackIfNeededLocked();
             mDispatchNativeCallbacks = true;
+            registerCallbackIfNeededLocked();
+            updateCallbackIfNeededLocked();
             DisplayInfo display = getDisplayInfoLocked(Display.DEFAULT_DISPLAY);
             if (display != null) {
                 // We need to tell AChoreographer instances the current refresh rate so that apps
@@ -1063,11 +1073,16 @@ public final class DisplayManagerGlobal {
         }
     }
 
-    // Called from AChoreographer via JNI.
-    // Unregisters AChoreographer from receiving refresh rate callbacks.
-    private void unregisterNativeChoreographerForRefreshRateCallbacks() {
+    /**
+     * Called from AChoreographer via JNI.
+     * Unregisters AChoreographer from receiving refresh rate callbacks.
+     * Public for unit testing to be able to call this method.
+     */
+    @VisibleForTesting
+    public void unregisterNativeChoreographerForRefreshRateCallbacks() {
         synchronized (mLock) {
             mDispatchNativeCallbacks = false;
+            updateCallbackIfNeededLocked();
         }
     }
 }
diff --git a/core/tests/coretests/src/android/hardware/display/DisplayManagerGlobalTest.java b/core/tests/coretests/src/android/hardware/display/DisplayManagerGlobalTest.java
index dfc9013e3c059..149f58f0a69b0 100644
--- a/core/tests/coretests/src/android/hardware/display/DisplayManagerGlobalTest.java
+++ b/core/tests/coretests/src/android/hardware/display/DisplayManagerGlobalTest.java
@@ -32,6 +32,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Captor;
+import org.mockito.InOrder;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
@@ -121,6 +122,46 @@ public class DisplayManagerGlobalTest {
         Mockito.verifyZeroInteractions(mListener);
     }
 
+    @Test
+    public void testDisplayManagerGlobalRegistersWithDisplayManager_WhenThereAreNoOtherListeners()
+            throws RemoteException {
+        mDisplayManagerGlobal.registerNativeChoreographerForRefreshRateCallbacks();
+        Mockito.verify(mDisplayManager)
+                .registerCallbackWithEventMask(mCallbackCaptor.capture(), eq(ALL_DISPLAY_EVENTS));
+
+        mDisplayManagerGlobal.unregisterNativeChoreographerForRefreshRateCallbacks();
+        Mockito.verify(mDisplayManager)
+                .registerCallbackWithEventMask(mCallbackCaptor.capture(), eq(0L));
+
+    }
+
+    @Test
+    public void testDisplayManagerGlobalRegistersWithDisplayManager_WhenThereAreListeners()
+            throws RemoteException {
+        mDisplayManagerGlobal.registerDisplayListener(mListener, mHandler,
+                DisplayManager.EVENT_FLAG_DISPLAY_BRIGHTNESS);
+        InOrder inOrder = Mockito.inOrder(mDisplayManager);
+
+        inOrder.verify(mDisplayManager)
+                .registerCallbackWithEventMask(mCallbackCaptor.capture(),
+                        eq(DisplayManager.EVENT_FLAG_DISPLAY_BRIGHTNESS));
+
+        mDisplayManagerGlobal.registerNativeChoreographerForRefreshRateCallbacks();
+        inOrder.verify(mDisplayManager)
+                .registerCallbackWithEventMask(mCallbackCaptor.capture(),
+                        eq(ALL_DISPLAY_EVENTS | DisplayManager.EVENT_FLAG_DISPLAY_BRIGHTNESS));
+
+        mDisplayManagerGlobal.unregisterNativeChoreographerForRefreshRateCallbacks();
+        inOrder.verify(mDisplayManager)
+                .registerCallbackWithEventMask(mCallbackCaptor.capture(),
+                        eq(DisplayManager.EVENT_FLAG_DISPLAY_BRIGHTNESS));
+
+        mDisplayManagerGlobal.unregisterDisplayListener(mListener);
+        inOrder.verify(mDisplayManager)
+                .registerCallbackWithEventMask(mCallbackCaptor.capture(), eq(0L));
+
+    }
+
     private void waitForHandler() {
         mHandler.runWithScissors(() -> { }, 0);
     }