SF: Cleanup creating / destroying virtual display APIs

Follow-up CL for cleanup requested by SF owner in http://ag/27202517.

Misc:
- Fix type conversion warnings for print format.
- Return status destroying virtual display in SF and add appropriate
checks in UT.
- Use static constexpr / const for compile-time effciciency.

Bug: 339525838
Bug: 137375833
Bug: 194863377
Test: atest libsurfaceflinger_unittest
Test: atest SurfaceFlinger_test
Flag: EXEMPT refactor
Change-Id: I7859720989c9244b26f8764c3323a8495064c888

17 files changed, 146 insertions, 132 deletions

diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 7aaaebbc8e..0a85cf8d8a 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -1280,18 +1280,22 @@ status_t SurfaceComposerClient::Transaction::sendSurfaceFlushJankDataTransaction
 }
 // ---------------------------------------------------------------------------
 
-sp<IBinder> SurfaceComposerClient::createDisplay(const String8& displayName, bool isSecure,
-                                                 const std::string& uniqueId,
-                                                 float requestedRefreshRate) {
+sp<IBinder> SurfaceComposerClient::createVirtualDisplay(const std::string& displayName,
+                                                        bool isSecure, const std::string& uniqueId,
+                                                        float requestedRefreshRate) {
     sp<IBinder> display = nullptr;
-    binder::Status status = ComposerServiceAIDL::getComposerService()
-                                    ->createDisplay(std::string(displayName.c_str()), isSecure,
-                                                    uniqueId, requestedRefreshRate, &display);
+    binder::Status status =
+            ComposerServiceAIDL::getComposerService()->createVirtualDisplay(displayName, isSecure,
+                                                                            uniqueId,
+                                                                            requestedRefreshRate,
+                                                                            &display);
     return status.isOk() ? display : nullptr;
 }
 
-void SurfaceComposerClient::destroyDisplay(const sp<IBinder>& display) {
-    ComposerServiceAIDL::getComposerService()->destroyDisplay(display);
+status_t SurfaceComposerClient::destroyVirtualDisplay(const sp<IBinder>& displayToken) {
+    return ComposerServiceAIDL::getComposerService()
+            ->destroyVirtualDisplay(displayToken)
+            .transactionError();
 }
 
 std::vector<PhysicalDisplayId> SurfaceComposerClient::getPhysicalDisplayIds() {
diff --git a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl
index c6e7197f24..11ccc9c2fa 100644
--- a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl
+++ b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl
@@ -105,14 +105,14 @@ interface ISurfaceComposer {
      *
      * requires ACCESS_SURFACE_FLINGER permission.
      */
-    @nullable IBinder createDisplay(@utf8InCpp String displayName, boolean isSecure,
+    @nullable IBinder createVirtualDisplay(@utf8InCpp String displayName, boolean isSecure,
             @utf8InCpp String uniqueId, float requestedRefreshRate);
 
     /**
      * Destroy a virtual display.
      * requires ACCESS_SURFACE_FLINGER permission.
      */
-    void destroyDisplay(IBinder display);
+    void destroyVirtualDisplay(IBinder displayToken);
 
     /**
      * Get stable IDs for connected physical displays.
diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h
index 738c73a24b..eb4a802c17 100644
--- a/libs/gui/include/gui/ISurfaceComposer.h
+++ b/libs/gui/include/gui/ISurfaceComposer.h
@@ -130,8 +130,8 @@ public:
         CREATE_CONNECTION,               // Deprecated. Autogenerated by .aidl now.
         GET_STATIC_DISPLAY_INFO,         // Deprecated. Autogenerated by .aidl now.
         CREATE_DISPLAY_EVENT_CONNECTION, // Deprecated. Autogenerated by .aidl now.
-        CREATE_DISPLAY,                  // Deprecated. Autogenerated by .aidl now.
-        DESTROY_DISPLAY,                 // Deprecated. Autogenerated by .aidl now.
+        CREATE_VIRTUAL_DISPLAY,          // Deprecated. Autogenerated by .aidl now.
+        DESTROY_VIRTUAL_DISPLAY,         // Deprecated. Autogenerated by .aidl now.
         GET_PHYSICAL_DISPLAY_TOKEN,      // Deprecated. Autogenerated by .aidl now.
         SET_TRANSACTION_STATE,
         AUTHENTICATE_SURFACE,           // Deprecated. Autogenerated by .aidl now.
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 987efe01ba..e2307ed60a 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -376,11 +376,11 @@ public:
     sp<SurfaceControl> mirrorDisplay(DisplayId displayId);
 
     static const std::string kEmpty;
-    static sp<IBinder> createDisplay(const String8& displayName, bool isSecure,
-                                     const std::string& uniqueId = kEmpty,
-                                     float requestedRefreshRate = 0);
+    static sp<IBinder> createVirtualDisplay(const std::string& displayName, bool isSecure,
+                                            const std::string& uniqueId = kEmpty,
+                                            float requestedRefreshRate = 0);
 
-    static void destroyDisplay(const sp<IBinder>& display);
+    static status_t destroyVirtualDisplay(const sp<IBinder>& displayToken);
 
     static std::vector<PhysicalDisplayId> getPhysicalDisplayIds();
 
diff --git a/libs/gui/tests/EndToEndNativeInputTest.cpp b/libs/gui/tests/EndToEndNativeInputTest.cpp
index c0e79655f8..b0db396872 100644
--- a/libs/gui/tests/EndToEndNativeInputTest.cpp
+++ b/libs/gui/tests/EndToEndNativeInputTest.cpp
@@ -1186,9 +1186,8 @@ public:
     MultiDisplayTests() : InputSurfacesTest() { ProcessState::self()->startThreadPool(); }
 
     void TearDown() override {
-        for (auto& token : mVirtualDisplays) {
-            SurfaceComposerClient::destroyDisplay(token);
-        }
+        std::for_each(mVirtualDisplays.begin(), mVirtualDisplays.end(),
+                      SurfaceComposerClient::destroyVirtualDisplay);
         InputSurfacesTest::TearDown();
     }
 
@@ -1203,7 +1202,7 @@ public:
 
         std::string name = "VirtualDisplay";
         name += std::to_string(mVirtualDisplays.size());
-        sp<IBinder> token = SurfaceComposerClient::createDisplay(String8(name.c_str()), isSecure);
+        sp<IBinder> token = SurfaceComposerClient::createVirtualDisplay(name, isSecure);
         SurfaceComposerClient::Transaction t;
         t.setDisplaySurface(token, producer);
         t.setDisplayFlags(token, receivesInput ? 0x01 /* DisplayDevice::eReceivesInput */ : 0);
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index eee4fb93c9..6c6a849544 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -673,13 +673,14 @@ public:
         return binder::Status::ok();
     }
 
-    binder::Status createDisplay(const std::string& /*displayName*/, bool /*isSecure*/,
-                                 const std::string& /*uniqueId*/, float /*requestedRefreshRate*/,
-                                 sp<IBinder>* /*outDisplay*/) override {
+    binder::Status createVirtualDisplay(const std::string& /*displayName*/, bool /*isSecure*/,
+                                        const std::string& /*uniqueId*/,
+                                        float /*requestedRefreshRate*/,
+                                        sp<IBinder>* /*outDisplay*/) override {
         return binder::Status::ok();
     }
 
-    binder::Status destroyDisplay(const sp<IBinder>& /*display*/) override {
+    binder::Status destroyVirtualDisplay(const sp<IBinder>& /*displayToken*/) override {
         return binder::Status::ok();
     }
 
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 5181fb8b56..9a575ca769 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -56,6 +56,7 @@
 #include <configstore/Utils.h>
 #include <cutils/compiler.h>
 #include <cutils/properties.h>
+#include <fmt/format.h>
 #include <ftl/algorithm.h>
 #include <ftl/concat.h>
 #include <ftl/fake_guard.h>
@@ -573,8 +574,9 @@ void SurfaceFlinger::run() {
     mScheduler->run();
 }
 
-sp<IBinder> SurfaceFlinger::createDisplay(const String8& displayName, bool isSecure,
-                                          const std::string& uniqueId, float requestedRefreshRate) {
+sp<IBinder> SurfaceFlinger::createVirtualDisplay(const std::string& displayName, bool isSecure,
+                                                 const std::string& uniqueId,
+                                                 float requestedRefreshRate) {
     // SurfaceComposerAIDL checks for some permissions, but adding an additional check here.
     // This is to ensure that only root, system, and graphics can request to create a secure
     // display. Secure displays can show secure content so we add an additional restriction on it.
@@ -614,22 +616,23 @@ sp<IBinder> SurfaceFlinger::createDisplay(const String8& displayName, bool isSec
     return token;
 }
 
-void SurfaceFlinger::destroyDisplay(const sp<IBinder>& displayToken) {
+status_t SurfaceFlinger::destroyVirtualDisplay(const sp<IBinder>& displayToken) {
     Mutex::Autolock lock(mStateLock);
 
     const ssize_t index = mCurrentState.displays.indexOfKey(displayToken);
     if (index < 0) {
         ALOGE("%s: Invalid display token %p", __func__, displayToken.get());
-        return;
+        return NAME_NOT_FOUND;
     }
 
     const DisplayDeviceState& state = mCurrentState.displays.valueAt(index);
     if (state.physical) {
         ALOGE("%s: Invalid operation on physical display", __func__);
-        return;
+        return INVALID_OPERATION;
     }
     mCurrentState.displays.removeItemsAt(index);
     setTransactionFlags(eDisplayTransactionNeeded);
+    return NO_ERROR;
 }
 
 void SurfaceFlinger::enableHalVirtualDisplays(bool enable) {
@@ -6531,18 +6534,19 @@ void SurfaceFlinger::dumpWideColorInfo(std::string& result) const {
     StringAppendF(&result, "DisplayColorSetting: %s\n",
                   decodeDisplayColorSetting(mDisplayColorSetting).c_str());
 
-    // TODO: print out if wide-color mode is active or not
+    // TODO: print out if wide-color mode is active or not.
 
     for (const auto& [id, display] : mPhysicalDisplays) {
         StringAppendF(&result, "Display %s color modes:\n", to_string(id).c_str());
         for (const auto mode : display.snapshot().colorModes()) {
-            StringAppendF(&result, "    %s (%d)\n", decodeColorMode(mode).c_str(), mode);
+            StringAppendF(&result, "    %s (%d)\n", decodeColorMode(mode).c_str(),
+                          fmt::underlying(mode));
         }
 
         if (const auto display = getDisplayDeviceLocked(id)) {
             ui::ColorMode currentMode = display->getCompositionDisplay()->getState().colorMode;
             StringAppendF(&result, "    Current color mode: %s (%d)\n",
-                          decodeColorMode(currentMode).c_str(), currentMode);
+                          decodeColorMode(currentMode).c_str(), fmt::underlying(currentMode));
         }
     }
     result.append("\n");
@@ -6999,8 +7003,8 @@ status_t SurfaceFlinger::CheckTransactCodeCredentials(uint32_t code) {
         // Used by apps to hook Choreographer to SurfaceFlinger.
         case CREATE_DISPLAY_EVENT_CONNECTION:
         case CREATE_CONNECTION:
-        case CREATE_DISPLAY:
-        case DESTROY_DISPLAY:
+        case CREATE_VIRTUAL_DISPLAY:
+        case DESTROY_VIRTUAL_DISPLAY:
         case GET_PRIMARY_PHYSICAL_DISPLAY_ID:
         case GET_PHYSICAL_DISPLAY_IDS:
         case GET_PHYSICAL_DISPLAY_TOKEN:
@@ -9553,26 +9557,25 @@ binder::Status SurfaceComposerAIDL::createConnection(sp<gui::ISurfaceComposerCli
     }
 }
 
-binder::Status SurfaceComposerAIDL::createDisplay(const std::string& displayName, bool isSecure,
-                                                  const std::string& uniqueId,
-                                                  float requestedRefreshRate,
-                                                  sp<IBinder>* outDisplay) {
+binder::Status SurfaceComposerAIDL::createVirtualDisplay(const std::string& displayName,
+                                                         bool isSecure, const std::string& uniqueId,
+                                                         float requestedRefreshRate,
+                                                         sp<IBinder>* outDisplay) {
     status_t status = checkAccessPermission();
     if (status != OK) {
         return binderStatusFromStatusT(status);
     }
-    String8 displayName8 = String8::format("%s", displayName.c_str());
-    *outDisplay = mFlinger->createDisplay(displayName8, isSecure, uniqueId, requestedRefreshRate);
+    *outDisplay =
+            mFlinger->createVirtualDisplay(displayName, isSecure, uniqueId, requestedRefreshRate);
     return binder::Status::ok();
 }
 
-binder::Status SurfaceComposerAIDL::destroyDisplay(const sp<IBinder>& display) {
+binder::Status SurfaceComposerAIDL::destroyVirtualDisplay(const sp<IBinder>& displayToken) {
     status_t status = checkAccessPermission();
     if (status != OK) {
         return binderStatusFromStatusT(status);
     }
-    mFlinger->destroyDisplay(display);
-    return binder::Status::ok();
+    return binder::Status::fromStatusT(mFlinger->destroyVirtualDisplay(displayToken));
 }
 
 binder::Status SurfaceComposerAIDL::getPhysicalDisplayIds(std::vector<int64_t>* outDisplayIds) {
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 84745150d0..2363cd7ebd 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -547,9 +547,10 @@ private:
     status_t dump(int fd, const Vector<String16>& args) override { return priorityDump(fd, args); }
 
     // ISurfaceComposer implementation:
-    sp<IBinder> createDisplay(const String8& displayName, bool isSecure,
-                              const std::string& uniqueId, float requestedRefreshRate = 0.0f);
-    void destroyDisplay(const sp<IBinder>& displayToken);
+    sp<IBinder> createVirtualDisplay(const std::string& displayName, bool isSecure,
+                                     const std::string& uniqueId,
+                                     float requestedRefreshRate = 0.0f);
+    status_t destroyVirtualDisplay(const sp<IBinder>& displayToken);
     std::vector<PhysicalDisplayId> getPhysicalDisplayIds() const EXCLUDES(mStateLock) {
         Mutex::Autolock lock(mStateLock);
         return getPhysicalDisplayIdsLocked();
@@ -1568,10 +1569,10 @@ public:
             const sp<IBinder>& layerHandle,
             sp<gui::IDisplayEventConnection>* outConnection) override;
     binder::Status createConnection(sp<gui::ISurfaceComposerClient>* outClient) override;
-    binder::Status createDisplay(const std::string& displayName, bool isSecure,
-                                 const std::string& uniqueId, float requestedRefreshRate,
-                                 sp<IBinder>* outDisplay) override;
-    binder::Status destroyDisplay(const sp<IBinder>& display) override;
+    binder::Status createVirtualDisplay(const std::string& displayName, bool isSecure,
+                                        const std::string& uniqueId, float requestedRefreshRate,
+                                        sp<IBinder>* outDisplay) override;
+    binder::Status destroyVirtualDisplay(const sp<IBinder>& displayToken) override;
     binder::Status getPhysicalDisplayIds(std::vector<int64_t>* outDisplayIds) override;
     binder::Status getPhysicalDisplayToken(int64_t displayId, sp<IBinder>* outDisplay) override;
     binder::Status setPowerMode(const sp<IBinder>& display, int mode) override;
diff --git a/services/surfaceflinger/tests/Credentials_test.cpp b/services/surfaceflinger/tests/Credentials_test.cpp
index c1ef48ec97..ebe11fb0f3 100644
--- a/services/surfaceflinger/tests/Credentials_test.cpp
+++ b/services/surfaceflinger/tests/Credentials_test.cpp
@@ -39,8 +39,8 @@ using gui::aidl_utils::statusTFromBinderStatus;
 using ui::ColorMode;
 
 namespace {
-const String8 DISPLAY_NAME("Credentials Display Test");
-const String8 SURFACE_NAME("Test Surface Name");
+const std::string kDisplayName("Credentials Display Test");
+const String8 kSurfaceName("Test Surface Name");
 } // namespace
 
 /**
@@ -99,7 +99,7 @@ protected:
         ASSERT_EQ(NO_ERROR, SurfaceComposerClient::getActiveDisplayMode(mDisplay, &mode));
 
         // Background surface
-        mBGSurfaceControl = mComposerClient->createSurface(SURFACE_NAME, mode.resolution.getWidth(),
+        mBGSurfaceControl = mComposerClient->createSurface(kSurfaceName, mode.resolution.getWidth(),
                                                            mode.resolution.getHeight(),
                                                            PIXEL_FORMAT_RGBA_8888, 0);
         ASSERT_TRUE(mBGSurfaceControl != nullptr);
@@ -232,7 +232,7 @@ TEST_F(CredentialsTest, SetActiveColorModeTest) {
 TEST_F(CredentialsTest, CreateDisplayTest) {
     // Only graphics and system processes can create a secure display.
     std::function<bool()> condition = [=]() {
-        sp<IBinder> testDisplay = SurfaceComposerClient::createDisplay(DISPLAY_NAME, true);
+        sp<IBinder> testDisplay = SurfaceComposerClient::createVirtualDisplay(kDisplayName, true);
         return testDisplay.get() != nullptr;
     };
 
@@ -267,7 +267,7 @@ TEST_F(CredentialsTest, CreateDisplayTest) {
     }
 
     condition = [=]() {
-        sp<IBinder> testDisplay = SurfaceComposerClient::createDisplay(DISPLAY_NAME, false);
+        sp<IBinder> testDisplay = SurfaceComposerClient::createVirtualDisplay(kDisplayName, false);
         return testDisplay.get() != nullptr;
     };
     ASSERT_NO_FATAL_FAILURE(checkWithPrivileges(condition, true, false));
diff --git a/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp b/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp
index 7fce7e9af9..56cf13d7fe 100644
--- a/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp
+++ b/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp
@@ -53,14 +53,15 @@ protected:
     }
 
     virtual void TearDown() {
-        SurfaceComposerClient::destroyDisplay(mVirtualDisplay);
+        EXPECT_EQ(NO_ERROR, SurfaceComposerClient::destroyVirtualDisplay(mVirtualDisplay));
         LayerTransactionTest::TearDown();
         mColorLayer = 0;
     }
 
     void createDisplay(const ui::Size& layerStackSize, ui::LayerStack layerStack) {
+        static const std::string kDisplayName("VirtualDisplay");
         mVirtualDisplay =
-                SurfaceComposerClient::createDisplay(String8("VirtualDisplay"), false /*secure*/);
+                SurfaceComposerClient::createVirtualDisplay(kDisplayName, false /*isSecure*/);
         asTransaction([&](Transaction& t) {
             t.setDisplaySurface(mVirtualDisplay, mProducer);
             t.setDisplayLayerStack(mVirtualDisplay, layerStack);
diff --git a/services/surfaceflinger/tests/TransactionTestHarnesses.h b/services/surfaceflinger/tests/TransactionTestHarnesses.h
index 87e6d3ee9f..af3cb9aec1 100644
--- a/services/surfaceflinger/tests/TransactionTestHarnesses.h
+++ b/services/surfaceflinger/tests/TransactionTestHarnesses.h
@@ -66,8 +66,9 @@ public:
                 sp<BufferListener> listener = sp<BufferListener>::make(this);
                 itemConsumer->setFrameAvailableListener(listener);
 
-                vDisplay = SurfaceComposerClient::createDisplay(String8("VirtualDisplay"),
-                                                                false /*secure*/);
+                static const std::string kDisplayName("VirtualDisplay");
+                vDisplay = SurfaceComposerClient::createVirtualDisplay(kDisplayName,
+                                                                       false /*isSecure*/);
 
                 constexpr ui::LayerStack layerStack{
                         848472}; // ASCII for TTH (TransactionTestHarnesses)
@@ -107,7 +108,7 @@ public:
                     t.setLayerStack(mirrorSc, ui::INVALID_LAYER_STACK);
                     t.apply(true);
                 }
-                SurfaceComposerClient::destroyDisplay(vDisplay);
+                SurfaceComposerClient::destroyVirtualDisplay(vDisplay);
                 return sc;
         }
     }
diff --git a/services/surfaceflinger/tests/VirtualDisplay_test.cpp b/services/surfaceflinger/tests/VirtualDisplay_test.cpp
index f31f582ffd..cd66dd20bb 100644
--- a/services/surfaceflinger/tests/VirtualDisplay_test.cpp
+++ b/services/surfaceflinger/tests/VirtualDisplay_test.cpp
@@ -41,14 +41,15 @@ protected:
 };
 
 TEST_F(VirtualDisplayTest, VirtualDisplayDestroyedSurfaceReuse) {
+    static const std::string kDisplayName("VirtualDisplay");
     sp<IBinder> virtualDisplay =
-            SurfaceComposerClient::createDisplay(String8("VirtualDisplay"), false /*secure*/);
+            SurfaceComposerClient::createVirtualDisplay(kDisplayName, false /*isSecure*/);
 
     SurfaceComposerClient::Transaction t;
     t.setDisplaySurface(virtualDisplay, mProducer);
     t.apply(true);
 
-    SurfaceComposerClient::destroyDisplay(virtualDisplay);
+    EXPECT_EQ(NO_ERROR, SurfaceComposerClient::destroyVirtualDisplay(virtualDisplay));
     virtualDisplay.clear();
     // Sync here to ensure the display was completely destroyed in SF
     t.apply(true);
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_ColorMatrixTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_ColorMatrixTest.cpp
index 5852b1c45f..ff7612e064 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_ColorMatrixTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_ColorMatrixTest.cpp
@@ -53,7 +53,8 @@ TEST_F(ColorMatrixTest, colorMatrixChangedAfterDisplayTransaction) {
     mFlinger.commitAndComposite();
     EXPECT_COLOR_MATRIX_CHANGED(false, false);
 
-    mFlinger.createDisplay(String8("Test Display"), false);
+    static const std::string kDisplayName("Test Display");
+    mFlinger.createVirtualDisplay(kDisplayName, false /*isSecure=*/);
 
     mFlinger.commit();
     EXPECT_COLOR_MATRIX_CHANGED(false, true);
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp
index bf5ae21f60..e5f2a9138d 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp
@@ -27,7 +27,7 @@ namespace {
 
 class CreateDisplayTest : public DisplayTransactionTest {
 public:
-    void createDisplayWithRequestedRefreshRate(const String8& name, uint64_t displayId,
+    void createDisplayWithRequestedRefreshRate(const std::string& name, uint64_t displayId,
                                                float pacesetterDisplayRefreshRate,
                                                float requestedRefreshRate,
                                                float expectedAdjustedRefreshRate) {
@@ -37,7 +37,7 @@ public:
         // --------------------------------------------------------------------
         // Invocation
 
-        sp<IBinder> displayToken = mFlinger.createDisplay(name, false, requestedRefreshRate);
+        sp<IBinder> displayToken = mFlinger.createVirtualDisplay(name, false, requestedRefreshRate);
 
         // --------------------------------------------------------------------
         // Postconditions
@@ -73,7 +73,7 @@ public:
 };
 
 TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForNonsecureDisplay) {
-    const String8 name("virtual.test");
+    static const std::string name("virtual.test");
 
     // --------------------------------------------------------------------
     // Call Expectations
@@ -81,7 +81,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForNonsecureDisplay) {
     // --------------------------------------------------------------------
     // Invocation
 
-    sp<IBinder> displayToken = mFlinger.createDisplay(name, false);
+    sp<IBinder> displayToken = mFlinger.createVirtualDisplay(name, false);
 
     // --------------------------------------------------------------------
     // Postconditions
@@ -101,7 +101,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForNonsecureDisplay) {
 }
 
 TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForSecureDisplay) {
-    const String8 name("virtual.test");
+    static const std::string kDisplayName("virtual.test");
 
     // --------------------------------------------------------------------
     // Call Expectations
@@ -112,7 +112,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForSecureDisplay) {
     // Set the calling identity to graphics so captureDisplay with secure is allowed.
     IPCThreadState::self()->restoreCallingIdentity(static_cast<int64_t>(AID_GRAPHICS) << 32 |
                                                    AID_GRAPHICS);
-    sp<IBinder> displayToken = mFlinger.createDisplay(name, true);
+    sp<IBinder> displayToken = mFlinger.createVirtualDisplay(kDisplayName, true);
     IPCThreadState::self()->restoreCallingIdentity(oldId);
 
     // --------------------------------------------------------------------
@@ -123,7 +123,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForSecureDisplay) {
     const auto& display = getCurrentDisplayState(displayToken);
     EXPECT_TRUE(display.isVirtual());
     EXPECT_TRUE(display.isSecure);
-    EXPECT_EQ(name.c_str(), display.displayName);
+    EXPECT_EQ(kDisplayName.c_str(), display.displayName);
 
     // --------------------------------------------------------------------
     // Cleanup conditions
@@ -133,8 +133,8 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForSecureDisplay) {
 }
 
 TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForUniqueId) {
-    const String8 name("virtual.test");
-    const std::string uniqueId = "virtual:package:id";
+    static const std::string kDisplayName("virtual.test");
+    static const std::string kUniqueId = "virtual:package:id";
 
     // --------------------------------------------------------------------
     // Call Expectations
@@ -142,7 +142,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForUniqueId) {
     // --------------------------------------------------------------------
     // Invocation
 
-    sp<IBinder> displayToken = mFlinger.createDisplay(name, false, uniqueId);
+    sp<IBinder> displayToken = mFlinger.createVirtualDisplay(kDisplayName, false, kUniqueId);
 
     // --------------------------------------------------------------------
     // Postconditions
@@ -153,7 +153,7 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForUniqueId) {
     EXPECT_TRUE(display.isVirtual());
     EXPECT_FALSE(display.isSecure);
     EXPECT_EQ(display.uniqueId, "virtual:package:id");
-    EXPECT_EQ(name.c_str(), display.displayName);
+    EXPECT_EQ(kDisplayName.c_str(), display.displayName);
 
     // --------------------------------------------------------------------
     // Cleanup conditions
@@ -164,78 +164,78 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForUniqueId) {
 
 // Requesting 0 tells SF not to do anything, i.e., default to refresh as physical displays
 TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRate0) {
-    const String8 displayName("virtual.test");
-    const uint64_t displayId = 123ull;
-    const float kPacesetterDisplayRefreshRate = 60.f;
-    const float kRequestedRefreshRate = 0.f;
-    const float kExpectedAdjustedRefreshRate = 0.f;
-    createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate,
+    static const std::string kDisplayName("virtual.test");
+    constexpr uint64_t kDisplayId = 123ull;
+    constexpr float kPacesetterDisplayRefreshRate = 60.f;
+    constexpr float kRequestedRefreshRate = 0.f;
+    constexpr float kExpectedAdjustedRefreshRate = 0.f;
+    createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate,
                                           kRequestedRefreshRate, kExpectedAdjustedRefreshRate);
 }
 
 // Requesting negative refresh rate, will be ignored, same as requesting 0
 TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateNegative) {
-    const String8 displayName("virtual.test");
-    const uint64_t displayId = 123ull;
-    const float kPacesetterDisplayRefreshRate = 60.f;
-    const float kRequestedRefreshRate = -60.f;
-    const float kExpectedAdjustedRefreshRate = 0.f;
-    createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate,
+    static const std::string kDisplayName("virtual.test");
+    constexpr uint64_t kDisplayId = 123ull;
+    constexpr float kPacesetterDisplayRefreshRate = 60.f;
+    constexpr float kRequestedRefreshRate = -60.f;
+    constexpr float kExpectedAdjustedRefreshRate = 0.f;
+    createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate,
                                           kRequestedRefreshRate, kExpectedAdjustedRefreshRate);
 }
 
 // Requesting a higher refresh rate than the pacesetter
 TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateHigh) {
-    const String8 displayName("virtual.test");
-    const uint64_t displayId = 123ull;
-    const float kPacesetterDisplayRefreshRate = 60.f;
-    const float kRequestedRefreshRate = 90.f;
-    const float kExpectedAdjustedRefreshRate = 60.f;
-    createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate,
+    static const std::string kDisplayName("virtual.test");
+    constexpr uint64_t kDisplayId = 123ull;
+    constexpr float kPacesetterDisplayRefreshRate = 60.f;
+    constexpr float kRequestedRefreshRate = 90.f;
+    constexpr float kExpectedAdjustedRefreshRate = 60.f;
+    createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate,
                                           kRequestedRefreshRate, kExpectedAdjustedRefreshRate);
 }
 
 // Requesting the same refresh rate as the pacesetter
 TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateSame) {
-    const String8 displayName("virtual.test");
-    const uint64_t displayId = 123ull;
-    const float kPacesetterDisplayRefreshRate = 60.f;
-    const float kRequestedRefreshRate = 60.f;
-    const float kExpectedAdjustedRefreshRate = 60.f;
-    createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate,
+    static const std::string kDisplayName("virtual.test");
+    constexpr uint64_t kDisplayId = 123ull;
+    constexpr float kPacesetterDisplayRefreshRate = 60.f;
+    constexpr float kRequestedRefreshRate = 60.f;
+    constexpr float kExpectedAdjustedRefreshRate = 60.f;
+    createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate,
                                           kRequestedRefreshRate, kExpectedAdjustedRefreshRate);
 }
 
 // Requesting a divisor (30) of the pacesetter (60) should be honored
 TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateDivisor) {
-    const String8 displayName("virtual.test");
-    const uint64_t displayId = 123ull;
-    const float kPacesetterDisplayRefreshRate = 60.f;
-    const float kRequestedRefreshRate = 30.f;
-    const float kExpectedAdjustedRefreshRate = 30.f;
-    createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate,
+    static const std::string kDisplayName("virtual.test");
+    constexpr uint64_t kDisplayId = 123ull;
+    constexpr float kPacesetterDisplayRefreshRate = 60.f;
+    constexpr float kRequestedRefreshRate = 30.f;
+    constexpr float kExpectedAdjustedRefreshRate = 30.f;
+    createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate,
                                           kRequestedRefreshRate, kExpectedAdjustedRefreshRate);
 }
 
 // Requesting a non divisor (45) of the pacesetter (120) should round up to a divisor (60)
 TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateNoneDivisor) {
-    const String8 displayName("virtual.test");
-    const uint64_t displayId = 123ull;
-    const float kPacesetterDisplayRefreshRate = 120.f;
-    const float kRequestedRefreshRate = 45.f;
-    const float kExpectedAdjustedRefreshRate = 60.f;
-    createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate,
+    static const std::string kDisplayName("virtual.test");
+    constexpr uint64_t kDisplayId = 123ull;
+    constexpr float kPacesetterDisplayRefreshRate = 120.f;
+    constexpr float kRequestedRefreshRate = 45.f;
+    constexpr float kExpectedAdjustedRefreshRate = 60.f;
+    createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate,
                                           kRequestedRefreshRate, kExpectedAdjustedRefreshRate);
 }
 
 // Requesting a non divisor (75) of the pacesetter (120) should round up to pacesetter (120)
 TEST_F(CreateDisplayTest, createDisplayWithRequestedRefreshRateNoneDivisorMax) {
-    const String8 displayName("virtual.test");
-    const uint64_t displayId = 123ull;
-    const float kPacesetterDisplayRefreshRate = 120.f;
-    const float kRequestedRefreshRate = 75.f;
-    const float kExpectedAdjustedRefreshRate = 120.f;
-    createDisplayWithRequestedRefreshRate(displayName, displayId, kPacesetterDisplayRefreshRate,
+    static const std::string kDisplayName("virtual.test");
+    constexpr uint64_t kDisplayId = 123ull;
+    constexpr float kPacesetterDisplayRefreshRate = 120.f;
+    constexpr float kRequestedRefreshRate = 75.f;
+    constexpr float kExpectedAdjustedRefreshRate = 120.f;
+    createDisplayWithRequestedRefreshRate(kDisplayName, kDisplayId, kPacesetterDisplayRefreshRate,
                                           kRequestedRefreshRate, kExpectedAdjustedRefreshRate);
 }
 
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp
index 93a3811172..f8ad8e1e1b 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp
@@ -43,18 +43,18 @@ TEST_F(DestroyDisplayTest, destroyDisplayClearsCurrentStateForDisplay) {
     // --------------------------------------------------------------------
     // Invocation
 
-    mFlinger.destroyDisplay(existing.token());
+    EXPECT_EQ(NO_ERROR, mFlinger.destroyVirtualDisplay(existing.token()));
 
     // --------------------------------------------------------------------
     // Postconditions
 
-    // The display should have been removed from the current state
+    // The display should have been removed from the current state.
     EXPECT_FALSE(hasCurrentDisplayState(existing.token()));
 
-    // Ths display should still exist in the drawing state
+    // Ths display should still exist in the drawing state.
     EXPECT_TRUE(hasDrawingDisplayState(existing.token()));
 
-    // The display transaction needed flasg should be set
+    // The display transaction needed flags should be set.
     EXPECT_TRUE(hasTransactionFlagSet(eDisplayTransactionNeeded));
 }
 
@@ -67,7 +67,7 @@ TEST_F(DestroyDisplayTest, destroyDisplayHandlesUnknownDisplay) {
     // --------------------------------------------------------------------
     // Invocation
 
-    mFlinger.destroyDisplay(displayToken);
+    EXPECT_EQ(NAME_NOT_FOUND, mFlinger.destroyVirtualDisplay(displayToken));
 }
 
 } // namespace
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_GetDisplayStatsTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_GetDisplayStatsTest.cpp
index 4e9fba7bda..f424133655 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_GetDisplayStatsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_GetDisplayStatsTest.cpp
@@ -42,8 +42,8 @@ TEST_F(SurfaceFlingerGetDisplayStatsTest, explicitToken) {
 }
 
 TEST_F(SurfaceFlingerGetDisplayStatsTest, invalidToken) {
-    const String8 displayName("fakeDisplay");
-    sp<IBinder> displayToken = mFlinger.createDisplay(displayName, false);
+    static const std::string kDisplayName("fakeDisplay");
+    sp<IBinder> displayToken = mFlinger.createVirtualDisplay(kDisplayName, false /*isSecure*/);
     DisplayStatInfo info;
     status_t status = mFlinger.getDisplayStats(displayToken, &info);
     EXPECT_EQ(status, NAME_NOT_FOUND);
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index b251e2cf0c..265f804fc8 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -420,19 +420,21 @@ public:
         commit(kComposite);
     }
 
