diff options
| -rw-r--r-- | libs/gui/SurfaceComposerClient.cpp | 15 | ||||
| -rw-r--r-- | libs/gui/aidl/android/gui/ISurfaceComposer.aidl | 18 | ||||
| -rw-r--r-- | libs/gui/include/gui/SurfaceComposerClient.h | 11 | ||||
| -rw-r--r-- | libs/gui/tests/Surface_test.cpp | 4 | ||||
| -rw-r--r-- | services/surfaceflinger/DisplayDevice.h | 1 | ||||
| -rw-r--r-- | services/surfaceflinger/SurfaceFlinger.cpp | 24 | ||||
| -rw-r--r-- | services/surfaceflinger/SurfaceFlinger.h | 24 | ||||
| -rw-r--r-- | services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp | 30 | ||||
| -rw-r--r-- | services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h | 11 |
9 files changed, 91 insertions, 47 deletions
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 3743025b7b..7aaaebbc8e 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -89,6 +89,8 @@ int64_t generateId() { void emptyCallback(nsecs_t, const sp<Fence>&, const std::vector<SurfaceControlStats>&) {} } // namespace +const std::string SurfaceComposerClient::kEmpty{}; + ComposerService::ComposerService() : Singleton<ComposerService>() { Mutex::Autolock _l(mLock); @@ -1278,14 +1280,13 @@ status_t SurfaceComposerClient::Transaction::sendSurfaceFlushJankDataTransaction } // --------------------------------------------------------------------------- -sp<IBinder> SurfaceComposerClient::createDisplay(const String8& displayName, bool secure, - float requestedRefereshRate) { +sp<IBinder> SurfaceComposerClient::createDisplay(const String8& displayName, bool isSecure, + const std::string& uniqueId, + float requestedRefreshRate) { sp<IBinder> display = nullptr; - binder::Status status = - ComposerServiceAIDL::getComposerService()->createDisplay(std::string( - displayName.c_str()), - secure, requestedRefereshRate, - &display); + binder::Status status = ComposerServiceAIDL::getComposerService() + ->createDisplay(std::string(displayName.c_str()), isSecure, + uniqueId, requestedRefreshRate, &display); return status.isOk() ? display : nullptr; } diff --git a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl index a2549e7e3f..c6e7197f24 100644 --- a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl +++ b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl @@ -72,7 +72,7 @@ interface ISurfaceComposer { void bootFinished(); /** - * Create a display event connection + * Create a display event connection. * * layerHandle * Optional binder handle representing a Layer in SF to associate the new @@ -89,12 +89,14 @@ interface ISurfaceComposer { @nullable ISurfaceComposerClient createConnection(); /** - * Create a virtual display + * Create a virtual display. * * displayName - * The name of the virtual display - * secure - * Whether this virtual display is secure + * The name of the virtual display. + * isSecure + * Whether this virtual display is secure. + * uniqueId + * The unique ID for the display. * requestedRefreshRate * The refresh rate, frames per second, to request on the virtual display. * This is just a request, the actual rate may be adjusted to align well @@ -103,11 +105,11 @@ interface ISurfaceComposer { * * requires ACCESS_SURFACE_FLINGER permission. */ - @nullable IBinder createDisplay(@utf8InCpp String displayName, boolean secure, - float requestedRefreshRate); + @nullable IBinder createDisplay(@utf8InCpp String displayName, boolean isSecure, + @utf8InCpp String uniqueId, float requestedRefreshRate); /** - * Destroy a virtual display + * Destroy a virtual display. * requires ACCESS_SURFACE_FLINGER permission. */ void destroyDisplay(IBinder display); diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 49b0a7d0c9..987efe01ba 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -18,6 +18,7 @@ #include <stdint.h> #include <sys/types.h> + #include <set> #include <thread> #include <unordered_map> @@ -374,17 +375,15 @@ public: sp<SurfaceControl> mirrorDisplay(DisplayId displayId); - //! Create a virtual display - static sp<IBinder> createDisplay(const String8& displayName, bool secure, - float requestedRefereshRate = 0); + static const std::string kEmpty; + static sp<IBinder> createDisplay(const String8& displayName, bool isSecure, + const std::string& uniqueId = kEmpty, + float requestedRefreshRate = 0); - //! Destroy a virtual display static void destroyDisplay(const sp<IBinder>& display); - //! Get stable IDs for connected physical displays static std::vector<PhysicalDisplayId> getPhysicalDisplayIds(); - //! Get token for a physical display given its stable ID static sp<IBinder> getPhysicalDisplayToken(PhysicalDisplayId displayId); // Returns StalledTransactionInfo if a transaction from the provided pid has not been applied diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp index f4b059c39b..eee4fb93c9 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -673,8 +673,8 @@ public: return binder::Status::ok(); } - binder::Status createDisplay(const std::string& /*displayName*/, bool /*secure*/, - float /*requestedRefreshRate*/, + binder::Status createDisplay(const std::string& /*displayName*/, bool /*isSecure*/, + const std::string& /*uniqueId*/, float /*requestedRefreshRate*/, sp<IBinder>* /*outDisplay*/) override { return binder::Status::ok(); } diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index fc5089b6d3..c2d09c910d 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -329,6 +329,7 @@ struct DisplayDeviceState { uint32_t width = 0; uint32_t height = 0; std::string displayName; + std::string uniqueId; bool isSecure = false; bool isProtected = false; // Refer to DisplayDevice::mRequestedRefreshRate, for virtual display only diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 0d2e5142bc..5181fb8b56 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -573,13 +573,13 @@ void SurfaceFlinger::run() { mScheduler->run(); } -sp<IBinder> SurfaceFlinger::createDisplay(const String8& displayName, bool secure, - float requestedRefreshRate) { +sp<IBinder> SurfaceFlinger::createDisplay(const String8& 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. - const int uid = IPCThreadState::self()->getCallingUid(); - if (secure && uid != AID_ROOT && uid != AID_GRAPHICS && uid != AID_SYSTEM) { + const uid_t uid = IPCThreadState::self()->getCallingUid(); + if (isSecure && uid != AID_ROOT && uid != AID_GRAPHICS && uid != AID_SYSTEM) { ALOGE("Only privileged processes can create a secure display"); return nullptr; } @@ -603,11 +603,12 @@ sp<IBinder> SurfaceFlinger::createDisplay(const String8& displayName, bool secur Mutex::Autolock _l(mStateLock); // Display ID is assigned when virtual display is allocated by HWC. DisplayDeviceState state; - state.isSecure = secure; + state.isSecure = isSecure; // Set display as protected when marked as secure to ensure no behavior change // TODO (b/314820005): separate as a different arg when creating the display. - state.isProtected = secure; + state.isProtected = isSecure; state.displayName = displayName; + state.uniqueId = uniqueId; state.requestedRefreshRate = Fps::fromValue(requestedRefreshRate); mCurrentState.displays.add(token, state); return token; @@ -9552,7 +9553,8 @@ binder::Status SurfaceComposerAIDL::createConnection(sp<gui::ISurfaceComposerCli } } -binder::Status SurfaceComposerAIDL::createDisplay(const std::string& displayName, bool secure, +binder::Status SurfaceComposerAIDL::createDisplay(const std::string& displayName, bool isSecure, + const std::string& uniqueId, float requestedRefreshRate, sp<IBinder>* outDisplay) { status_t status = checkAccessPermission(); @@ -9560,7 +9562,7 @@ binder::Status SurfaceComposerAIDL::createDisplay(const std::string& displayName return binderStatusFromStatusT(status); } String8 displayName8 = String8::format("%s", displayName.c_str()); - *outDisplay = mFlinger->createDisplay(displayName8, secure, requestedRefreshRate); + *outDisplay = mFlinger->createDisplay(displayName8, isSecure, uniqueId, requestedRefreshRate); return binder::Status::ok(); } @@ -9577,10 +9579,10 @@ binder::Status SurfaceComposerAIDL::getPhysicalDisplayIds(std::vector<int64_t>* std::vector<PhysicalDisplayId> physicalDisplayIds = mFlinger->getPhysicalDisplayIds(); std::vector<int64_t> displayIds; displayIds.reserve(physicalDisplayIds.size()); - for (auto item : physicalDisplayIds) { - displayIds.push_back(static_cast<int64_t>(item.value)); + for (const auto id : physicalDisplayIds) { + displayIds.push_back(static_cast<int64_t>(id.value)); } - *outDisplayIds = displayIds; + *outDisplayIds = std::move(displayIds); return binder::Status::ok(); } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 4cb5aa3da5..84745150d0 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -539,15 +539,16 @@ private: static const size_t MAX_LAYERS = 4096; - // Implements IBinder. + static bool callingThreadHasUnscopedSurfaceFlingerAccess(bool usePermissionCache = true) + EXCLUDES(mStateLock); + + // IBinder overrides: status_t onTransact(uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags) override; status_t dump(int fd, const Vector<String16>& args) override { return priorityDump(fd, args); } - bool callingThreadHasUnscopedSurfaceFlingerAccess(bool usePermissionCache = true) - EXCLUDES(mStateLock); - // Implements ISurfaceComposer - sp<IBinder> createDisplay(const String8& displayName, bool secure, - float requestedRefreshRate = 0.0f); + // ISurfaceComposer implementation: + sp<IBinder> createDisplay(const String8& displayName, bool isSecure, + const std::string& uniqueId, float requestedRefreshRate = 0.0f); void destroyDisplay(const sp<IBinder>& displayToken); std::vector<PhysicalDisplayId> getPhysicalDisplayIds() const EXCLUDES(mStateLock) { Mutex::Autolock lock(mStateLock); @@ -667,7 +668,7 @@ private: void updateHdcpLevels(hal::HWDisplayId hwcDisplayId, int32_t connectedLevel, int32_t maxLevel); - // Implements IBinder::DeathRecipient. + // IBinder::DeathRecipient overrides: void binderDied(const wp<IBinder>& who) override; // HWC2::ComposerCallback overrides: @@ -1559,7 +1560,7 @@ private: class SurfaceComposerAIDL : public gui::BnSurfaceComposer { public: - SurfaceComposerAIDL(sp<SurfaceFlinger> sf) : mFlinger(std::move(sf)) {} + explicit SurfaceComposerAIDL(sp<SurfaceFlinger> sf) : mFlinger(std::move(sf)) {} binder::Status bootFinished() override; binder::Status createDisplayEventConnection( @@ -1567,8 +1568,9 @@ 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 secure, - float requestedRefreshRate, sp<IBinder>* outDisplay) 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 getPhysicalDisplayIds(std::vector<int64_t>* outDisplayIds) override; binder::Status getPhysicalDisplayToken(int64_t displayId, sp<IBinder>* outDisplay) override; @@ -1690,7 +1692,7 @@ private: gui::DynamicDisplayInfo*& outInfo); private: - sp<SurfaceFlinger> mFlinger; + const sp<SurfaceFlinger> mFlinger; }; } // namespace android diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp index 28162f4d6b..bf5ae21f60 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp @@ -132,6 +132,36 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForSecureDisplay) { EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); } +TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForUniqueId) { + const String8 name("virtual.test"); + const std::string uniqueId = "virtual:package:id"; + + // -------------------------------------------------------------------- + // Call Expectations + + // -------------------------------------------------------------------- + // Invocation + + sp<IBinder> displayToken = mFlinger.createDisplay(name, false, uniqueId); + + // -------------------------------------------------------------------- + // Postconditions + + // The display should have been added to the current state + ASSERT_TRUE(hasCurrentDisplayState(displayToken)); + const auto& display = getCurrentDisplayState(displayToken); + EXPECT_TRUE(display.isVirtual()); + EXPECT_FALSE(display.isSecure); + EXPECT_EQ(display.uniqueId, "virtual:package:id"); + EXPECT_EQ(name.c_str(), display.displayName); + + // -------------------------------------------------------------------- + // Cleanup conditions + + // Creating the display commits a display transaction. + EXPECT_CALL(*mFlinger.scheduler(), scheduleFrame()).Times(1); +} + // 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"); diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index a0c13720c0..b251e2cf0c 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -420,8 +420,15 @@ public: commit(kComposite); } - auto createDisplay(const String8& displayName, bool secure, float requestedRefreshRate = 0.0f) { - return mFlinger->createDisplay(displayName, secure, requestedRefreshRate); + 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 createDisplay(const String8& displayName, bool isSecure, const std::string& uniqueId, + float requestedRefreshRate = 0.0f) { + return mFlinger->createDisplay(displayName, isSecure, uniqueId, requestedRefreshRate); } auto destroyDisplay(const sp<IBinder>& displayToken) { |