Merge "Fix HighHint touch signal for override." into main
diff --git a/cmds/evemu-record/README.md b/cmds/evemu-record/README.md
new file mode 100644
index 0000000..5d16d51
--- /dev/null
+++ b/cmds/evemu-record/README.md
@@ -0,0 +1,48 @@
+# `evemu-record`
+
+This is a Rust implementation of the `evemu-record` command from the [FreeDesktop project's evemu
+suite][FreeDesktop]. It records the descriptor and events produced by a single input device in a
+[simple text-based format][format] that can be replayed using the [`uinput` command on
+Android][uinput] or the FreeDesktop evemu tools on other Linux-based platforms. It is included by
+default with `userdebug` and `eng` builds of Android.
+
+The command-line interface is the same as that of the FreeDesktop version, except for
+Android-specific features. For usage instructions, run `evemu-record --help`.
+
+## Usage example
+
+From a computer connected to the device over ADB, you can start a recording:
+
+```
+$ adb shell evemu-record > my-recording.evemu
+Available devices:
+/dev/input/event0: gpio_keys
+/dev/input/event1: s2mpg12-power-keys
+/dev/input/event2: NVTCapacitiveTouchScreen
+/dev/input/event3: NVTCapacitivePen
+/dev/input/event4: uinput-folio
+/dev/input/event5: ACME Touchpad
+Select the device event number [0-5]: 5
+```
+
+...then use the input device for a while, and press Ctrl+C to finish. You will now have a
+`my-recording.evemu` file that you can examine in a text editor. To replay it, use the [`uinput`
+command][uinput]:
+
+```
+$ adb shell uinput - < my-recording.evemu
+```
+
+## Android-specific features
+
+### Timestamp bases
+
+By default, event timestamps are recorded relative to the time of the first event received during
+the recording. Passing `--timestamp-base=boot` causes the timestamps to be recorded relative to the
+system boot time instead. While this does not affect the playback of the recording, it can be useful
+for matching recorded events with other logs that use such timestamps, such as `dmesg` or the
+touchpad gesture debug logs emitted by `TouchpadInputMapper`.
+
+[FreeDesktop]: https://gitlab.freedesktop.org/libevdev/evemu
+[format]: https://gitlab.freedesktop.org/libevdev/evemu#device-description-format
+[uinput]: https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/cmds/uinput/README.md
diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp
index 7478f29..4486bd6 100644
--- a/cmds/installd/InstalldNativeService.cpp
+++ b/cmds/installd/InstalldNativeService.cpp
@@ -4017,24 +4017,37 @@
return exception(binder::Status::EX_ILLEGAL_ARGUMENT, "Received a null auth token");
}
- // Authenticate to check the targeting file is the same inode as the authFd.
+ // Authenticate to check the targeting file is the same inode as the authFd. With O_PATH, we
+ // prevent a malicious client from blocking installd by providing a path to FIFO. After the
+ // authentication, the actual open is safe.
sp<IBinder> authTokenBinder = IInterface::asBinder(authToken)->localBinder();
if (authTokenBinder == nullptr) {
return exception(binder::Status::EX_SECURITY, "Received a non-local auth token");
}
- auto authTokenInstance = sp<FsveritySetupAuthToken>::cast(authTokenBinder);
- unique_fd rfd(open(filePath.c_str(), O_RDONLY | O_CLOEXEC | O_NOFOLLOW));
- struct stat stFromPath;
- if (fstat(rfd.get(), &stFromPath) < 0) {
- *_aidl_return = errno;
+ unique_fd pathFd(open(filePath.c_str(), O_RDONLY | O_CLOEXEC | O_NOFOLLOW | O_PATH));
+ // Returns a constant errno to avoid one app probing file existence of the others, before the
+ // authentication is done.
+ const int kFixedErrno = EPERM;
+ if (pathFd.get() < 0) {
+ PLOG(DEBUG) << "Failed to open the path";
+ *_aidl_return = kFixedErrno;
return ok();
}
+ std::string procFdPath(StringPrintf("/proc/self/fd/%d", pathFd.get()));
+ struct stat stFromPath;
+ if (stat(procFdPath.c_str(), &stFromPath) < 0) {
+ PLOG(DEBUG) << "Failed to stat proc fd " << pathFd.get() << " -> " << filePath;
+ *_aidl_return = kFixedErrno;
+ return ok();
+ }
+ auto authTokenInstance = sp<FsveritySetupAuthToken>::cast(authTokenBinder);
if (!authTokenInstance->isSameStat(stFromPath)) {
LOG(DEBUG) << "FD authentication failed";
- *_aidl_return = EPERM;
+ *_aidl_return = kFixedErrno;
return ok();
}
+ unique_fd rfd(open(procFdPath.c_str(), O_RDONLY | O_CLOEXEC));
fsverity_enable_arg arg = {};
arg.version = 1;
arg.hash_algorithm = FS_VERITY_HASH_ALG_SHA256;
diff --git a/cmds/installd/tests/installd_service_test.cpp b/cmds/installd/tests/installd_service_test.cpp
index f2b578a..023491f 100644
--- a/cmds/installd/tests/installd_service_test.cpp
+++ b/cmds/installd/tests/installd_service_test.cpp
@@ -194,6 +194,12 @@
});
}
+static void unlink_path(const std::string& path) {
+ if (unlink(path.c_str()) < 0) {
+ PLOG(DEBUG) << "Failed to unlink " + path;
+ }
+}
+
class ServiceTest : public testing::Test {
protected:
InstalldNativeService* service;
@@ -555,7 +561,7 @@
TEST_F(FsverityTest, enableFsverity) {
const std::string path = kTestPath + "/foo";
create_with_content(path, kTestAppUid, kTestAppUid, 0600, "content");
- UniqueFile raii(/*fd=*/-1, path, [](const std::string& path) { unlink(path.c_str()); });
+ UniqueFile raii(/*fd=*/-1, path, &unlink_path);
// Expect to fs-verity setup to succeed
sp<IFsveritySetupAuthToken> authToken;
@@ -573,7 +579,7 @@
TEST_F(FsverityTest, enableFsverity_nullAuthToken) {
const std::string path = kTestPath + "/foo";
create_with_content(path, kTestAppUid, kTestAppUid, 0600, "content");
- UniqueFile raii(/*fd=*/-1, path, [](const std::string& path) { unlink(path.c_str()); });
+ UniqueFile raii(/*fd=*/-1, path, &unlink_path);
// Verity null auth token fails
sp<IFsveritySetupAuthToken> authToken;
@@ -586,7 +592,7 @@
TEST_F(FsverityTest, enableFsverity_differentFile) {
const std::string path = kTestPath + "/foo";
create_with_content(path, kTestAppUid, kTestAppUid, 0600, "content");
- UniqueFile raii(/*fd=*/-1, path, [](const std::string& path) { unlink(path.c_str()); });
+ UniqueFile raii(/*fd=*/-1, path, &unlink_path);
// Expect to fs-verity setup to succeed
sp<IFsveritySetupAuthToken> authToken;
@@ -597,17 +603,36 @@
// Verity auth token does not work for a different file
const std::string anotherPath = kTestPath + "/bar";
ASSERT_TRUE(android::base::WriteStringToFile("content", anotherPath));
- UniqueFile raii2(/*fd=*/-1, anotherPath, [](const std::string& path) { unlink(path.c_str()); });
+ UniqueFile raii2(/*fd=*/-1, anotherPath, &unlink_path);
int32_t errno_local;
status = service->enableFsverity(authToken, anotherPath, "fake.package.name", &errno_local);
EXPECT_TRUE(status.isOk());
EXPECT_NE(errno_local, 0);
}
+TEST_F(FsverityTest, enableFsverity_errnoBeforeAuthenticated) {
+ const std::string path = kTestPath + "/foo";
+ create_with_content(path, kTestAppUid, kTestAppUid, 0600, "content");
+ UniqueFile raii(/*fd=*/-1, path, &unlink_path);
+
+ // Expect to fs-verity setup to succeed
+ sp<IFsveritySetupAuthToken> authToken;
+ binder::Status status = createFsveritySetupAuthToken(path, O_RDWR, &authToken);
+ EXPECT_TRUE(status.isOk());
+ EXPECT_TRUE(authToken != nullptr);
+
+ // Verity errno before the fd authentication is constant (EPERM)
+ int32_t errno_local;
+ status = service->enableFsverity(authToken, path + "-non-exist", "fake.package.name",
+ &errno_local);
+ EXPECT_TRUE(status.isOk());
+ EXPECT_EQ(errno_local, EPERM);
+}
+
TEST_F(FsverityTest, createFsveritySetupAuthToken_ReadonlyFdDoesNotAuthenticate) {
const std::string path = kTestPath + "/foo";
create_with_content(path, kTestAppUid, kTestAppUid, 0600, "content");
- UniqueFile raii(/*fd=*/-1, path, [](const std::string& path) { unlink(path.c_str()); });
+ UniqueFile raii(/*fd=*/-1, path, &unlink_path);
// Expect the fs-verity setup to fail
sp<IFsveritySetupAuthToken> authToken;
@@ -619,7 +644,7 @@
const std::string path = kTestPath + "/foo";
// Simulate world-writable file owned by another app
create_with_content(path, kTestAppUid + 1, kTestAppUid + 1, 0666, "content");
- UniqueFile raii(/*fd=*/-1, path, [](const std::string& path) { unlink(path.c_str()); });
+ UniqueFile raii(/*fd=*/-1, path, &unlink_path);
// Expect the fs-verity setup to fail
sp<IFsveritySetupAuthToken> authToken;
diff --git a/cmds/lshal/Lshal.cpp b/cmds/lshal/Lshal.cpp
index 6115da7..5cdcb23 100644
--- a/cmds/lshal/Lshal.cpp
+++ b/cmds/lshal/Lshal.cpp
@@ -232,6 +232,11 @@
return static_cast<HelpCommand*>(help)->usageOfCommand(mCommand);
}
+ // After Lshal::main() finishes, caller may call _exit(), causing debug
+ // information to prematurely ends. Hence flush().
+ err().flush();
+ out().flush();
+
return status;
}
diff --git a/cmds/lshal/NullableOStream.h b/cmds/lshal/NullableOStream.h
index 7cffcf8..1576486 100644
--- a/cmds/lshal/NullableOStream.h
+++ b/cmds/lshal/NullableOStream.h
@@ -59,6 +59,11 @@
operator bool() const { // NOLINT(google-explicit-constructor)
return mOs != nullptr;
}
+ void flush() {
+ if (mOs) {
+ mOs->flush();
+ }
+ }
private:
template<typename>
friend class NullableOStream;
diff --git a/cmds/lshal/Timeout.h b/cmds/lshal/Timeout.h
index 37f41be..d97ba89 100644
--- a/cmds/lshal/Timeout.h
+++ b/cmds/lshal/Timeout.h
@@ -16,83 +16,44 @@
#pragma once
-#include <condition_variable>
#include <chrono>
-#include <functional>
-#include <mutex>
-#include <thread>
+#include <future>
#include <hidl/Status.h>
+#include <utils/Errors.h>
namespace android {
namespace lshal {
-class BackgroundTaskState {
-public:
- explicit BackgroundTaskState(std::function<void(void)> &&func)
- : mFunc(std::forward<decltype(func)>(func)) {}
- void notify() {
- std::unique_lock<std::mutex> lock(mMutex);
- mFinished = true;
- lock.unlock();
- mCondVar.notify_all();
- }
- template<class C, class D>
- bool wait(std::chrono::time_point<C, D> end) {
- std::unique_lock<std::mutex> lock(mMutex);
- mCondVar.wait_until(lock, end, [this](){ return this->mFinished; });
- return mFinished;
- }
- void operator()() {
- mFunc();
- }
-private:
- std::mutex mMutex;
- std::condition_variable mCondVar;
- bool mFinished = false;
- std::function<void(void)> mFunc;
-};
-
-void *callAndNotify(void *data) {
- BackgroundTaskState &state = *static_cast<BackgroundTaskState *>(data);
- state();
- state.notify();
- return nullptr;
-}
-
-template<class R, class P>
-bool timeout(std::chrono::duration<R, P> delay, std::function<void(void)> &&func) {
- auto now = std::chrono::system_clock::now();
- BackgroundTaskState state{std::forward<decltype(func)>(func)};
- pthread_t thread;
- if (pthread_create(&thread, nullptr, callAndNotify, &state)) {
- std::cerr << "FATAL: could not create background thread." << std::endl;
- return false;
- }
- bool success = state.wait(now + delay);
- if (!success) {
- pthread_kill(thread, SIGINT);
- }
- pthread_join(thread, nullptr);
- return success;
-}
-
+// Call function on interfaceObject and wait for result until the given timeout has reached.
+// Callback functions pass to timeoutIPC() may be executed after the this function
+// has returned, especially if deadline has been reached. Hence, care must be taken when passing
+// data between the background thread and the main thread. See b/311143089.
template<class R, class P, class Function, class I, class... Args>
typename std::invoke_result<Function, I *, Args...>::type
timeoutIPC(std::chrono::duration<R, P> wait, const sp<I> &interfaceObject, Function &&func,
Args &&... args) {
using ::android::hardware::Status;
- typename std::result_of<Function(I *, Args...)>::type ret{Status::ok()};
- auto boundFunc = std::bind(std::forward<Function>(func),
- interfaceObject.get(), std::forward<Args>(args)...);
- bool success = timeout(wait, [&ret, &boundFunc] {
- ret = std::move(boundFunc());
- });
- if (!success) {
+
+ // Execute on a background thread but do not defer execution.
+ auto future =
+ std::async(std::launch::async, func, interfaceObject, std::forward<Args>(args)...);
+ auto status = future.wait_for(wait);
+ if (status == std::future_status::ready) {
+ return future.get();
+ }
+
+ // This future belongs to a background thread that we no longer care about.
+ // Putting this in the global list avoids std::future::~future() that may wait for the
+ // result to come back.
+ // This leaks memory, but lshal is a debugging tool, so this is fine.
+ static std::vector<decltype(future)> gDeadPool{};
+ gDeadPool.emplace_back(std::move(future));
+
+ if (status == std::future_status::timeout) {
return Status::fromStatusT(TIMED_OUT);
}
- return ret;
+ return Status::fromExceptionCode(Status::Exception::EX_ILLEGAL_STATE, "Illegal future_status");
}
-
-} // namespace lshal
-} // namespace android
+} // namespace lshal
+} // namespace android
diff --git a/cmds/lshal/main.cpp b/cmds/lshal/main.cpp
index 366c938..bd5fa32 100644
--- a/cmds/lshal/main.cpp
+++ b/cmds/lshal/main.cpp
@@ -18,5 +18,6 @@
int main(int argc, char **argv) {
using namespace ::android::lshal;
- return Lshal{}.main(Arg{argc, argv});
+ // Use _exit() to force terminate background threads in Timeout.h
+ _exit(Lshal{}.main(Arg{argc, argv}));
}
diff --git a/cmds/lshal/test.cpp b/cmds/lshal/test.cpp
index cba7c4b..c24f827 100644
--- a/cmds/lshal/test.cpp
+++ b/cmds/lshal/test.cpp
@@ -14,6 +14,10 @@
* limitations under the License.
*/
+#include <chrono>
+#include <future>
+#include <mutex>
+#include "android/hidl/base/1.0/IBase.h"
#define LOG_TAG "Lshal"
#include <android-base/logging.h>
@@ -36,6 +40,8 @@
using namespace testing;
+using std::chrono_literals::operator""ms;
+
using ::android::hidl::base::V1_0::DebugInfo;
using ::android::hidl::base::V1_0::IBase;
using ::android::hidl::manager::V1_0::IServiceManager;
@@ -934,12 +940,9 @@
return hardware::Void();
}));
EXPECT_CALL(*serviceManager, get(_, _))
- .WillRepeatedly(
- Invoke([&](const hidl_string&, const hidl_string& instance) -> sp<IBase> {
- int id = getIdFromInstanceName(instance);
- if (id > inheritanceLevel) return nullptr;
- return sp<IBase>(service);
- }));
+ .WillRepeatedly(Invoke([&](const hidl_string&, const hidl_string&) -> sp<IBase> {
+ return sp<IBase>(service);
+ }));
const std::string expected = "[fake description 0]\n"
"Interface\n"
@@ -957,6 +960,110 @@
EXPECT_EQ("", err.str());
}
+// In SlowService, everything goes slooooooow. Each IPC call will wait for
+// the specified time before calling the callback function or returning.
+class SlowService : public IBase {
+public:
+ explicit SlowService(std::chrono::milliseconds wait) : mWait(wait) {}
+ android::hardware::Return<void> interfaceDescriptor(interfaceDescriptor_cb cb) override {
+ std::this_thread::sleep_for(mWait);
+ cb(getInterfaceName(1));
+ storeHistory("interfaceDescriptor");
+ return hardware::Void();
+ }
+ android::hardware::Return<void> interfaceChain(interfaceChain_cb cb) override {
+ std::this_thread::sleep_for(mWait);
+ std::vector<hidl_string> ret;
+ ret.push_back(getInterfaceName(1));
+ ret.push_back(IBase::descriptor);
+ cb(ret);
+ storeHistory("interfaceChain");
+ return hardware::Void();
+ }
+ android::hardware::Return<void> getHashChain(getHashChain_cb cb) override {
+ std::this_thread::sleep_for(mWait);
+ std::vector<hidl_hash> ret;
+ ret.push_back(getHashFromId(0));
+ ret.push_back(getHashFromId(0xff));
+ cb(ret);
+ storeHistory("getHashChain");
+ return hardware::Void();
+ }
+ android::hardware::Return<void> debug(const hidl_handle&,
+ const hidl_vec<hidl_string>&) override {
+ std::this_thread::sleep_for(mWait);
+ storeHistory("debug");
+ return Void();
+ }
+
+ template <class R, class P, class Pred>
+ bool waitForHistory(std::chrono::duration<R, P> wait, Pred predicate) {
+ std::unique_lock<std::mutex> lock(mLock);
+ return mCv.wait_for(lock, wait, [&]() { return predicate(mCallHistory); });
+ }
+
+private:
+ void storeHistory(std::string hist) {
+ {
+ std::lock_guard<std::mutex> lock(mLock);
+ mCallHistory.emplace_back(std::move(hist));
+ }
+ mCv.notify_all();
+ }
+
+ const std::chrono::milliseconds mWait;
+ std::mutex mLock;
+ std::condition_variable mCv;
+ // List of functions that have finished being called on this interface.
+ std::vector<std::string> mCallHistory;
+};
+
+class TimeoutTest : public ListTest {
+public:
+ void setMockServiceManager(sp<IBase> service) {
+ EXPECT_CALL(*serviceManager, list(_))
+ .WillRepeatedly(Invoke([&](IServiceManager::list_cb cb) {
+ std::vector<hidl_string> ret;
+ ret.push_back(getInterfaceName(1) + "/default");
+ cb(ret);
+ return hardware::Void();
+ }));
+ EXPECT_CALL(*serviceManager, get(_, _))
+ .WillRepeatedly(Invoke([&](const hidl_string&, const hidl_string&) -> sp<IBase> {
+ return service;
+ }));
+ }
+};
+
+TEST_F(TimeoutTest, BackgroundThreadIsKept) {
+ auto lshalIpcTimeout = 100ms;
+ auto serviceIpcTimeout = 200ms;
+ lshal->setWaitTimeForTest(lshalIpcTimeout, lshalIpcTimeout);
+ sp<SlowService> service = new SlowService(serviceIpcTimeout);
+ setMockServiceManager(service);
+
+ optind = 1; // mimic Lshal::parseArg()
+ EXPECT_NE(0u, mockList->main(createArg({"lshal", "--types=b", "-i", "--neat"})));
+ EXPECT_THAT(err.str(), HasSubstr("Skipping \"a.h.foo1@1.0::IFoo/default\""));
+ EXPECT_TRUE(service->waitForHistory(serviceIpcTimeout * 5, [](const auto& hist) {
+ return hist.size() == 1 && hist[0] == "interfaceChain";
+ })) << "The background thread should continue after the main thread moves on, but it is killed";
+}
+
+TEST_F(TimeoutTest, BackgroundThreadDoesNotBlockMainThread) {
+ auto lshalIpcTimeout = 100ms;
+ auto serviceIpcTimeout = 2000ms;
+ auto start = std::chrono::system_clock::now();
+ lshal->setWaitTimeForTest(lshalIpcTimeout, lshalIpcTimeout);
+ sp<SlowService> service = new SlowService(serviceIpcTimeout);
+ setMockServiceManager(service);
+
+ optind = 1; // mimic Lshal::parseArg()
+ EXPECT_NE(0u, mockList->main(createArg({"lshal", "--types=b", "-i", "--neat"})));
+ EXPECT_LE(std::chrono::system_clock::now(), start + 5 * lshalIpcTimeout)
+ << "The main thread should not be blocked by the background task";
+}
+
class ListVintfTest : public ListTest {
public:
virtual void SetUp() override {
@@ -1079,5 +1186,6 @@
int main(int argc, char **argv) {
::testing::InitGoogleMock(&argc, argv);
- return RUN_ALL_TESTS();
+ // Use _exit() to force terminate background threads in Timeout.h
+ _exit(RUN_ALL_TESTS());
}
diff --git a/data/etc/wearable_core_hardware.xml b/data/etc/wearable_core_hardware.xml
index 855b110..4c9932d 100644
--- a/data/etc/wearable_core_hardware.xml
+++ b/data/etc/wearable_core_hardware.xml
@@ -36,6 +36,7 @@
<feature name="android.hardware.security.model.compatible" />
<!-- basic system services -->
+ <feature name="android.software.credentials" />
<feature name="android.software.home_screen" />
<feature name="android.software.secure_lock_screen" />
diff --git a/libs/binder/ProcessState.cpp b/libs/binder/ProcessState.cpp
index 7de94e3..fb2781b 100644
--- a/libs/binder/ProcessState.cpp
+++ b/libs/binder/ProcessState.cpp
@@ -492,6 +492,7 @@
if (read(fd, &on, sizeof(on)) == -1) {
ALOGE("%s: error reading to %s: %s", __func__,
names[static_cast<int>(feature)], strerror(errno));
+ close(fd);
return false;
}
close(fd);
diff --git a/libs/binder/RecordedTransaction.cpp b/libs/binder/RecordedTransaction.cpp
index 525ba2e..de2a69f 100644
--- a/libs/binder/RecordedTransaction.cpp
+++ b/libs/binder/RecordedTransaction.cpp
@@ -114,8 +114,8 @@
RecordedTransaction::RecordedTransaction(RecordedTransaction&& t) noexcept {
mData = t.mData;
- mSent.setData(t.getDataParcel().data(), t.getDataParcel().dataSize());
- mReply.setData(t.getReplyParcel().data(), t.getReplyParcel().dataSize());
+ mSentDataOnly.setData(t.getDataParcel().data(), t.getDataParcel().dataSize());
+ mReplyDataOnly.setData(t.getReplyParcel().data(), t.getReplyParcel().dataSize());
}
std::optional<RecordedTransaction> RecordedTransaction::fromDetails(
@@ -136,12 +136,21 @@
return std::nullopt;
}
- if (t.mSent.setData(dataParcel.data(), dataParcel.dataBufferSize()) != android::NO_ERROR) {
+ if (const auto* kernelFields = dataParcel.maybeKernelFields()) {
+ for (size_t i = 0; i < kernelFields->mObjectsSize; i++) {
+ uint64_t offset = kernelFields->mObjects[i];
+ t.mData.mSentObjectData.push_back(offset);
+ }
+ }
+
+ if (t.mSentDataOnly.setData(dataParcel.data(), dataParcel.dataBufferSize()) !=
+ android::NO_ERROR) {
ALOGE("Failed to set sent parcel data.");
return std::nullopt;
}
- if (t.mReply.setData(replyParcel.data(), replyParcel.dataBufferSize()) != android::NO_ERROR) {
+ if (t.mReplyDataOnly.setData(replyParcel.data(), replyParcel.dataBufferSize()) !=
+ android::NO_ERROR) {
ALOGE("Failed to set reply parcel data.");
return std::nullopt;
}
@@ -154,6 +163,7 @@
DATA_PARCEL_CHUNK = 2,
REPLY_PARCEL_CHUNK = 3,
INTERFACE_NAME_CHUNK = 4,
+ DATA_PARCEL_OBJECT_CHUNK = 5,
END_CHUNK = 0x00ffffff,
};
@@ -265,21 +275,30 @@
break;
}
case DATA_PARCEL_CHUNK: {
- if (t.mSent.setData(reinterpret_cast<const unsigned char*>(payloadMap),
- chunk.dataSize) != android::NO_ERROR) {
+ if (t.mSentDataOnly.setData(reinterpret_cast<const unsigned char*>(payloadMap),
+ chunk.dataSize) != android::NO_ERROR) {
ALOGE("Failed to set sent parcel data.");
return std::nullopt;
}
break;
}
case REPLY_PARCEL_CHUNK: {
- if (t.mReply.setData(reinterpret_cast<const unsigned char*>(payloadMap),
- chunk.dataSize) != android::NO_ERROR) {
+ if (t.mReplyDataOnly.setData(reinterpret_cast<const unsigned char*>(payloadMap),
+ chunk.dataSize) != android::NO_ERROR) {
ALOGE("Failed to set reply parcel data.");
return std::nullopt;
}
break;
}
+ case DATA_PARCEL_OBJECT_CHUNK: {
+ const uint64_t* objects = reinterpret_cast<const uint64_t*>(payloadMap);
+ size_t metaDataSize = (chunk.dataSize / sizeof(uint64_t));
+ ALOGI("Total objects found in saved parcel %zu", metaDataSize);
+ for (size_t index = 0; index < metaDataSize; ++index) {
+ t.mData.mSentObjectData.push_back(objects[index]);
+ }
+ break;
+ }
case END_CHUNK:
break;
default:
@@ -343,14 +362,26 @@
return UNKNOWN_ERROR;
}
- if (NO_ERROR != writeChunk(fd, DATA_PARCEL_CHUNK, mSent.dataBufferSize(), mSent.data())) {
+ if (NO_ERROR !=
+ writeChunk(fd, DATA_PARCEL_CHUNK, mSentDataOnly.dataBufferSize(), mSentDataOnly.data())) {
ALOGE("Failed to write sent Parcel to fd %d", fd.get());
return UNKNOWN_ERROR;
}
- if (NO_ERROR != writeChunk(fd, REPLY_PARCEL_CHUNK, mReply.dataBufferSize(), mReply.data())) {
+
+ if (NO_ERROR !=
+ writeChunk(fd, REPLY_PARCEL_CHUNK, mReplyDataOnly.dataBufferSize(),
+ mReplyDataOnly.data())) {
ALOGE("Failed to write reply Parcel to fd %d", fd.get());
return UNKNOWN_ERROR;
}
+
+ if (NO_ERROR !=
+ writeChunk(fd, DATA_PARCEL_OBJECT_CHUNK, mData.mSentObjectData.size() * sizeof(uint64_t),
+ reinterpret_cast<const uint8_t*>(mData.mSentObjectData.data()))) {
+ ALOGE("Failed to write sent parcel object metadata to fd %d", fd.get());
+ return UNKNOWN_ERROR;
+ }
+
if (NO_ERROR != writeChunk(fd, END_CHUNK, 0, NULL)) {
ALOGE("Failed to write end chunk to fd %d", fd.get());
return UNKNOWN_ERROR;
@@ -384,10 +415,14 @@
return mData.mHeader.version;
}
+const std::vector<uint64_t>& RecordedTransaction::getObjectOffsets() const {
+ return mData.mSentObjectData;
+}
+
const Parcel& RecordedTransaction::getDataParcel() const {
- return mSent;
+ return mSentDataOnly;
}
const Parcel& RecordedTransaction::getReplyParcel() const {
- return mReply;
+ return mReplyDataOnly;
}
diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h
index 09da6e3..d7096d8 100644
--- a/libs/binder/include/binder/Parcel.h
+++ b/libs/binder/include/binder/Parcel.h
@@ -55,6 +55,9 @@
class TextOutput;
namespace binder {
class Status;
+namespace debug {
+class RecordedTransaction;
+}
}
class Parcel {
@@ -1443,6 +1446,9 @@
// TODO(b/202029388): Remove 'getBlobAshmemSize' once no prebuilts reference
// this
size_t getBlobAshmemSize() const;
+
+ // Needed so that we can save object metadata to the disk
+ friend class android::binder::debug::RecordedTransaction;
};
// ---------------------------------------------------------------------------
diff --git a/libs/binder/include/binder/RecordedTransaction.h b/libs/binder/include/binder/RecordedTransaction.h
index 505c199..f0bee7f 100644
--- a/libs/binder/include/binder/RecordedTransaction.h
+++ b/libs/binder/include/binder/RecordedTransaction.h
@@ -50,6 +50,7 @@
uint32_t getVersion() const;
const Parcel& getDataParcel() const;
const Parcel& getReplyParcel() const;
+ const std::vector<uint64_t>& getObjectOffsets() const;
private:
RecordedTransaction() = default;
@@ -75,10 +76,11 @@
struct MovableData { // movable
TransactionHeader mHeader;
std::string mInterfaceName;
+ std::vector<uint64_t> mSentObjectData; /* Object Offsets */
};
MovableData mData;
- Parcel mSent;
- Parcel mReply;
+ Parcel mSentDataOnly;
+ Parcel mReplyDataOnly;
};
} // namespace binder::debug
diff --git a/libs/binder/tests/Android.bp b/libs/binder/tests/Android.bp
index dd2be94..aba2319 100644
--- a/libs/binder/tests/Android.bp
+++ b/libs/binder/tests/Android.bp
@@ -69,10 +69,14 @@
cc_test {
name: "binderRecordReplayTest",
srcs: ["binderRecordReplayTest.cpp"],
+ cflags: [
+ "-DBINDER_WITH_KERNEL_IPC",
+ ],
shared_libs: [
"libbinder",
"libcutils",
"libutils",
+ "liblog",
],
static_libs: [
"binderRecordReplayTestIface-cpp",
@@ -96,6 +100,9 @@
enabled: true,
platform_apis: true,
},
+ ndk: {
+ enabled: false,
+ },
},
}
diff --git a/libs/binder/tests/IBinderRecordReplayTest.aidl b/libs/binder/tests/IBinderRecordReplayTest.aidl
index bd6b03c..29267e9 100644
--- a/libs/binder/tests/IBinderRecordReplayTest.aidl
+++ b/libs/binder/tests/IBinderRecordReplayTest.aidl
@@ -69,4 +69,10 @@
void setSingleDataParcelableArray(in SingleDataParcelable[] input);
SingleDataParcelable[] getSingleDataParcelableArray();
+
+ void setBinder(in IBinder binder);
+ IBinder getBinder();
+
+ void setFileDescriptor(in FileDescriptor fd);
+ FileDescriptor getFileDescriptor();
}
diff --git a/libs/binder/tests/binderRecordReplayTest.cpp b/libs/binder/tests/binderRecordReplayTest.cpp
index 73c0a94..b975fad 100644
--- a/libs/binder/tests/binderRecordReplayTest.cpp
+++ b/libs/binder/tests/binderRecordReplayTest.cpp
@@ -24,7 +24,10 @@
#include <binder/RecordedTransaction.h>
#include <binder/unique_fd.h>
+#include <cutils/ashmem.h>
+
#include <fuzzbinder/libbinder_driver.h>
+#include <fuzzbinder/random_binder.h>
#include <fuzzer/FuzzedDataProvider.h>
#include <fuzzseeds/random_parcel_seeds.h>
@@ -37,6 +40,7 @@
using namespace android;
using android::generateSeedsFromRecording;
+using android::RandomBinder;
using android::binder::borrowed_fd;
using android::binder::Status;
using android::binder::unique_fd;
@@ -44,6 +48,7 @@
using parcelables::SingleDataParcelable;
const String16 kServerName = String16("binderRecordReplay");
+extern std::string kRandomInterfaceName;
#define GENERATE_GETTER_SETTER_PRIMITIVE(name, T) \
Status set##name(T input) { \
@@ -81,6 +86,7 @@
GENERATE_GETTER_SETTER(String, String16);
GENERATE_GETTER_SETTER(SingleDataParcelable, SingleDataParcelable);
+ GENERATE_GETTER_SETTER(Binder, sp<IBinder>);
GENERATE_GETTER_SETTER(BooleanArray, std::vector<bool>);
GENERATE_GETTER_SETTER(ByteArray, std::vector<uint8_t>);
@@ -91,12 +97,22 @@
GENERATE_GETTER_SETTER(DoubleArray, std::vector<double>);
GENERATE_GETTER_SETTER(StringArray, std::vector<::android::String16>);
GENERATE_GETTER_SETTER(SingleDataParcelableArray, std::vector<SingleDataParcelable>);
+
+ Status setFileDescriptor(unique_fd input) {
+ mFd = std::move(unique_fd(dup(input)));
+ return Status::ok();
+ }
+
+ Status getFileDescriptor(unique_fd* output) {
+ *output = std::move(unique_fd(dup(mFd)));
+ return Status::ok();
+ }
+ unique_fd mFd;
};
std::vector<uint8_t> retrieveData(borrowed_fd fd) {
struct stat fdStat;
EXPECT_TRUE(fstat(fd.get(), &fdStat) != -1);
- EXPECT_TRUE(fdStat.st_size != 0);
std::vector<uint8_t> buffer(fdStat.st_size);
auto readResult = android::base::ReadFully(fd, buffer.data(), fdStat.st_size);
@@ -115,6 +131,7 @@
// Read the data which has been written to seed corpus
ASSERT_EQ(0, lseek(seedFd.get(), 0, SEEK_SET));
std::vector<uint8_t> seedData = retrieveData(seedFd);
+ EXPECT_TRUE(seedData.size() != 0);
// use fuzzService to replay the corpus
FuzzedDataProvider provider(seedData.data(), seedData.size());
@@ -148,7 +165,14 @@
template <typename T, typename U>
void recordReplay(Status (IBinderRecordReplayTest::*set)(T), U recordedValue,
Status (IBinderRecordReplayTest::*get)(U*), U changedValue) {
- auto replayFunctions = {&replayBinder, &replayFuzzService};
+ using ReplayFunc = decltype(&replayFuzzService);
+ vector<ReplayFunc> replayFunctions = {&replayFuzzService};
+ if (!std::is_same_v<U, unique_fd> && !std::is_same_v<U, sp<IBinder>>) {
+ // Parcel retrieved from record replay doesn't have object information. use it for
+ // replaying primitive types only.
+ replayFunctions.push_back(&replayBinder);
+ }
+
for (auto replayFunc : replayFunctions) {
unique_fd fd(open("/data/local/tmp/binderRecordReplayTest.rec",
O_RDWR | O_CREAT | O_CLOEXEC, 0666));
@@ -156,7 +180,7 @@
// record a transaction
mBpBinder->startRecordingBinder(fd);
- auto status = (*mInterface.*set)(recordedValue);
+ auto status = (*mInterface.*set)(std::move(recordedValue));
EXPECT_TRUE(status.isOk());
mBpBinder->stopRecordingBinder();
@@ -164,16 +188,22 @@
U output;
status = (*mInterface.*get)(&output);
EXPECT_TRUE(status.isOk());
- EXPECT_EQ(output, recordedValue);
+
+ // Expect this equal only if types are primitives
+ if (!std::is_same_v<U, unique_fd> && !std::is_same_v<U, sp<IBinder>>) {
+ EXPECT_EQ(output, recordedValue);
+ }
// write over the existing state
- status = (*mInterface.*set)(changedValue);
+ status = (*mInterface.*set)(std::move(changedValue));
EXPECT_TRUE(status.isOk());
status = (*mInterface.*get)(&output);
EXPECT_TRUE(status.isOk());
- EXPECT_EQ(output, changedValue);
+ if (!std::is_same_v<U, unique_fd> && !std::is_same_v<U, sp<IBinder>>) {
+ EXPECT_EQ(output, changedValue);
+ }
// replay transaction
ASSERT_EQ(0, lseek(fd.get(), 0, SEEK_SET));
@@ -186,7 +216,23 @@
status = (*mInterface.*get)(&output);
EXPECT_TRUE(status.isOk());
- EXPECT_EQ(output, recordedValue);
+
+ // FDs and binders will be replaced with random fd and random binders
+ if constexpr (std::is_same_v<U, unique_fd>) {
+ // verify that replayed fd is /dev/null. This is being replayed from random_fd.cpp
+ // and choosing /dav/null while generating seed in binder2corpus
+ std::string fdPath = "/proc/self/fd/" + std::to_string(output.get());
+ char path[PATH_MAX];
+ ASSERT_GT(readlink(fdPath.c_str(), path, sizeof(path)), 0);
+ EXPECT_EQ(strcmp("/dev/null", path), 0);
+ } else if constexpr (std::is_same_v<U, sp<IBinder>>) {
+ // This is binder is replayed from random_binder.cpp using seed data which writes
+ // this interface.
+ EXPECT_EQ(String16(kRandomInterfaceName.c_str(), kRandomInterfaceName.size()),
+ output->getInterfaceDescriptor());
+ } else {
+ ASSERT_EQ(recordedValue, output);
+ }
}
}
@@ -319,6 +365,32 @@
&IBinderRecordReplayTest::getSingleDataParcelableArray, changed);
}
+TEST_F(BinderRecordReplayTest, ReplayBinder) {
+ vector<uint8_t> data = {0x8A, 0x19, 0x0D, 0x44, 0x37, 0x0D, 0x38, 0x5E, 0x9B, 0xAA, 0xF3, 0xDA};
+ sp<IBinder> saved = new RandomBinder(String16("random_interface"), std::move(data));
+ sp<IBinder> changed = IInterface::asBinder(defaultServiceManager());
+ recordReplay(&IBinderRecordReplayTest::setBinder, saved, &IBinderRecordReplayTest::getBinder,
+ changed);
+}
+
+TEST_F(BinderRecordReplayTest, ReplayFd) {
+ // Write something to both fds we are setting
+ unique_fd saved(open("/data/local/tmp/test_fd", O_RDWR | O_CREAT | O_CLOEXEC, 0666));
+ std::string contentSaved = "This will be never read again for recorded fd!";
+ CHECK(android::base::WriteFully(saved, contentSaved.data(), contentSaved.size()))
+ << saved.get();
+
+ unique_fd changed(open("/data/local/tmp/test_des", O_RDWR | O_CREAT | O_CLOEXEC, 0666));
+ std::string contentChanged = "This will be never read again from changed fd!";
+ CHECK(android::base::WriteFully(changed, contentChanged.data(), contentChanged.size()))
+ << changed.get();
+
+ // When fds are replayed, it will be replaced by /dev/null..reading from it should yield
+ // null data
+ recordReplay(&IBinderRecordReplayTest::setFileDescriptor, std::move(unique_fd(dup(saved))),
+ &IBinderRecordReplayTest::getFileDescriptor, std::move(unique_fd(dup(changed))));
+}
+
int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv);
diff --git a/libs/binder/tests/parcel_fuzzer/Android.bp b/libs/binder/tests/parcel_fuzzer/Android.bp
index 83db6c9..fbab8f0 100644
--- a/libs/binder/tests/parcel_fuzzer/Android.bp
+++ b/libs/binder/tests/parcel_fuzzer/Android.bp
@@ -131,6 +131,13 @@
"libcutils",
"libutils",
],
+ static_libs: [
+ "libbinder_random_parcel",
+ ],
+ include_dirs: [
+ "bionic/libc/kernel/android/uapi/",
+ "bionic/libc/kernel/uapi/",
+ ],
local_include_dirs: [
"include_random_parcel_seeds",
],
@@ -140,8 +147,12 @@
cc_binary_host {
name: "binder2corpus",
static_libs: [
+ "libbinder_random_parcel",
"libbinder_random_parcel_seeds",
],
+ cflags: [
+ "-DBINDER_WITH_KERNEL_IPC",
+ ],
srcs: [
"binder2corpus/binder2corpus.cpp",
],
@@ -149,5 +160,6 @@
"libbase",
"libbinder",
"libutils",
+ "libcutils",
],
}
diff --git a/libs/binder/tests/parcel_fuzzer/include_random_parcel/fuzzbinder/random_binder.h b/libs/binder/tests/parcel_fuzzer/include_random_parcel/fuzzbinder/random_binder.h
index 8fc9263..7a1688b 100644
--- a/libs/binder/tests/parcel_fuzzer/include_random_parcel/fuzzbinder/random_binder.h
+++ b/libs/binder/tests/parcel_fuzzer/include_random_parcel/fuzzbinder/random_binder.h
@@ -16,11 +16,25 @@
#pragma once
+#include <binder/Binder.h>
#include <binder/IBinder.h>
#include <fuzzer/FuzzedDataProvider.h>
namespace android {
+class RandomBinder : public BBinder {
+public:
+ RandomBinder(const String16& descriptor, std::vector<uint8_t>&& bytes);
+ const String16& getInterfaceDescriptor() const override;
+ status_t onTransact(uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags) override;
+
+private:
+ String16 mDescriptor;
+ // note may not all be used
+ std::vector<uint8_t> mBytes;
+ FuzzedDataProvider mProvider;
+};
+
// Get a random binder object for use in fuzzing.
//
// May return nullptr.
diff --git a/libs/binder/tests/parcel_fuzzer/random_binder.cpp b/libs/binder/tests/parcel_fuzzer/random_binder.cpp
index 8a1fecb..f41c35b 100644
--- a/libs/binder/tests/parcel_fuzzer/random_binder.cpp
+++ b/libs/binder/tests/parcel_fuzzer/random_binder.cpp
@@ -21,56 +21,52 @@
#include <binder/IInterface.h>
#include <binder/IServiceManager.h>
+size_t kRandomInterfaceLength = 50;
namespace android {
-class RandomBinder : public BBinder {
-public:
- RandomBinder(const String16& descriptor, std::vector<uint8_t>&& bytes)
- : mDescriptor(descriptor),
- mBytes(std::move(bytes)),
- mProvider(mBytes.data(), mBytes.size()) {}
- const String16& getInterfaceDescriptor() const override { return mDescriptor; }
+RandomBinder::RandomBinder(const String16& descriptor, std::vector<uint8_t>&& bytes)
+ : mDescriptor(descriptor),
+ mBytes(std::move(bytes)),
+ mProvider(mBytes.data(), mBytes.size()) {}
- status_t onTransact(uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags) override {
- (void)code;
- (void)data;
- (void)reply;
- (void)flags; // note - for maximum coverage even ignore if oneway
+const String16& RandomBinder::getInterfaceDescriptor() const {
+ return mDescriptor;
+}
- if (mProvider.ConsumeBool()) {
- return mProvider.ConsumeIntegral<status_t>();
- }
+status_t RandomBinder::onTransact(uint32_t code, const Parcel& data, Parcel* reply,
+ uint32_t flags) {
+ (void)code;
+ (void)data;
+ (void)reply;
+ (void)flags; // note - for maximum coverage even ignore if oneway
- if (reply == nullptr) return OK;
-
- // TODO: things we could do to increase state space
- // - also pull FDs and binders from 'data'
- // (optionally combine these into random parcel 'options')
- // - also pull FDs and binders from random parcel 'options'
- RandomParcelOptions options;
-
- // random output
- std::vector<uint8_t> subData = mProvider.ConsumeBytes<uint8_t>(
- mProvider.ConsumeIntegralInRange<size_t>(0, mProvider.remaining_bytes()));
- fillRandomParcel(reply, FuzzedDataProvider(subData.data(), subData.size()), &options);
-
- return OK;
+ if (mProvider.ConsumeBool()) {
+ return mProvider.ConsumeIntegral<status_t>();
}
-private:
- String16 mDescriptor;
+ if (reply == nullptr) return OK;
- // note may not all be used
- std::vector<uint8_t> mBytes;
- FuzzedDataProvider mProvider;
-};
+ // TODO: things we could do to increase state space
+ // - also pull FDs and binders from 'data'
+ // (optionally combine these into random parcel 'options')
+ // - also pull FDs and binders from random parcel 'options'
+ RandomParcelOptions options;
+
+ // random output
+ std::vector<uint8_t> subData = mProvider.ConsumeBytes<uint8_t>(
+ mProvider.ConsumeIntegralInRange<size_t>(0, mProvider.remaining_bytes()));
+ fillRandomParcel(reply, FuzzedDataProvider(subData.data(), subData.size()), &options);
+
+ return OK;
+}
sp<IBinder> getRandomBinder(FuzzedDataProvider* provider) {
auto makeFunc = provider->PickValueInArray<const std::function<sp<IBinder>()>>({
[&]() {
// descriptor is the length of a class name, e.g.
// "some.package.Foo"
- std::string str = provider->ConsumeRandomLengthString(100 /*max length*/);
+ std::string str =
+ provider->ConsumeRandomLengthString(kRandomInterfaceLength /*max length*/);
// arbitrarily consume remaining data to create a binder that can return
// random results - coverage guided fuzzer should ensure all of the remaining
diff --git a/libs/binder/tests/parcel_fuzzer/random_parcel.cpp b/libs/binder/tests/parcel_fuzzer/random_parcel.cpp
index 4e58dc4..62b8433 100644
--- a/libs/binder/tests/parcel_fuzzer/random_parcel.cpp
+++ b/libs/binder/tests/parcel_fuzzer/random_parcel.cpp
@@ -73,7 +73,7 @@
return;
}
- if (options->extraFds.size() > 0 && provider.ConsumeBool()) {
+ if (provider.ConsumeBool() && options->extraFds.size() > 0) {
const unique_fd& fd = options->extraFds.at(
provider.ConsumeIntegralInRange<size_t>(0,
options->extraFds.size() -
@@ -102,7 +102,7 @@
}
sp<IBinder> binder;
- if (options->extraBinders.size() > 0 && provider.ConsumeBool()) {
+ if (provider.ConsumeBool() && options->extraBinders.size() > 0) {
binder = options->extraBinders.at(
provider.ConsumeIntegralInRange<size_t>(0,
options->extraBinders
diff --git a/libs/binder/tests/parcel_fuzzer/random_parcel_seeds.cpp b/libs/binder/tests/parcel_fuzzer/random_parcel_seeds.cpp
index 7b3c806..fd9777a 100644
--- a/libs/binder/tests/parcel_fuzzer/random_parcel_seeds.cpp
+++ b/libs/binder/tests/parcel_fuzzer/random_parcel_seeds.cpp
@@ -14,16 +14,26 @@
* limitations under the License.
*/
+#include <linux/android/binder.h>
+
#include <android-base/logging.h>
+#include <binder/Parcel.h>
#include <binder/RecordedTransaction.h>
#include <fuzzseeds/random_parcel_seeds.h>
+#include <stack>
+#include <string>
#include "../../file.h"
using android::binder::borrowed_fd;
using android::binder::WriteFully;
+using std::stack;
+
+extern size_t kRandomInterfaceLength;
+// Keep this in sync with max_length in random_binder.cpp while creating a RandomBinder
+std::string kRandomInterfaceName(kRandomInterfaceLength, 'i');
namespace android {
namespace impl {
@@ -66,6 +76,162 @@
} // namespace impl
+struct ProviderMetadata {
+ size_t position;
+ size_t value;
+
+ ProviderMetadata() {
+ value = 0;
+ position = 0;
+ }
+};
+
+// Assuming current seed path is inside the fillRandomParcel function, start of the loop.
+void writeRandomBinder(borrowed_fd fd, vector<uint8_t>& fillParcelBuffer,
+ stack<ProviderMetadata>& remainingPositions) {
+ // Choose 2 index in array
+ size_t fillFuncIndex = 2;
+ impl::writeReversedBuffer(fillParcelBuffer, static_cast<size_t>(0), static_cast<size_t>(2),
+ fillFuncIndex);
+
+ // navigate to getRandomBinder. provide consume bool false
+ bool flag = false;
+ impl::writeReversedBuffer(fillParcelBuffer, flag);
+
+ // selecting RandomBinder, other binders in the list are not recorded as KernelObjects
+ size_t randomBinderIndex = 0;
+ impl::writeReversedBuffer(fillParcelBuffer, static_cast<size_t>(0), static_cast<size_t>(2),
+ randomBinderIndex);
+
+ // write random string of length 100 in actual buffer array.
+ CHECK(WriteFully(fd, kRandomInterfaceName.c_str(), kRandomInterfaceName.size())) << fd.get();
+
+ // These will be bytes which are used inside of RandomBinder
+ // simplest path for these bytes is going to be consume bool -> return random status
+ vector<uint8_t> randomBinderBuffer;
+
+ bool returnRandomInt = true;
+ impl::writeReversedBuffer(randomBinderBuffer, returnRandomInt);
+
+ status_t randomStatus = 0;
+ impl::writeReversedBuffer(randomBinderBuffer, randomStatus);
+
+ // write integral in range to consume bytes for random binder
+ ProviderMetadata providerData;
+ providerData.position = fillParcelBuffer.size();
+ providerData.value = randomBinderBuffer.size();
+ remainingPositions.push(providerData);
+
+ // Write to fd
+ CHECK(WriteFully(fd, randomBinderBuffer.data(), randomBinderBuffer.size())) << fd.get();
+}
+
+// Assuming current seed path is inside the fillRandomParcelFunction, start of the loop.
+void writeRandomFd(vector<uint8_t>& fillParcelBuffer) {
+ // path to random fd
+ size_t fillFuncIndex = 1;
+ impl::writeReversedBuffer(fillParcelBuffer, static_cast<size_t>(0), static_cast<size_t>(2),
+ fillFuncIndex);
+
+ bool flag = false;
+ impl::writeReversedBuffer(fillParcelBuffer, flag);
+
+ // go for /dev/null index 1
+ size_t fdIndex = 1;
+ impl::writeReversedBuffer(fillParcelBuffer, static_cast<size_t>(0), static_cast<size_t>(3),
+ fdIndex);
+}
+
+void writeParcelData(borrowed_fd fd, vector<uint8_t>& fillParcelBuffer,
+ stack<ProviderMetadata>& remainingPositions, const uint8_t* data, size_t start,
+ size_t length) {
+ // need to write parcel data till next offset with instructions to pick random bytes till offset
+ size_t fillFuncIndex = 0;
+ impl::writeReversedBuffer(fillParcelBuffer, static_cast<size_t>(0), static_cast<size_t>(2),
+ fillFuncIndex);
+
+ // provide how much bytes to read in control buffer
+ ProviderMetadata providerData;
+ providerData.position = fillParcelBuffer.size();
+ providerData.value = length;
+ remainingPositions.push(providerData);
+
+ // provide actual bytes
+ CHECK(WriteFully(fd, data + start, length)) << fd.get();
+}
+
+/**
+ * Generate sequence of copy data, write fd and write binder instructions and required data.
+ * Data which will be read using consumeBytes is written to fd directly. Data which is read in
+ * form integer is consumed from rear end FuzzedDataProvider. So insert it in fillParcelBuffer and
+ * then write to fd
+ */
+size_t regenerateParcel(borrowed_fd fd, vector<uint8_t>& fillParcelBuffer, const Parcel& p,
+ size_t dataSize, const vector<uint64_t>& objectOffsets) {
+ stack<ProviderMetadata> remainingPositions;
+ size_t copiedDataPosition = 0;
+ const uint8_t* parcelData = p.data();
+ size_t numBinders = 0;
+ size_t numFds = 0;
+
+ for (auto offset : objectOffsets) {
+ // Check what type of object is present here
+ const flat_binder_object* flatObject =
+ reinterpret_cast<const flat_binder_object*>(parcelData + offset);
+ // Copy till the object offset
+ writeParcelData(fd, fillParcelBuffer, remainingPositions, parcelData, copiedDataPosition,
+ offset - copiedDataPosition);
+ copiedDataPosition = offset;
+ if (flatObject->hdr.type == BINDER_TYPE_BINDER ||
+ flatObject->hdr.type == BINDER_TYPE_HANDLE) {
+ writeRandomBinder(fd, fillParcelBuffer, remainingPositions);
+ numBinders++;
+ // In case of binder, stability is written after the binder object.
+ // We want to move the copiedDataPosition further to account for this stability field
+ copiedDataPosition += sizeof(int32_t) + sizeof(flat_binder_object);
+ } else if (flatObject->hdr.type == BINDER_TYPE_FD) {
+ writeRandomFd(fillParcelBuffer);
+ numFds++;
+ copiedDataPosition += sizeof(flat_binder_object);
+ }
+ }
+
+ if (copiedDataPosition < dataSize) {
+ // copy remaining data from recorded parcel -> last Object to end of the data
+ writeParcelData(fd, fillParcelBuffer, remainingPositions, parcelData, copiedDataPosition,
+ dataSize - copiedDataPosition);
+ }
+
+ // We need to write bytes for selecting integer within range of 0 to provide.remaining_bytes()
+ // is called.
+ size_t totalWrittenBytes = dataSize - (sizeof(flat_binder_object) * objectOffsets.size()) -
+ (sizeof(int32_t) * numBinders) +
+ (kRandomInterfaceName.size() /*Interface String*/ + sizeof(bool) + sizeof(status_t)) *
+ numBinders;
+
+ // Code in fuzzService relies on provider.remaining_bytes() to select random bytes using
+ // consume integer. use the calculated remaining_bytes to generate byte buffer which can
+ // generate required fds and binders in fillRandomParcel function.
+ while (!remainingPositions.empty()) {
+ auto meta = remainingPositions.top();
+ remainingPositions.pop();
+ size_t remainingBytes = totalWrittenBytes + fillParcelBuffer.size() - meta.position;
+
+ vector<uint8_t> remReversedBytes;
+ impl::writeReversedBuffer(remReversedBytes, static_cast<size_t>(0), remainingBytes,
+ meta.value);
+ // Check the order of buffer which is being written
+ fillParcelBuffer.insert(fillParcelBuffer.end() - meta.position, remReversedBytes.begin(),
+ remReversedBytes.end());
+ }
+
+ return totalWrittenBytes;
+}
+
+/**
+ * Current corpus format
+ * |Reserved bytes(8)|parcel data|fillParcelBuffer|integralBuffer|
+ */
void generateSeedsFromRecording(borrowed_fd fd,
const binder::debug::RecordedTransaction& transaction) {
// Write Reserved bytes for future use
@@ -123,17 +289,9 @@
uint8_t writeHeaderInternal = 0;
impl::writeReversedBuffer(fillParcelBuffer, writeHeaderInternal);
- // Choose to write data in parcel
- size_t fillFuncIndex = 0;
- impl::writeReversedBuffer(fillParcelBuffer, static_cast<size_t>(0), static_cast<size_t>(2),
- fillFuncIndex);
-
- // Write parcel data size from recorded transaction
- size_t toWrite = transaction.getDataParcel().dataBufferSize();
- impl::writeReversedBuffer(fillParcelBuffer, static_cast<size_t>(0), toWrite, toWrite);
-
- // Write parcel data with size towrite from recorded transaction
- CHECK(WriteFully(fd, dataParcel.data(), toWrite)) << fd.get();
+ auto objectMetadata = transaction.getObjectOffsets();
+ size_t toWrite = regenerateParcel(fd, fillParcelBuffer, dataParcel, dataParcel.dataBufferSize(),
+ objectMetadata);
// Write Fill Parcel buffer size in integralBuffer so that fuzzService knows size of data
size_t subDataSize = toWrite + fillParcelBuffer.size();
diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp
index 19693e3..fb69fda 100644
--- a/libs/gui/BufferQueueProducer.cpp
+++ b/libs/gui/BufferQueueProducer.cpp
@@ -887,6 +887,9 @@
int callbackTicket = 0;
uint64_t currentFrameNumber = 0;
BufferItem item;
+ int connectedApi;
+ sp<Fence> lastQueuedFence;
+
{ // Autolock scope
std::lock_guard<std::mutex> lock(mCore->mMutex);
@@ -1056,6 +1059,13 @@
callbackTicket = mNextCallbackTicket++;
VALIDATE_CONSISTENCY();
+
+ connectedApi = mCore->mConnectedApi;
+ lastQueuedFence = std::move(mLastQueueBufferFence);
+
+ mLastQueueBufferFence = std::move(acquireFence);
+ mLastQueuedCrop = item.mCrop;
+ mLastQueuedTransform = item.mTransform;
} // Autolock scope
// It is okay not to clear the GraphicBuffer when the consumer is SurfaceFlinger because
@@ -1079,9 +1089,6 @@
// Call back without the main BufferQueue lock held, but with the callback
// lock held so we can ensure that callbacks occur in order
- int connectedApi;
- sp<Fence> lastQueuedFence;
-
{ // scope for the lock
std::unique_lock<std::mutex> lock(mCallbackMutex);
while (callbackTicket != mCurrentCallbackTicket) {
@@ -1094,13 +1101,6 @@
frameReplacedListener->onFrameReplaced(item);
}
- connectedApi = mCore->mConnectedApi;
- lastQueuedFence = std::move(mLastQueueBufferFence);
-
- mLastQueueBufferFence = std::move(acquireFence);
- mLastQueuedCrop = item.mCrop;
- mLastQueuedTransform = item.mTransform;
-
++mCurrentCallbackTicket;
mCallbackCondition.notify_all();
}
@@ -1653,9 +1653,10 @@
status_t BufferQueueProducer::getLastQueuedBuffer(sp<GraphicBuffer>* outBuffer,
sp<Fence>* outFence, float outTransformMatrix[16]) {
ATRACE_CALL();
- BQ_LOGV("getLastQueuedBuffer");
std::lock_guard<std::mutex> lock(mCore->mMutex);
+ BQ_LOGV("getLastQueuedBuffer, slot=%d", mCore->mLastQueuedSlot);
+
if (mCore->mLastQueuedSlot == BufferItem::INVALID_BUFFER_SLOT) {
*outBuffer = nullptr;
*outFence = Fence::NO_FENCE;
@@ -1679,10 +1680,11 @@
status_t BufferQueueProducer::getLastQueuedBuffer(sp<GraphicBuffer>* outBuffer, sp<Fence>* outFence,
Rect* outRect, uint32_t* outTransform) {
ATRACE_CALL();
- BQ_LOGV("getLastQueuedBuffer");
std::lock_guard<std::mutex> lock(mCore->mMutex);
- if (mCore->mLastQueuedSlot == BufferItem::INVALID_BUFFER_SLOT) {
+ BQ_LOGV("getLastQueuedBuffer, slot=%d", mCore->mLastQueuedSlot);
+ if (mCore->mLastQueuedSlot == BufferItem::INVALID_BUFFER_SLOT ||
+ mSlots[mCore->mLastQueuedSlot].mBufferState.isDequeued()) {
*outBuffer = nullptr;
*outFence = Fence::NO_FENCE;
return NO_ERROR;
diff --git a/libs/renderengine/Android.bp b/libs/renderengine/Android.bp
index 9494ee5..fd45840 100644
--- a/libs/renderengine/Android.bp
+++ b/libs/renderengine/Android.bp
@@ -48,6 +48,7 @@
static_libs: [
"libshaders",
"libtonemap",
+ "libsurfaceflinger_common",
],
local_include_dirs: ["include"],
export_include_dirs: ["include"],
diff --git a/libs/renderengine/benchmark/Android.bp b/libs/renderengine/benchmark/Android.bp
index 55c34cd..87e21c2 100644
--- a/libs/renderengine/benchmark/Android.bp
+++ b/libs/renderengine/benchmark/Android.bp
@@ -37,6 +37,7 @@
static_libs: [
"librenderengine",
"libshaders",
+ "libsurfaceflinger_common",
"libtonemap",
],
cflags: [
@@ -54,6 +55,7 @@
"libsync",
"libui",
"libutils",
+ "server_configurable_flags",
],
data: ["resources/*"],
diff --git a/libs/renderengine/skia/SkiaRenderEngine.cpp b/libs/renderengine/skia/SkiaRenderEngine.cpp
index 330cc66..6e393f0 100644
--- a/libs/renderengine/skia/SkiaRenderEngine.cpp
+++ b/libs/renderengine/skia/SkiaRenderEngine.cpp
@@ -53,6 +53,7 @@
#include <SkSurface.h>
#include <SkTileMode.h>
#include <android-base/stringprintf.h>
+#include <common/FlagManager.h>
#include <gui/FenceMonitor.h>
#include <gui/TraceUtils.h>
#include <include/gpu/ganesh/SkSurfaceGanesh.h>
@@ -419,6 +420,9 @@
mGraphicBufferExternalRefs[buffer->getId()]++;
if (const auto& iter = cache.find(buffer->getId()); iter == cache.end()) {
+ if (FlagManager::getInstance().renderable_buffer_usage()) {
+ isRenderable = buffer->getUsage() & GRALLOC_USAGE_HW_RENDER;
+ }
std::shared_ptr<AutoBackendTexture::LocalRef> imageTextureRef =
std::make_shared<AutoBackendTexture::LocalRef>(grContext,
buffer->toAHardwareBuffer(),
@@ -760,10 +764,11 @@
// save a snapshot of the activeSurface to use as input to the blur shaders
blurInput = activeSurface->makeImageSnapshot();
- // blit the offscreen framebuffer into the destination AHB, but only
- // if there are blur regions. backgroundBlurRadius blurs the entire
- // image below, so it can skip this step.
- if (layer.blurRegions.size()) {
+ // blit the offscreen framebuffer into the destination AHB. This ensures that
+ // even if the blurred image does not cover the screen (for example, during
+ // a rotation animation, or if blur regions are used), the entire screen is
+ // initialized.
+ if (layer.blurRegions.size() || FlagManager::getInstance().restore_blur_step()) {
SkPaint paint;
paint.setBlendMode(SkBlendMode::kSrc);
if (CC_UNLIKELY(mCapture->isCaptureRunning())) {
diff --git a/libs/renderengine/tests/Android.bp b/libs/renderengine/tests/Android.bp
index 50e166d..473e1d4 100644
--- a/libs/renderengine/tests/Android.bp
+++ b/libs/renderengine/tests/Android.bp
@@ -44,6 +44,7 @@
"librenderengine_mocks",
"libshaders",
"libtonemap",
+ "libsurfaceflinger_common",
],
header_libs: [
"libtonemap_headers",
@@ -61,5 +62,6 @@
"libsync",
"libui",
"libutils",
+ "server_configurable_flags",
],
}
diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp
index 77c2222..3e999c7 100644
--- a/services/inputflinger/dispatcher/InputDispatcher.cpp
+++ b/services/inputflinger/dispatcher/InputDispatcher.cpp
@@ -804,6 +804,20 @@
}
}
+std::pair<bool /*cancelPointers*/, bool /*cancelNonPointers*/> expandCancellationMode(
+ CancelationOptions::Mode mode) {
+ switch (mode) {
+ case CancelationOptions::Mode::CANCEL_ALL_EVENTS:
+ return {true, true};
+ case CancelationOptions::Mode::CANCEL_POINTER_EVENTS:
+ return {true, false};
+ case CancelationOptions::Mode::CANCEL_NON_POINTER_EVENTS:
+ return {false, true};
+ case CancelationOptions::Mode::CANCEL_FALLBACK_EVENTS:
+ return {false, true};
+ }
+}
+
} // namespace
// --- InputDispatcher ---
@@ -2078,7 +2092,9 @@
if (connection->status == Connection::Status::NORMAL) {
CancelationOptions options(CancelationOptions::Mode::CANCEL_ALL_EVENTS,
"application not responding");
- synthesizeCancelationEventsForConnectionLocked(connection, options);
+ synthesizeCancelationEventsForConnectionLocked(connection, options,
+ getWindowHandleLocked(
+ connection->getToken()));
}
}
@@ -3328,7 +3344,13 @@
void InputDispatcher::enqueueDispatchEntryLocked(const std::shared_ptr<Connection>& connection,
std::shared_ptr<const EventEntry> eventEntry,
const InputTarget& inputTarget) {
- // TODO(b/210460522): Verify all targets excluding global monitors are associated with a window.
+ const bool isKeyOrMotion = eventEntry->type == EventEntry::Type::KEY ||
+ eventEntry->type == EventEntry::Type::MOTION;
+ if (isKeyOrMotion && !inputTarget.windowHandle && !connection->monitor) {
+ LOG(FATAL) << "All InputTargets for non-monitors must be associated with a window; target: "
+ << inputTarget << " connection: " << connection->getInputChannelName()
+ << " entry: " << eventEntry->getDescription();
+ }
// This is a new event.
// Enqueue a new dispatch entry onto the outbound queue for this connection.
std::unique_ptr<DispatchEntry> dispatchEntry =
@@ -3977,22 +3999,78 @@
void InputDispatcher::synthesizeCancelationEventsForAllConnectionsLocked(
const CancelationOptions& options) {
- for (const auto& [token, connection] : mConnectionsByToken) {
- synthesizeCancelationEventsForConnectionLocked(connection, options);
+ // Cancel windows (i.e. non-monitors).
+ // A channel must have at least one window to receive any input. If a window was removed, the
+ // event streams directed to the window will already have been canceled during window removal.
+ // So there is no need to generate cancellations for connections without any windows.
+ const auto [cancelPointers, cancelNonPointers] = expandCancellationMode(options.mode);
+ // Generate cancellations for touched windows first. This is to avoid generating cancellations
+ // through a non-touched window if there are more than one window for an input channel.
+ if (cancelPointers) {
+ for (const auto& [displayId, touchState] : mTouchStatesByDisplay) {
+ if (options.displayId.has_value() && options.displayId != displayId) {
+ continue;
+ }
+ for (const auto& touchedWindow : touchState.windows) {
+ synthesizeCancelationEventsForWindowLocked(touchedWindow.windowHandle, options);
+ }
+ }
}
+ // Follow up by generating cancellations for all windows, because we don't explicitly track
+ // the windows that have an ongoing focus event stream.
+ if (cancelNonPointers) {
+ for (const auto& [_, handles] : mWindowHandlesByDisplay) {
+ for (const auto& windowHandle : handles) {
+ synthesizeCancelationEventsForWindowLocked(windowHandle, options);
+ }
+ }
+ }
+
+ // Cancel monitors.
+ synthesizeCancelationEventsForMonitorsLocked(options);
}
void InputDispatcher::synthesizeCancelationEventsForMonitorsLocked(
const CancelationOptions& options) {
for (const auto& [_, monitors] : mGlobalMonitorsByDisplay) {
for (const Monitor& monitor : monitors) {
- synthesizeCancelationEventsForConnectionLocked(monitor.connection, options);
+ synthesizeCancelationEventsForConnectionLocked(monitor.connection, options,
+ /*window=*/nullptr);
}
}
}
+void InputDispatcher::synthesizeCancelationEventsForWindowLocked(
+ const sp<WindowInfoHandle>& windowHandle, const CancelationOptions& options,
+ const std::shared_ptr<Connection>& connection) {
+ if (windowHandle == nullptr) {
+ LOG(FATAL) << __func__ << ": Window handle must not be null";
+ }
+ if (connection) {
+ // The connection can be optionally provided to avoid multiple lookups.
+ if (windowHandle->getToken() != connection->getToken()) {
+ LOG(FATAL) << __func__
+ << ": Wrong connection provided for window: " << windowHandle->getName();
+ }
+ }
+
+ std::shared_ptr<Connection> resolvedConnection =
+ connection ? connection : getConnectionLocked(windowHandle->getToken());
+ if (!resolvedConnection) {
+ LOG(DEBUG) << __func__ << "No connection found for window: " << windowHandle->getName();
+ return;
+ }
+ synthesizeCancelationEventsForConnectionLocked(resolvedConnection, options, windowHandle);
+}
+
void InputDispatcher::synthesizeCancelationEventsForConnectionLocked(
- const std::shared_ptr<Connection>& connection, const CancelationOptions& options) {
+ const std::shared_ptr<Connection>& connection, const CancelationOptions& options,
+ const sp<WindowInfoHandle>& window) {
+ if (!connection->monitor && window == nullptr) {
+ LOG(FATAL) << __func__
+ << ": Cannot send event to non-monitor channel without a window - channel: "
+ << connection->getInputChannelName();
+ }
if (connection->status != Connection::Status::NORMAL) {
return;
}
@@ -4029,10 +4107,7 @@
switch (cancelationEventEntry->type) {
case EventEntry::Type::KEY: {
const auto& keyEntry = static_cast<const KeyEntry&>(*cancelationEventEntry);
- const std::optional<int32_t> targetDisplay = keyEntry.displayId != ADISPLAY_ID_NONE
- ? std::make_optional(keyEntry.displayId)
- : std::nullopt;
- if (const auto& window = getWindowHandleLocked(token, targetDisplay); window) {
+ if (window) {
addWindowTargetLocked(window, InputTarget::DispatchMode::AS_IS,
/*targetFlags=*/{}, keyEntry.downTime, targets);
} else {
@@ -4043,11 +4118,7 @@
}
case EventEntry::Type::MOTION: {
const auto& motionEntry = static_cast<const MotionEntry&>(*cancelationEventEntry);
- const std::optional<int32_t> targetDisplay =
- motionEntry.displayId != ADISPLAY_ID_NONE
- ? std::make_optional(motionEntry.displayId)
- : std::nullopt;
- if (const auto& window = getWindowHandleLocked(token, targetDisplay); window) {
+ if (window) {
std::bitset<MAX_POINTER_ID + 1> pointerIds;
for (uint32_t pointerIndex = 0; pointerIndex < motionEntry.getPointerCount();
pointerIndex++) {
@@ -4121,7 +4192,12 @@
connection->getInputChannelName().c_str(), downEvents.size());
}
- sp<WindowInfoHandle> windowHandle = getWindowHandleLocked(connection->getToken());
+ const auto [_, touchedWindowState, displayId] =
+ findTouchStateWindowAndDisplayLocked(connection->getToken());
+ if (touchedWindowState == nullptr) {
+ LOG(FATAL) << __func__ << ": Touch state is out of sync: No touched window for token";
+ }
+ const auto& windowHandle = touchedWindowState->windowHandle;
const bool wasEmpty = connection->outboundQueue.empty();
for (std::unique_ptr<EventEntry>& downEventEntry : downEvents) {
@@ -4175,17 +4251,6 @@
}
}
-void InputDispatcher::synthesizeCancelationEventsForWindowLocked(
- const sp<WindowInfoHandle>& windowHandle, const CancelationOptions& options) {
- if (windowHandle != nullptr) {
- std::shared_ptr<Connection> wallpaperConnection =
- getConnectionLocked(windowHandle->getToken());
- if (wallpaperConnection != nullptr) {
- synthesizeCancelationEventsForConnectionLocked(wallpaperConnection, options);
- }
- }
-}
-
std::unique_ptr<MotionEntry> InputDispatcher::splitMotionEvent(
const MotionEntry& originalMotionEntry, std::bitset<MAX_POINTER_ID + 1> pointerIds,
nsecs_t splitDownTime) {
@@ -5195,6 +5260,7 @@
// Copy old handles for release if they are no longer present.
const std::vector<sp<WindowInfoHandle>> oldWindowHandles = getWindowHandlesLocked(displayId);
+ const sp<WindowInfoHandle> removedFocusedWindowHandle = getFocusedWindowHandleLocked(displayId);
updateWindowHandlesForDisplayLocked(windowInfoHandles, displayId);
@@ -5203,7 +5269,7 @@
std::optional<FocusResolver::FocusChanges> changes =
mFocusResolver.setInputWindows(displayId, windowHandles);
if (changes) {
- onFocusChangedLocked(*changes);
+ onFocusChangedLocked(*changes, removedFocusedWindowHandle);
}
std::unordered_map<int32_t, TouchState>::iterator stateIt =
@@ -5215,19 +5281,16 @@
if (getWindowHandleLocked(touchedWindow.windowHandle) == nullptr) {
LOG(INFO) << "Touched window was removed: " << touchedWindow.windowHandle->getName()
<< " in display %" << displayId;
- std::shared_ptr<Connection> touchedConnection =
- getConnectionLocked(touchedWindow.windowHandle->getToken());
- if (touchedConnection != nullptr) {
- CancelationOptions options(CancelationOptions::Mode::CANCEL_POINTER_EVENTS,
- "touched window was removed");
- synthesizeCancelationEventsForConnectionLocked(touchedConnection, options);
- // Since we are about to drop the touch, cancel the events for the wallpaper as
- // well.
- if (touchedWindow.targetFlags.test(InputTarget::Flags::FOREGROUND) &&
- touchedWindow.windowHandle->getInfo()->inputConfig.test(
- gui::WindowInfo::InputConfig::DUPLICATE_TOUCH_TO_WALLPAPER)) {
- sp<WindowInfoHandle> wallpaper = state.getWallpaperWindow();
- synthesizeCancelationEventsForWindowLocked(wallpaper, options);
+ CancelationOptions options(CancelationOptions::Mode::CANCEL_POINTER_EVENTS,
+ "touched window was removed");
+ synthesizeCancelationEventsForWindowLocked(touchedWindow.windowHandle, options);
+ // Since we are about to drop the touch, cancel the events for the wallpaper as
+ // well.
+ if (touchedWindow.targetFlags.test(InputTarget::Flags::FOREGROUND) &&
+ touchedWindow.windowHandle->getInfo()->inputConfig.test(
+ gui::WindowInfo::InputConfig::DUPLICATE_TOUCH_TO_WALLPAPER)) {
+ if (const auto& ww = state.getWallpaperWindow(); ww) {
+ synthesizeCancelationEventsForWindowLocked(ww, options);
}
}
state.windows.erase(state.windows.begin() + i);
@@ -5325,14 +5388,16 @@
sp<IBinder> oldFocusedWindowToken =
mFocusResolver.getFocusedWindowToken(mFocusedDisplayId);
if (oldFocusedWindowToken != nullptr) {
- std::shared_ptr<Connection> connection = getConnectionLocked(oldFocusedWindowToken);
- if (connection != nullptr) {
- CancelationOptions
- options(CancelationOptions::Mode::CANCEL_NON_POINTER_EVENTS,
- "The display which contains this window no longer has focus.");
- options.displayId = ADISPLAY_ID_NONE;
- synthesizeCancelationEventsForConnectionLocked(connection, options);
+ const auto windowHandle =
+ getWindowHandleLocked(oldFocusedWindowToken, mFocusedDisplayId);
+ if (windowHandle == nullptr) {
+ LOG(FATAL) << __func__ << ": Previously focused token did not have a window";
}
+ CancelationOptions
+ options(CancelationOptions::Mode::CANCEL_NON_POINTER_EVENTS,
+ "The display which contains this window no longer has focus.");
+ options.displayId = ADISPLAY_ID_NONE;
+ synthesizeCancelationEventsForWindowLocked(windowHandle, options);
}
mFocusedDisplayId = displayId;
@@ -5512,9 +5577,10 @@
}
const int32_t deviceId = *deviceIds.begin();
- sp<WindowInfoHandle> toWindowHandle = getWindowHandleLocked(toToken, displayId);
- if (toWindowHandle == nullptr) {
- ALOGW("Cannot transfer touch because to window not found.");
+ const sp<WindowInfoHandle> fromWindowHandle = touchedWindow->windowHandle;
+ const sp<WindowInfoHandle> toWindowHandle = getWindowHandleLocked(toToken, displayId);
+ if (!toWindowHandle) {
+ ALOGW("Cannot transfer touch because the transfer target window was not found.");
return false;
}
@@ -5527,7 +5593,6 @@
// Erase old window.
ftl::Flags<InputTarget::Flags> oldTargetFlags = touchedWindow->targetFlags;
std::vector<PointerProperties> pointers = touchedWindow->getTouchingPointers(deviceId);
- sp<WindowInfoHandle> fromWindowHandle = touchedWindow->windowHandle;
state->removeWindowByToken(fromToken);
// Add new window.
@@ -5559,7 +5624,7 @@
fromConnection->inputState.mergePointerStateTo(toConnection->inputState);
CancelationOptions options(CancelationOptions::Mode::CANCEL_POINTER_EVENTS,
"transferring touch from this window to another window");
- synthesizeCancelationEventsForConnectionLocked(fromConnection, options);
+ synthesizeCancelationEventsForWindowLocked(fromWindowHandle, options, fromConnection);
synthesizePointerDownEventsForConnectionLocked(downTimeInTarget, toConnection,
newTargetFlags);
@@ -6041,12 +6106,10 @@
std::string canceledWindows;
for (const TouchedWindow& w : state.windows) {
- const std::shared_ptr<Connection> connection =
- getConnectionLocked(w.windowHandle->getToken());
- if (connection != nullptr && connection->getToken() != token) {
- synthesizeCancelationEventsForConnectionLocked(connection, options);
+ if (w.windowHandle->getToken() != token) {
+ synthesizeCancelationEventsForWindowLocked(w.windowHandle, options);
canceledWindows += canceledWindows.empty() ? "[" : ", ";
- canceledWindows += connection->getInputChannelName();
+ canceledWindows += w.windowHandle->getName();
}
}
canceledWindows += canceledWindows.empty() ? "[]" : "]";
@@ -6207,8 +6270,18 @@
}
traceWaitQueueLength(*connection);
if (fallbackKeyEntry && connection->status == Connection::Status::NORMAL) {
- const InputTarget target{.connection = connection, .flags = dispatchEntry->targetFlags};
- enqueueDispatchEntryLocked(connection, std::move(fallbackKeyEntry), target);
+ const auto windowHandle = getWindowHandleLocked(connection->getToken());
+ // Only dispatch fallbacks if there is a window for the connection.
+ if (windowHandle != nullptr) {
+ const auto inputTarget =
+ createInputTargetLocked(windowHandle, InputTarget::DispatchMode::AS_IS,
+ dispatchEntry->targetFlags,
+ fallbackKeyEntry->downTime);
+ if (inputTarget.has_value()) {
+ enqueueDispatchEntryLocked(connection, std::move(fallbackKeyEntry),
+ *inputTarget);
+ }
+ }
}
releaseDispatchEntry(std::move(dispatchEntry));
}
@@ -6440,14 +6513,18 @@
mLock.lock();
- // Cancel the fallback key.
+ // Cancel the fallback key, but only if we still have a window for the channel.
+ // It could have been removed during the policy call.
if (*fallbackKeyCode != AKEYCODE_UNKNOWN) {
- CancelationOptions options(CancelationOptions::Mode::CANCEL_FALLBACK_EVENTS,
- "application handled the original non-fallback key "
- "or is no longer a foreground target, "
- "canceling previously dispatched fallback key");
- options.keyCode = *fallbackKeyCode;
- synthesizeCancelationEventsForConnectionLocked(connection, options);
+ const auto windowHandle = getWindowHandleLocked(connection->getToken());
+ if (windowHandle != nullptr) {
+ CancelationOptions options(CancelationOptions::Mode::CANCEL_FALLBACK_EVENTS,
+ "application handled the original non-fallback key "
+ "or is no longer a foreground target, "
+ "canceling previously dispatched fallback key");
+ options.keyCode = *fallbackKeyCode;
+ synthesizeCancelationEventsForWindowLocked(windowHandle, options, connection);
+ }
}
connection->inputState.removeFallbackKey(originalKeyCode);
}
@@ -6523,10 +6600,13 @@
}
}
- CancelationOptions options(CancelationOptions::Mode::CANCEL_FALLBACK_EVENTS,
- "canceling fallback, policy no longer desires it");
- options.keyCode = *fallbackKeyCode;
- synthesizeCancelationEventsForConnectionLocked(connection, options);
+ const auto windowHandle = getWindowHandleLocked(connection->getToken());
+ if (windowHandle != nullptr) {
+ CancelationOptions options(CancelationOptions::Mode::CANCEL_FALLBACK_EVENTS,
+ "canceling fallback, policy no longer desires it");
+ options.keyCode = *fallbackKeyCode;
+ synthesizeCancelationEventsForWindowLocked(windowHandle, options, connection);
+ }
fallback = false;
*fallbackKeyCode = AKEYCODE_UNKNOWN;
@@ -6665,15 +6745,19 @@
mLooper->wake();
}
-void InputDispatcher::onFocusChangedLocked(const FocusResolver::FocusChanges& changes) {
+void InputDispatcher::onFocusChangedLocked(const FocusResolver::FocusChanges& changes,
+ const sp<WindowInfoHandle> removedFocusedWindowHandle) {
if (changes.oldFocus) {
- std::shared_ptr<Connection> focusedConnection = getConnectionLocked(changes.oldFocus);
- if (focusedConnection) {
- CancelationOptions options(CancelationOptions::Mode::CANCEL_NON_POINTER_EVENTS,
- "focus left window");
- synthesizeCancelationEventsForConnectionLocked(focusedConnection, options);
- enqueueFocusEventLocked(changes.oldFocus, /*hasFocus=*/false, changes.reason);
+ const auto resolvedWindow = removedFocusedWindowHandle != nullptr
+ ? removedFocusedWindowHandle
+ : getWindowHandleLocked(changes.oldFocus, changes.displayId);
+ if (resolvedWindow == nullptr) {
+ LOG(FATAL) << __func__ << ": Previously focused token did not have a window";
}
+ CancelationOptions options(CancelationOptions::Mode::CANCEL_NON_POINTER_EVENTS,
+ "focus left window");
+ synthesizeCancelationEventsForWindowLocked(resolvedWindow, options);
+ enqueueFocusEventLocked(changes.oldFocus, /*hasFocus=*/false, changes.reason);
}
if (changes.newFocus) {
resetNoFocusedWindowTimeoutLocked();
diff --git a/services/inputflinger/dispatcher/InputDispatcher.h b/services/inputflinger/dispatcher/InputDispatcher.h
index dcd1566..f2fd0ca 100644
--- a/services/inputflinger/dispatcher/InputDispatcher.h
+++ b/services/inputflinger/dispatcher/InputDispatcher.h
@@ -615,18 +615,21 @@
REQUIRES(mLock);
void synthesizeCancelationEventsForMonitorsLocked(const CancelationOptions& options)
REQUIRES(mLock);
- void synthesizeCancelationEventsForConnectionLocked(
- const std::shared_ptr<Connection>& connection, const CancelationOptions& options)
+ void synthesizeCancelationEventsForWindowLocked(const sp<gui::WindowInfoHandle>&,
+ const CancelationOptions&,
+ const std::shared_ptr<Connection>& = nullptr)
REQUIRES(mLock);
+ // This is a convenience function used to generate cancellation for a connection without having
+ // to check whether it's a monitor or a window. For non-monitors, the window handle must not be
+ // null. Always prefer the "-ForWindow" method above when explicitly dealing with windows.
+ void synthesizeCancelationEventsForConnectionLocked(
+ const std::shared_ptr<Connection>& connection, const CancelationOptions& options,
+ const sp<gui::WindowInfoHandle>& window) REQUIRES(mLock);
void synthesizePointerDownEventsForConnectionLocked(
const nsecs_t downTime, const std::shared_ptr<Connection>& connection,
ftl::Flags<InputTarget::Flags> targetFlags) REQUIRES(mLock);
- void synthesizeCancelationEventsForWindowLocked(
- const sp<android::gui::WindowInfoHandle>& windowHandle,
- const CancelationOptions& options) REQUIRES(mLock);
-
// Splitting motion events across windows. When splitting motion event for a target,
// splitDownTime refers to the time of first 'down' event on that particular target
std::unique_ptr<MotionEntry> splitMotionEvent(const MotionEntry& originalMotionEntry,
@@ -653,7 +656,9 @@
bool handled, nsecs_t consumeTime) REQUIRES(mLock);
void doInterceptKeyBeforeDispatchingCommand(const sp<IBinder>& focusedWindowToken,
const KeyEntry& entry) REQUIRES(mLock);
- void onFocusChangedLocked(const FocusResolver::FocusChanges& changes) REQUIRES(mLock);
+ void onFocusChangedLocked(const FocusResolver::FocusChanges& changes,
+ const sp<gui::WindowInfoHandle> removedFocusedWindowHandle = nullptr)
+ REQUIRES(mLock);
void sendFocusChangedCommandLocked(const sp<IBinder>& oldToken, const sp<IBinder>& newToken)
REQUIRES(mLock);
void sendDropWindowCommandLocked(const sp<IBinder>& token, float x, float y) REQUIRES(mLock);
diff --git a/services/inputflinger/dispatcher/trace/InputTracer.cpp b/services/inputflinger/dispatcher/trace/InputTracer.cpp
index b065729..8a855c2 100644
--- a/services/inputflinger/dispatcher/trace/InputTracer.cpp
+++ b/services/inputflinger/dispatcher/trace/InputTracer.cpp
@@ -154,19 +154,21 @@
void InputTracer::threadLoop() {
androidSetThreadName("InputTracer");
+ std::vector<const EventState> eventsToTrace;
+ std::vector<const WindowDispatchArgs> dispatchEventsToTrace;
+
while (true) {
- std::vector<const EventState> eventsToTrace;
- std::vector<const WindowDispatchArgs> dispatchEventsToTrace;
- {
+ { // acquire lock
std::unique_lock lock(mLock);
base::ScopedLockAssertion assumeLocked(mLock);
+
+ // Wait until we need to process more events or exit.
+ mThreadWakeCondition.wait(lock, [&]() REQUIRES(mLock) {
+ return mThreadExit || !mTraceQueue.empty() || !mDispatchTraceQueue.empty();
+ });
if (mThreadExit) {
return;
}
- if (mTraceQueue.empty() && mDispatchTraceQueue.empty()) {
- // Wait indefinitely until the thread is awoken.
- mThreadWakeCondition.wait(lock);
- }
mTraceQueue.swap(eventsToTrace);
mDispatchTraceQueue.swap(dispatchEventsToTrace);
diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp
index 0582649..9608210 100644
--- a/services/inputflinger/reader/InputReader.cpp
+++ b/services/inputflinger/reader/InputReader.cpp
@@ -164,19 +164,6 @@
std::swap(notifyArgs, mPendingArgs);
} // release lock
- // Send out a message that the describes the changed input devices.
- if (inputDevicesChanged) {
- mPolicy->notifyInputDevicesChanged(inputDevices);
- }
-
- // Notify the policy of the start of every new stylus gesture outside the lock.
- for (const auto& args : notifyArgs) {
- const auto* motionArgs = std::get_if<NotifyMotionArgs>(&args);
- if (motionArgs != nullptr && isStylusPointerGestureStart(*motionArgs)) {
- mPolicy->notifyStylusGestureStarted(motionArgs->deviceId, motionArgs->eventTime);
- }
- }
-
// Flush queued events out to the listener.
// This must happen outside of the lock because the listener could potentially call
// back into the InputReader's methods, such as getScanCodeState, or become blocked
@@ -187,6 +174,21 @@
for (const NotifyArgs& args : notifyArgs) {
mNextListener.notify(args);
}
+
+ // Notify the policy that input devices have changed.
+ // This must be done after flushing events down the listener chain to ensure that the rest of
+ // the listeners are synchronized with the changes before the policy reacts to them.
+ if (inputDevicesChanged) {
+ mPolicy->notifyInputDevicesChanged(inputDevices);
+ }
+
+ // Notify the policy of the start of every new stylus gesture.
+ for (const auto& args : notifyArgs) {
+ const auto* motionArgs = std::get_if<NotifyMotionArgs>(&args);
+ if (motionArgs != nullptr && isStylusPointerGestureStart(*motionArgs)) {
+ mPolicy->notifyStylusGestureStarted(motionArgs->deviceId, motionArgs->eventTime);
+ }
+ }
}
std::list<NotifyArgs> InputReader::processEventsLocked(const RawEvent* rawEvents, size_t count) {
diff --git a/services/inputflinger/tests/Android.bp b/services/inputflinger/tests/Android.bp
index 0544757..a26153e 100644
--- a/services/inputflinger/tests/Android.bp
+++ b/services/inputflinger/tests/Android.bp
@@ -45,6 +45,7 @@
"EventHub_test.cpp",
"FakeEventHub.cpp",
"FakeInputReaderPolicy.cpp",
+ "FakeInputTracingBackend.cpp",
"FakePointerController.cpp",
"FocusResolver_test.cpp",
"GestureConverter_test.cpp",
diff --git a/services/inputflinger/tests/FakeInputReaderPolicy.cpp b/services/inputflinger/tests/FakeInputReaderPolicy.cpp
index 88f514f..9e93712 100644
--- a/services/inputflinger/tests/FakeInputReaderPolicy.cpp
+++ b/services/inputflinger/tests/FakeInputReaderPolicy.cpp
@@ -41,15 +41,21 @@
}
void FakeInputReaderPolicy::assertStylusGestureNotified(int32_t deviceId) {
- std::scoped_lock lock(mLock);
- ASSERT_TRUE(mStylusGestureNotified);
- ASSERT_EQ(deviceId, *mStylusGestureNotified);
- mStylusGestureNotified.reset();
+ std::unique_lock lock(mLock);
+ base::ScopedLockAssertion assumeLocked(mLock);
+
+ const bool success =
+ mStylusGestureNotifiedCondition.wait_for(lock, WAIT_TIMEOUT, [this]() REQUIRES(mLock) {
+ return mDeviceIdOfNotifiedStylusGesture.has_value();
+ });
+ ASSERT_TRUE(success) << "Timed out waiting for stylus gesture to be notified";
+ ASSERT_EQ(deviceId, *mDeviceIdOfNotifiedStylusGesture);
+ mDeviceIdOfNotifiedStylusGesture.reset();
}
void FakeInputReaderPolicy::assertStylusGestureNotNotified() {
std::scoped_lock lock(mLock);
- ASSERT_FALSE(mStylusGestureNotified);
+ ASSERT_FALSE(mDeviceIdOfNotifiedStylusGesture);
}
void FakeInputReaderPolicy::clearViewports() {
@@ -258,7 +264,8 @@
void FakeInputReaderPolicy::notifyStylusGestureStarted(int32_t deviceId, nsecs_t eventTime) {
std::scoped_lock lock(mLock);
- mStylusGestureNotified = deviceId;
+ mDeviceIdOfNotifiedStylusGesture = deviceId;
+ mStylusGestureNotifiedCondition.notify_all();
}
std::optional<DisplayViewport> FakeInputReaderPolicy::getPointerViewportForAssociatedDisplay(
diff --git a/services/inputflinger/tests/FakeInputReaderPolicy.h b/services/inputflinger/tests/FakeInputReaderPolicy.h
index 4ef9c3e..da5085d 100644
--- a/services/inputflinger/tests/FakeInputReaderPolicy.h
+++ b/services/inputflinger/tests/FakeInputReaderPolicy.h
@@ -102,9 +102,11 @@
bool mInputDevicesChanged GUARDED_BY(mLock){false};
std::vector<DisplayViewport> mViewports;
TouchAffineTransformation transform;
- std::optional<int32_t /*deviceId*/> mStylusGestureNotified GUARDED_BY(mLock){};
bool mIsInputMethodConnectionActive{false};
+ std::condition_variable mStylusGestureNotifiedCondition;
+ std::optional<DeviceId> mDeviceIdOfNotifiedStylusGesture GUARDED_BY(mLock){};
+
uint32_t mNextPointerCaptureSequenceNumber{0};
};
diff --git a/services/inputflinger/tests/FakeInputTracingBackend.cpp b/services/inputflinger/tests/FakeInputTracingBackend.cpp
new file mode 100644
index 0000000..1d27107
--- /dev/null
+++ b/services/inputflinger/tests/FakeInputTracingBackend.cpp
@@ -0,0 +1,111 @@
+/*
+ * Copyright 2024 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.
+ */
+
+#include "FakeInputTracingBackend.h"
+
+#include <android-base/logging.h>
+#include <utils/Errors.h>
+
+namespace android::inputdispatcher {
+
+namespace {
+
+// Use a larger timeout while waiting for events to be traced, compared to the timeout used while
+// waiting to receive events through the input channel. Events are traced from a separate thread,
+// which does not have the same high thread priority as the InputDispatcher's thread, so the tracer
+// is expected to lag behind the Dispatcher at times.
+constexpr auto TRACE_TIMEOUT = std::chrono::seconds(5);
+
+base::ResultError<> error(const std::ostringstream& ss) {
+ return base::ResultError(ss.str(), BAD_VALUE);
+}
+
+} // namespace
+
+// --- VerifyingTrace ---
+
+void VerifyingTrace::expectKeyDispatchTraced(const KeyEvent& event) {
+ std::scoped_lock lock(mLock);
+ mExpectedEvents.emplace_back(event);
+}
+
+void VerifyingTrace::expectMotionDispatchTraced(const MotionEvent& event) {
+ std::scoped_lock lock(mLock);
+ mExpectedEvents.emplace_back(event);
+}
+
+void VerifyingTrace::verifyExpectedEventsTraced() {
+ std::unique_lock lock(mLock);
+ base::ScopedLockAssertion assumeLocked(mLock);
+
+ base::Result<void> result;
+ mEventTracedCondition.wait_for(lock, TRACE_TIMEOUT, [&]() REQUIRES(mLock) {
+ for (const auto& expectedEvent : mExpectedEvents) {
+ std::visit([&](const auto& event)
+ REQUIRES(mLock) { result = verifyEventTraced(event); },
+ expectedEvent);
+ if (!result.ok()) {
+ return false;
+ }
+ }
+ return true;
+ });
+
+ EXPECT_TRUE(result.ok())
+ << "Timed out waiting for all expected events to be traced successfully: "
+ << result.error().message();
+}
+
+void VerifyingTrace::reset() {
+ std::scoped_lock lock(mLock);
+ mTracedEvents.clear();
+ mExpectedEvents.clear();
+}
+
+template <typename Event>
+base::Result<void> VerifyingTrace::verifyEventTraced(const Event& expectedEvent) const {
+ std::ostringstream msg;
+
+ auto tracedEventsIt = mTracedEvents.find(expectedEvent.getId());
+ if (tracedEventsIt == mTracedEvents.end()) {
+ msg << "Expected event with ID 0x" << std::hex << expectedEvent.getId()
+ << " to be traced, but it was not.\n"
+ << "Expected event: " << expectedEvent;
+ return error(msg);
+ }
+
+ return {};
+}
+
+// --- FakeInputTracingBackend ---
+
+void FakeInputTracingBackend::traceKeyEvent(const trace::TracedKeyEvent& event) const {
+ {
+ std::scoped_lock lock(mTrace->mLock);
+ mTrace->mTracedEvents.emplace(event.id);
+ }
+ mTrace->mEventTracedCondition.notify_all();
+}
+
+void FakeInputTracingBackend::traceMotionEvent(const trace::TracedMotionEvent& event) const {
+ {
+ std::scoped_lock lock(mTrace->mLock);
+ mTrace->mTracedEvents.emplace(event.id);
+ }
+ mTrace->mEventTracedCondition.notify_all();
+}
+
+} // namespace android::inputdispatcher
diff --git a/services/inputflinger/tests/FakeInputTracingBackend.h b/services/inputflinger/tests/FakeInputTracingBackend.h
new file mode 100644
index 0000000..e5dd546
--- /dev/null
+++ b/services/inputflinger/tests/FakeInputTracingBackend.h
@@ -0,0 +1,88 @@
+/*
+ * Copyright 2024 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 "../dispatcher/trace/InputTracingBackendInterface.h"
+
+#include <android-base/result.h>
+#include <android-base/thread_annotations.h>
+#include <gtest/gtest.h>
+#include <input/Input.h>
+#include <condition_variable>
+#include <memory>
+#include <mutex>
+#include <unordered_set>
+#include <vector>
+
+namespace android::inputdispatcher {
+
+/**
+ * A class representing an input trace, used to make assertions on what was traced by
+ * InputDispatcher in tests. This class is thread-safe.
+ */
+class VerifyingTrace {
+public:
+ VerifyingTrace() = default;
+
+ /** Add an expectation for a key event to be traced. */
+ void expectKeyDispatchTraced(const KeyEvent& event);
+
+ /** Add an expectation for a motion event to be traced. */
+ void expectMotionDispatchTraced(const MotionEvent& event);
+
+ /**
+ * Wait and verify that all expected events are traced.
+ * This is a lenient verifier that does not expect the events to be traced in the order
+ * that the events were expected, and does not fail if there are events that are traced that
+ * were not expected. Verifying does not clear the expectations.
+ */
+ void verifyExpectedEventsTraced();
+
+ /** Reset the trace and clear all expectations. */
+ void reset();
+
+private:
+ std::mutex mLock;
+ std::condition_variable mEventTracedCondition;
+ std::unordered_set<uint32_t /*eventId*/> mTracedEvents GUARDED_BY(mLock);
+ std::vector<std::variant<KeyEvent, MotionEvent>> mExpectedEvents GUARDED_BY(mLock);
+
+ friend class FakeInputTracingBackend;
+
+ // Helper to verify that the given event appears as expected in the trace. If the verification
+ // fails, the error message describes why.
+ template <typename Event>
+ base::Result<void> verifyEventTraced(const Event&) const REQUIRES(mLock);
+};
+
+/**
+ * A backend implementation for input tracing that records events to the provided
+ * VerifyingTrace used for testing.
+ */
+class FakeInputTracingBackend : public trace::InputTracingBackendInterface {
+public:
+ FakeInputTracingBackend(std::shared_ptr<VerifyingTrace> trace) : mTrace(trace) {}
+
+private:
+ std::shared_ptr<VerifyingTrace> mTrace;
+
+ void traceKeyEvent(const trace::TracedKeyEvent& entry) const override;
+ void traceMotionEvent(const trace::TracedMotionEvent& entry) const override;
+ void traceWindowDispatch(const WindowDispatchArgs& entry) const override {}
+};
+
+} // namespace android::inputdispatcher
diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp
index ab667d0..1c37da0 100644
--- a/services/inputflinger/tests/InputDispatcher_test.cpp
+++ b/services/inputflinger/tests/InputDispatcher_test.cpp
@@ -17,6 +17,7 @@
#include "../dispatcher/InputDispatcher.h"
#include "../BlockingQueue.h"
#include "FakeApplicationHandle.h"
+#include "FakeInputTracingBackend.h"
#include "TestEventMatchers.h"
#include <NotifyArgsBuilders.h>
@@ -660,14 +661,22 @@
// --- InputDispatcherTest ---
+// The trace is a global variable for now, to avoid having to pass it into all of the
+// FakeWindowHandles created throughout the tests.
+// TODO(b/210460522): Update the tests to avoid the need to have the trace be a global variable.
+static std::shared_ptr<VerifyingTrace> gVerifyingTrace = std::make_shared<VerifyingTrace>();
+
class InputDispatcherTest : public testing::Test {
protected:
std::unique_ptr<FakeInputDispatcherPolicy> mFakePolicy;
std::unique_ptr<InputDispatcher> mDispatcher;
void SetUp() override {
+ gVerifyingTrace->reset();
mFakePolicy = std::make_unique<FakeInputDispatcherPolicy>();
- mDispatcher = std::make_unique<InputDispatcher>(*mFakePolicy, nullptr);
+ mDispatcher = std::make_unique<InputDispatcher>(*mFakePolicy,
+ std::make_unique<FakeInputTracingBackend>(
+ gVerifyingTrace));
mDispatcher->setInputDispatchMode(/*enabled=*/true, /*frozen=*/false);
// Start InputDispatcher thread
@@ -675,6 +684,7 @@
}
void TearDown() override {
+ ASSERT_NO_FATAL_FAILURE(gVerifyingTrace->verifyExpectedEventsTraced());
ASSERT_EQ(OK, mDispatcher->stop());
mFakePolicy.reset();
mDispatcher.reset();
@@ -1397,11 +1407,7 @@
}
std::pair<std::optional<uint32_t>, std::unique_ptr<InputEvent>> receiveEvent() {
- if (mInputReceiver == nullptr) {
- ADD_FAILURE() << "Invalid receive event on window with no receiver";
- return std::make_pair(std::nullopt, nullptr);
- }
- return mInputReceiver->receiveEvent(CONSUME_TIMEOUT_EVENT_EXPECTED);
+ return receive();
}
void finishEvent(uint32_t sequenceNum) {
@@ -1439,6 +1445,7 @@
int getChannelFd() { return mInputReceiver->getChannelFd(); }
+ // FakeWindowHandle uses this consume method to ensure received events are added to the trace.
std::unique_ptr<InputEvent> consume(std::chrono::milliseconds timeout, bool handled = true) {
if (mInputReceiver == nullptr) {
LOG(FATAL) << "Cannot consume event from a window with no input event receiver";
@@ -1447,6 +1454,7 @@
if (event == nullptr) {
ADD_FAILURE() << "Consume failed: no event";
}
+ expectReceivedEventTraced(event);
return event;
}
@@ -1456,6 +1464,37 @@
std::shared_ptr<FakeInputReceiver> mInputReceiver;
static std::atomic<int32_t> sId; // each window gets a unique id, like in surfaceflinger
friend class sp<FakeWindowHandle>;
+
+ // FakeWindowHandle uses this receive method to ensure received events are added to the trace.
+ std::pair<std::optional<uint32_t /*seq*/>, std::unique_ptr<InputEvent>> receive() {
+ if (mInputReceiver == nullptr) {
+ ADD_FAILURE() << "Invalid receive event on window with no receiver";
+ return std::make_pair(std::nullopt, nullptr);
+ }
+ auto out = mInputReceiver->receiveEvent(CONSUME_TIMEOUT_EVENT_EXPECTED);
+ const auto& [_, event] = out;
+ expectReceivedEventTraced(event);
+ return std::move(out);
+ }
+
+ void expectReceivedEventTraced(const std::unique_ptr<InputEvent>& event) {
+ if (!event) {
+ return;
+ }
+
+ switch (event->getType()) {
+ case InputEventType::KEY: {
+ gVerifyingTrace->expectKeyDispatchTraced(static_cast<KeyEvent&>(*event));
+ break;
+ }
+ case InputEventType::MOTION: {
+ gVerifyingTrace->expectMotionDispatchTraced(static_cast<MotionEvent&>(*event));
+ break;
+ }
+ default:
+ break;
+ }
+ }
};
std::atomic<int32_t> FakeWindowHandle::sId{1};
@@ -5779,27 +5818,27 @@
<< "Inject motion event should return InputEventInjectionResult::SUCCEEDED";
// Window should receive motion event.
- firstWindowInPrimary->consumeMotionDown(SECOND_DISPLAY_ID);
+ firstWindowInSecondary->consumeMotionDown(SECOND_DISPLAY_ID);
// Transfer touch focus
ASSERT_TRUE(mDispatcher->transferTouch(secondWindowInSecondary->getToken(), SECOND_DISPLAY_ID));
// The first window gets cancel.
- firstWindowInPrimary->consumeMotionCancel(SECOND_DISPLAY_ID);
- secondWindowInPrimary->consumeMotionDown(SECOND_DISPLAY_ID);
+ firstWindowInSecondary->consumeMotionCancel(SECOND_DISPLAY_ID);
+ secondWindowInSecondary->consumeMotionDown(SECOND_DISPLAY_ID);
ASSERT_EQ(InputEventInjectionResult::SUCCEEDED,
injectMotionEvent(*mDispatcher, AMOTION_EVENT_ACTION_MOVE, AINPUT_SOURCE_TOUCHSCREEN,
SECOND_DISPLAY_ID, {150, 50}))
<< "Inject motion event should return InputEventInjectionResult::SUCCEEDED";
- firstWindowInPrimary->assertNoEvents();
- secondWindowInPrimary->consumeMotionMove(SECOND_DISPLAY_ID);
+ firstWindowInSecondary->assertNoEvents();
+ secondWindowInSecondary->consumeMotionMove(SECOND_DISPLAY_ID);
ASSERT_EQ(InputEventInjectionResult::SUCCEEDED,
injectMotionUp(*mDispatcher, AINPUT_SOURCE_TOUCHSCREEN, SECOND_DISPLAY_ID, {150, 50}))
<< "Inject motion event should return InputEventInjectionResult::SUCCEEDED";
- firstWindowInPrimary->assertNoEvents();
- secondWindowInPrimary->consumeMotionUp(SECOND_DISPLAY_ID);
+ firstWindowInSecondary->assertNoEvents();
+ secondWindowInSecondary->consumeMotionUp(SECOND_DISPLAY_ID);
}
TEST_F(InputDispatcherTest, FocusedWindow_ReceivesFocusEventAndKeyEvent) {
@@ -7263,7 +7302,7 @@
mWindow->assertNoEvents();
}
-TEST_F(InputDispatcherFallbackKeyTest, WindowRemovedDuringPolicyCall) {
+TEST_F(InputDispatcherFallbackKeyTest, InputChannelRemovedDuringPolicyCall) {
setFallback(AKEYCODE_B);
mDispatcher->notifyKey(
KeyArgsBuilder(ACTION_DOWN, AINPUT_SOURCE_KEYBOARD).keyCode(AKEYCODE_A).build());
@@ -7296,6 +7335,71 @@
ASSERT_NO_FATAL_FAILURE(mFakePolicy->assertUnhandledKeyReported(AKEYCODE_A));
}
+TEST_F(InputDispatcherFallbackKeyTest, WindowRemovedDuringPolicyCall) {
+ setFallback(AKEYCODE_B);
+ mDispatcher->notifyKey(
+ KeyArgsBuilder(ACTION_DOWN, AINPUT_SOURCE_KEYBOARD).keyCode(AKEYCODE_A).build());
+
+ // Do not handle this key event.
+ consumeKey(/*handled=*/false,
+ AllOf(WithKeyAction(ACTION_DOWN), WithKeyCode(AKEYCODE_A), WithFlags(0)));
+ ASSERT_NO_FATAL_FAILURE(mFakePolicy->assertUnhandledKeyReported(AKEYCODE_A));
+ consumeKey(/*handled=*/true,
+ AllOf(WithKeyAction(ACTION_DOWN), WithKeyCode(AKEYCODE_B),
+ WithFlags(AKEY_EVENT_FLAG_FALLBACK)));
+
+ mFakePolicy->setUnhandledKeyHandler([&](const KeyEvent& event) {
+ // When the unhandled key is reported to the policy next, remove the window.
+ mDispatcher->onWindowInfosChanged({{}, {}, 0, 0});
+ return KeyEventBuilder(event).keyCode(AKEYCODE_B).build();
+ });
+ // Release the original key, which the app will not handle. When this unhandled key is reported
+ // to the policy, the window will be removed.
+ mDispatcher->notifyKey(
+ KeyArgsBuilder(ACTION_UP, AINPUT_SOURCE_KEYBOARD).keyCode(AKEYCODE_A).build());
+ consumeKey(/*handled=*/false,
+ AllOf(WithKeyAction(ACTION_UP), WithKeyCode(AKEYCODE_A), WithFlags(0)));
+ ASSERT_NO_FATAL_FAILURE(mFakePolicy->assertUnhandledKeyReported(AKEYCODE_A));
+
+ // Since the window was removed, it loses focus, and the channel state will be reset.
+ consumeKey(/*handled=*/true,
+ AllOf(WithKeyAction(ACTION_UP), WithKeyCode(AKEYCODE_B),
+ WithFlags(AKEY_EVENT_FLAG_FALLBACK | AKEY_EVENT_FLAG_CANCELED)));
+ mWindow->consumeFocusEvent(false);
+ mWindow->assertNoEvents();
+}
+
+TEST_F(InputDispatcherFallbackKeyTest, WindowRemovedWhileAwaitingFinishedSignal) {
+ setFallback(AKEYCODE_B);
+ mDispatcher->notifyKey(
+ KeyArgsBuilder(ACTION_DOWN, AINPUT_SOURCE_KEYBOARD).keyCode(AKEYCODE_A).build());
+
+ // Do not handle this key event.
+ consumeKey(/*handled=*/false,
+ AllOf(WithKeyAction(ACTION_DOWN), WithKeyCode(AKEYCODE_A), WithFlags(0)));
+ ASSERT_NO_FATAL_FAILURE(mFakePolicy->assertUnhandledKeyReported(AKEYCODE_A));
+ const auto [seq, event] = mWindow->receiveEvent();
+ ASSERT_TRUE(seq.has_value() && event != nullptr) << "Failed to receive fallback event";
+ ASSERT_EQ(event->getType(), InputEventType::KEY);
+ ASSERT_THAT(static_cast<const KeyEvent&>(*event),
+ AllOf(WithKeyAction(ACTION_DOWN), WithKeyCode(AKEYCODE_B),
+ WithFlags(AKEY_EVENT_FLAG_FALLBACK)));
+
+ // Remove the window now, which should generate a cancellations and make the window lose focus.
+ mDispatcher->onWindowInfosChanged({{}, {}, 0, 0});
+ consumeKey(/*handled=*/true,
+ AllOf(WithKeyAction(ACTION_UP), WithKeyCode(AKEYCODE_A),
+ WithFlags(AKEY_EVENT_FLAG_CANCELED)));
+ consumeKey(/*handled=*/true,
+ AllOf(WithKeyAction(ACTION_UP), WithKeyCode(AKEYCODE_B),
+ WithFlags(AKEY_EVENT_FLAG_FALLBACK | AKEY_EVENT_FLAG_CANCELED)));
+ mWindow->consumeFocusEvent(false);
+
+ // Finish the event by reporting it as handled.
+ mWindow->finishEvent(*seq);
+ mWindow->assertNoEvents();
+}
+
class InputDispatcherKeyRepeatTest : public InputDispatcherTest {
protected:
static constexpr std::chrono::nanoseconds KEY_REPEAT_TIMEOUT = 40ms;
@@ -8225,13 +8329,13 @@
}
}
- void touchAndAssertPositions(int32_t action, const std::vector<PointF>& touchedPoints,
+ void touchAndAssertPositions(sp<FakeWindowHandle> touchedWindow, int32_t action,
+ const std::vector<PointF>& touchedPoints,
std::vector<PointF> expectedPoints) {
mDispatcher->notifyMotion(generateMotionArgs(action, AINPUT_SOURCE_TOUCHSCREEN,
ADISPLAY_ID_DEFAULT, touchedPoints));
- // Always consume from window1 since it's the window that has the InputReceiver
- consumeMotionEvent(mWindow1, action, expectedPoints);
+ consumeMotionEvent(touchedWindow, action, expectedPoints);
}
};
@@ -8239,15 +8343,15 @@
// Touch Window 1
PointF touchedPoint = {10, 10};
PointF expectedPoint = getPointInWindow(mWindow1->getInfo(), touchedPoint);
- touchAndAssertPositions(AMOTION_EVENT_ACTION_DOWN, {touchedPoint}, {expectedPoint});
+ touchAndAssertPositions(mWindow1, AMOTION_EVENT_ACTION_DOWN, {touchedPoint}, {expectedPoint});
// Release touch on Window 1
- touchAndAssertPositions(AMOTION_EVENT_ACTION_UP, {touchedPoint}, {expectedPoint});
+ touchAndAssertPositions(mWindow1, AMOTION_EVENT_ACTION_UP, {touchedPoint}, {expectedPoint});
// Touch Window 2
touchedPoint = {150, 150};
expectedPoint = getPointInWindow(mWindow2->getInfo(), touchedPoint);
- touchAndAssertPositions(AMOTION_EVENT_ACTION_DOWN, {touchedPoint}, {expectedPoint});
+ touchAndAssertPositions(mWindow2, AMOTION_EVENT_ACTION_DOWN, {touchedPoint}, {expectedPoint});
}
TEST_F(InputDispatcherMultiWindowSameTokenTests, SingleTouchDifferentTransform) {
@@ -8258,21 +8362,21 @@
// Touch Window 1
PointF touchedPoint = {10, 10};
PointF expectedPoint = getPointInWindow(mWindow1->getInfo(), touchedPoint);
- touchAndAssertPositions(AMOTION_EVENT_ACTION_DOWN, {touchedPoint}, {expectedPoint});
+ touchAndAssertPositions(mWindow1, AMOTION_EVENT_ACTION_DOWN, {touchedPoint}, {expectedPoint});
// Release touch on Window 1
- touchAndAssertPositions(AMOTION_EVENT_ACTION_UP, {touchedPoint}, {expectedPoint});
+ touchAndAssertPositions(mWindow1, AMOTION_EVENT_ACTION_UP, {touchedPoint}, {expectedPoint});
// Touch Window 2
touchedPoint = {150, 150};
expectedPoint = getPointInWindow(mWindow2->getInfo(), touchedPoint);
- touchAndAssertPositions(AMOTION_EVENT_ACTION_DOWN, {touchedPoint}, {expectedPoint});
- touchAndAssertPositions(AMOTION_EVENT_ACTION_UP, {touchedPoint}, {expectedPoint});
+ touchAndAssertPositions(mWindow2, AMOTION_EVENT_ACTION_DOWN, {touchedPoint}, {expectedPoint});
+ touchAndAssertPositions(mWindow2, AMOTION_EVENT_ACTION_UP, {touchedPoint}, {expectedPoint});
// Update the transform so rotation is set
mWindow2->setWindowTransform(0, -1, 1, 0);
mDispatcher->onWindowInfosChanged({{*mWindow1->getInfo(), *mWindow2->getInfo()}, {}, 0, 0});
expectedPoint = getPointInWindow(mWindow2->getInfo(), touchedPoint);
- touchAndAssertPositions(AMOTION_EVENT_ACTION_DOWN, {touchedPoint}, {expectedPoint});
+ touchAndAssertPositions(mWindow2, AMOTION_EVENT_ACTION_DOWN, {touchedPoint}, {expectedPoint});
}
TEST_F(InputDispatcherMultiWindowSameTokenTests, MultipleTouchDifferentTransform) {
@@ -8282,22 +8386,25 @@
// Touch Window 1
std::vector<PointF> touchedPoints = {PointF{10, 10}};
std::vector<PointF> expectedPoints = {getPointInWindow(mWindow1->getInfo(), touchedPoints[0])};
- touchAndAssertPositions(AMOTION_EVENT_ACTION_DOWN, touchedPoints, expectedPoints);
+ touchAndAssertPositions(mWindow1, AMOTION_EVENT_ACTION_DOWN, touchedPoints, expectedPoints);
// Touch Window 2
+ // Since this is part of the same touch gesture that has already been dispatched to Window 1,
+ // the touch stream from Window 2 will be merged with the stream in Window 1. The merged stream
+ // will continue to be dispatched through Window 1.
touchedPoints.push_back(PointF{150, 150});
expectedPoints.push_back(getPointInWindow(mWindow2->getInfo(), touchedPoints[1]));
- touchAndAssertPositions(POINTER_1_DOWN, touchedPoints, expectedPoints);
+ touchAndAssertPositions(mWindow1, POINTER_1_DOWN, touchedPoints, expectedPoints);
// Release Window 2
- touchAndAssertPositions(POINTER_1_UP, touchedPoints, expectedPoints);
+ touchAndAssertPositions(mWindow1, POINTER_1_UP, touchedPoints, expectedPoints);
expectedPoints.pop_back();
// Update the transform so rotation is set for Window 2
mWindow2->setWindowTransform(0, -1, 1, 0);
mDispatcher->onWindowInfosChanged({{*mWindow1->getInfo(), *mWindow2->getInfo()}, {}, 0, 0});
expectedPoints.push_back(getPointInWindow(mWindow2->getInfo(), touchedPoints[1]));
- touchAndAssertPositions(POINTER_1_DOWN, touchedPoints, expectedPoints);
+ touchAndAssertPositions(mWindow1, POINTER_1_DOWN, touchedPoints, expectedPoints);
}
TEST_F(InputDispatcherMultiWindowSameTokenTests, MultipleTouchMoveDifferentTransform) {
@@ -8307,37 +8414,37 @@
// Touch Window 1
std::vector<PointF> touchedPoints = {PointF{10, 10}};
std::vector<PointF> expectedPoints = {getPointInWindow(mWindow1->getInfo(), touchedPoints[0])};
- touchAndAssertPositions(AMOTION_EVENT_ACTION_DOWN, touchedPoints, expectedPoints);
+ touchAndAssertPositions(mWindow1, AMOTION_EVENT_ACTION_DOWN, touchedPoints, expectedPoints);
// Touch Window 2
touchedPoints.push_back(PointF{150, 150});
expectedPoints.push_back(getPointInWindow(mWindow2->getInfo(), touchedPoints[1]));
- touchAndAssertPositions(POINTER_1_DOWN, touchedPoints, expectedPoints);
+ touchAndAssertPositions(mWindow1, POINTER_1_DOWN, touchedPoints, expectedPoints);
// Move both windows
touchedPoints = {{20, 20}, {175, 175}};
expectedPoints = {getPointInWindow(mWindow1->getInfo(), touchedPoints[0]),
getPointInWindow(mWindow2->getInfo(), touchedPoints[1])};
- touchAndAssertPositions(AMOTION_EVENT_ACTION_MOVE, touchedPoints, expectedPoints);
+ touchAndAssertPositions(mWindow1, AMOTION_EVENT_ACTION_MOVE, touchedPoints, expectedPoints);
// Release Window 2
- touchAndAssertPositions(POINTER_1_UP, touchedPoints, expectedPoints);
+ touchAndAssertPositions(mWindow1, POINTER_1_UP, touchedPoints, expectedPoints);
expectedPoints.pop_back();
// Touch Window 2
mWindow2->setWindowTransform(0, -1, 1, 0);
mDispatcher->onWindowInfosChanged({{*mWindow1->getInfo(), *mWindow2->getInfo()}, {}, 0, 0});
expectedPoints.push_back(getPointInWindow(mWindow2->getInfo(), touchedPoints[1]));
- touchAndAssertPositions(POINTER_1_DOWN, touchedPoints, expectedPoints);
+ touchAndAssertPositions(mWindow1, POINTER_1_DOWN, touchedPoints, expectedPoints);
// Move both windows
touchedPoints = {{20, 20}, {175, 175}};
expectedPoints = {getPointInWindow(mWindow1->getInfo(), touchedPoints[0]),
getPointInWindow(mWindow2->getInfo(), touchedPoints[1])};
- touchAndAssertPositions(AMOTION_EVENT_ACTION_MOVE, touchedPoints, expectedPoints);
+ touchAndAssertPositions(mWindow1, AMOTION_EVENT_ACTION_MOVE, touchedPoints, expectedPoints);
}
TEST_F(InputDispatcherMultiWindowSameTokenTests, MultipleWindowsFirstTouchWithScale) {
@@ -8347,20 +8454,20 @@
// Touch Window 1
std::vector<PointF> touchedPoints = {PointF{10, 10}};
std::vector<PointF> expectedPoints = {getPointInWindow(mWindow1->getInfo(), touchedPoints[0])};
- touchAndAssertPositions(AMOTION_EVENT_ACTION_DOWN, touchedPoints, expectedPoints);
+ touchAndAssertPositions(mWindow1, AMOTION_EVENT_ACTION_DOWN, touchedPoints, expectedPoints);
// Touch Window 2
touchedPoints.push_back(PointF{150, 150});
expectedPoints.push_back(getPointInWindow(mWindow2->getInfo(), touchedPoints[1]));
- touchAndAssertPositions(POINTER_1_DOWN, touchedPoints, expectedPoints);
+ touchAndAssertPositions(mWindow1, POINTER_1_DOWN, touchedPoints, expectedPoints);
// Move both windows
touchedPoints = {{20, 20}, {175, 175}};
expectedPoints = {getPointInWindow(mWindow1->getInfo(), touchedPoints[0]),
getPointInWindow(mWindow2->getInfo(), touchedPoints[1])};
- touchAndAssertPositions(AMOTION_EVENT_ACTION_MOVE, touchedPoints, expectedPoints);
+ touchAndAssertPositions(mWindow1, AMOTION_EVENT_ACTION_MOVE, touchedPoints, expectedPoints);
}
/**
@@ -8404,7 +8511,7 @@
.pointer(PointerBuilder(0, ToolType::FINGER).x(150).y(150))
.build());
consumeMotionEvent(mWindow1, ACTION_HOVER_EXIT, {{50, 50}});
- consumeMotionEvent(mWindow1, ACTION_HOVER_ENTER,
+ consumeMotionEvent(mWindow2, ACTION_HOVER_ENTER,
{getPointInWindow(mWindow2->getInfo(), PointF{150, 150})});
}
@@ -9543,7 +9650,7 @@
TEST_F(InputDispatcherMirrorWindowFocusTests, FocusedIfAllWindowsFocusable) {
setFocusedWindow(mMirror);
- // window gets focused
+ // window gets focused because it is above the mirror
mWindow->consumeFocusEvent(true);
ASSERT_EQ(InputEventInjectionResult::SUCCEEDED, injectKeyDown(*mDispatcher))
<< "Inject key event should return InputEventInjectionResult::SUCCEEDED";
@@ -9616,10 +9723,10 @@
ASSERT_EQ(InputEventInjectionResult::SUCCEEDED, injectKeyDown(*mDispatcher))
<< "Inject key event should return InputEventInjectionResult::SUCCEEDED";
- mWindow->consumeKeyDown(ADISPLAY_ID_NONE);
+ mMirror->consumeKeyDown(ADISPLAY_ID_NONE);
ASSERT_EQ(InputEventInjectionResult::SUCCEEDED, injectKeyUp(*mDispatcher))
<< "Inject key event should return InputEventInjectionResult::SUCCEEDED";
- mWindow->consumeKeyUp(ADISPLAY_ID_NONE);
+ mMirror->consumeKeyUp(ADISPLAY_ID_NONE);
// Both windows are removed
mDispatcher->onWindowInfosChanged({{}, {}, 0, 0});
diff --git a/services/surfaceflinger/Display/DisplayModeRequest.h b/services/surfaceflinger/Display/DisplayModeRequest.h
index c0e77bb..d07cdf5 100644
--- a/services/surfaceflinger/Display/DisplayModeRequest.h
+++ b/services/surfaceflinger/Display/DisplayModeRequest.h
@@ -27,9 +27,6 @@
// Whether to emit DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE.
bool emitEvent = false;
-
- // Whether to force the request to be applied, even if the mode is unchanged.
- bool force = false;
};
inline bool operator==(const DisplayModeRequest& lhs, const DisplayModeRequest& rhs) {
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 45f08a4..5f20cd9 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -24,7 +24,6 @@
#define ATRACE_TAG ATRACE_TAG_GRAPHICS
-#include <common/FlagManager.h>
#include <compositionengine/CompositionEngine.h>
#include <compositionengine/Display.h>
#include <compositionengine/DisplayColorProfile.h>
@@ -215,17 +214,6 @@
bool DisplayDevice::initiateModeChange(display::DisplayModeRequest&& desiredMode,
const hal::VsyncPeriodChangeConstraints& constraints,
hal::VsyncPeriodChangeTimeline& outTimeline) {
- // TODO(b/255635711): Flow the DisplayModeRequest through the desired/pending/active states. For
- // now, `desiredMode` and `mDesiredModeOpt` are one and the same, but the latter is not cleared
- // until the next `SF::initiateDisplayModeChanges`. However, the desired mode has been consumed
- // at this point, so clear the `force` flag to prevent an endless loop of `initiateModeChange`.
- if (FlagManager::getInstance().connected_display()) {
- std::scoped_lock lock(mDesiredModeLock);
- if (mDesiredModeOpt) {
- mDesiredModeOpt->force = false;
- }
- }
-
mPendingModeOpt = std::move(desiredMode);
mIsModeSetPending = true;
@@ -529,7 +517,8 @@
}
}
-auto DisplayDevice::setDesiredMode(display::DisplayModeRequest&& desiredMode) -> DesiredModeAction {
+auto DisplayDevice::setDesiredMode(display::DisplayModeRequest&& desiredMode, bool force)
+ -> DesiredModeAction {
ATRACE_CALL();
const auto& desiredModePtr = desiredMode.mode.modePtr;
@@ -537,26 +526,20 @@
LOG_ALWAYS_FATAL_IF(getPhysicalId() != desiredModePtr->getPhysicalDisplayId(),
"DisplayId mismatch");
- // TODO (b/318533819): Stringize DisplayModeRequest.
- ALOGD("%s(%s, force=%s)", __func__, to_string(*desiredModePtr).c_str(),
- desiredMode.force ? "true" : "false");
+ ALOGV("%s(%s)", __func__, to_string(*desiredModePtr).c_str());
std::scoped_lock lock(mDesiredModeLock);
if (mDesiredModeOpt) {
// A mode transition was already scheduled, so just override the desired mode.
const bool emitEvent = mDesiredModeOpt->emitEvent;
- const bool force = mDesiredModeOpt->force;
mDesiredModeOpt = std::move(desiredMode);
mDesiredModeOpt->emitEvent |= emitEvent;
- if (FlagManager::getInstance().connected_display()) {
- mDesiredModeOpt->force |= force;
- }
return DesiredModeAction::None;
}
// If the desired mode is already active...
const auto activeMode = refreshRateSelector().getActiveMode();
- if (!desiredMode.force && activeMode.modePtr->getId() == desiredModePtr->getId()) {
+ if (!force && activeMode.modePtr->getId() == desiredModePtr->getId()) {
if (activeMode == desiredMode.mode) {
return DesiredModeAction::None;
}
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index edd57cc..4ab6321 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -189,7 +189,8 @@
enum class DesiredModeAction { None, InitiateDisplayModeSwitch, InitiateRenderRateSwitch };
- DesiredModeAction setDesiredMode(display::DisplayModeRequest&&) EXCLUDES(mDesiredModeLock);
+ DesiredModeAction setDesiredMode(display::DisplayModeRequest&&, bool force = false)
+ EXCLUDES(mDesiredModeLock);
using DisplayModeRequestOpt = ftl::Optional<display::DisplayModeRequest>;
diff --git a/services/surfaceflinger/DisplayHardware/DisplayMode.h b/services/surfaceflinger/DisplayHardware/DisplayMode.h
index ba0825c..224f50e 100644
--- a/services/surfaceflinger/DisplayHardware/DisplayMode.h
+++ b/services/surfaceflinger/DisplayHardware/DisplayMode.h
@@ -21,17 +21,17 @@
#include <android-base/stringprintf.h>
#include <android/configuration.h>
+#include <ftl/mixins.h>
#include <ftl/small_map.h>
#include <ui/DisplayId.h>
#include <ui/DisplayMode.h>
#include <ui/Size.h>
#include <utils/Timers.h>
+#include <common/FlagManager.h>
#include <scheduler/Fps.h>
-#include <common/FlagManager.h>
#include "DisplayHardware/Hal.h"
-#include "Scheduler/StrongTyping.h"
namespace android {
@@ -46,7 +46,12 @@
bool operator<=(const DisplayModePtr&, const DisplayModePtr&) = delete;
bool operator>=(const DisplayModePtr&, const DisplayModePtr&) = delete;
-using DisplayModeId = StrongTyping<ui::DisplayModeId, struct DisplayModeIdTag, Compare>;
+struct DisplayModeId : ftl::DefaultConstructible<DisplayModeId, ui::DisplayModeId>,
+ ftl::Incrementable<DisplayModeId>,
+ ftl::Equatable<DisplayModeId>,
+ ftl::Orderable<DisplayModeId> {
+ using DefaultConstructible::DefaultConstructible;
+};
using DisplayModes = ftl::SmallMap<DisplayModeId, DisplayModePtr, 3>;
using DisplayModeIterator = DisplayModes::const_iterator;
@@ -185,7 +190,7 @@
inline std::string to_string(const DisplayMode& mode) {
return base::StringPrintf("{id=%d, hwcId=%d, resolution=%dx%d, vsyncRate=%s, "
"dpi=%.2fx%.2f, group=%d, vrrConfig=%s}",
- mode.getId().value(), mode.getHwcId(), mode.getWidth(),
+ ftl::to_underlying(mode.getId()), mode.getHwcId(), mode.getWidth(),
mode.getHeight(), to_string(mode.getVsyncRate()).c_str(),
mode.getDpi().x, mode.getDpi().y, mode.getGroup(),
to_string(mode.getVrrConfig()).c_str());
diff --git a/services/surfaceflinger/DisplayHardware/HWC2.cpp b/services/surfaceflinger/DisplayHardware/HWC2.cpp
index db66f5b..704ece5 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2.cpp
+++ b/services/surfaceflinger/DisplayHardware/HWC2.cpp
@@ -27,7 +27,6 @@
#include "HWC2.h"
#include <android/configuration.h>
-#include <common/FlagManager.h>
#include <ui/Fence.h>
#include <ui/FloatRect.h>
#include <ui/GraphicBuffer.h>
@@ -417,19 +416,7 @@
VsyncPeriodChangeTimeline* outTimeline) {
ALOGV("[%" PRIu64 "] setActiveConfigWithConstraints", mId);
- // FIXME (b/319505580): At least the first config set on an external display must be
- // `setActiveConfig`, so skip over the block that calls `setActiveConfigWithConstraints`
- // for simplicity.
- ui::DisplayConnectionType type = ui::DisplayConnectionType::Internal;
- const bool connected_display = FlagManager::getInstance().connected_display();
- if (connected_display) {
- if (auto err = getConnectionType(&type); err != Error::NONE) {
- return err;
- }
- }
-
- if (isVsyncPeriodSwitchSupported() &&
- (!connected_display || type != ui::DisplayConnectionType::External)) {
+ if (isVsyncPeriodSwitchSupported()) {
Hwc2::IComposerClient::VsyncPeriodChangeConstraints hwc2Constraints;
hwc2Constraints.desiredTimeNanos = constraints.desiredTimeNanos;
hwc2Constraints.seamlessRequired = constraints.seamlessRequired;
diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp
index ccd1c0f..96eccf2 100644
--- a/services/surfaceflinger/Scheduler/EventThread.cpp
+++ b/services/surfaceflinger/Scheduler/EventThread.cpp
@@ -148,7 +148,7 @@
DisplayEventReceiver::Event event;
event.header = {DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE,
mode.modePtr->getPhysicalDisplayId(), systemTime()};
- event.modeChange.modeId = mode.modePtr->getId().value();
+ event.modeChange.modeId = ftl::to_underlying(mode.modePtr->getId());
event.modeChange.vsyncPeriod = mode.fps.getPeriodNsecs();
return event;
}
diff --git a/services/surfaceflinger/Scheduler/LayerHistory.cpp b/services/surfaceflinger/Scheduler/LayerHistory.cpp
index 5ce883c..dcb6254 100644
--- a/services/surfaceflinger/Scheduler/LayerHistory.cpp
+++ b/services/surfaceflinger/Scheduler/LayerHistory.cpp
@@ -221,9 +221,8 @@
const std::string categoryString = vote.category == FrameRateCategory::Default
? ""
: base::StringPrintf("category=%s", ftl::enum_string(vote.category).c_str());
- ATRACE_FORMAT_INSTANT("%s %s %s (%d%)", ftl::enum_string(vote.type).c_str(),
- to_string(vote.fps).c_str(), categoryString.c_str(),
- weight * 100);
+ ATRACE_FORMAT_INSTANT("%s %s %s (%.2f)", ftl::enum_string(vote.type).c_str(),
+ to_string(vote.fps).c_str(), categoryString.c_str(), weight);
summary.push_back({info->getName(), info->getOwnerUid(), vote.type, vote.fps,
vote.seamlessness, vote.category, vote.categorySmoothSwitchOnly,
weight, layerFocused});
diff --git a/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp b/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp
index bdeff8d..e696e8c 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp
+++ b/services/surfaceflinger/Scheduler/RefreshRateSelector.cpp
@@ -286,7 +286,8 @@
std::string RefreshRateSelector::Policy::toString() const {
return base::StringPrintf("{defaultModeId=%d, allowGroupSwitching=%s"
", primaryRanges=%s, appRequestRanges=%s}",
- defaultMode.value(), allowGroupSwitching ? "true" : "false",
+ ftl::to_underlying(defaultMode),
+ allowGroupSwitching ? "true" : "false",
to_string(primaryRanges).c_str(),
to_string(appRequestRanges).c_str());
}
diff --git a/services/surfaceflinger/Scheduler/RefreshRateSelector.h b/services/surfaceflinger/Scheduler/RefreshRateSelector.h
index a1a7c28..6051e89 100644
--- a/services/surfaceflinger/Scheduler/RefreshRateSelector.h
+++ b/services/surfaceflinger/Scheduler/RefreshRateSelector.h
@@ -31,7 +31,6 @@
#include "DisplayHardware/DisplayMode.h"
#include "Scheduler/OneShotTimer.h"
-#include "Scheduler/StrongTyping.h"
#include "ThreadContext.h"
#include "Utils/Dumper.h"
diff --git a/services/surfaceflinger/Scheduler/StrongTyping.h b/services/surfaceflinger/Scheduler/StrongTyping.h
deleted file mode 100644
index a05c123..0000000
--- a/services/surfaceflinger/Scheduler/StrongTyping.h
+++ /dev/null
@@ -1,89 +0,0 @@
-/*
- * Copyright 2019 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
-namespace android {
-
-template <typename T, template <typename> class AbilityType>
-struct Ability {
- T& base() { return static_cast<T&>(*this); }
- T const& base() const { return static_cast<T const&>(*this); }
-};
-
-template <typename T>
-struct Add : Ability<T, Add> {
- inline T operator+(T const& other) const { return T(this->base().value() + other.value()); }
- inline T& operator++() {
- ++this->base().value();
- return this->base();
- };
- inline T operator++(int) {
- T tmp(this->base());
- operator++();
- return tmp;
- };
- inline T& operator+=(T const& other) {
- this->base().value() += other.value();
- return this->base();
- };
-};
-
-template <typename T>
-struct Compare : Ability<T, Compare> {
- inline bool operator==(T const& other) const { return this->base().value() == other.value(); };
- inline bool operator<(T const& other) const { return this->base().value() < other.value(); }
- inline bool operator<=(T const& other) const { return (*this < other) || (*this == other); }
- inline bool operator!=(T const& other) const { return !(*this == other); }
- inline bool operator>=(T const& other) const { return !(*this < other); }
- inline bool operator>(T const& other) const { return !(*this < other || *this == other); }
-};
-
-template <typename T>
-struct Hash : Ability<T, Hash> {
- [[nodiscard]] std::size_t hash() const {
- return std::hash<typename std::remove_const<
- typename std::remove_reference<decltype(this->base().value())>::type>::type>{}(
- this->base().value());
- }
-};
-
-template <typename T, typename W, template <typename> class... Ability>
-struct StrongTyping : Ability<StrongTyping<T, W, Ability...>>... {
- constexpr StrongTyping() = default;
- constexpr explicit StrongTyping(T const& value) : mValue(value) {}
- StrongTyping(StrongTyping const&) = default;
- StrongTyping& operator=(StrongTyping const&) = default;
- explicit inline operator T() const { return mValue; }
- T const& value() const { return mValue; }
- T& value() { return mValue; }
-
- friend std::ostream& operator<<(std::ostream& os, const StrongTyping<T, W, Ability...>& value) {
- return os << value.value();
- }
-
-private:
- T mValue{0};
-};
-} // namespace android
-
-namespace std {
-template <typename T, typename W, template <typename> class... Ability>
-struct hash<android::StrongTyping<T, W, Ability...>> {
- std::size_t operator()(android::StrongTyping<T, W, Ability...> const& k) const {
- return k.hash();
- }
-};
-} // namespace std
diff --git a/services/surfaceflinger/Scheduler/VSyncDispatch.h b/services/surfaceflinger/Scheduler/VSyncDispatch.h
index f978016..ed8f8fe 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatch.h
+++ b/services/surfaceflinger/Scheduler/VSyncDispatch.h
@@ -20,10 +20,9 @@
#include <optional>
#include <string>
+#include <ftl/mixins.h>
#include <utils/Timers.h>
-#include "StrongTyping.h"
-
namespace android::scheduler {
using ScheduleResult = std::optional<nsecs_t>;
@@ -35,7 +34,11 @@
*/
class VSyncDispatch {
public:
- using CallbackToken = StrongTyping<size_t, class CallbackTokenTag, Compare, Hash>;
+ struct CallbackToken : ftl::DefaultConstructible<CallbackToken, size_t>,
+ ftl::Equatable<CallbackToken>,
+ ftl::Incrementable<CallbackToken> {
+ using DefaultConstructible::DefaultConstructible;
+ };
virtual ~VSyncDispatch();
diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
index 5cb0ffb..963f9e9 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
@@ -267,15 +267,15 @@
}
void VSyncDispatchTimerQueue::rearmTimer(nsecs_t now) {
- rearmTimerSkippingUpdateFor(now, mCallbacks.end());
+ rearmTimerSkippingUpdateFor(now, mCallbacks.cend());
}
void VSyncDispatchTimerQueue::rearmTimerSkippingUpdateFor(
- nsecs_t now, CallbackMap::iterator const& skipUpdateIt) {
+ nsecs_t now, CallbackMap::const_iterator skipUpdateIt) {
std::optional<nsecs_t> min;
std::optional<nsecs_t> targetVsync;
std::optional<std::string_view> nextWakeupName;
- for (auto it = mCallbacks.begin(); it != mCallbacks.end(); it++) {
+ for (auto it = mCallbacks.cbegin(); it != mCallbacks.cend(); ++it) {
auto& callback = it->second;
if (!callback->wakeupTime() && !callback->hasPendingWorkloadUpdate()) {
continue;
@@ -351,13 +351,12 @@
VSyncDispatchTimerQueue::CallbackToken VSyncDispatchTimerQueue::registerCallback(
Callback callback, std::string callbackName) {
std::lock_guard lock(mMutex);
- return CallbackToken{
- mCallbacks
- .emplace(++mCallbackToken,
- std::make_shared<VSyncDispatchTimerQueueEntry>(std::move(callbackName),
- std::move(callback),
- mMinVsyncDistance))
- .first->first};
+ return mCallbacks
+ .try_emplace(++mCallbackToken,
+ std::make_shared<VSyncDispatchTimerQueueEntry>(std::move(callbackName),
+ std::move(callback),
+ mMinVsyncDistance))
+ .first->first;
}
void VSyncDispatchTimerQueue::unregisterCallback(CallbackToken token) {
@@ -367,7 +366,7 @@
auto it = mCallbacks.find(token);
if (it != mCallbacks.end()) {
entry = it->second;
- mCallbacks.erase(it);
+ mCallbacks.erase(it->first);
}
}
diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h
index 3d08410..81c746e 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h
+++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h
@@ -16,14 +16,13 @@
#pragma once
-#include <functional>
#include <memory>
#include <mutex>
#include <string>
#include <string_view>
-#include <unordered_map>
#include <android-base/thread_annotations.h>
+#include <ftl/small_map.h>
#include "VSyncDispatch.h"
#include "VsyncSchedule.h"
@@ -135,13 +134,14 @@
VSyncDispatchTimerQueue(const VSyncDispatchTimerQueue&) = delete;
VSyncDispatchTimerQueue& operator=(const VSyncDispatchTimerQueue&) = delete;
+ // The static capacity was chosen to exceed the expected number of callbacks.
using CallbackMap =
- std::unordered_map<CallbackToken, std::shared_ptr<VSyncDispatchTimerQueueEntry>>;
+ ftl::SmallMap<CallbackToken, std::shared_ptr<VSyncDispatchTimerQueueEntry>, 5>;
void timerCallback();
void setTimer(nsecs_t, nsecs_t) REQUIRES(mMutex);
void rearmTimer(nsecs_t now) REQUIRES(mMutex);
- void rearmTimerSkippingUpdateFor(nsecs_t now, CallbackMap::iterator const& skipUpdate)
+ void rearmTimerSkippingUpdateFor(nsecs_t now, CallbackMap::const_iterator skipUpdate)
REQUIRES(mMutex);
void cancelTimer() REQUIRES(mMutex);
ScheduleResult scheduleLocked(CallbackToken, ScheduleTiming) REQUIRES(mMutex);
@@ -158,7 +158,7 @@
nsecs_t const mTimerSlack;
nsecs_t const mMinVsyncDistance;
- size_t mCallbackToken GUARDED_BY(mMutex) = 0;
+ CallbackToken mCallbackToken GUARDED_BY(mMutex);
CallbackMap mCallbacks GUARDED_BY(mMutex);
nsecs_t mIntendedWakeupTime GUARDED_BY(mMutex) = kInvalidTime;
diff --git a/services/surfaceflinger/Scheduler/src/Timer.cpp b/services/surfaceflinger/Scheduler/src/Timer.cpp
index 09e8a1e..eeb9c60 100644
--- a/services/surfaceflinger/Scheduler/src/Timer.cpp
+++ b/services/surfaceflinger/Scheduler/src/Timer.cpp
@@ -159,7 +159,7 @@
ALOGW("Failed to set SCHED_FIFO on dispatch thread");
}
- if (pthread_setname_np(pthread_self(), "TimerDispatch")) {
+ if (pthread_setname_np(pthread_self(), "TimerDispatch") != 0) {
ALOGW("Failed to set thread name on dispatch thread");
}
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 89a341f..173b6fe 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1083,7 +1083,7 @@
for (const auto& [id, mode] : displayModes) {
ui::DisplayMode outMode;
- outMode.id = static_cast<int32_t>(id.value());
+ outMode.id = ftl::to_underlying(id);
auto [width, height] = mode->getResolution();
auto [xDpi, yDpi] = mode->getDpi();
@@ -1132,7 +1132,7 @@
const PhysicalDisplayId displayId = snapshot.displayId();
const auto mode = display->refreshRateSelector().getActiveMode();
- info->activeDisplayModeId = mode.modePtr->getId().value();
+ info->activeDisplayModeId = ftl::to_underlying(mode.modePtr->getId());
info->renderFrameRate = mode.fps.getValue();
info->activeColorMode = display->getCompositionDisplay()->getState().colorMode;
info->hdrCapabilities = filterOut4k30(display->getHdrCapabilities());
@@ -1148,7 +1148,7 @@
if (getHwComposer().hasCapability(Capability::BOOT_DISPLAY_CONFIG)) {
if (const auto hwcId = getHwComposer().getPreferredBootDisplayMode(displayId)) {
if (const auto modeId = snapshot.translateModeId(*hwcId)) {
- info->preferredBootDisplayMode = modeId->value();
+ info->preferredBootDisplayMode = ftl::to_underlying(*modeId);
}
}
}
@@ -1230,10 +1230,8 @@
return NO_ERROR;
}
-void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& desiredMode) {
- const auto mode = desiredMode.mode;
- const auto displayId = mode.modePtr->getPhysicalDisplayId();
-
+void SurfaceFlinger::setDesiredMode(display::DisplayModeRequest&& request, bool force) {
+ const auto displayId = request.mode.modePtr->getPhysicalDisplayId();
ATRACE_NAME(ftl::Concat(__func__, ' ', displayId.value).c_str());
const auto display = getDisplayDeviceLocked(displayId);
@@ -1242,9 +1240,10 @@
return;
}
- const bool emitEvent = desiredMode.emitEvent;
+ const auto mode = request.mode;
+ const bool emitEvent = request.emitEvent;
- switch (display->setDesiredMode(std::move(desiredMode))) {
+ switch (display->setDesiredMode(std::move(request), force)) {
case DisplayDevice::DesiredModeAction::InitiateDisplayModeSwitch:
// DisplayDevice::setDesiredMode updated the render rate, so inform Scheduler.
mScheduler->setRenderRate(displayId,
@@ -1312,7 +1311,7 @@
[](const DisplayModePtr& mode) { return mode->getPeakFps(); });
if (!fpsOpt) {
- ALOGE("%s: Invalid mode %d for display %s", whence, modeId.value(),
+ ALOGE("%s: Invalid mode %d for display %s", whence, ftl::to_underlying(modeId),
to_string(snapshot.displayId()).c_str());
return BAD_VALUE;
}
@@ -1421,17 +1420,16 @@
if (!displayModePtrOpt) {
ALOGW("Desired display mode is no longer supported. Mode ID = %d",
- desiredModeId.value());
- dropModeRequest(display);
+ ftl::to_underlying(desiredModeId));
continue;
}
- ALOGV("%s changing active mode to %d(%s) for display %s", __func__, desiredModeId.value(),
+ ALOGV("%s changing active mode to %d(%s) for display %s", __func__,
+ ftl::to_underlying(desiredModeId),
to_string(displayModePtrOpt->get()->getVsyncRate()).c_str(),
to_string(display->getId()).c_str());
- if ((!FlagManager::getInstance().connected_display() || !desiredModeOpt->force) &&
- display->getActiveMode() == desiredModeOpt->mode) {
+ if (display->getActiveMode() == desiredModeOpt->mode) {
applyActiveMode(display);
continue;
}
@@ -1619,7 +1617,7 @@
[](const DisplayModePtr& mode) { return mode->getHwcId(); });
if (!hwcIdOpt) {
- ALOGE("%s: Invalid mode %d for display %s", whence, modeId.value(),
+ ALOGE("%s: Invalid mode %d for display %s", whence, ftl::to_underlying(modeId),
to_string(snapshot.displayId()).c_str());
return BAD_VALUE;
}
@@ -3286,88 +3284,13 @@
std::vector<HWComposer::HWCDisplayMode> hwcModes;
std::optional<hal::HWConfigId> activeModeHwcIdOpt;
- const bool isExternalDisplay = FlagManager::getInstance().connected_display() &&
- getHwComposer().getDisplayConnectionType(displayId) ==
- ui::DisplayConnectionType::External;
-
int attempt = 0;
constexpr int kMaxAttempts = 3;
do {
hwcModes = getHwComposer().getModes(displayId,
scheduler::RefreshRateSelector::kMinSupportedFrameRate
.getPeriodNsecs());
- const auto activeModeHwcIdExp = getHwComposer().getActiveMode(displayId);
- activeModeHwcIdOpt = activeModeHwcIdExp.value_opt();
-
- if (isExternalDisplay &&
- activeModeHwcIdExp.has_error([](status_t error) { return error == NO_INIT; })) {
- constexpr nsecs_t k59HzVsyncPeriod = 16949153;
- constexpr nsecs_t k60HzVsyncPeriod = 16666667;
-
- // DM sets the initial mode for an external display to 1080p@60, but
- // this comes after SF creates its own state (including the
- // DisplayDevice). For now, pick the same mode in order to avoid
- // inconsistent state and unnecessary mode switching.
- // TODO (b/318534874): Let DM decide the initial mode.
- //
- // Try to find 1920x1080 @ 60 Hz
- if (const auto iter = std::find_if(hwcModes.begin(), hwcModes.end(),
- [](const auto& mode) {
- return mode.width == 1920 &&
- mode.height == 1080 &&
- mode.vsyncPeriod == k60HzVsyncPeriod;
- });
- iter != hwcModes.end()) {
- activeModeHwcIdOpt = iter->hwcId;
- break;
- }
-
- // Try to find 1920x1080 @ 59-60 Hz
- if (const auto iter = std::find_if(hwcModes.begin(), hwcModes.end(),
- [](const auto& mode) {
- return mode.width == 1920 &&
- mode.height == 1080 &&
- mode.vsyncPeriod >= k60HzVsyncPeriod &&
- mode.vsyncPeriod <= k59HzVsyncPeriod;
- });
- iter != hwcModes.end()) {
- activeModeHwcIdOpt = iter->hwcId;
- break;
- }
-
- // The display does not support 1080p@60, and this is the last attempt to pick a display
- // mode. Prefer 60 Hz if available, with the closest resolution to 1080p.
- if (attempt + 1 == kMaxAttempts) {
- std::vector<HWComposer::HWCDisplayMode> hwcModeOpts;
-
- for (const auto& mode : hwcModes) {
- if (mode.width <= 1920 && mode.height <= 1080 &&
- mode.vsyncPeriod >= k60HzVsyncPeriod &&
- mode.vsyncPeriod <= k59HzVsyncPeriod) {
- hwcModeOpts.push_back(mode);
- }
- }
-
- if (const auto iter = std::max_element(hwcModeOpts.begin(), hwcModeOpts.end(),
- [](const auto& a, const auto& b) {
- const auto aSize = a.width * a.height;
- const auto bSize = b.width * b.height;
- if (aSize < bSize)
- return true;
- else if (aSize == bSize)
- return a.vsyncPeriod > b.vsyncPeriod;
- else
- return false;
- });
- iter != hwcModeOpts.end()) {
- activeModeHwcIdOpt = iter->hwcId;
- break;
- }
-
- // hwcModeOpts was empty, use hwcModes[0] as the last resort
- activeModeHwcIdOpt = hwcModes[0].hwcId;
- }
- }
+ activeModeHwcIdOpt = getHwComposer().getActiveMode(displayId).value_opt();
const auto isActiveMode = [activeModeHwcIdOpt](const HWComposer::HWCDisplayMode& mode) {
return mode.hwcId == activeModeHwcIdOpt;
@@ -3393,15 +3316,15 @@
})
.value_or(DisplayModes{});
- ui::DisplayModeId nextModeId = 1 +
- std::accumulate(oldModes.begin(), oldModes.end(), static_cast<ui::DisplayModeId>(-1),
- [](ui::DisplayModeId max, const auto& pair) {
- return std::max(max, pair.first.value());
- });
+ DisplayModeId nextModeId = std::accumulate(oldModes.begin(), oldModes.end(), DisplayModeId(-1),
+ [](DisplayModeId max, const auto& pair) {
+ return std::max(max, pair.first);
+ });
+ ++nextModeId;
DisplayModes newModes;
for (const auto& hwcMode : hwcModes) {
- const DisplayModeId id{nextModeId++};
+ const auto id = nextModeId++;
newModes.try_emplace(id,
DisplayMode::Builder(hwcMode.hwcId)
.setId(id)
@@ -3428,10 +3351,6 @@
return pair.second->getHwcId() == activeModeHwcIdOpt;
})->second;
- if (isExternalDisplay) {
- ALOGI("External display %s initial mode: {%s}", to_string(displayId).c_str(),
- to_string(*activeMode).c_str());
- }
return {modes, activeMode};
}
@@ -3476,8 +3395,12 @@
auto [displayModes, activeMode] = loadDisplayModes(displayId);
if (!activeMode) {
- // TODO(b/241286153): Report hotplug failure to the framework.
ALOGE("Failed to hotplug display %s", to_string(displayId).c_str());
+ if (FlagManager::getInstance().hotplug2()) {
+ mScheduler->onHotplugConnectionError(mAppConnectionHandle,
+ static_cast<int32_t>(
+ DisplayHotplugEvent::ERROR_UNKNOWN));
+ }
getHwComposer().disconnectDisplay(displayId);
return nullptr;
}
@@ -3736,27 +3659,6 @@
}
mDisplays.try_emplace(displayToken, std::move(display));
-
- // For an external display, loadDisplayModes already attempted to select the same mode
- // as DM, but SF still needs to be updated to match.
- // TODO (b/318534874): Let DM decide the initial mode.
- if (const auto& physical = state.physical;
- mScheduler && physical && FlagManager::getInstance().connected_display()) {
- const bool isInternalDisplay = mPhysicalDisplays.get(physical->id)
- .transform(&PhysicalDisplay::isInternal)
- .value_or(false);
-
- if (!isInternalDisplay) {
- auto activeModePtr = physical->activeMode;
- const auto fps = activeModePtr->getPeakFps();
-
- setDesiredMode(
- {.mode = scheduler::FrameRateMode{fps,
- ftl::as_non_null(std::move(activeModePtr))},
- .emitEvent = false,
- .force = true});
- }
- }
}
void SurfaceFlinger::processDisplayRemoved(const wp<IBinder>& displayToken) {
@@ -4178,8 +4080,8 @@
if (display->refreshRateSelector().isModeAllowed(request.mode)) {
setDesiredMode(std::move(request));
} else {
- ALOGV("%s: Mode %d is disallowed for display %s", __func__, modePtr->getId().value(),
- to_string(displayId).c_str());
+ ALOGV("%s: Mode %d is disallowed for display %s", __func__,
+ ftl::to_underlying(modePtr->getId()), to_string(displayId).c_str());
}
}
}
@@ -8473,15 +8375,15 @@
const auto preferredModeId = preferredMode.modePtr->getId();
const Fps preferredFps = preferredMode.fps;
- ALOGV("Switching to Scheduler preferred mode %d (%s)", preferredModeId.value(),
+ ALOGV("Switching to Scheduler preferred mode %d (%s)", ftl::to_underlying(preferredModeId),
to_string(preferredFps).c_str());
if (!selector.isModeAllowed(preferredMode)) {
- ALOGE("%s: Preferred mode %d is disallowed", __func__, preferredModeId.value());
+ ALOGE("%s: Preferred mode %d is disallowed", __func__, ftl::to_underlying(preferredModeId));
return INVALID_OPERATION;
}
- setDesiredMode({std::move(preferredMode), .emitEvent = true, .force = force});
+ setDesiredMode({std::move(preferredMode), .emitEvent = true}, force);
// Update the frameRateOverride list as the display render rate might have changed
if (mScheduler->updateFrameRateOverrides(scheduler::GlobalSignals{}, preferredFps)) {
@@ -8565,7 +8467,7 @@
scheduler::RefreshRateSelector::Policy policy =
display->refreshRateSelector().getDisplayManagerPolicy();
- outSpecs->defaultMode = policy.defaultMode.value();
+ outSpecs->defaultMode = ftl::to_underlying(policy.defaultMode);
outSpecs->allowGroupSwitching = policy.allowGroupSwitching;
outSpecs->primaryRanges = translate(policy.primaryRanges);
outSpecs->appRequestRanges = translate(policy.appRequestRanges);
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index be05797..992bc00 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -716,7 +716,7 @@
// Show hdr sdr ratio overlay
bool mHdrSdrRatioOverlay = false;
- void setDesiredMode(display::DisplayModeRequest&&) REQUIRES(mStateLock);
+ void setDesiredMode(display::DisplayModeRequest&&, bool force = false) REQUIRES(mStateLock);
status_t setActiveModeFromBackdoor(const sp<display::DisplayToken>&, DisplayModeId, Fps minFps,
Fps maxFps);
diff --git a/services/surfaceflinger/common/FlagManager.cpp b/services/surfaceflinger/common/FlagManager.cpp
index b07e7ac..425d2da 100644
--- a/services/surfaceflinger/common/FlagManager.cpp
+++ b/services/surfaceflinger/common/FlagManager.cpp
@@ -130,6 +130,8 @@
DUMP_READ_ONLY_FLAG(enable_layer_command_batching);
DUMP_READ_ONLY_FLAG(screenshot_fence_preservation);
DUMP_READ_ONLY_FLAG(vulkan_renderengine);
+ DUMP_READ_ONLY_FLAG(renderable_buffer_usage);
+ DUMP_READ_ONLY_FLAG(restore_blur_step);
#undef DUMP_READ_ONLY_FLAG
#undef DUMP_SERVER_FLAG
#undef DUMP_FLAG_INTERVAL
@@ -206,6 +208,8 @@
FLAG_MANAGER_READ_ONLY_FLAG(enable_layer_command_batching, "")
FLAG_MANAGER_READ_ONLY_FLAG(screenshot_fence_preservation, "debug.sf.screenshot_fence_preservation")
FLAG_MANAGER_READ_ONLY_FLAG(vulkan_renderengine, "debug.renderengine.vulkan")
+FLAG_MANAGER_READ_ONLY_FLAG(renderable_buffer_usage, "")
+FLAG_MANAGER_READ_ONLY_FLAG(restore_blur_step, "debug.renderengine.restore_blur_step")
/// Trunk stable server flags ///
FLAG_MANAGER_SERVER_FLAG(late_boot_misc2, "")
diff --git a/services/surfaceflinger/common/include/common/FlagManager.h b/services/surfaceflinger/common/include/common/FlagManager.h
index 2a30a40..86efd30 100644
--- a/services/surfaceflinger/common/include/common/FlagManager.h
+++ b/services/surfaceflinger/common/include/common/FlagManager.h
@@ -70,6 +70,8 @@
bool enable_layer_command_batching() const;
bool screenshot_fence_preservation() const;
bool vulkan_renderengine() const;
+ bool renderable_buffer_usage() const;
+ bool restore_blur_step() const;
protected:
// overridden for unit tests
diff --git a/services/surfaceflinger/surfaceflinger_flags.aconfig b/services/surfaceflinger/surfaceflinger_flags.aconfig
index fcbef01..0ebc41b 100644
--- a/services/surfaceflinger/surfaceflinger_flags.aconfig
+++ b/services/surfaceflinger/surfaceflinger_flags.aconfig
@@ -174,3 +174,25 @@
bug: "302703346"
is_fixed_read_only: true
}
+
+flag {
+ name: "renderable_buffer_usage"
+ namespace: "core_graphics"
+ description: "Decide whether an ExternalTexture isRenderable based on its buffer's usage."
+ bug: "305445199"
+ is_fixed_read_only: true
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
+}
+
+flag {
+ name: "restore_blur_step"
+ namespace: "core_graphics"
+ description: "Restore drawing the blur input prior to drawing blurred content."
+ bug: "255921628"
+ is_fixed_read_only: true
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
+}
diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp
index 7c191a5..da4e47f 100644
--- a/services/surfaceflinger/tests/unittests/Android.bp
+++ b/services/surfaceflinger/tests/unittests/Android.bp
@@ -129,7 +129,6 @@
"TransactionTraceWriterTest.cpp",
"TransactionTracingTest.cpp",
"TunnelModeEnabledReporterTest.cpp",
- "StrongTypingTest.cpp",
"VSyncCallbackRegistrationTest.cpp",
"VSyncDispatchTimerQueueTest.cpp",
"VSyncDispatchRealtimeTest.cpp",
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
index f26336a..387d2f2 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h
@@ -461,11 +461,9 @@
? IComposerClient::DisplayConnectionType::INTERNAL
: IComposerClient::DisplayConnectionType::EXTERNAL;
- using ::testing::AtLeast;
EXPECT_CALL(*test->mComposer, getDisplayConnectionType(HWC_DISPLAY_ID, _))
- .Times(AtLeast(1))
- .WillRepeatedly(DoAll(SetArgPointee<1>(CONNECTION_TYPE),
- Return(hal::V2_4::Error::NONE)));
+ .WillOnce(DoAll(SetArgPointee<1>(CONNECTION_TYPE),
+ Return(hal::V2_4::Error::NONE)));
}
EXPECT_CALL(*test->mComposer, setClientTargetSlotCount(_))
diff --git a/services/surfaceflinger/tests/unittests/LayerHistoryIntegrationTest.cpp b/services/surfaceflinger/tests/unittests/LayerHistoryIntegrationTest.cpp
index 01762c1..110f324 100644
--- a/services/surfaceflinger/tests/unittests/LayerHistoryIntegrationTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerHistoryIntegrationTest.cpp
@@ -1026,6 +1026,7 @@
};
TEST_F(SmallAreaDetectionTest, SmallDirtyLayer) {
+ SET_FLAG_FOR_TEST(flags::enable_small_area_detection, true);
auto layer = createLegacyAndFrontedEndLayer(1);
nsecs_t time = systemTime();
@@ -1043,6 +1044,7 @@
}
TEST_F(SmallAreaDetectionTest, NotSmallDirtyLayer) {
+ SET_FLAG_FOR_TEST(flags::enable_small_area_detection, true);
auto layer = createLegacyAndFrontedEndLayer(1);
nsecs_t time = systemTime();
@@ -1060,6 +1062,7 @@
}
TEST_F(SmallAreaDetectionTest, smallDirtyLayerWithMatrix) {
+ SET_FLAG_FOR_TEST(flags::enable_small_area_detection, true);
auto layer = createLegacyAndFrontedEndLayer(1);
nsecs_t time = systemTime();
diff --git a/services/surfaceflinger/tests/unittests/StrongTypingTest.cpp b/services/surfaceflinger/tests/unittests/StrongTypingTest.cpp
deleted file mode 100644
index 45b7610..0000000
--- a/services/surfaceflinger/tests/unittests/StrongTypingTest.cpp
+++ /dev/null
@@ -1,72 +0,0 @@
-/*
- * Copyright 2019 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.
- */
-
-#include <gmock/gmock.h>
-#include <gtest/gtest.h>
-#include "Scheduler/StrongTyping.h"
-
-using namespace testing;
-
-namespace android {
-
-TEST(StrongTypeTest, comparison) {
- using SpunkyType = StrongTyping<int, struct SpunkyTypeTag, Compare>;
- SpunkyType f1(10);
-
- EXPECT_TRUE(f1 == f1);
- EXPECT_TRUE(SpunkyType(10) != SpunkyType(11));
- EXPECT_FALSE(SpunkyType(31) != SpunkyType(31));
-
- EXPECT_TRUE(SpunkyType(10) < SpunkyType(11));
- EXPECT_TRUE(SpunkyType(-1) < SpunkyType(0));
- EXPECT_FALSE(SpunkyType(-10) < SpunkyType(-20));
-
- EXPECT_TRUE(SpunkyType(10) <= SpunkyType(11));
- EXPECT_TRUE(SpunkyType(10) <= SpunkyType(10));
- EXPECT_TRUE(SpunkyType(-10) <= SpunkyType(1));
- EXPECT_FALSE(SpunkyType(10) <= SpunkyType(9));
-
- EXPECT_TRUE(SpunkyType(11) >= SpunkyType(11));
- EXPECT_TRUE(SpunkyType(12) >= SpunkyType(11));
- EXPECT_FALSE(SpunkyType(11) >= SpunkyType(12));
-
- EXPECT_FALSE(SpunkyType(11) > SpunkyType(12));
- EXPECT_TRUE(SpunkyType(-11) < SpunkyType(7));
-}
-
-TEST(StrongTypeTest, addition) {
- using FunkyType = StrongTyping<int, struct FunkyTypeTag, Compare, Add>;
- FunkyType f2(22);
- FunkyType f1(10);
-
- EXPECT_THAT(f1 + f2, Eq(FunkyType(32)));
- EXPECT_THAT(f2 + f1, Eq(FunkyType(32)));
-
- EXPECT_THAT(++f1.value(), Eq(11));
- EXPECT_THAT(f1.value(), Eq(11));
- EXPECT_THAT(f1++.value(), Eq(11));
- EXPECT_THAT(f1++.value(), Eq(12));
- EXPECT_THAT(f1.value(), Eq(13));
-
- auto f3 = f1;
- EXPECT_THAT(f1, Eq(f3));
- EXPECT_THAT(f1, Lt(f2));
-
- f3 += f1;
- EXPECT_THAT(f1.value(), Eq(13));
- EXPECT_THAT(f3.value(), Eq(26));
-}
-} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
index 82b4ad0..9efb503 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp
@@ -21,12 +21,16 @@
#include "mock/DisplayHardware/MockDisplayMode.h"
#include "mock/MockDisplayModeSpecs.h"
-#include <com_android_graphics_surfaceflinger_flags.h>
-#include <common/test/FlagUtils.h>
#include <ftl/fake_guard.h>
#include <scheduler/Fps.h>
-using namespace com::android::graphics::surfaceflinger;
+#define EXPECT_SET_ACTIVE_CONFIG(displayId, modeId) \
+ EXPECT_CALL(*mComposer, \
+ setActiveConfigWithConstraints(displayId, \
+ static_cast<hal::HWConfigId>( \
+ ftl::to_underlying(modeId)), \
+ _, _)) \
+ .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)))
namespace android {
namespace {
@@ -165,8 +169,7 @@
mFlinger.onActiveDisplayChanged(nullptr, *mDisplay);
mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
- mock::createDisplayModeSpecs(kModeId90.value(), false, 0,
- 120));
+ mock::createDisplayModeSpecs(kModeId90, false, 0, 120));
ASSERT_TRUE(mDisplay->getDesiredMode());
EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId90);
@@ -174,10 +177,7 @@
// Verify that next commit will call setActiveConfigWithConstraints in HWC
const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};
- EXPECT_CALL(*mComposer,
- setActiveConfigWithConstraints(PrimaryDisplayVariant::HWC_DISPLAY_ID,
- hal::HWConfigId(kModeId90.value()), _, _))
- .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+ EXPECT_SET_ACTIVE_CONFIG(PrimaryDisplayVariant::HWC_DISPLAY_ID, kModeId90);
mFlinger.commit();
@@ -206,8 +206,7 @@
mFlinger.onActiveDisplayChanged(nullptr, *mDisplay);
mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
- mock::createDisplayModeSpecs(kModeId90.value(), true, 0,
- 120));
+ mock::createDisplayModeSpecs(kModeId90, true, 0, 120));
ASSERT_TRUE(mDisplay->getDesiredMode());
EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId90);
@@ -216,10 +215,7 @@
// Verify that next commit will call setActiveConfigWithConstraints in HWC
// and complete the mode change.
const VsyncPeriodChangeTimeline timeline{.refreshRequired = false};
- EXPECT_CALL(*mComposer,
- setActiveConfigWithConstraints(PrimaryDisplayVariant::HWC_DISPLAY_ID,
- hal::HWConfigId(kModeId90.value()), _, _))
- .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+ EXPECT_SET_ACTIVE_CONFIG(PrimaryDisplayVariant::HWC_DISPLAY_ID, kModeId90);
EXPECT_CALL(*mAppEventThread,
onModeChanged(scheduler::FrameRateMode{90_Hz, ftl::as_non_null(kMode90)}));
@@ -242,28 +238,20 @@
mFlinger.onActiveDisplayChanged(nullptr, *mDisplay);
mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
- mock::createDisplayModeSpecs(kModeId90.value(), false, 0,
- 120));
+ mock::createDisplayModeSpecs(kModeId90, false, 0, 120));
const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};
- EXPECT_CALL(*mComposer,
- setActiveConfigWithConstraints(PrimaryDisplayVariant::HWC_DISPLAY_ID,
- hal::HWConfigId(kModeId90.value()), _, _))
- .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+ EXPECT_SET_ACTIVE_CONFIG(PrimaryDisplayVariant::HWC_DISPLAY_ID, kModeId90);
mFlinger.commit();
mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
- mock::createDisplayModeSpecs(kModeId120.value(), false, 0,
- 180));
+ mock::createDisplayModeSpecs(kModeId120, false, 0, 180));
ASSERT_TRUE(mDisplay->getDesiredMode());
EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId120);
- EXPECT_CALL(*mComposer,
- setActiveConfigWithConstraints(PrimaryDisplayVariant::HWC_DISPLAY_ID,
- hal::HWConfigId(kModeId120.value()), _, _))
- .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+ EXPECT_SET_ACTIVE_CONFIG(PrimaryDisplayVariant::HWC_DISPLAY_ID, kModeId120);
mFlinger.commit();
@@ -285,8 +273,7 @@
mFlinger.onActiveDisplayChanged(nullptr, *mDisplay);
mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
- mock::createDisplayModeSpecs(kModeId90_4K.value(), false, 0,
- 120));
+ mock::createDisplayModeSpecs(kModeId90_4K, false, 0, 120));
ASSERT_TRUE(mDisplay->getDesiredMode());
EXPECT_EQ(mDisplay->getDesiredMode()->mode.modePtr->getId(), kModeId90_4K);
@@ -295,10 +282,7 @@
// Verify that next commit will call setActiveConfigWithConstraints in HWC
// and complete the mode change.
const VsyncPeriodChangeTimeline timeline{.refreshRequired = false};
- EXPECT_CALL(*mComposer,
- setActiveConfigWithConstraints(PrimaryDisplayVariant::HWC_DISPLAY_ID,
- hal::HWConfigId(kModeId90_4K.value()), _, _))
- .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+ EXPECT_SET_ACTIVE_CONFIG(PrimaryDisplayVariant::HWC_DISPLAY_ID, kModeId90_4K);
EXPECT_CALL(*mAppEventThread, onHotplugReceived(mDisplay->getPhysicalId(), true));
@@ -335,7 +319,7 @@
}
if (arg->getDesiredMode()->mode.modePtr->getId() != modeId) {
- *result_listener << "Unexpected desired mode " << modeId;
+ *result_listener << "Unexpected desired mode " << ftl::to_underlying(modeId);
return false;
}
@@ -349,14 +333,15 @@
MATCHER_P(ModeSettledTo, modeId, "") {
if (const auto desiredOpt = arg->getDesiredMode()) {
- *result_listener << "Unsettled desired mode " << desiredOpt->mode.modePtr->getId();
+ *result_listener << "Unsettled desired mode "
+ << ftl::to_underlying(desiredOpt->mode.modePtr->getId());
return false;
}
ftl::FakeGuard guard(kMainThreadContext);
if (arg->getActiveMode().modePtr->getId() != modeId) {
- *result_listener << "Settled to unexpected active mode " << modeId;
+ *result_listener << "Settled to unexpected active mode " << ftl::to_underlying(modeId);
return false;
}
@@ -364,13 +349,6 @@
}
TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) {
- SET_FLAG_FOR_TEST(flags::connected_display, true);
-
- // For the inner display, this is handled by setupHwcHotplugCallExpectations.
- EXPECT_CALL(*mComposer, getDisplayConnectionType(kOuterDisplayHwcId, _))
- .WillOnce(DoAll(SetArgPointee<1>(IComposerClient::DisplayConnectionType::INTERNAL),
- Return(hal::V2_4::Error::NONE)));
-
const auto [innerDisplay, outerDisplay] = injectOuterDisplay();
EXPECT_TRUE(innerDisplay->isPoweredOn());
@@ -387,22 +365,19 @@
EXPECT_EQ(NO_ERROR,
mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(),
- mock::createDisplayModeSpecs(kModeId90.value(),
- false, 0.f, 120.f)));
+ mock::createDisplayModeSpecs(kModeId90, false,
+ 0.f, 120.f)));
EXPECT_EQ(NO_ERROR,
mFlinger.setDesiredDisplayModeSpecs(outerDisplay->getDisplayToken().promote(),
- mock::createDisplayModeSpecs(kModeId60.value(),
- false, 0.f, 120.f)));
+ mock::createDisplayModeSpecs(kModeId60, false,
+ 0.f, 120.f)));
EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120));
const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};
- EXPECT_CALL(*mComposer,
- setActiveConfigWithConstraints(kInnerDisplayHwcId,
- hal::HWConfigId(kModeId90.value()), _, _))
- .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+ EXPECT_SET_ACTIVE_CONFIG(kInnerDisplayHwcId, kModeId90);
mFlinger.commit();
@@ -423,10 +398,7 @@
EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90));
EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));
- EXPECT_CALL(*mComposer,
- setActiveConfigWithConstraints(kOuterDisplayHwcId,
- hal::HWConfigId(kModeId60.value()), _, _))
- .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+ EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId60);
mFlinger.commit();
@@ -440,12 +412,6 @@
}
TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) {
- SET_FLAG_FOR_TEST(flags::connected_display, true);
-
- // For the inner display, this is handled by setupHwcHotplugCallExpectations.
- EXPECT_CALL(*mComposer, getDisplayConnectionType(kOuterDisplayHwcId, _))
- .WillOnce(DoAll(SetArgPointee<1>(IComposerClient::DisplayConnectionType::INTERNAL),
- Return(hal::V2_4::Error::NONE)));
const auto [innerDisplay, outerDisplay] = injectOuterDisplay();
EXPECT_TRUE(innerDisplay->isPoweredOn());
@@ -464,27 +430,20 @@
EXPECT_EQ(NO_ERROR,
mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(),
- mock::createDisplayModeSpecs(kModeId90.value(),
- false, 0.f, 120.f)));
+ mock::createDisplayModeSpecs(kModeId90, false,
+ 0.f, 120.f)));
EXPECT_EQ(NO_ERROR,
mFlinger.setDesiredDisplayModeSpecs(outerDisplay->getDisplayToken().promote(),
- mock::createDisplayModeSpecs(kModeId60.value(),
- false, 0.f, 120.f)));
+ mock::createDisplayModeSpecs(kModeId60, false,
+ 0.f, 120.f)));
EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));
const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};
- EXPECT_CALL(*mComposer,
- setActiveConfigWithConstraints(kInnerDisplayHwcId,
- hal::HWConfigId(kModeId90.value()), _, _))
- .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
-
- EXPECT_CALL(*mComposer,
- setActiveConfigWithConstraints(kOuterDisplayHwcId,
- hal::HWConfigId(kModeId60.value()), _, _))
- .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+ EXPECT_SET_ACTIVE_CONFIG(kInnerDisplayHwcId, kModeId90);
+ EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId60);
mFlinger.commit();
@@ -503,8 +462,8 @@
EXPECT_EQ(NO_ERROR,
mFlinger.setDesiredDisplayModeSpecs(mDisplay->getDisplayToken().promote(),
- mock::createDisplayModeSpecs(kModeId90.value(),
- false, 0.f, 120.f)));
+ mock::createDisplayModeSpecs(kModeId90, false,
+ 0.f, 120.f)));
EXPECT_THAT(mDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
@@ -512,10 +471,7 @@
mDisplay->setPowerMode(hal::PowerMode::OFF);
const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};
- EXPECT_CALL(*mComposer,
- setActiveConfigWithConstraints(kInnerDisplayHwcId,
- hal::HWConfigId(kModeId90.value()), _, _))
- .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+ EXPECT_SET_ACTIVE_CONFIG(kInnerDisplayHwcId, kModeId90);
mFlinger.commit();
@@ -529,13 +485,6 @@
}
TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) {
- SET_FLAG_FOR_TEST(flags::connected_display, true);
-
- // For the inner display, this is handled by setupHwcHotplugCallExpectations.
- EXPECT_CALL(*mComposer, getDisplayConnectionType(kOuterDisplayHwcId, _))
- .WillOnce(DoAll(SetArgPointee<1>(IComposerClient::DisplayConnectionType::INTERNAL),
- Return(hal::V2_4::Error::NONE)));
-
const auto [innerDisplay, outerDisplay] = injectOuterDisplay();
EXPECT_TRUE(innerDisplay->isPoweredOn());
@@ -554,13 +503,13 @@
EXPECT_EQ(NO_ERROR,
mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(),
- mock::createDisplayModeSpecs(kModeId90.value(),
- false, 0.f, 120.f)));
+ mock::createDisplayModeSpecs(kModeId90, false,
+ 0.f, 120.f)));
EXPECT_EQ(NO_ERROR,
mFlinger.setDesiredDisplayModeSpecs(outerDisplay->getDisplayToken().promote(),
- mock::createDisplayModeSpecs(kModeId60.value(),
- false, 0.f, 120.f)));
+ mock::createDisplayModeSpecs(kModeId60, false,
+ 0.f, 120.f)));
EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90));
EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60));
@@ -569,10 +518,7 @@
outerDisplay->setPowerMode(hal::PowerMode::OFF);
const VsyncPeriodChangeTimeline timeline{.refreshRequired = true};
- EXPECT_CALL(*mComposer,
- setActiveConfigWithConstraints(kInnerDisplayHwcId,
- hal::HWConfigId(kModeId90.value()), _, _))
- .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+ EXPECT_SET_ACTIVE_CONFIG(kInnerDisplayHwcId, kModeId90);
mFlinger.commit();
@@ -591,10 +537,7 @@
// Only the outer display is powered on.
mFlinger.onActiveDisplayChanged(innerDisplay.get(), *outerDisplay);
- EXPECT_CALL(*mComposer,
- setActiveConfigWithConstraints(kOuterDisplayHwcId,
- hal::HWConfigId(kModeId60.value()), _, _))
- .WillOnce(DoAll(SetArgPointee<3>(timeline), Return(Error::NONE)));
+ EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId60);
mFlinger.commit();
diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp
index a270dc9..897f9a0 100644
--- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp
@@ -17,8 +17,14 @@
#undef LOG_TAG
#define LOG_TAG "LibSurfaceFlingerUnittests"
+#include <aidl/android/hardware/graphics/common/DisplayHotplugEvent.h>
+#include <com_android_graphics_surfaceflinger_flags.h>
+#include <common/test/FlagUtils.h>
#include "DisplayTransactionTestHelpers.h"
+using namespace com::android::graphics::surfaceflinger;
+using ::aidl::android::hardware::graphics::common::DisplayHotplugEvent;
+
namespace android {
class HotplugTest : public DisplayTransactionTest {};
@@ -87,6 +93,8 @@
}
TEST_F(HotplugTest, rejectsHotplugIfFailedToLoadDisplayModes) {
+ SET_FLAG_FOR_TEST(flags::connected_display, true);
+
// Inject a primary display.
PrimaryDisplayVariant::injectHwcDisplay(this);
@@ -94,6 +102,10 @@
constexpr bool kFailedHotplug = true;
ExternalDisplay::setupHwcHotplugCallExpectations<kFailedHotplug>(this);
+ EXPECT_CALL(*mEventThread,
+ onHotplugConnectionError(static_cast<int32_t>(DisplayHotplugEvent::ERROR_UNKNOWN)))
+ .Times(1);
+
// Simulate a connect event that fails to load display modes due to HWC already having
// disconnected the display but SF yet having to process the queued disconnect event.
EXPECT_CALL(*mComposer, getActiveConfig(ExternalDisplay::HWC_DISPLAY_ID, _))
diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockDisplayMode.h b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockDisplayMode.h
index e298e7c..685d8f9 100644
--- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockDisplayMode.h
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockDisplayMode.h
@@ -24,7 +24,7 @@
DisplayModeId modeId, Fps displayRefreshRate, int32_t group = 0,
ui::Size resolution = ui::Size(1920, 1080),
PhysicalDisplayId displayId = PhysicalDisplayId::fromPort(0)) {
- return DisplayMode::Builder(hal::HWConfigId(modeId.value()))
+ return DisplayMode::Builder(hal::HWConfigId(ftl::to_underlying(modeId)))
.setId(modeId)
.setPhysicalDisplayId(displayId)
.setVsyncPeriod(displayRefreshRate.getPeriodNsecs())
diff --git a/services/surfaceflinger/tests/unittests/mock/MockDisplayModeSpecs.h b/services/surfaceflinger/tests/unittests/mock/MockDisplayModeSpecs.h
index a71e82c..7b18a82 100644
--- a/services/surfaceflinger/tests/unittests/mock/MockDisplayModeSpecs.h
+++ b/services/surfaceflinger/tests/unittests/mock/MockDisplayModeSpecs.h
@@ -18,12 +18,15 @@
#include <android/gui/DisplayModeSpecs.h>
+#include "DisplayHardware/DisplayMode.h"
+
namespace android::mock {
-inline gui::DisplayModeSpecs createDisplayModeSpecs(int32_t defaultMode, bool allowGroupSwitching,
- float minFps, float maxFps) {
+inline gui::DisplayModeSpecs createDisplayModeSpecs(DisplayModeId defaultMode,
+ bool allowGroupSwitching, float minFps,
+ float maxFps) {
gui::DisplayModeSpecs specs;
- specs.defaultMode = defaultMode;
+ specs.defaultMode = ftl::to_underlying(defaultMode);
specs.allowGroupSwitching = allowGroupSwitching;
specs.primaryRanges.physical.min = minFps;
specs.primaryRanges.physical.max = maxFps;
diff --git a/vulkan/libvulkan/swapchain.cpp b/vulkan/libvulkan/swapchain.cpp
index 867fc45..892cd19 100644
--- a/vulkan/libvulkan/swapchain.cpp
+++ b/vulkan/libvulkan/swapchain.cpp
@@ -786,6 +786,9 @@
all_formats.emplace_back(
VkSurfaceFormatKHR{VK_FORMAT_R16G16B16A16_SFLOAT,
VK_COLOR_SPACE_EXTENDED_SRGB_NONLINEAR_EXT});
+ all_formats.emplace_back(
+ VkSurfaceFormatKHR{VK_FORMAT_R16G16B16A16_SFLOAT,
+ VK_COLOR_SPACE_DISPLAY_P3_NONLINEAR_EXT});
}
}