SF: Fix condition to activate display on power-on

Since [1], getDefaultDisplayDeviceLocked's fallback when the active
display is outdated is to return the primary display, not `nullptr`.
During the initial call to setPowerModeInternal, `activeDisplay` is
no longer `nullptr`, but the primary display.

After [2], onActiveDisplayChangedLocked is expected to be called for
the primary display during boot. This was not the case though, since
the DisplayDevice::setPowerMode() update precedes the DisplayDevice::
isPoweredOn() query.

Fix this by querying before updating. Also, clean up `nullptr` checks
around onActiveDisplayChangedLocked.

[1] If83c908446e5e5267dfcb15189a26b779d75b216
[2] I15e0f5a280e62baf6d4e6ea2748d95342e79ac44

Fixes: 260663220
Test: The leader display ID is logged during single-display boot.
Test: SetPowerModeInternalTest.designatesLeaderDisplay
Change-Id: I0d20ce7f7ab3f9e90ef1c72263b8166865486d62
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 365ffb7..1bbaf7b 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4685,18 +4685,18 @@
                                            .value_or(false);
 
     const auto activeDisplay = getDisplayDeviceLocked(mActiveDisplayId);
-    if (isInternalDisplay && activeDisplay != display && activeDisplay &&
-        activeDisplay->isPoweredOn()) {
-        ALOGW("Trying to change power mode on non active display while the active display is ON");
-    }
+    const bool isActiveDisplayPoweredOn = activeDisplay && activeDisplay->isPoweredOn();
+
+    ALOGW_IF(display != activeDisplay && isInternalDisplay && isActiveDisplayPoweredOn,
+             "Trying to change power mode on inactive display without powering off active display");
 
     display->setPowerMode(mode);
 
     const auto refreshRate = display->refreshRateSelector().getActiveMode().modePtr->getFps();
     if (!currentModeOpt || *currentModeOpt == hal::PowerMode::OFF) {
         // Turn on the display
-        if (isInternalDisplay && (!activeDisplay || !activeDisplay->isPoweredOn())) {
-            onActiveDisplayChangedLocked(display);
+        if (isInternalDisplay && !isActiveDisplayPoweredOn) {
+            onActiveDisplayChangedLocked(activeDisplay, display);
         }
         // Keep uclamp in a separate syscall and set it before changing to RT due to b/190237315.
         // We can merge the syscall later.
@@ -6995,17 +6995,14 @@
     getRenderEngine().onActiveDisplaySizeChanged(activeDisplay->getSize());
 }
 
-void SurfaceFlinger::onActiveDisplayChangedLocked(const sp<DisplayDevice>& activeDisplay) {
+void SurfaceFlinger::onActiveDisplayChangedLocked(const sp<DisplayDevice>& inactiveDisplay,
+                                                  const sp<DisplayDevice>& activeDisplay) {
     ATRACE_CALL();
 
-    if (const auto display = getDisplayDeviceLocked(mActiveDisplayId)) {
-        display->getCompositionDisplay()->setLayerCachingTexturePoolEnabled(false);
+    if (inactiveDisplay) {
+        inactiveDisplay->getCompositionDisplay()->setLayerCachingTexturePoolEnabled(false);
     }
 
-    if (!activeDisplay) {
-        ALOGE("%s: activeDisplay is null", __func__);
-        return;
-    }
     mActiveDisplayId = activeDisplay->getPhysicalId();
     activeDisplay->getCompositionDisplay()->setLayerCachingTexturePoolEnabled(true);
 
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index e3217a3..ff69245 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -1000,7 +1000,10 @@
     VirtualDisplayId acquireVirtualDisplay(ui::Size, ui::PixelFormat) REQUIRES(mStateLock);
     void releaseVirtualDisplay(VirtualDisplayId);
 
-    void onActiveDisplayChangedLocked(const sp<DisplayDevice>& activeDisplay)
+    // TODO(b/255635821): Replace pointers with references. `inactiveDisplay` is only ever `nullptr`
+    // in tests, and `activeDisplay` must not be `nullptr` as a precondition.
+    void onActiveDisplayChangedLocked(const sp<DisplayDevice>& inactiveDisplay,
+                                      const sp<DisplayDevice>& activeDisplay)
             REQUIRES(mStateLock, kMainThreadContext);
 
     void onActiveDisplaySizeChanged(const sp<const DisplayDevice>&);
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
index 19c7d5c..d58e644 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
@@ -359,6 +359,7 @@
     }
 
     // Called by tests to inject a HWC display setup
+    template <bool kInitPowerMode = true>
     static void injectHwcDisplayWithNoDefaultCapabilities(DisplayTransactionTest* test) {
         const auto displayId = DisplayVariant::DISPLAY_ID::get();
         ASSERT_FALSE(GpuVirtualDisplayId::tryCast(displayId));
@@ -367,18 +368,21 @@
                 .setHwcDisplayId(HWC_DISPLAY_ID)
                 .setResolution(DisplayVariant::RESOLUTION)
                 .setActiveConfig(HWC_ACTIVE_CONFIG_ID)
-                .setPowerMode(INIT_POWER_MODE)
+                .setPowerMode(kInitPowerMode ? std::make_optional(INIT_POWER_MODE) : std::nullopt)
                 .inject(&test->mFlinger, test->mComposer);
     }
 
     // Called by tests to inject a HWC display setup
+    template <bool kInitPowerMode = true>
     static void injectHwcDisplay(DisplayTransactionTest* test) {
         EXPECT_CALL(*test->mComposer, getDisplayCapabilities(HWC_DISPLAY_ID, _))
                 .WillOnce(DoAll(SetArgPointee<1>(std::vector<DisplayCapability>({})),
                                 Return(Error::NONE)));
-        EXPECT_CALL(*test->mComposer, setPowerMode(HWC_DISPLAY_ID, INIT_POWER_MODE))
-                .WillOnce(Return(Error::NONE));
-        injectHwcDisplayWithNoDefaultCapabilities(test);
+        if constexpr (kInitPowerMode) {
+            EXPECT_CALL(*test->mComposer, setPowerMode(HWC_DISPLAY_ID, INIT_POWER_MODE))
+                    .WillOnce(Return(Error::NONE));
+        }
+        injectHwcDisplayWithNoDefaultCapabilities<kInitPowerMode>(test);
     }
 
     static std::shared_ptr<compositionengine::Display> injectCompositionDisplay(
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_PowerHintTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_PowerHintTest.cpp
index bc66961..622717f 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_PowerHintTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_PowerHintTest.cpp
@@ -97,7 +97,6 @@
                     .setNativeWindow(mNativeWindow)
                     .setPowerMode(hal::PowerMode::ON)
                     .inject();
-    mFlinger.mutableActiveDisplayId() = mDisplay->getPhysicalId();
 }
 
 void SurfaceFlingerPowerHintTest::setupScheduler() {
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
index 25857ec..0fb8e2b 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp
@@ -259,12 +259,6 @@
         auto injector = Display::makeFakeExistingDisplayInjector(test);
         const auto display = injector.inject();
         display->setPowerMode(mode);
-        if (injector.physicalDisplay()
-                    .transform(&display::PhysicalDisplay::isInternal)
-                    .value_or(false)) {
-            test->mFlinger.mutableActiveDisplayId() = display->getPhysicalId();
-        }
-
         return display;
     }
 
@@ -490,5 +484,37 @@
     transitionDisplayCommon<ExternalDisplayPowerCase<TransitionOnToUnknownVariant>>();
 }
 
+TEST_F(SetPowerModeInternalTest, designatesLeaderDisplay) {
+    using Case = SimplePrimaryDisplayCase;
+
+    // --------------------------------------------------------------------
+    // Preconditions
+
+    // Inject a primary display with uninitialized power mode.
+    constexpr bool kInitPowerMode = false;
+    Case::Display::injectHwcDisplay<kInitPowerMode>(this);
+    auto injector = Case::Display::makeFakeExistingDisplayInjector(this);
+    injector.setPowerMode(std::nullopt);
+    const auto display = injector.inject();
+
+    // --------------------------------------------------------------------
+    // Invocation
+
+    // FakeDisplayDeviceInjector registers the display with Scheduler, so it has already been
+    // designated as the leader. Set an arbitrary leader to verify that `setPowerModeInternal`
+    // designates a leader regardless of any preceding `Scheduler::registerDisplay` call(s).
+    constexpr PhysicalDisplayId kPlaceholderId = PhysicalDisplayId::fromPort(42);
+    ASSERT_NE(display->getPhysicalId(), kPlaceholderId);
+    mFlinger.scheduler()->setLeaderDisplay(kPlaceholderId);
+
+    mFlinger.setPowerModeInternal(display, PowerMode::ON);
+
+    // --------------------------------------------------------------------
+    // Postconditions
+
+    // The primary display should be designated as the leader.
+    EXPECT_EQ(mFlinger.scheduler()->leaderDisplayId(), display->getPhysicalId());
+}
+
 } // namespace
 } // namespace android
diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h
index 54c10c5..b8a6063 100644
--- a/services/surfaceflinger/tests/unittests/TestableScheduler.h
+++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h
@@ -87,6 +87,10 @@
         Scheduler::unregisterDisplay(displayId);
     }
 
+    std::optional<PhysicalDisplayId> leaderDisplayId() const NO_THREAD_SAFETY_ANALYSIS {
+        return mLeaderDisplayId;
+    }
+
     void setLeaderDisplay(PhysicalDisplayId displayId) {
         ftl::FakeGuard guard(kMainThreadContext);
         Scheduler::setLeaderDisplay(displayId);
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index e29dd67..fb473f5 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -470,7 +470,7 @@
     void onActiveDisplayChanged(const sp<DisplayDevice>& activeDisplay) {
         Mutex::Autolock lock(mFlinger->mStateLock);
         ftl::FakeGuard guard(kMainThreadContext);
-        mFlinger->onActiveDisplayChangedLocked(activeDisplay);
+        mFlinger->onActiveDisplayChangedLocked(nullptr, activeDisplay);
     }
 
     auto createLayer(LayerCreationArgs& args, const sp<IBinder>& parentHandle,
@@ -621,7 +621,7 @@
             return *this;
         }
 
-        auto& setPowerMode(hal::PowerMode mode) {
+        auto& setPowerMode(std::optional<hal::PowerMode> mode) {
             mPowerMode = mode;
             return *this;
         }
@@ -645,7 +645,11 @@
                                                          mHwcDisplayType);
 
             display->mutableIsConnected() = true;
-            display->setPowerMode(mPowerMode);
+
+            if (mPowerMode) {
+                display->setPowerMode(*mPowerMode);
+            }
+
             flinger->mutableHwcDisplayData()[mDisplayId].hwcDisplay = std::move(display);
 
             EXPECT_CALL(*composer, getDisplayConfigs(mHwcDisplayId, _))
@@ -711,7 +715,7 @@
         int32_t mDpiY = DEFAULT_DPI;
         int32_t mConfigGroup = DEFAULT_CONFIG_GROUP;
         hal::HWConfigId mActiveConfig = DEFAULT_ACTIVE_CONFIG;
-        hal::PowerMode mPowerMode = DEFAULT_POWER_MODE;
+        std::optional<hal::PowerMode> mPowerMode = DEFAULT_POWER_MODE;
         const std::unordered_set<aidl::android::hardware::graphics::composer3::Capability>*
                 mCapabilities = nullptr;
     };
@@ -788,7 +792,7 @@
             return *this;
         }
 
-        auto& setPowerMode(hal::PowerMode mode) {
+        auto& setPowerMode(std::optional<hal::PowerMode> mode) {
             mCreationArgs.initialPowerMode = mode;
             return *this;
         }
@@ -853,6 +857,10 @@
                 LOG_ALWAYS_FATAL_IF(!physicalIdOpt);
                 const auto physicalId = *physicalIdOpt;
 
+                if (mCreationArgs.isPrimary) {
+                    mFlinger.mutableActiveDisplayId() = physicalId;
+                }
+
                 LOG_ALWAYS_FATAL_IF(!mHwcDisplayId);
 
                 const auto activeMode = modes.get(activeModeId);