-    auto createDisplay(const String8& displayName, bool isSecure,
-                       float requestedRefreshRate = 0.0f) {
-        const std::string testId = "virtual:libsurfaceflinger_unittest:TestableSurfaceFlinger";
-        return mFlinger->createDisplay(displayName, isSecure, testId, requestedRefreshRate);
+    auto createVirtualDisplay(const std::string& displayName, bool isSecure,
+                              float requestedRefreshRate = 0.0f) {
+        static const std::string kTestId =
+                "virtual:libsurfaceflinger_unittest:TestableSurfaceFlinger";
+        return mFlinger->createVirtualDisplay(displayName, isSecure, kTestId, requestedRefreshRate);
     }
 
-    auto createDisplay(const String8& displayName, bool isSecure, const std::string& uniqueId,
-                       float requestedRefreshRate = 0.0f) {
-        return mFlinger->createDisplay(displayName, isSecure, uniqueId, requestedRefreshRate);
+    auto createVirtualDisplay(const std::string& displayName, bool isSecure,
+                              const std::string& uniqueId, float requestedRefreshRate = 0.0f) {
+        return mFlinger->createVirtualDisplay(displayName, isSecure, uniqueId,
+                                              requestedRefreshRate);
     }
 
-    auto destroyDisplay(const sp<IBinder>& displayToken) {
-        return mFlinger->destroyDisplay(displayToken);
+    auto destroyVirtualDisplay(const sp<IBinder>& displayToken) {
+        return mFlinger->destroyVirtualDisplay(displayToken);
     }
 
     auto getDisplay(const sp<IBinder>& displayToken) {