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
diff --git a/services/surfaceflinger/tests/Credentials_test.cpp b/services/surfaceflinger/tests/Credentials_test.cpp
index c1ef48e..ebe11fb 100644
--- a/services/surfaceflinger/tests/Credentials_test.cpp
+++ b/services/surfaceflinger/tests/Credentials_test.cpp
@@ -39,8 +39,8 @@
 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 @@
         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, 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 @@
     }
 
     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 7fce7e9..56cf13d 100644
--- a/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp
+++ b/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp
@@ -53,14 +53,15 @@
     }
 
     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 87e6d3e..af3cb9a 100644
--- a/services/surfaceflinger/tests/TransactionTestHarnesses.h
+++ b/services/surfaceflinger/tests/TransactionTestHarnesses.h
@@ -66,8 +66,9 @@
                 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 @@
                     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 f31f582..cd66dd2 100644
--- a/services/surfaceflinger/tests/VirtualDisplay_test.cpp
+++ b/services/surfaceflinger/tests/VirtualDisplay_test.cpp
@@ -41,14 +41,15 @@
 };
 
 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 5852b1c..ff7612e 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_ColorMatrixTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_ColorMatrixTest.cpp
@@ -53,7 +53,8 @@
     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 bf5ae21..e5f2a91 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp
@@ -27,7 +27,7 @@
 
 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 @@
         // --------------------------------------------------------------------
         // Invocation
 
-        sp<IBinder> displayToken = mFlinger.createDisplay(name, false, requestedRefreshRate);
+        sp<IBinder> displayToken = mFlinger.createVirtualDisplay(name, false, requestedRefreshRate);
 
         // --------------------------------------------------------------------
         // Postconditions
@@ -73,7 +73,7 @@
 };
 
 TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForNonsecureDisplay) {
-    const String8 name("virtual.test");
+    static const std::string name("virtual.test");
 
     // --------------------------------------------------------------------
     // Call Expectations
@@ -81,7 +81,7 @@
     // --------------------------------------------------------------------
     // Invocation
 
-    sp<IBinder> displayToken = mFlinger.createDisplay(name, false);
+    sp<IBinder> displayToken = mFlinger.createVirtualDisplay(name, false);
 
     // --------------------------------------------------------------------
     // Postconditions
@@ -101,7 +101,7 @@
 }
 
 TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForSecureDisplay) {
-    const String8 name("virtual.test");
+    static const std::string kDisplayName("virtual.test");
 
     // --------------------------------------------------------------------
     // Call Expectations
@@ -112,7 +112,7 @@
     // 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 @@
     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, 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 @@
     // --------------------------------------------------------------------
     // Invocation
 
-    sp<IBinder> displayToken = mFlinger.createDisplay(name, false, uniqueId);
+    sp<IBinder> displayToken = mFlinger.createVirtualDisplay(kDisplayName, false, kUniqueId);
 
     // --------------------------------------------------------------------
     // Postconditions
@@ -153,7 +153,7 @@
     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 @@
 
 // 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 93a3811..f8ad8e1 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp
@@ -43,18 +43,18 @@
     // --------------------------------------------------------------------
     // 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 @@
     // --------------------------------------------------------------------
     // 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 4e9fba7..f424133 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_GetDisplayStatsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_GetDisplayStatsTest.cpp
@@ -42,8 +42,8 @@
 }
 
 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 b251e2c..265f804 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -420,19 +420,21 @@
         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) {