From 6fc8f3282f0021cfac3f145b97fe46b4d406dfb3 Mon Sep 17 00:00:00 2001
From: dakinola <dakinola@google.com>
Date: Mon, 31 Jul 2023 14:38:55 +0000
Subject: [PATCH] Always updateRecording in
 ContentRecordingController#setContentRecordingSession

Race condition in DisplayManagerService meant that sometimes the boolean check gating updateRecording was unintentionally false, so now we always updateRecording to ensure that there is always an attempt to add new content to a freshly created DisplayContent used for mirroring.

Bug: 290911074
Test: atest CtsSurfaceControlTests:android.view.surfacecontrol.cts.SurfaceViewSyncTest#testSurfaceViewFixedSizeChanges --iterations 10
Test: atest CtsWindowManagerDeviceTestCases:android.server.wm.SurfaceSyncGroupContinuousTest#testSurfaceControlViewHostIPCSync_Slow --iterations 10
Test: atest WmTests:ContentRecorderTests
Test: atest WmTests:ContentRecordingControllerTests
Change-Id: I0db6f4fdb173fdab704f8cc93556ea65bc50d84c
---
 .../android/server/wm/ContentRecordingController.java    | 9 +++++----
 .../server/wm/ContentRecordingControllerTests.java       | 3 ++-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/services/core/java/com/android/server/wm/ContentRecordingController.java b/services/core/java/com/android/server/wm/ContentRecordingController.java
index f24ba5a458859..b5890856fa6f4 100644
--- a/services/core/java/com/android/server/wm/ContentRecordingController.java
+++ b/services/core/java/com/android/server/wm/ContentRecordingController.java
@@ -117,10 +117,11 @@ final class ContentRecordingController {
             }
             incomingDisplayContent.setContentRecordingSession(incomingSession);
             // Updating scenario: Explicitly ask ContentRecorder to update, since no config or
-            // display change will trigger an update from the DisplayContent.
-            if (hasSessionUpdatedWithConsent) {
-                incomingDisplayContent.updateRecording();
-            }
+            // display change will trigger an update from the DisplayContent. There exists a
+            // scenario where a DisplayContent is created, but it's ContentRecordingSession hasn't
+            // been set yet due to a race condition. On creation, updateRecording fails to start
+            // recording, so now this call guarantees recording will be started from somewhere.
+            incomingDisplayContent.updateRecording();
         }
         // Takeover and stopping scenario: stop recording on the pre-existing session.
         if (mSession != null && !hasSessionUpdatedWithConsent) {
diff --git a/services/tests/wmtests/src/com/android/server/wm/ContentRecordingControllerTests.java b/services/tests/wmtests/src/com/android/server/wm/ContentRecordingControllerTests.java
index 52226c2be2980..4473a31f05130 100644
--- a/services/tests/wmtests/src/com/android/server/wm/ContentRecordingControllerTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/ContentRecordingControllerTests.java
@@ -123,6 +123,7 @@ public class ContentRecordingControllerTests extends WindowTestsBase {
         controller.setContentRecordingSessionLocked(mWaitingDisplaySession, mWm);
         verify(mVirtualDisplayContent, atLeastOnce()).setContentRecordingSession(
                 mWaitingDisplaySession);
+        verify(mVirtualDisplayContent).updateRecording();
 
         // WHEN updating the session on the same display, so no longer waiting to record.
         ContentRecordingSession sessionUpdate = ContentRecordingSession.createTaskSession(
@@ -135,7 +136,7 @@ public class ContentRecordingControllerTests extends WindowTestsBase {
         // THEN the session was accepted.
         assertThat(resultingSession).isEqualTo(sessionUpdate);
         verify(mVirtualDisplayContent, atLeastOnce()).setContentRecordingSession(sessionUpdate);
-        verify(mVirtualDisplayContent).updateRecording();
+        verify(mVirtualDisplayContent, atLeastOnce()).updateRecording();
     }
 
     @Test