SF: Propagate uniqueID when creating virtual displays
This is part of Trunk Stable effort to upstream SF-ARC screen record
capabiliy (undiverging http://ag/20003031) which requires mirroring
virtual displays to be associated with screen capture sessions using
the package name within the unique ID.
Creating virtual display with unique ID specified is optional such that
it doesn't affect existing consumers who don't need it (i.e. av).
Bug: 137375833
Bug: 194863377
Test: libsurfaceflinger_unittest
Change-Id: Ia3cd13df07f701593ddc94c196df0b04844cf502
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 4f1356b..07664ca 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -89,6 +89,8 @@
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);
@@ -1276,14 +1278,13 @@
}
// ---------------------------------------------------------------------------
-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 51e0193..cf6752f 100644
--- a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl
+++ b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl
@@ -73,7 +73,7 @@
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
@@ -90,12 +90,14 @@
@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
@@ -104,11 +106,11 @@
*
* 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 2888826..82090fa 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 @@
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 577d239..8a7ecab 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -673,8 +673,8 @@
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 edd57cc..543a16a 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -329,6 +329,7 @@
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 3ec5f21..6b117b8 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -577,13 +577,13 @@
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;
}
@@ -607,11 +607,12 @@
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;
@@ -9530,7 +9531,8 @@
}
}
-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();
@@ -9538,7 +9540,7 @@
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();
}
@@ -9555,10 +9557,10 @@
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 c106abd..4c53fae 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -536,15 +536,16 @@
static const size_t MAX_LAYERS = 4096;
- // Implements IBinder.
- 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)
+ static bool callingThreadHasUnscopedSurfaceFlingerAccess(bool usePermissionCache = true)
EXCLUDES(mStateLock);
- // Implements ISurfaceComposer
- sp<IBinder> createDisplay(const String8& displayName, bool secure,
- float requestedRefreshRate = 0.0f);
+ // 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); }
+
+ // 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);
@@ -665,7 +666,7 @@
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:
@@ -1543,7 +1544,7 @@
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(
@@ -1551,8 +1552,9 @@
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;
@@ -1675,7 +1677,7 @@
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 28162f4..bf5ae21 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp
@@ -132,6 +132,36 @@
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 82023b0..3b03f92 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -411,8 +411,15 @@
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) {