Merge "Remove rotation and use flag useIdentityTransform for screenshots."
diff --git a/cmds/servicemanager/ServiceManager.cpp b/cmds/servicemanager/ServiceManager.cpp
index 1f9892a..e80c321 100644
--- a/cmds/servicemanager/ServiceManager.cpp
+++ b/cmds/servicemanager/ServiceManager.cpp
@@ -213,17 +213,18 @@
return Status::fromExceptionCode(Status::EX_ILLEGAL_STATE);
}
- auto entry = mNameToService.emplace(name, Service {
+ // Overwrite the old service if it exists
+ mNameToService[name] = Service {
.binder = binder,
.allowIsolated = allowIsolated,
.dumpPriority = dumpPriority,
.debugPid = ctx.debugPid,
- });
+ };
auto it = mNameToRegistrationCallback.find(name);
if (it != mNameToRegistrationCallback.end()) {
for (const sp<IServiceCallback>& cb : it->second) {
- entry.first->second.guaranteeClient = true;
+ mNameToService[name].guaranteeClient = true;
// permission checked in registerForNotifications
cb->onRegistration(name, binder);
}
diff --git a/cmds/servicemanager/test_sm.cpp b/cmds/servicemanager/test_sm.cpp
index 25245be..fb9f9df 100644
--- a/cmds/servicemanager/test_sm.cpp
+++ b/cmds/servicemanager/test_sm.cpp
@@ -135,6 +135,26 @@
IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
}
+TEST(AddService, OverwriteExistingService) {
+ auto sm = getPermissiveServiceManager();
+ sp<IBinder> serviceA = getBinder();
+ EXPECT_TRUE(sm->addService("foo", serviceA, false /*allowIsolated*/,
+ IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
+
+ sp<IBinder> outA;
+ EXPECT_TRUE(sm->getService("foo", &outA).isOk());
+ EXPECT_EQ(serviceA, outA);
+
+ // serviceA should be overwritten by serviceB
+ sp<IBinder> serviceB = getBinder();
+ EXPECT_TRUE(sm->addService("foo", serviceB, false /*allowIsolated*/,
+ IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
+
+ sp<IBinder> outB;
+ EXPECT_TRUE(sm->getService("foo", &outB).isOk());
+ EXPECT_EQ(serviceB, outB);
+}
+
TEST(AddService, NoPermissions) {
std::unique_ptr<MockAccess> access = std::make_unique<NiceMock<MockAccess>>();
diff --git a/libs/binder/include/binder/Nullable.h b/libs/binder/include/binder/Nullable.h
deleted file mode 100644
index a98583d..0000000
--- a/libs/binder/include/binder/Nullable.h
+++ /dev/null
@@ -1,44 +0,0 @@
-/*
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-#pragma once
-
-#include <optional>
-#include <utility>
-
-namespace android {
-
-namespace aidl {
-
-// nullable/make_nullable provide source-level compatibility between std::opional and std::unique_ptr
-// usage:
-// nullable<Foo> a;
-// nullable<Foo> b = make_nullable<Foo>(...);
-// auto c = make_nullable<Foo>(...);
-// c.reset();
-// c = make_nullable<Foo>(...);
-// c = std::move(a);
-
-template <typename T>
-using nullable = std::optional<T>;
-
-template <typename T, typename... Args>
-inline nullable<T> make_nullable(Args&&... args) {
- return std::make_optional<T>(std::forward<Args>(args)...);
-}
-
-} // namespace aidl
-
-} // namespace android
\ No newline at end of file
diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp
index d287290..7d9fd51 100644
--- a/libs/binder/ndk/ibinder.cpp
+++ b/libs/binder/ndk/ibinder.cpp
@@ -168,7 +168,7 @@
binder_status_t status = getClass()->onTransact(this, code, &in, &out);
return PruneStatusT(status);
- } else if (code == SHELL_COMMAND_TRANSACTION) {
+ } else if (code == SHELL_COMMAND_TRANSACTION && getClass()->handleShellCommand != nullptr) {
int in = data.readFileDescriptor();
int out = data.readFileDescriptor();
int err = data.readFileDescriptor();
diff --git a/libs/binder/ndk/ibinder_internal.h b/libs/binder/ndk/ibinder_internal.h
index 5779427..902fe79 100644
--- a/libs/binder/ndk/ibinder_internal.h
+++ b/libs/binder/ndk/ibinder_internal.h
@@ -110,13 +110,13 @@
const ::android::String16& getInterfaceDescriptor() const { return mInterfaceDescriptor; }
// required to be non-null, implemented for every class
- const AIBinder_Class_onCreate onCreate;
- const AIBinder_Class_onDestroy onDestroy;
- const AIBinder_Class_onTransact onTransact;
+ const AIBinder_Class_onCreate onCreate = nullptr;
+ const AIBinder_Class_onDestroy onDestroy = nullptr;
+ const AIBinder_Class_onTransact onTransact = nullptr;
// optional methods for a class
- AIBinder_onDump onDump;
- AIBinder_handleShellCommand handleShellCommand;
+ AIBinder_onDump onDump = nullptr;
+ AIBinder_handleShellCommand handleShellCommand = nullptr;
private:
// This must be a String16 since BBinder virtual getInterfaceDescriptor returns a reference to
diff --git a/libs/binder/ndk/tests/iface.cpp b/libs/binder/ndk/tests/iface.cpp
index 64832f3..a588985 100644
--- a/libs/binder/ndk/tests/iface.cpp
+++ b/libs/binder/ndk/tests/iface.cpp
@@ -118,7 +118,7 @@
AIBinder_Weak_delete(mWeakBinder);
}
-binder_status_t IFoo::addService(const char* instance) {
+AIBinder* IFoo::getBinder() {
AIBinder* binder = nullptr;
if (mWeakBinder != nullptr) {
@@ -132,8 +132,18 @@
AIBinder_Weak_delete(mWeakBinder);
}
mWeakBinder = AIBinder_Weak_new(binder);
+
+ // WARNING: it is important that this class does not implement debug or
+ // shell functions because it does not use special C++ wrapper
+ // functions, and so this is how we test those functions.
}
+ return binder;
+}
+
+binder_status_t IFoo::addService(const char* instance) {
+ AIBinder* binder = getBinder();
+
binder_status_t status = AServiceManager_addService(binder, instance);
// Strong references we care about kept by remote process
AIBinder_decStrong(binder);
diff --git a/libs/binder/ndk/tests/include/iface/iface.h b/libs/binder/ndk/tests/include/iface/iface.h
index cdf5493..d9dd64b 100644
--- a/libs/binder/ndk/tests/include/iface/iface.h
+++ b/libs/binder/ndk/tests/include/iface/iface.h
@@ -30,6 +30,9 @@
static AIBinder_Class* kClass;
+ // binder representing this interface with one reference count
+ AIBinder* getBinder();
+
// Takes ownership of IFoo
binder_status_t addService(const char* instance);
static ::android::sp<IFoo> getService(const char* instance, AIBinder** outBinder = nullptr);
diff --git a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
index 332b4ca..1424b6c 100644
--- a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
+++ b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp
@@ -165,11 +165,10 @@
return 1; // should not return
}
-// This is too slow
-// TEST(NdkBinder, GetServiceThatDoesntExist) {
-// sp<IFoo> foo = IFoo::getService("asdfghkl;");
-// EXPECT_EQ(nullptr, foo.get());
-// }
+TEST(NdkBinder, GetServiceThatDoesntExist) {
+ sp<IFoo> foo = IFoo::getService("asdfghkl;");
+ EXPECT_EQ(nullptr, foo.get());
+}
TEST(NdkBinder, CheckServiceThatDoesntExist) {
AIBinder* binder = AServiceManager_checkService("asdfghkl;");
@@ -184,6 +183,26 @@
AIBinder_decStrong(binder);
}
+TEST(NdkBinder, UnimplementedDump) {
+ sp<IFoo> foo = IFoo::getService(IFoo::kSomeInstanceName);
+ ASSERT_NE(foo, nullptr);
+ AIBinder* binder = foo->getBinder();
+ EXPECT_EQ(OK, AIBinder_dump(binder, STDOUT_FILENO, nullptr, 0));
+ AIBinder_decStrong(binder);
+}
+
+TEST(NdkBinder, UnimplementedShell) {
+ // libbinder_ndk doesn't support calling shell, so we are calling from the
+ // libbinder across processes to the NDK service which doesn't implement
+ // shell
+ static const sp<android::IServiceManager> sm(android::defaultServiceManager());
+ sp<IBinder> testService = sm->getService(String16(IFoo::kSomeInstanceName));
+
+ Vector<String16> argsVec;
+ EXPECT_EQ(OK, IBinder::shellCommand(testService, STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO,
+ argsVec, nullptr, nullptr));
+}
+
TEST(NdkBinder, DoubleNumber) {
sp<IFoo> foo = IFoo::getService(IFoo::kSomeInstanceName);
ASSERT_NE(foo, nullptr);
diff --git a/libs/gui/BitTube.cpp b/libs/gui/BitTube.cpp
index ef7a6f5..351af65 100644
--- a/libs/gui/BitTube.cpp
+++ b/libs/gui/BitTube.cpp
@@ -86,6 +86,10 @@
mReceiveFd = std::move(receiveFd);
}
+void BitTube::setSendFd(base::unique_fd&& sendFd) {
+ mSendFd = std::move(sendFd);
+}
+
ssize_t BitTube::write(void const* vaddr, size_t size) {
ssize_t err, len;
do {
@@ -115,6 +119,11 @@
status_t result = reply->writeDupFileDescriptor(mReceiveFd);
mReceiveFd.reset();
+ if (result != NO_ERROR) {
+ return result;
+ }
+ result = reply->writeDupFileDescriptor(mSendFd);
+ mSendFd.reset();
return result;
}
@@ -126,6 +135,13 @@
ALOGE("BitTube::readFromParcel: can't dup file descriptor (%s)", strerror(error));
return -error;
}
+ mSendFd.reset(dup(parcel->readFileDescriptor()));
+ if (mSendFd < 0) {
+ mSendFd.reset();
+ int error = errno;
+ ALOGE("BitTube::readFromParcel: can't dup file descriptor (%s)", strerror(error));
+ return -error;
+ }
return NO_ERROR;
}
diff --git a/libs/gui/DisplayEventDispatcher.cpp b/libs/gui/DisplayEventDispatcher.cpp
index 51fbb97..2cc7c34 100644
--- a/libs/gui/DisplayEventDispatcher.cpp
+++ b/libs/gui/DisplayEventDispatcher.cpp
@@ -89,12 +89,8 @@
return OK;
}
-void DisplayEventDispatcher::requestLatestConfig() {
- status_t status = mReceiver.requestLatestConfig();
- if (status) {
- ALOGW("Failed enable config events, status=%d", status);
- return;
- }
+void DisplayEventDispatcher::injectEvent(const DisplayEventReceiver::Event& event) {
+ mReceiver.sendEvents(&event, 1);
}
int DisplayEventDispatcher::getFd() const {
@@ -156,6 +152,9 @@
dispatchConfigChanged(ev.header.timestamp, ev.header.displayId,
ev.config.configId, ev.config.vsyncPeriod);
break;
+ case DisplayEventReceiver::DISPLAY_EVENT_NULL:
+ dispatchNullEvent(ev.header.timestamp, ev.header.displayId);
+ break;
default:
ALOGW("dispatcher %p ~ ignoring unknown event type %#x", this, ev.header.type);
break;
@@ -167,4 +166,5 @@
}
return gotVsync;
}
+
} // namespace android
diff --git a/libs/gui/DisplayEventReceiver.cpp b/libs/gui/DisplayEventReceiver.cpp
index 1fed509..f2b0962 100644
--- a/libs/gui/DisplayEventReceiver.cpp
+++ b/libs/gui/DisplayEventReceiver.cpp
@@ -79,14 +79,6 @@
return NO_INIT;
}
-status_t DisplayEventReceiver::requestLatestConfig() {
- if (mEventConnection != nullptr) {
- mEventConnection->requestLatestConfig();
- return NO_ERROR;
- }
- return NO_INIT;
-}
-
ssize_t DisplayEventReceiver::getEvents(DisplayEventReceiver::Event* events,
size_t count) {
return DisplayEventReceiver::getEvents(mDataChannel.get(), events, count);
@@ -98,6 +90,10 @@
return gui::BitTube::recvObjects(dataChannel, events, count);
}
+ssize_t DisplayEventReceiver::sendEvents(Event const* events, size_t count) {
+ return DisplayEventReceiver::sendEvents(mDataChannel.get(), events, count);
+}
+
ssize_t DisplayEventReceiver::sendEvents(gui::BitTube* dataChannel,
Event const* events, size_t count)
{
diff --git a/libs/gui/IDisplayEventConnection.cpp b/libs/gui/IDisplayEventConnection.cpp
index aa74bfd..c0e246f 100644
--- a/libs/gui/IDisplayEventConnection.cpp
+++ b/libs/gui/IDisplayEventConnection.cpp
@@ -26,8 +26,7 @@
STEAL_RECEIVE_CHANNEL = IBinder::FIRST_CALL_TRANSACTION,
SET_VSYNC_RATE,
REQUEST_NEXT_VSYNC,
- REQUEST_LATEST_CONFIG,
- LAST = REQUEST_LATEST_CONFIG,
+ LAST = REQUEST_NEXT_VSYNC,
};
} // Anonymous namespace
@@ -54,11 +53,6 @@
callRemoteAsync<decltype(&IDisplayEventConnection::requestNextVsync)>(
Tag::REQUEST_NEXT_VSYNC);
}
-
- void requestLatestConfig() override {
- callRemoteAsync<decltype(&IDisplayEventConnection::requestLatestConfig)>(
- Tag::REQUEST_LATEST_CONFIG);
- }
};
// Out-of-line virtual method definition to trigger vtable emission in this translation unit (see
@@ -80,8 +74,6 @@
return callLocal(data, reply, &IDisplayEventConnection::setVsyncRate);
case Tag::REQUEST_NEXT_VSYNC:
return callLocalAsync(data, reply, &IDisplayEventConnection::requestNextVsync);
- case Tag::REQUEST_LATEST_CONFIG:
- return callLocalAsync(data, reply, &IDisplayEventConnection::requestLatestConfig);
}
}
diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index 9c0ad9d..7742503 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -509,7 +509,8 @@
status_t status = output.writeInt32(static_cast<int32_t>(pixelFormat)) ?:
output.write(sourceCrop) ?:
output.writeFloat(frameScale) ?:
- output.writeBool(captureSecureLayers);
+ output.writeBool(captureSecureLayers) ?:
+ output.writeInt32(uid);
return status;
}
@@ -518,7 +519,8 @@
status_t status = input.readInt32(&format) ?:
input.read(sourceCrop) ?:
input.readFloat(&frameScale) ?:
- input.readBool(&captureSecureLayers);
+ input.readBool(&captureSecureLayers) ?:
+ input.readInt32(&uid);
pixelFormat = static_cast<ui::PixelFormat>(format);
return status;
diff --git a/libs/gui/include/gui/DisplayEventDispatcher.h b/libs/gui/include/gui/DisplayEventDispatcher.h
index f210c34..eb5b004 100644
--- a/libs/gui/include/gui/DisplayEventDispatcher.h
+++ b/libs/gui/include/gui/DisplayEventDispatcher.h
@@ -31,7 +31,7 @@
status_t initialize();
void dispose();
status_t scheduleVsync();
- void requestLatestConfig();
+ void injectEvent(const DisplayEventReceiver::Event& event);
int getFd() const;
virtual int handleEvent(int receiveFd, int events, void* data);
@@ -48,6 +48,9 @@
bool connected) = 0;
virtual void dispatchConfigChanged(nsecs_t timestamp, PhysicalDisplayId displayId,
int32_t configId, nsecs_t vsyncPeriod) = 0;
+ // AChoreographer-specific hook for processing null-events so that looper
+ // can be properly poked.
+ virtual void dispatchNullEvent(nsecs_t timestamp, PhysicalDisplayId displayId) = 0;
bool processPendingEvents(nsecs_t* outTimestamp, PhysicalDisplayId* outDisplayId,
uint32_t* outCount);
diff --git a/libs/gui/include/gui/DisplayEventReceiver.h b/libs/gui/include/gui/DisplayEventReceiver.h
index 8d49184..0e10d1a 100644
--- a/libs/gui/include/gui/DisplayEventReceiver.h
+++ b/libs/gui/include/gui/DisplayEventReceiver.h
@@ -53,6 +53,7 @@
DISPLAY_EVENT_VSYNC = fourcc('v', 's', 'y', 'n'),
DISPLAY_EVENT_HOTPLUG = fourcc('p', 'l', 'u', 'g'),
DISPLAY_EVENT_CONFIG_CHANGED = fourcc('c', 'o', 'n', 'f'),
+ DISPLAY_EVENT_NULL = fourcc('n', 'u', 'l', 'l'),
};
struct Event {
@@ -130,6 +131,7 @@
* sendEvents write events to the queue and returns how many events were
* written.
*/
+ ssize_t sendEvents(Event const* events, size_t count);
static ssize_t sendEvents(gui::BitTube* dataChannel, Event const* events, size_t count);
/*
@@ -146,12 +148,6 @@
*/
status_t requestNextVsync();
- /*
- * requestLatestConfig() force-requests the current config for the primary
- * display.
- */
- status_t requestLatestConfig();
-
private:
sp<IDisplayEventConnection> mEventConnection;
std::unique_ptr<gui::BitTube> mDataChannel;
diff --git a/libs/gui/include/gui/IDisplayEventConnection.h b/libs/gui/include/gui/IDisplayEventConnection.h
index 674aafd..cff22a3 100644
--- a/libs/gui/include/gui/IDisplayEventConnection.h
+++ b/libs/gui/include/gui/IDisplayEventConnection.h
@@ -51,11 +51,6 @@
* requestNextVsync() schedules the next vsync event. It has no effect if the vsync rate is > 0.
*/
virtual void requestNextVsync() = 0; // Asynchronous
-
- /*
- * requestLatestConfig() requests the config for the primary display.
- */
- virtual void requestLatestConfig() = 0; // Asynchronous
};
class BnDisplayEventConnection : public SafeBnInterface<IDisplayEventConnection> {
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index bbf827f..6a304ef 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -314,12 +314,14 @@
bool ValidateFrameRate(float frameRate, int8_t compatibility, const char* functionName);
struct CaptureArgs {
+ const static int32_t UNSET_UID = -1;
virtual ~CaptureArgs() = default;
ui::PixelFormat pixelFormat{ui::PixelFormat::RGBA_8888};
Rect sourceCrop;
float frameScale{1};
bool captureSecureLayers{false};
+ int32_t uid{UNSET_UID};
virtual status_t write(Parcel& output) const;
virtual status_t read(const Parcel& input);
diff --git a/libs/gui/include/private/gui/BitTube.h b/libs/gui/include/private/gui/BitTube.h
index 13c0162..8048518 100644
--- a/libs/gui/include/private/gui/BitTube.h
+++ b/libs/gui/include/private/gui/BitTube.h
@@ -58,6 +58,9 @@
// resets this BitTube's receive file descriptor to receiveFd
void setReceiveFd(base::unique_fd&& receiveFd);
+ // resets this BitTube's send file descriptor to sendFd
+ void setSendFd(base::unique_fd&& sendFd);
+
// send objects (sized blobs). All objects are guaranteed to be written or the call fails.
template <typename T>
static ssize_t sendObjects(BitTube* tube, T const* events, size_t count) {
@@ -85,7 +88,7 @@
// the message, excess data is silently discarded.
ssize_t read(void* vaddr, size_t size);
- base::unique_fd mSendFd;
+ mutable base::unique_fd mSendFd;
mutable base::unique_fd mReceiveFd;
static ssize_t sendObjects(BitTube* tube, void const* events, size_t count, size_t objSize);
diff --git a/libs/nativedisplay/AChoreographer.cpp b/libs/nativedisplay/AChoreographer.cpp
index ebc8909..ff1b5e6 100644
--- a/libs/nativedisplay/AChoreographer.cpp
+++ b/libs/nativedisplay/AChoreographer.cpp
@@ -136,6 +136,7 @@
void dispatchHotplug(nsecs_t timestamp, PhysicalDisplayId displayId, bool connected) override;
void dispatchConfigChanged(nsecs_t timestamp, PhysicalDisplayId displayId, int32_t configId,
nsecs_t vsyncPeriod) override;
+ void dispatchNullEvent(nsecs_t, PhysicalDisplayId) override;
void scheduleCallbacks();
@@ -170,7 +171,7 @@
Choreographer::Choreographer(const sp<Looper>& looper)
: DisplayEventDispatcher(looper, ISurfaceComposer::VsyncSource::eVsyncSourceApp,
- ISurfaceComposer::ConfigChanged::eConfigChangedDispatch),
+ ISurfaceComposer::ConfigChanged::eConfigChangedSuppress),
mLooper(looper),
mThreadId(std::this_thread::get_id()) {
std::lock_guard<std::mutex> _l(gChoreographers.lock);
@@ -294,8 +295,14 @@
} else {
// If the looper thread is detached from Choreographer, then refresh rate
// changes will be handled in AChoreographer_handlePendingEvents, so we
- // need to redispatch a config from SF
- requestLatestConfig();
+ // need to wake up the looper thread by writing to the write-end of the
+ // socket the looper is listening on.
+ // Fortunately, these events are small so sending packets across the
+ // socket should be atomic across processes.
+ DisplayEventReceiver::Event event;
+ event.header = DisplayEventReceiver::Event::Header{DisplayEventReceiver::DISPLAY_EVENT_NULL,
+ PhysicalDisplayId(0), systemTime()};
+ injectEvent(event);
}
}
@@ -373,27 +380,14 @@
// displays. When multi-display choreographer is properly supported, then
// PhysicalDisplayId should no longer be ignored.
void Choreographer::dispatchConfigChanged(nsecs_t, PhysicalDisplayId displayId, int32_t configId,
- nsecs_t vsyncPeriod) {
+ nsecs_t) {
ALOGV("choreographer %p ~ received config change event (displayId=%s, configId=%d).",
this, to_string(displayId).c_str(), configId);
+}
- const nsecs_t lastPeriod = mLatestVsyncPeriod;
- std::vector<RefreshRateCallback> callbacks{};
- {
- std::lock_guard<std::mutex> _l{mLock};
- for (auto& cb : mRefreshRateCallbacks) {
- callbacks.push_back(cb);
- cb.firstCallbackFired = true;
- }
- }
-
- for (auto& cb : callbacks) {
- if (!cb.firstCallbackFired || (vsyncPeriod > 0 && vsyncPeriod != lastPeriod)) {
- cb.callback(vsyncPeriod, cb.data);
- }
- }
-
- mLatestVsyncPeriod = vsyncPeriod;
+void Choreographer::dispatchNullEvent(nsecs_t, PhysicalDisplayId) {
+ ALOGV("choreographer %p ~ received null event.", this);
+ handleRefreshRateUpdates();
}
void Choreographer::handleMessage(const Message& message) {
diff --git a/libs/renderengine/OWNERS b/libs/renderengine/OWNERS
index c00fbba..b44456b 100644
--- a/libs/renderengine/OWNERS
+++ b/libs/renderengine/OWNERS
@@ -1,2 +1,4 @@
+alecmouri@google.com
+jreck@google.com
lpy@google.com
stoza@google.com
diff --git a/libs/ui/OWNERS b/libs/ui/OWNERS
index 203a739..b1317b1 100644
--- a/libs/ui/OWNERS
+++ b/libs/ui/OWNERS
@@ -1,6 +1,7 @@
+alecmouri@google.com
chrisforbes@google.com
+jreck@google.com
lpy@google.com
mathias@google.com
romainguy@google.com
stoza@google.com
-vhau@google.com
diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp
index 5ba5ad8..44f26b0 100644
--- a/services/inputflinger/dispatcher/InputDispatcher.cpp
+++ b/services/inputflinger/dispatcher/InputDispatcher.cpp
@@ -3947,7 +3947,7 @@
}
mFocusedDisplayId = displayId;
- // Sanity check
+ // Find new focused window and validate
sp<InputWindowHandle> newFocusedWindowHandle =
getValueByKey(mFocusedWindowHandlesByDisplay, displayId);
notifyFocusChangedLocked(oldFocusedWindowHandle, newFocusedWindowHandle);
diff --git a/services/inputflinger/tests/BlockingQueue_test.cpp b/services/inputflinger/tests/BlockingQueue_test.cpp
index 0dea8d7..fd9d9d5 100644
--- a/services/inputflinger/tests/BlockingQueue_test.cpp
+++ b/services/inputflinger/tests/BlockingQueue_test.cpp
@@ -26,7 +26,7 @@
// --- BlockingQueueTest ---
/**
- * Sanity check of basic pop and push operation.
+ * Validate basic pop and push operation.
*/
TEST(BlockingQueueTest, Queue_AddAndRemove) {
constexpr size_t capacity = 10;
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 001eab7..eb33175 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -39,6 +39,7 @@
#include <gui/LayerDebugInfo.h>
#include <gui/Surface.h>
#include <math.h>
+#include <private/android_filesystem_config.h>
#include <renderengine/RenderEngine.h>
#include <stdint.h>
#include <stdlib.h>
@@ -139,6 +140,14 @@
mCallingPid = args.callingPid;
mCallingUid = args.callingUid;
+
+ if (mCallingUid == AID_GRAPHICS || mCallingUid == AID_SYSTEM) {
+ // If the system didn't send an ownerUid, use the callingUid for the ownerUid.
+ mOwnerUid = args.metadata.getInt32(METADATA_OWNER_UID, mCallingUid);
+ } else {
+ // A create layer request from a non system request cannot specify the owner uid
+ mOwnerUid = mCallingUid;
+ }
}
void Layer::onFirstRef() {
@@ -1665,8 +1674,8 @@
}
void Layer::dumpCallingUidPid(std::string& result) const {
- StringAppendF(&result, "Layer %s (%s) pid:%d uid:%d\n", getName().c_str(), getType(),
- mCallingPid, mCallingUid);
+ StringAppendF(&result, "Layer %s (%s) callingPid:%d callingUid:%d ownerUid:%d\n",
+ getName().c_str(), getType(), mCallingPid, mCallingUid, mOwnerUid);
}
void Layer::onDisconnect() {
@@ -2339,6 +2348,8 @@
}
layerInfo->set_is_relative_of(state.isRelativeOf);
+
+ layerInfo->set_owner_uid(mOwnerUid);
}
if (traceFlags & SurfaceTracing::TRACE_INPUT) {
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 99b1bb1..8d8ab6d 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -436,9 +436,7 @@
// Deprecated, please use compositionengine::Output::belongsInOutput()
// instead.
// TODO(lpique): Move the remaining callers (screencap) to the new function.
- bool belongsToDisplay(uint32_t layerStack, bool isPrimaryDisplay) const {
- return getLayerStack() == layerStack && (!mPrimaryDisplayOnly || isPrimaryDisplay);
- }
+ bool belongsToDisplay(uint32_t layerStack) const { return getLayerStack() == layerStack; }
FloatRect getBounds(const Region& activeTransparentRegion) const;
FloatRect getBounds() const;
@@ -967,6 +965,8 @@
*/
virtual bool needsInputInfo() const { return hasInputInfo(); }
+ uid_t getOwnerUid() { return mOwnerUid; }
+
protected:
compositionengine::OutputLayer* findOutputLayerForDisplay(const DisplayDevice*) const;
@@ -1089,6 +1089,10 @@
pid_t mCallingPid;
uid_t mCallingUid;
+ // The owner of the layer. If created from a non system process, it will be the calling uid.
+ // If created from a system process, the value can be passed in.
+ uid_t mOwnerUid;
+
// The current layer is a clone of mClonedFrom. This means that this layer will update it's
// properties based on mClonedFrom. When mClonedFrom latches a new buffer for BufferLayers,
// this layer will update it's buffer. When mClonedFrom updates it's drawing state, children,
diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp
index 255a1f2..0157a7f 100644
--- a/services/surfaceflinger/RegionSamplingThread.cpp
+++ b/services/surfaceflinger/RegionSamplingThread.cpp
@@ -429,7 +429,7 @@
bounds.top, bounds.right, bounds.bottom);
visitor(layer);
};
- mFlinger.traverseLayersInLayerStack(layerStack, filterVisitor);
+ mFlinger.traverseLayersInLayerStack(layerStack, CaptureArgs::UNSET_UID, filterVisitor);
};
sp<GraphicBuffer> buffer = nullptr;
diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp
index 2020e0c..846190c 100644
--- a/services/surfaceflinger/Scheduler/EventThread.cpp
+++ b/services/surfaceflinger/Scheduler/EventThread.cpp
@@ -137,6 +137,7 @@
status_t EventThreadConnection::stealReceiveChannel(gui::BitTube* outChannel) {
outChannel->setReceiveFd(mChannel.moveReceiveFd());
+ outChannel->setSendFd(base::unique_fd(dup(mChannel.getSendFd())));
return NO_ERROR;
}
@@ -150,11 +151,6 @@
mEventThread->requestNextVsync(this);
}
-void EventThreadConnection::requestLatestConfig() {
- ATRACE_NAME("requestLatestConfig");
- mEventThread->requestLatestConfig(this);
-}
-
status_t EventThreadConnection::postEvent(const DisplayEventReceiver::Event& event) {
ssize_t size = DisplayEventReceiver::sendEvents(&mChannel, &event, 1);
return size < 0 ? status_t(size) : status_t(NO_ERROR);
@@ -267,28 +263,6 @@
}
}
-void EventThread::requestLatestConfig(const sp<EventThreadConnection>& connection) {
- std::lock_guard<std::mutex> lock(mMutex);
- if (connection->mForcedConfigChangeDispatch) {
- return;
- }
- connection->mForcedConfigChangeDispatch = true;
- auto pendingConfigChange =
- std::find_if(std::begin(mPendingEvents), std::end(mPendingEvents),
- [&](const DisplayEventReceiver::Event& event) {
- return event.header.type ==
- DisplayEventReceiver::DISPLAY_EVENT_CONFIG_CHANGED;
- });
-
- // If we didn't find a pending config change event, then push out the
- // latest one we've ever seen.
- if (pendingConfigChange == std::end(mPendingEvents)) {
- mPendingEvents.push_back(mLastConfigChangeEvent);
- }
-
- mCondition.notify_all();
-}
-
void EventThread::onScreenReleased() {
std::lock_guard<std::mutex> lock(mMutex);
if (!mVSyncState || mVSyncState->synthetic) {
@@ -364,9 +338,6 @@
mInterceptVSyncsCallback(event->header.timestamp);
}
break;
- case DisplayEventReceiver::DISPLAY_EVENT_CONFIG_CHANGED:
- mLastConfigChangeEvent = *event;
- break;
}
}
@@ -379,10 +350,6 @@
vsyncRequested |= connection->vsyncRequest != VSyncRequest::None;
if (event && shouldConsumeEvent(*event, connection)) {
- if (event->header.type == DisplayEventReceiver::DISPLAY_EVENT_CONFIG_CHANGED &&
- connection->mForcedConfigChangeDispatch) {
- connection->mForcedConfigChangeDispatch = false;
- }
consumers.push_back(connection);
}
@@ -458,9 +425,7 @@
return true;
case DisplayEventReceiver::DISPLAY_EVENT_CONFIG_CHANGED: {
- const bool oneTimeDispatch = connection->mForcedConfigChangeDispatch;
- return oneTimeDispatch ||
- connection->mConfigChanged == ISurfaceComposer::eConfigChangedDispatch;
+ return connection->mConfigChanged == ISurfaceComposer::eConfigChangedDispatch;
}
case DisplayEventReceiver::DISPLAY_EVENT_VSYNC:
diff --git a/services/surfaceflinger/Scheduler/EventThread.h b/services/surfaceflinger/Scheduler/EventThread.h
index 64acbd7..49f624c 100644
--- a/services/surfaceflinger/Scheduler/EventThread.h
+++ b/services/surfaceflinger/Scheduler/EventThread.h
@@ -81,19 +81,13 @@
status_t stealReceiveChannel(gui::BitTube* outChannel) override;
status_t setVsyncRate(uint32_t rate) override;
void requestNextVsync() override; // asynchronous
- void requestLatestConfig() override; // asynchronous
// Called in response to requestNextVsync.
const ResyncCallback resyncCallback;
VSyncRequest vsyncRequest = VSyncRequest::None;
- ISurfaceComposer::ConfigChanged mConfigChanged =
+ const ISurfaceComposer::ConfigChanged mConfigChanged =
ISurfaceComposer::ConfigChanged::eConfigChangedSuppress;
- // Store whether we need to force dispatching a config change separately -
- // if mConfigChanged ever changes before the config change is dispatched
- // then we still need to propagate an initial config to the app if we
- // haven't already.
- bool mForcedConfigChangeDispatch = false;
private:
virtual void onFirstRef();
@@ -129,10 +123,6 @@
virtual void setVsyncRate(uint32_t rate, const sp<EventThreadConnection>& connection) = 0;
// Requests the next vsync. If resetIdleTimer is set to true, it resets the idle timer.
virtual void requestNextVsync(const sp<EventThreadConnection>& connection) = 0;
- // Dispatches the most recent configuration
- // Usage of this method assumes that only the primary internal display
- // supports multiple display configurations.
- virtual void requestLatestConfig(const sp<EventThreadConnection>& connection) = 0;
// Retrieves the number of event connections tracked by this EventThread.
virtual size_t getEventThreadConnectionCount() = 0;
@@ -153,7 +143,6 @@
status_t registerDisplayEventConnection(const sp<EventThreadConnection>& connection) override;
void setVsyncRate(uint32_t rate, const sp<EventThreadConnection>& connection) override;
void requestNextVsync(const sp<EventThreadConnection>& connection) override;
- void requestLatestConfig(const sp<EventThreadConnection>& connection) override;
// called before the screen is turned off from main thread
void onScreenReleased() override;
@@ -201,7 +190,6 @@
std::vector<wp<EventThreadConnection>> mDisplayEventConnections GUARDED_BY(mMutex);
std::deque<DisplayEventReceiver::Event> mPendingEvents GUARDED_BY(mMutex);
- DisplayEventReceiver::Event mLastConfigChangeEvent GUARDED_BY(mMutex);
// VSYNC state of connected display.
struct VSyncState {
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 9ff57df..ca9f629 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4958,11 +4958,13 @@
// special permissions.
case SET_FRAME_RATE:
case GET_DISPLAY_BRIGHTNESS_SUPPORT:
+ // captureLayers and captureDisplay will handle the permission check in the function
+ case CAPTURE_LAYERS:
+ case CAPTURE_DISPLAY:
case SET_DISPLAY_BRIGHTNESS: {
return OK;
}
- case CAPTURE_LAYERS:
- case CAPTURE_DISPLAY:
+
case ADD_REGION_SAMPLING_LISTENER:
case REMOVE_REGION_SAMPLING_LISTENER: {
// codes that require permission check
@@ -5434,10 +5436,33 @@
}
}
+static status_t validateScreenshotPermissions(const CaptureArgs& captureArgs) {
+ IPCThreadState* ipc = IPCThreadState::self();
+ const int pid = ipc->getCallingPid();
+ const int uid = ipc->getCallingUid();
+ if (uid == AID_GRAPHICS || PermissionCache::checkPermission(sReadFramebuffer, pid, uid)) {
+ return OK;
+ }
+
+ // If the caller doesn't have the correct permissions but is only attempting to screenshot
+ // itself, we allow it to continue.
+ if (captureArgs.uid == uid) {
+ return OK;
+ }
+
+ ALOGE("Permission Denial: can't take screenshot pid=%d, uid=%d", pid, uid);
+ return PERMISSION_DENIED;
+}
+
status_t SurfaceFlinger::captureDisplay(const DisplayCaptureArgs& args,
ScreenCaptureResults& captureResults) {
ATRACE_CALL();
+ status_t validate = validateScreenshotPermissions(args);
+ if (validate != OK) {
+ return validate;
+ }
+
if (!args.displayToken) return BAD_VALUE;
wp<DisplayDevice> displayWeak;
@@ -5466,8 +5491,8 @@
args.useIdentityTransform, args.captureSecureLayers);
});
- auto traverseLayers = [this, layerStack](const LayerVector::Visitor& visitor) {
- traverseLayersInLayerStack(layerStack, visitor);
+ auto traverseLayers = [this, args, layerStack](const LayerVector::Visitor& visitor) {
+ traverseLayersInLayerStack(layerStack, args.uid, visitor);
};
return captureScreenCommon(std::move(renderAreaFuture), traverseLayers, reqSize,
args.pixelFormat, captureResults);
@@ -5541,7 +5566,7 @@
});
auto traverseLayers = [this, layerStack](const LayerVector::Visitor& visitor) {
- traverseLayersInLayerStack(layerStack, visitor);
+ traverseLayersInLayerStack(layerStack, CaptureArgs::UNSET_UID, visitor);
};
return captureScreenCommon(std::move(renderAreaFuture), traverseLayers, size,
@@ -5552,6 +5577,11 @@
ScreenCaptureResults& captureResults) {
ATRACE_CALL();
+ status_t validate = validateScreenshotPermissions(args);
+ if (validate != OK) {
+ return validate;
+ }
+
ui::Size reqSize;
sp<Layer> parent;
Rect crop(args.sourceCrop);
@@ -5625,19 +5655,19 @@
}
bool childrenOnly = args.childrenOnly;
-
RenderAreaFuture renderAreaFuture = promise::defer([=]() -> std::unique_ptr<RenderArea> {
return std::make_unique<LayerRenderArea>(*this, parent, crop, reqSize, dataspace,
childrenOnly, displayViewport,
captureSecureLayers);
});
- auto traverseLayers = [parent, childrenOnly,
- &excludeLayers](const LayerVector::Visitor& visitor) {
+ auto traverseLayers = [parent, args, &excludeLayers](const LayerVector::Visitor& visitor) {
parent->traverseChildrenInZOrder(LayerVector::StateSet::Drawing, [&](Layer* layer) {
if (!layer->isVisible()) {
return;
- } else if (childrenOnly && layer == parent.get()) {
+ } else if (args.childrenOnly && layer == parent.get()) {
+ return;
+ } else if (args.uid != CaptureArgs::UNSET_UID && args.uid != layer->getOwnerUid()) {
return;
}
@@ -5859,22 +5889,25 @@
layersSortedByZ.traverseInReverseZOrder(stateSet, visitor);
}
-void SurfaceFlinger::traverseLayersInLayerStack(ui::LayerStack layerStack,
+void SurfaceFlinger::traverseLayersInLayerStack(ui::LayerStack layerStack, const int32_t uid,
const LayerVector::Visitor& visitor) {
// We loop through the first level of layers without traversing,
// as we need to determine which layers belong to the requested display.
for (const auto& layer : mDrawingState.layersSortedByZ) {
- if (!layer->belongsToDisplay(layerStack, false)) {
+ if (!layer->belongsToDisplay(layerStack)) {
continue;
}
// relative layers are traversed in Layer::traverseInZOrder
layer->traverseInZOrder(LayerVector::StateSet::Drawing, [&](Layer* layer) {
- if (!layer->belongsToDisplay(layerStack, false)) {
+ if (layer->getPrimaryDisplayOnly()) {
return;
}
if (!layer->isVisible()) {
return;
}
+ if (uid != CaptureArgs::UNSET_UID && layer->getOwnerUid() != uid) {
+ return;
+ }
visitor(layer);
});
}
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 2e85c1b..1acfda9 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -728,7 +728,9 @@
sp<DisplayDevice> getDisplayByIdOrLayerStack(uint64_t displayOrLayerStack) REQUIRES(mStateLock);
sp<DisplayDevice> getDisplayByLayerStack(uint64_t layerStack) REQUIRES(mStateLock);
- void traverseLayersInLayerStack(ui::LayerStack, const LayerVector::Visitor&);
+ // If the uid provided is not UNSET_UID, the traverse will skip any layers that don't have a
+ // matching ownerUid
+ void traverseLayersInLayerStack(ui::LayerStack, const int32_t uid, const LayerVector::Visitor&);
sp<StartPropertySetThread> mStartPropertySetThread;
diff --git a/services/surfaceflinger/layerproto/LayerProtoParser.cpp b/services/surfaceflinger/layerproto/LayerProtoParser.cpp
index 8fce0c9..aef670d 100644
--- a/services/surfaceflinger/layerproto/LayerProtoParser.cpp
+++ b/services/surfaceflinger/layerproto/LayerProtoParser.cpp
@@ -115,6 +115,7 @@
}
layer.cornerRadiusCrop = generateFloatRect(layerProto.corner_radius_crop());
layer.shadowRadius = layerProto.shadow_radius();
+ layer.ownerUid = layerProto.owner_uid();
return layer;
}
@@ -276,7 +277,7 @@
std::string LayerProtoParser::Layer::to_string() const {
std::string result;
- StringAppendF(&result, "+ %s (%s)\n", type.c_str(), name.c_str());
+ StringAppendF(&result, "+ %s (%s) uid=%d\n", type.c_str(), name.c_str(), ownerUid);
result.append(transparentRegion.to_string("TransparentRegion").c_str());
result.append(visibleRegion.to_string("VisibleRegion").c_str());
result.append(damageRegion.to_string("SurfaceDamageRegion").c_str());
diff --git a/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h b/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h
index 52b9165..c48354f 100644
--- a/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h
+++ b/services/surfaceflinger/layerproto/include/layerproto/LayerProtoParser.h
@@ -114,6 +114,7 @@
LayerMetadata metadata;
LayerProtoParser::FloatRect cornerRadiusCrop;
float shadowRadius;
+ uid_t ownerUid;
std::string to_string() const;
};
diff --git a/services/surfaceflinger/layerproto/layers.proto b/services/surfaceflinger/layerproto/layers.proto
index 8458d54..41d6d08 100644
--- a/services/surfaceflinger/layerproto/layers.proto
+++ b/services/surfaceflinger/layerproto/layers.proto
@@ -123,6 +123,8 @@
bool is_relative_of = 51;
// Layer's background blur radius in pixels.
int32 background_blur_radius = 52;
+
+ uint32 owner_uid = 53;
}
message PositionProto {
diff --git a/services/surfaceflinger/tests/LayerTransaction_test.cpp b/services/surfaceflinger/tests/LayerTransaction_test.cpp
index 0ef4150..8d715e1 100644
--- a/services/surfaceflinger/tests/LayerTransaction_test.cpp
+++ b/services/surfaceflinger/tests/LayerTransaction_test.cpp
@@ -18,7 +18,6 @@
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wconversion"
-#include <private/android_filesystem_config.h>
#include <thread>
#include "LayerTransactionTest.h"
@@ -26,40 +25,6 @@
using android::hardware::graphics::common::V1_1::BufferUsage;
-TEST_F(LayerTransactionTest, SetFlagsSecureEUidSystem) {
- sp<SurfaceControl> layer;
- ASSERT_NO_FATAL_FAILURE(layer = createLayer("test", 32, 32));
- ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(layer, Color::RED, 32, 32));
-
- sp<ISurfaceComposer> composer = ComposerService::getComposerService();
- Transaction()
- .setFlags(layer, layer_state_t::eLayerSecure, layer_state_t::eLayerSecure)
- .apply(true);
- ASSERT_EQ(PERMISSION_DENIED, composer->captureDisplay(mCaptureArgs, mCaptureResults));
-
- UIDFaker f(AID_SYSTEM);
-
- // By default the system can capture screenshots with secure layers but they
- // will be blacked out
- ASSERT_EQ(NO_ERROR, composer->captureDisplay(mCaptureArgs, mCaptureResults));
-
- {
- SCOPED_TRACE("as system");
- auto shot = screenshot();
- shot->expectColor(Rect(0, 0, 32, 32), Color::BLACK);
- }
-
- // Here we pass captureSecureLayers = true and since we are AID_SYSTEM we should be able
- // to receive them...we are expected to take care with the results.
- DisplayCaptureArgs args;
- args.displayToken = mDisplay;
- args.captureSecureLayers = true;
- ASSERT_EQ(NO_ERROR, composer->captureDisplay(args, mCaptureResults));
- ASSERT_EQ(true, mCaptureResults.capturedSecureLayers);
- ScreenCapture sc(mCaptureResults.buffer);
- sc.expectColor(Rect(0, 0, 32, 32), Color::RED);
-}
-
TEST_F(LayerTransactionTest, SetTransformToDisplayInverse_BufferState) {
sp<SurfaceControl> layer;
ASSERT_NO_FATAL_FAILURE(
diff --git a/services/surfaceflinger/tests/ScreenCapture_test.cpp b/services/surfaceflinger/tests/ScreenCapture_test.cpp
index 149e4d7..690f758 100644
--- a/services/surfaceflinger/tests/ScreenCapture_test.cpp
+++ b/services/surfaceflinger/tests/ScreenCapture_test.cpp
@@ -74,6 +74,42 @@
std::unique_ptr<ScreenCapture> mCapture;
};
+TEST_F(ScreenCaptureTest, SetFlagsSecureEUidSystem) {
+ sp<SurfaceControl> layer;
+ ASSERT_NO_FATAL_FAILURE(
+ layer = createLayer("test", 32, 32,
+ ISurfaceComposerClient::eSecure |
+ ISurfaceComposerClient::eFXSurfaceBufferQueue));
+ ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(layer, Color::RED, 32, 32));
+
+ Transaction().show(layer).setLayer(layer, INT32_MAX).apply(true);
+
+ sp<ISurfaceComposer> composer = ComposerService::getComposerService();
+ ASSERT_EQ(PERMISSION_DENIED, composer->captureDisplay(mCaptureArgs, mCaptureResults));
+
+ UIDFaker f(AID_SYSTEM);
+
+ // By default the system can capture screenshots with secure layers but they
+ // will be blacked out
+ ASSERT_EQ(NO_ERROR, composer->captureDisplay(mCaptureArgs, mCaptureResults));
+
+ {
+ SCOPED_TRACE("as system");
+ auto shot = screenshot();
+ shot->expectColor(Rect(0, 0, 32, 32), Color::BLACK);
+ }
+
+ // Here we pass captureSecureLayers = true and since we are AID_SYSTEM we should be able
+ // to receive them...we are expected to take care with the results.
+ DisplayCaptureArgs args;
+ args.displayToken = mDisplay;
+ args.captureSecureLayers = true;
+ ASSERT_EQ(NO_ERROR, composer->captureDisplay(args, mCaptureResults));
+ ASSERT_TRUE(mCaptureResults.capturedSecureLayers);
+ ScreenCapture sc(mCaptureResults.buffer);
+ sc.expectColor(Rect(0, 0, 32, 32), Color::RED);
+}
+
TEST_F(ScreenCaptureTest, CaptureSingleLayer) {
LayerCaptureArgs captureArgs;
captureArgs.layerHandle = mBGSurfaceControl->getHandle();
@@ -494,6 +530,128 @@
mCapture->expectColor(Rect(30, 30, 60, 60), Color::RED);
}
+TEST_F(ScreenCaptureTest, CaptureDisplayWithUid) {
+ uid_t fakeUid = 12345;
+
+ DisplayCaptureArgs captureArgs;
+ captureArgs.displayToken = mDisplay;
+
+ sp<SurfaceControl> layer;
+ ASSERT_NO_FATAL_FAILURE(layer = createLayer("test layer", 32, 32,
+ ISurfaceComposerClient::eFXSurfaceBufferQueue,
+ mBGSurfaceControl.get()));
+ ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(layer, Color::RED, 32, 32));
+
+ Transaction().show(layer).setLayer(layer, INT32_MAX).apply();
+
+ // Make sure red layer with the background layer is screenshot.
+ ScreenCapture::captureDisplay(&mCapture, captureArgs);
+ mCapture->expectColor(Rect(0, 0, 32, 32), Color::RED);
+ mCapture->expectBorder(Rect(0, 0, 32, 32), {63, 63, 195, 255});
+
+ // From non system uid, can't request screenshot without a specified uid.
+ UIDFaker f(fakeUid);
+ sp<ISurfaceComposer> composer = ComposerService::getComposerService();
+ ASSERT_EQ(PERMISSION_DENIED, composer->captureDisplay(captureArgs, mCaptureResults));
+
+ // Make screenshot request with current uid set. No layers were created with the current
+ // uid so screenshot will be black.
+ captureArgs.uid = fakeUid;
+ ScreenCapture::captureDisplay(&mCapture, captureArgs);
+ mCapture->expectColor(Rect(0, 0, 32, 32), Color::BLACK);
+ mCapture->expectBorder(Rect(0, 0, 32, 32), Color::BLACK);
+
+ sp<SurfaceControl> layerWithFakeUid;
+ // Create a new layer with the current uid
+ ASSERT_NO_FATAL_FAILURE(layerWithFakeUid =
+ createLayer("new test layer", 32, 32,
+ ISurfaceComposerClient::eFXSurfaceBufferQueue,
+ mBGSurfaceControl.get()));
+ ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(layerWithFakeUid, Color::GREEN, 32, 32));
+ Transaction()
+ .show(layerWithFakeUid)
+ .setLayer(layerWithFakeUid, INT32_MAX)
+ .setPosition(layerWithFakeUid, 128, 128)
+ .apply();
+
+ // Screenshot from the fakeUid caller with the uid requested allows the layer
+ // with that uid to be screenshotted. Everything else is black
+ ScreenCapture::captureDisplay(&mCapture, captureArgs);
+ mCapture->expectColor(Rect(128, 128, 160, 160), Color::GREEN);
+ mCapture->expectBorder(Rect(128, 128, 160, 160), Color::BLACK);
+}
+
+TEST_F(ScreenCaptureTest, CaptureLayerWithUid) {
+ uid_t fakeUid = 12345;
+
+ sp<SurfaceControl> layer;
+ ASSERT_NO_FATAL_FAILURE(layer = createLayer("test layer", 32, 32,
+ ISurfaceComposerClient::eFXSurfaceBufferQueue,
+ mBGSurfaceControl.get()));
+ ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(layer, Color::RED, 32, 32));
+
+ Transaction().show(layer).setLayer(layer, INT32_MAX).apply();
+
+ LayerCaptureArgs captureArgs;
+ captureArgs.layerHandle = mBGSurfaceControl->getHandle();
+ captureArgs.childrenOnly = false;
+
+ // Make sure red layer with the background layer is screenshot.
+ ScreenCapture::captureLayers(&mCapture, captureArgs);
+ mCapture->expectColor(Rect(0, 0, 32, 32), Color::RED);
+ mCapture->expectBorder(Rect(0, 0, 32, 32), {63, 63, 195, 255});
+
+ // From non system uid, can't request screenshot without a specified uid.
+ std::unique_ptr<UIDFaker> uidFaker = std::make_unique<UIDFaker>(fakeUid);
+
+ sp<ISurfaceComposer> composer = ComposerService::getComposerService();
+ ASSERT_EQ(PERMISSION_DENIED, composer->captureLayers(captureArgs, mCaptureResults));
+
+ // Make screenshot request with current uid set. No layers were created with the current
+ // uid so screenshot will be black.
+ captureArgs.uid = fakeUid;
+ ScreenCapture::captureLayers(&mCapture, captureArgs);
+ mCapture->expectColor(Rect(0, 0, 32, 32), Color::TRANSPARENT);
+ mCapture->expectBorder(Rect(0, 0, 32, 32), Color::TRANSPARENT);
+
+ sp<SurfaceControl> layerWithFakeUid;
+ // Create a new layer with the current uid
+ ASSERT_NO_FATAL_FAILURE(layerWithFakeUid =
+ createLayer("new test layer", 32, 32,
+ ISurfaceComposerClient::eFXSurfaceBufferQueue,
+ mBGSurfaceControl.get()));
+ ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(layerWithFakeUid, Color::GREEN, 32, 32));
+ Transaction()
+ .show(layerWithFakeUid)
+ .setLayer(layerWithFakeUid, INT32_MAX)
+ .setPosition(layerWithFakeUid, 128, 128)
+ // reparent a layer that was created with a different uid to the new layer.
+ .reparent(layer, layerWithFakeUid->getHandle())
+ .apply();
+
+ // Screenshot from the fakeUid caller with the uid requested allows the layer
+ // with that uid to be screenshotted. The child layer is skipped since it was created
+ // from a different uid.
+ ScreenCapture::captureLayers(&mCapture, captureArgs);
+ mCapture->expectColor(Rect(128, 128, 160, 160), Color::GREEN);
+ mCapture->expectBorder(Rect(128, 128, 160, 160), Color::TRANSPARENT);
+
+ // Clear fake calling uid so it's back to system.
+ uidFaker = nullptr;
+ // Screenshot from the test caller with the uid requested allows the layer
+ // with that uid to be screenshotted. The child layer is skipped since it was created
+ // from a different uid.
+ ScreenCapture::captureLayers(&mCapture, captureArgs);
+ mCapture->expectColor(Rect(128, 128, 160, 160), Color::GREEN);
+ mCapture->expectBorder(Rect(128, 128, 160, 160), Color::TRANSPARENT);
+
+ // Screenshot from the fakeUid caller with no uid requested allows everything to be screenshot.
+ captureArgs.uid = -1;
+ ScreenCapture::captureLayers(&mCapture, captureArgs);
+ mCapture->expectColor(Rect(128, 128, 160, 160), Color::RED);
+ mCapture->expectBorder(Rect(128, 128, 160, 160), {63, 63, 195, 255});
+}
+
// In the following tests we verify successful skipping of a parent layer,
// so we use the same verification logic and only change how we mutate
// the parent layer to verify that various properties are ignored.
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index 6472428..4843f05 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -240,7 +240,8 @@
ui::Dataspace::V0_SRGB, ui::Transform::ROT_0);
auto traverseLayers = [this](const LayerVector::Visitor& visitor) {
- return mFlinger.traverseLayersInLayerStack(mDisplay->getLayerStack(), visitor);
+ return mFlinger.traverseLayersInLayerStack(mDisplay->getLayerStack(),
+ CaptureArgs::UNSET_UID, visitor);
};
// TODO: Eliminate expensive/real allocation if possible.
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 345577d..b349144 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -342,9 +342,9 @@
outSyncFd, regionSampling, captureResults);
}
- auto traverseLayersInLayerStack(ui::LayerStack layerStack,
+ auto traverseLayersInLayerStack(ui::LayerStack layerStack, int32_t uid,
const LayerVector::Visitor& visitor) {
- return mFlinger->SurfaceFlinger::traverseLayersInLayerStack(layerStack, visitor);
+ return mFlinger->SurfaceFlinger::traverseLayersInLayerStack(layerStack, uid, visitor);
}
auto getDisplayNativePrimaries(const sp<IBinder>& displayToken,
diff --git a/services/surfaceflinger/tests/utils/ScreenshotUtils.h b/services/surfaceflinger/tests/utils/ScreenshotUtils.h
index 56628f8..081d18b 100644
--- a/services/surfaceflinger/tests/utils/ScreenshotUtils.h
+++ b/services/surfaceflinger/tests/utils/ScreenshotUtils.h
@@ -31,16 +31,20 @@
captureScreen(sc, SurfaceComposerClient::getInternalDisplayToken());
}
- static void captureScreen(std::unique_ptr<ScreenCapture>* sc, sp<IBinder> displayToken) {
+ static void captureDisplay(std::unique_ptr<ScreenCapture>* sc,
+ const DisplayCaptureArgs& captureArgs) {
const auto sf = ComposerService::getComposerService();
SurfaceComposerClient::Transaction().apply(true);
+ ScreenCaptureResults captureResults;
+ ASSERT_EQ(NO_ERROR, sf->captureDisplay(captureArgs, captureResults));
+ *sc = std::make_unique<ScreenCapture>(captureResults.buffer);
+ }
+
+ static void captureScreen(std::unique_ptr<ScreenCapture>* sc, sp<IBinder> displayToken) {
DisplayCaptureArgs args;
args.displayToken = displayToken;
-
- ScreenCaptureResults captureResults;
- ASSERT_EQ(NO_ERROR, sf->captureDisplay(args, captureResults));
- *sc = std::make_unique<ScreenCapture>(captureResults.buffer);
+ captureDisplay(sc, args);
}
static void captureLayers(std::unique_ptr<ScreenCapture>* sc,
diff --git a/services/vibratorservice/VibratorHalController.cpp b/services/vibratorservice/VibratorHalController.cpp
index 03e51ae..a9da74f 100644
--- a/services/vibratorservice/VibratorHalController.cpp
+++ b/services/vibratorservice/VibratorHalController.cpp
@@ -91,7 +91,7 @@
template <typename T>
HalResult<T> HalController::processHalResult(HalResult<T> result, const char* functionName) {
if (result.isFailed()) {
- ALOGE("%s failed: Vibrator HAL not available", functionName);
+ ALOGE("%s failed: %s", functionName, result.errorMessage());
std::lock_guard<std::mutex> lock(mConnectedHalMutex);
mConnectedHal->tryReconnect();
}
diff --git a/services/vibratorservice/VibratorHalWrapper.cpp b/services/vibratorservice/VibratorHalWrapper.cpp
index 1210110..ee891de 100644
--- a/services/vibratorservice/VibratorHalWrapper.cpp
+++ b/services/vibratorservice/VibratorHalWrapper.cpp
@@ -67,6 +67,10 @@
// -------------------------------------------------------------------------------------------------
+const constexpr char* STATUS_T_ERROR_MESSAGE_PREFIX = "status_t = ";
+const constexpr char* STATUS_V_1_0_ERROR_MESSAGE_PREFIX =
+ "android::hardware::vibrator::V1_0::Status = ";
+
template <typename T>
HalResult<T> HalResult<T>::fromStatus(binder::Status status, T data) {
if (status.exceptionCode() == binder::Status::EX_UNSUPPORTED_OPERATION) {
@@ -75,7 +79,7 @@
if (status.isOk()) {
return HalResult<T>::ok(data);
}
- return HalResult<T>::failed();
+ return HalResult<T>::failed(std::string(status.toString8().c_str()));
}
template <typename T>
@@ -86,24 +90,32 @@
case V1_0::Status::UNSUPPORTED_OPERATION:
return HalResult<T>::unsupported();
default:
- return HalResult<T>::failed();
+ return HalResult<T>::failed(STATUS_V_1_0_ERROR_MESSAGE_PREFIX + toString(status));
}
}
template <typename T>
template <typename R>
HalResult<T> HalResult<T>::fromReturn(hardware::Return<R>& ret, T data) {
- return ret.isOk() ? HalResult<T>::ok(data) : HalResult<T>::failed();
+ return ret.isOk() ? HalResult<T>::ok(data) : HalResult<T>::failed(ret.description());
}
template <typename T>
template <typename R>
HalResult<T> HalResult<T>::fromReturn(hardware::Return<R>& ret, V1_0::Status status, T data) {
- return ret.isOk() ? HalResult<T>::fromStatus(status, data) : HalResult<T>::failed();
+ return ret.isOk() ? HalResult<T>::fromStatus(status, data)
+ : HalResult<T>::failed(ret.description());
}
// -------------------------------------------------------------------------------------------------
+HalResult<void> HalResult<void>::fromStatus(status_t status) {
+ if (status == android::OK) {
+ return HalResult<void>::ok();
+ }
+ return HalResult<void>::failed(STATUS_T_ERROR_MESSAGE_PREFIX + statusToString(status));
+}
+
HalResult<void> HalResult<void>::fromStatus(binder::Status status) {
if (status.exceptionCode() == binder::Status::EX_UNSUPPORTED_OPERATION) {
return HalResult<void>::unsupported();
@@ -111,7 +123,7 @@
if (status.isOk()) {
return HalResult<void>::ok();
}
- return HalResult<void>::failed();
+ return HalResult<void>::failed(std::string(status.toString8().c_str()));
}
HalResult<void> HalResult<void>::fromStatus(V1_0::Status status) {
@@ -121,13 +133,13 @@
case V1_0::Status::UNSUPPORTED_OPERATION:
return HalResult<void>::unsupported();
default:
- return HalResult<void>::failed();
+ return HalResult<void>::failed(STATUS_V_1_0_ERROR_MESSAGE_PREFIX + toString(status));
}
}
template <typename R>
HalResult<void> HalResult<void>::fromReturn(hardware::Return<R>& ret) {
- return ret.isOk() ? HalResult<void>::ok() : HalResult<void>::failed();
+ return ret.isOk() ? HalResult<void>::ok() : HalResult<void>::failed(ret.description());
}
// -------------------------------------------------------------------------------------------------
@@ -149,8 +161,7 @@
// -------------------------------------------------------------------------------------------------
HalResult<void> AidlHalWrapper::ping() {
- // TODO(b/153415251): Investigate why IBinder::pingBinder() is returning false even on success.
- return getCapabilitiesInternal().isFailed() ? HalResult<void>::failed() : HalResult<void>::ok();
+ return HalResult<void>::fromStatus(IInterface::asBinder(getHal())->pingBinder());
}
void AidlHalWrapper::tryReconnect() {
@@ -456,15 +467,15 @@
}
HalResult<Capabilities> HidlHalWrapperV1_3::getCapabilitiesInternal() {
+ Capabilities capabilities = Capabilities::NONE;
+
sp<V1_3::IVibrator> hal = getHal();
auto amplitudeResult = hal->supportsAmplitudeControl();
if (!amplitudeResult.isOk()) {
- return HalResult<Capabilities>::failed();
+ return HalResult<Capabilities>::fromReturn(amplitudeResult, capabilities);
}
auto externalControlResult = hal->supportsExternalControl();
- Capabilities capabilities = Capabilities::NONE;
-
if (amplitudeResult.withDefault(false)) {
capabilities |= Capabilities::AMPLITUDE_CONTROL;
}
diff --git a/services/vibratorservice/include/vibratorservice/VibratorHalWrapper.h b/services/vibratorservice/include/vibratorservice/VibratorHalWrapper.h
index a4fa869..6e36bd6 100644
--- a/services/vibratorservice/include/vibratorservice/VibratorHalWrapper.h
+++ b/services/vibratorservice/include/vibratorservice/VibratorHalWrapper.h
@@ -35,8 +35,10 @@
class HalResult {
public:
static HalResult<T> ok(T value) { return HalResult(value); }
- static HalResult<T> failed() { return HalResult(/* unsupported= */ false); }
- static HalResult<T> unsupported() { return HalResult(/* unsupported= */ true); }
+ static HalResult<T> failed(std::string msg) {
+ return HalResult(std::move(msg), /* unsupported= */ false);
+ }
+ static HalResult<T> unsupported() { return HalResult("", /* unsupported= */ true); }
static HalResult<T> fromStatus(binder::Status status, T data);
static HalResult<T> fromStatus(hardware::vibrator::V1_0::Status status, T data);
@@ -53,13 +55,17 @@
bool isOk() const { return !mUnsupported && mValue.has_value(); }
bool isFailed() const { return !mUnsupported && !mValue.has_value(); }
bool isUnsupported() const { return mUnsupported; }
+ const char* errorMessage() const { return mErrorMessage.c_str(); }
private:
std::optional<T> mValue;
+ std::string mErrorMessage;
bool mUnsupported;
- explicit HalResult(T value) : mValue(std::make_optional(value)), mUnsupported(false) {}
- explicit HalResult(bool unsupported) : mValue(), mUnsupported(unsupported) {}
+ explicit HalResult(T value)
+ : mValue(std::make_optional(value)), mErrorMessage(), mUnsupported(false) {}
+ explicit HalResult(std::string errorMessage, bool unsupported)
+ : mValue(), mErrorMessage(std::move(errorMessage)), mUnsupported(unsupported) {}
};
// Empty result of a call to the Vibrator HAL wrapper.
@@ -67,11 +73,10 @@
class HalResult<void> {
public:
static HalResult<void> ok() { return HalResult(); }
- static HalResult<void> failed() { return HalResult(/* failed= */ true); }
- static HalResult<void> unsupported() {
- return HalResult(/* failed= */ false, /* unsupported= */ true);
- }
+ static HalResult<void> failed(std::string msg) { return HalResult(std::move(msg)); }
+ static HalResult<void> unsupported() { return HalResult(/* unsupported= */ true); }
+ static HalResult<void> fromStatus(status_t status);
static HalResult<void> fromStatus(binder::Status status);
static HalResult<void> fromStatus(hardware::vibrator::V1_0::Status status);
@@ -81,13 +86,17 @@
bool isOk() const { return !mUnsupported && !mFailed; }
bool isFailed() const { return !mUnsupported && mFailed; }
bool isUnsupported() const { return mUnsupported; }
+ const char* errorMessage() const { return mErrorMessage.c_str(); }
private:
+ std::string mErrorMessage;
bool mFailed;
bool mUnsupported;
- explicit HalResult(bool failed = false, bool unsupported = false)
- : mFailed(failed), mUnsupported(unsupported) {}
+ explicit HalResult(bool unsupported = false)
+ : mErrorMessage(), mFailed(false), mUnsupported(unsupported) {}
+ explicit HalResult(std::string errorMessage)
+ : mErrorMessage(std::move(errorMessage)), mFailed(true), mUnsupported(false) {}
};
// -------------------------------------------------------------------------------------------------
diff --git a/services/vibratorservice/test/VibratorHalControllerTest.cpp b/services/vibratorservice/test/VibratorHalControllerTest.cpp
index 2d55549..8155df0 100644
--- a/services/vibratorservice/test/VibratorHalControllerTest.cpp
+++ b/services/vibratorservice/test/VibratorHalControllerTest.cpp
@@ -257,10 +257,10 @@
TEST_F(VibratorHalControllerTest, TestFailedApiResultResetsHalConnection) {
setHalExpectations(MAX_ATTEMPTS, std::vector<CompositeEffect>(),
- vibrator::HalResult<void>::failed(),
- vibrator::HalResult<vibrator::Capabilities>::failed(),
- vibrator::HalResult<std::vector<Effect>>::failed(),
- vibrator::HalResult<milliseconds>::failed());
+ vibrator::HalResult<void>::failed("message"),
+ vibrator::HalResult<vibrator::Capabilities>::failed("message"),
+ vibrator::HalResult<std::vector<Effect>>::failed("message"),
+ vibrator::HalResult<milliseconds>::failed("message"));
ASSERT_EQ(0, mConnectCounter);
@@ -286,7 +286,7 @@
InSequence seq;
EXPECT_CALL(*mMockHal.get(), ping())
.Times(Exactly(1))
- .WillRepeatedly(Return(vibrator::HalResult<void>::failed()));
+ .WillRepeatedly(Return(vibrator::HalResult<void>::failed("message")));
EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(1));
EXPECT_CALL(*mMockHal.get(), ping())
.Times(Exactly(1))
@@ -351,11 +351,11 @@
});
EXPECT_CALL(*mMockHal.get(), ping())
.Times(Exactly(1))
- .WillRepeatedly(Return(vibrator::HalResult<void>::failed()));
+ .WillRepeatedly(Return(vibrator::HalResult<void>::failed("message")));
EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(1));
EXPECT_CALL(*mMockHal.get(), ping())
.Times(Exactly(1))
- .WillRepeatedly(Return(vibrator::HalResult<void>::failed()));
+ .WillRepeatedly(Return(vibrator::HalResult<void>::failed("message")));
EXPECT_CALL(*mMockHal.get(), tryReconnect()).Times(Exactly(1));
}
diff --git a/services/vibratorservice/test/VibratorHalWrapperAidlTest.cpp b/services/vibratorservice/test/VibratorHalWrapperAidlTest.cpp
index ab5ab1d..3e06c95 100644
--- a/services/vibratorservice/test/VibratorHalWrapperAidlTest.cpp
+++ b/services/vibratorservice/test/VibratorHalWrapperAidlTest.cpp
@@ -116,19 +116,16 @@
}
TEST_F(VibratorHalWrapperAidlTest, TestPing) {
- {
- InSequence seq;
- EXPECT_CALL(*mMockHal.get(), getCapabilities(_))
- .Times(Exactly(3))
- .WillOnce(Return(Status::fromExceptionCode(Status::Exception::EX_SECURITY)))
- .WillOnce(Return(
- Status::fromExceptionCode(Status::Exception::EX_UNSUPPORTED_OPERATION)))
- .WillRepeatedly(Return(Status()));
- }
+ EXPECT_CALL(*mMockHal.get(), onAsBinder())
+ .Times(Exactly(2))
+ .WillRepeatedly(Return(mMockBinder.get()));
+ EXPECT_CALL(*mMockBinder.get(), pingBinder())
+ .Times(Exactly(2))
+ .WillOnce(Return(android::OK))
+ .WillRepeatedly(Return(android::DEAD_OBJECT));
+ ASSERT_TRUE(mWrapper->ping().isOk());
ASSERT_TRUE(mWrapper->ping().isFailed());
- ASSERT_TRUE(mWrapper->ping().isOk());
- ASSERT_TRUE(mWrapper->ping().isOk());
}
TEST_F(VibratorHalWrapperAidlTest, TestOnWithCallbackSupport) {