Merge "SF: Introduce TransactionTrace testsuite" into tm-dev
diff --git a/cmds/installd/dexopt.cpp b/cmds/installd/dexopt.cpp
index c796da6..9647865 100644
--- a/cmds/installd/dexopt.cpp
+++ b/cmds/installd/dexopt.cpp
@@ -74,10 +74,19 @@
using android::base::ReadFully;
using android::base::StringPrintf;
using android::base::WriteFully;
+using android::base::borrowed_fd;
using android::base::unique_fd;
namespace {
+// Timeout for short operations, such as merging profiles.
+constexpr int kShortTimeoutMs = 60000; // 1 minute.
+
+// Timeout for long operations, such as compilation. This should be smaller than the Package Manager
+// watchdog (PackageManagerService.WATCHDOG_TIMEOUT, 10 minutes), so that the operation will be
+// aborted before that watchdog would take down the system server.
+constexpr int kLongTimeoutMs = 570000; // 9.5 minutes.
+
class DexOptStatus {
public:
// Check if dexopt is cancelled and fork if it is not cancelled.
@@ -486,6 +495,25 @@
}
}
+// Cleans up an output file specified by a file descriptor. This function should be called whenever
+// a subprocess that modifies a system-managed file crashes.
+// If the subprocess crashes while it's writing to the file, the file is likely corrupted, so we
+// should remove it.
+// If the subprocess times out and is killed while it's acquiring a flock on the file, there is
+// probably a deadlock, so it's also good to remove the file so that later operations won't
+// encounter the same problem. It's safe to do so because the process that is holding the flock will
+// still have access to the file until the file descriptor is closed.
+// Note that we can't do `clear_reference_profile` here even if the fd points to a reference profile
+// because that also requires a flock and is therefore likely to be stuck in the second case.
+static bool cleanup_output_fd(int fd) {
+ std::string path;
+ bool ret = remove_file_at_fd(fd, &path);
+ if (ret) {
+ LOG(INFO) << "Removed file at path " << path;
+ }
+ return ret;
+}
+
static constexpr int PROFMAN_BIN_RETURN_CODE_SUCCESS = 0;
static constexpr int PROFMAN_BIN_RETURN_CODE_COMPILE = 1;
static constexpr int PROFMAN_BIN_RETURN_CODE_SKIP_COMPILATION_NOT_ENOUGH_DELTA = 2;
@@ -497,13 +525,14 @@
class RunProfman : public ExecVHelper {
public:
- void SetupArgs(const std::vector<unique_fd>& profile_fds,
- const unique_fd& reference_profile_fd,
- const std::vector<unique_fd>& apk_fds,
- const std::vector<std::string>& dex_locations,
- bool copy_and_update,
- bool for_snapshot,
- bool for_boot_image) {
+ template <typename T, typename U>
+ void SetupArgs(const std::vector<T>& profile_fds,
+ const unique_fd& reference_profile_fd,
+ const std::vector<U>& apk_fds,
+ const std::vector<std::string>& dex_locations,
+ bool copy_and_update,
+ bool for_snapshot,
+ bool for_boot_image) {
// TODO(calin): Assume for now we run in the bg compile job (which is in
// most of the invocation). With the current data flow, is not very easy or
@@ -519,11 +548,11 @@
AddArg("--reference-profile-file-fd=" + std::to_string(reference_profile_fd.get()));
}
- for (const unique_fd& fd : profile_fds) {
+ for (const T& fd : profile_fds) {
AddArg("--profile-file-fd=" + std::to_string(fd.get()));
}
- for (const unique_fd& fd : apk_fds) {
+ for (const U& fd : apk_fds) {
AddArg("--apk-fd=" + std::to_string(fd.get()));
}
@@ -582,20 +611,14 @@
for_boot_image);
}
- void SetupCopyAndUpdate(unique_fd&& profile_fd,
- unique_fd&& reference_profile_fd,
- unique_fd&& apk_fd,
+ void SetupCopyAndUpdate(const unique_fd& profile_fd,
+ const unique_fd& reference_profile_fd,
+ const unique_fd& apk_fd,
const std::string& dex_location) {
- // The fds need to stay open longer than the scope of the function, so put them into a local
- // variable vector.
- profiles_fd_.push_back(std::move(profile_fd));
- apk_fds_.push_back(std::move(apk_fd));
- reference_profile_fd_ = std::move(reference_profile_fd);
- std::vector<std::string> dex_locations = {dex_location};
- SetupArgs(profiles_fd_,
- reference_profile_fd_,
- apk_fds_,
- dex_locations,
+ SetupArgs(std::vector<borrowed_fd>{profile_fd},
+ reference_profile_fd,
+ std::vector<borrowed_fd>{apk_fd},
+ {dex_location},
/*copy_and_update=*/true,
/*for_snapshot*/false,
/*for_boot_image*/false);
@@ -621,11 +644,6 @@
void Exec() {
ExecVHelper::Exec(DexoptReturnCodes::kProfmanExec);
}
-
- private:
- unique_fd reference_profile_fd_;
- std::vector<unique_fd> profiles_fd_;
- std::vector<unique_fd> apk_fds_;
};
static int analyze_profiles(uid_t uid, const std::string& package_name,
@@ -657,13 +675,14 @@
profman_merge.Exec();
}
/* parent */
- int return_code = wait_child(pid);
+ int return_code = wait_child_with_timeout(pid, kShortTimeoutMs);
bool need_to_compile = false;
bool empty_profiles = false;
bool should_clear_current_profiles = false;
bool should_clear_reference_profile = false;
if (!WIFEXITED(return_code)) {
LOG(WARNING) << "profman failed for location " << location << ": " << return_code;
+ cleanup_output_fd(reference_profile_fd.get());
} else {
return_code = WEXITSTATUS(return_code);
switch (return_code) {
@@ -797,10 +816,10 @@
profman_dump.Exec();
}
/* parent */
- int return_code = wait_child(pid);
+ int return_code = wait_child_with_timeout(pid, kShortTimeoutMs);
if (!WIFEXITED(return_code)) {
- LOG(WARNING) << "profman failed for package " << pkgname << ": "
- << return_code;
+ LOG(WARNING) << "profman failed for package " << pkgname << ": " << return_code;
+ cleanup_output_fd(output_fd.get());
return false;
}
return true;
@@ -871,7 +890,11 @@
_exit(0);
}
/* parent */
- int return_code = wait_child(pid);
+ int return_code = wait_child_with_timeout(pid, kShortTimeoutMs);
+ if (!WIFEXITED(return_code)) {
+ cleanup_output_fd(out_fd.get());
+ return false;
+ }
return return_code == 0;
}
@@ -1521,7 +1544,7 @@
}
pipe_read.reset();
- int return_code = wait_child(pid);
+ int return_code = wait_child_with_timeout(pid, kShortTimeoutMs);
if (!WIFEXITED(return_code)) {
PLOG(ERROR) << "Error waiting for child dexoptanalyzer process";
return false;
@@ -1695,7 +1718,7 @@
}
/* parent */
- int result = wait_child(pid);
+ int result = wait_child_with_timeout(pid, kShortTimeoutMs);
cancelled = dexopt_status_->check_if_killed_and_remove_dexopt_pid(pid);
if (!WIFEXITED(result)) {
if ((WTERMSIG(result) == SIGKILL) && cancelled) {
@@ -1954,7 +1977,7 @@
runner.Exec(DexoptReturnCodes::kDex2oatExec);
} else {
- int res = wait_child(pid);
+ int res = wait_child_with_timeout(pid, kLongTimeoutMs);
bool cancelled = dexopt_status_->check_if_killed_and_remove_dexopt_pid(pid);
if (res == 0) {
LOG(VERBOSE) << "DexInv: --- END '" << dex_path << "' (success) ---";
@@ -2143,7 +2166,7 @@
_exit(result ? kReconcileSecondaryDexCleanedUp : kReconcileSecondaryDexAccessIOError);
}
- int return_code = wait_child(pid);
+ int return_code = wait_child_with_timeout(pid, kShortTimeoutMs);
if (!WIFEXITED(return_code)) {
LOG(WARNING) << "reconcile dex failed for location " << dex_path << ": " << return_code;
} else {
@@ -2261,7 +2284,7 @@
if (!ReadFully(pipe_read, out_secondary_dex_hash->data(), out_secondary_dex_hash->size())) {
out_secondary_dex_hash->clear();
}
- return wait_child(pid) == 0;
+ return wait_child_with_timeout(pid, kShortTimeoutMs) == 0;
}
// Helper for move_ab, so that we can have common failure-case cleanup.
@@ -2591,9 +2614,10 @@
}
/* parent */
- int return_code = wait_child(pid);
+ int return_code = wait_child_with_timeout(pid, kShortTimeoutMs);
if (!WIFEXITED(return_code)) {
LOG(WARNING) << "profman failed for " << package_name << ":" << profile_name;
+ cleanup_output_fd(snapshot_fd.get());
return false;
}
@@ -2700,10 +2724,11 @@
}
/* parent */
- int return_code = wait_child(pid);
+ int return_code = wait_child_with_timeout(pid, kShortTimeoutMs);
if (!WIFEXITED(return_code)) {
PLOG(WARNING) << "profman failed for " << package_name << ":" << profile_name;
+ cleanup_output_fd(snapshot_fd.get());
return false;
}
@@ -2774,9 +2799,9 @@
}
RunProfman args;
- args.SetupCopyAndUpdate(std::move(dex_metadata_fd),
- std::move(ref_profile_fd),
- std::move(apk_fd),
+ args.SetupCopyAndUpdate(dex_metadata_fd,
+ ref_profile_fd,
+ apk_fd,
code_path);
pid_t pid = fork();
if (pid == 0) {
@@ -2789,9 +2814,10 @@
}
/* parent */
- int return_code = wait_child(pid);
+ int return_code = wait_child_with_timeout(pid, kShortTimeoutMs);
if (!WIFEXITED(return_code)) {
PLOG(WARNING) << "profman failed for " << package_name << ":" << profile_name;
+ cleanup_output_fd(ref_profile_fd.get());
return false;
}
return true;
diff --git a/cmds/installd/tests/installd_dexopt_test.cpp b/cmds/installd/tests/installd_dexopt_test.cpp
index bb36c39..f21a304 100644
--- a/cmds/installd/tests/installd_dexopt_test.cpp
+++ b/cmds/installd/tests/installd_dexopt_test.cpp
@@ -50,6 +50,8 @@
namespace android {
namespace installd {
+constexpr int kTimeoutMs = 60000;
+
// TODO(calin): try to dedup this code.
#if defined(__arm__)
static const std::string kRuntimeIsa = "arm";
@@ -1073,7 +1075,7 @@
_exit(0);
}
/* parent */
- ASSERT_TRUE(WIFEXITED(wait_child(pid)));
+ ASSERT_TRUE(WIFEXITED(wait_child_with_timeout(pid, kTimeoutMs)));
}
void mergePackageProfiles(const std::string& package_name,
@@ -1377,7 +1379,7 @@
_exit(0);
}
/* parent */
- ASSERT_TRUE(WIFEXITED(wait_child(pid)));
+ ASSERT_TRUE(WIFEXITED(wait_child_with_timeout(pid, kTimeoutMs)));
}
protected:
std::string intial_android_profiles_dir;
diff --git a/cmds/installd/tests/installd_utils_test.cpp b/cmds/installd/tests/installd_utils_test.cpp
index a7a8624..8d1ccdc 100644
--- a/cmds/installd/tests/installd_utils_test.cpp
+++ b/cmds/installd/tests/installd_utils_test.cpp
@@ -14,8 +14,10 @@
* limitations under the License.
*/
+#include <errno.h>
#include <stdlib.h>
#include <string.h>
+#include <unistd.h>
#include <android-base/logging.h>
#include <android-base/scopeguard.h>
@@ -690,5 +692,45 @@
create_data_misc_supplemental_shared_path(nullptr, false, 10, "com.foo"));
}
+TEST_F(UtilsTest, WaitChild) {
+ pid_t pid = fork();
+ if (pid == 0) {
+ /* child */
+ // Do nothing.
+ _exit(0);
+ }
+ /* parent */
+ int return_code = wait_child_with_timeout(pid, /*timeout_ms=*/100);
+ EXPECT_TRUE(WIFEXITED(return_code));
+ EXPECT_EQ(WEXITSTATUS(return_code), 0);
+}
+
+TEST_F(UtilsTest, WaitChildTimeout) {
+ pid_t pid = fork();
+ if (pid == 0) {
+ /* child */
+ sleep(1);
+ _exit(0);
+ }
+ /* parent */
+ int return_code = wait_child_with_timeout(pid, /*timeout_ms=*/1);
+ EXPECT_FALSE(WIFEXITED(return_code));
+ EXPECT_EQ(WTERMSIG(return_code), SIGKILL);
+}
+
+TEST_F(UtilsTest, RemoveFileAtFd) {
+ std::string filename = "/data/local/tmp/tempfile-XXXXXX";
+ int fd = mkstemp(filename.data());
+ ASSERT_GE(fd, 0);
+ ASSERT_EQ(access(filename.c_str(), F_OK), 0);
+
+ std::string actual_filename;
+ remove_file_at_fd(fd, &actual_filename);
+ EXPECT_NE(access(filename.c_str(), F_OK), 0);
+ EXPECT_EQ(filename, actual_filename);
+
+ close(fd);
+}
+
} // namespace installd
} // namespace android
diff --git a/cmds/installd/utils.cpp b/cmds/installd/utils.cpp
index 3ce4b67..6650b76 100644
--- a/cmds/installd/utils.cpp
+++ b/cmds/installd/utils.cpp
@@ -19,18 +19,21 @@
#include <errno.h>
#include <fcntl.h>
#include <fts.h>
+#include <poll.h>
#include <stdlib.h>
#include <sys/capability.h>
+#include <sys/pidfd.h>
#include <sys/stat.h>
#include <sys/statvfs.h>
#include <sys/wait.h>
#include <sys/xattr.h>
+#include <unistd.h>
#include <uuid/uuid.h>
#include <android-base/file.h>
#include <android-base/logging.h>
-#include <android-base/strings.h>
#include <android-base/stringprintf.h>
+#include <android-base/strings.h>
#include <android-base/unique_fd.h>
#include <cutils/fs.h>
#include <cutils/properties.h>
@@ -1096,30 +1099,45 @@
return fs_prepare_dir(path.c_str(), 0750, uid, gid);
}
-int wait_child(pid_t pid)
-{
+static int wait_child(pid_t pid) {
int status;
- pid_t got_pid;
+ pid_t got_pid = TEMP_FAILURE_RETRY(waitpid(pid, &status, /*options=*/0));
- while (1) {
- got_pid = waitpid(pid, &status, 0);
- if (got_pid == -1 && errno == EINTR) {
- printf("waitpid interrupted, retrying\n");
- } else {
- break;
- }
- }
if (got_pid != pid) {
- ALOGW("waitpid failed: wanted %d, got %d: %s\n",
- (int) pid, (int) got_pid, strerror(errno));
- return 1;
+ PLOG(ERROR) << "waitpid failed: wanted " << pid << ", got " << got_pid;
+ return W_EXITCODE(/*exit_code=*/255, /*signal_number=*/0);
}
- if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
- return 0;
- } else {
- return status; /* always nonzero */
+ return status;
+}
+
+int wait_child_with_timeout(pid_t pid, int timeout_ms) {
+ int pidfd = pidfd_open(pid, /*flags=*/0);
+ if (pidfd < 0) {
+ PLOG(ERROR) << "pidfd_open failed for pid " << pid;
+ kill(pid, SIGKILL);
+ return wait_child(pid);
}
+
+ struct pollfd pfd;
+ pfd.fd = pidfd;
+ pfd.events = POLLIN;
+ int poll_ret = TEMP_FAILURE_RETRY(poll(&pfd, /*nfds=*/1, timeout_ms));
+
+ close(pidfd);
+
+ if (poll_ret < 0) {
+ PLOG(ERROR) << "poll failed for pid " << pid;
+ kill(pid, SIGKILL);
+ return wait_child(pid);
+ }
+ if (poll_ret == 0) {
+ LOG(WARNING) << "Child process " << pid << " timed out after " << timeout_ms
+ << "ms. Killing it";
+ kill(pid, SIGKILL);
+ return wait_child(pid);
+ }
+ return wait_child(pid);
}
/**
@@ -1332,5 +1350,27 @@
}
}
+bool remove_file_at_fd(int fd, /*out*/ std::string* path) {
+ char path_buffer[PATH_MAX + 1];
+ std::string proc_path = android::base::StringPrintf("/proc/self/fd/%d", fd);
+ ssize_t len = readlink(proc_path.c_str(), path_buffer, PATH_MAX);
+ if (len < 0) {
+ PLOG(WARNING) << "Could not remove file at fd " << fd << ": Failed to get file path";
+ return false;
+ }
+ path_buffer[len] = '\0';
+ if (path != nullptr) {
+ *path = path_buffer;
+ }
+ if (unlink(path_buffer) != 0) {
+ if (errno == ENOENT) {
+ return true;
+ }
+ PLOG(WARNING) << "Could not remove file at path " << path_buffer;
+ return false;
+ }
+ return true;
+}
+
} // namespace installd
} // namespace android
diff --git a/cmds/installd/utils.h b/cmds/installd/utils.h
index 2db1623..2d00845 100644
--- a/cmds/installd/utils.h
+++ b/cmds/installd/utils.h
@@ -160,7 +160,8 @@
int ensure_config_user_dirs(userid_t userid);
-int wait_child(pid_t pid);
+// Waits for a child process, or kills it if it times out. Returns the exit code.
+int wait_child_with_timeout(pid_t pid, int timeout_ms);
int prepare_app_cache_dir(const std::string& parent, const char* name, mode_t target_mode,
uid_t uid, gid_t gid);
@@ -177,6 +178,10 @@
void drop_capabilities(uid_t uid);
+// Removes a file specified by a file descriptor. Returns true on success. Reports the file path to
+// `path` if present.
+bool remove_file_at_fd(int fd, /*out*/ std::string* path = nullptr);
+
} // namespace installd
} // namespace android
diff --git a/cmds/installd/view_compiler.cpp b/cmds/installd/view_compiler.cpp
index 60d6492..8c000a1 100644
--- a/cmds/installd/view_compiler.cpp
+++ b/cmds/installd/view_compiler.cpp
@@ -33,7 +33,13 @@
namespace android {
namespace installd {
-using base::unique_fd;
+namespace {
+
+using ::android::base::unique_fd;
+
+constexpr int kTimeoutMs = 300000;
+
+} // namespace
bool view_compiler(const char* apk_path, const char* package_name, const char* out_dex_file,
int uid) {
@@ -88,7 +94,17 @@
_exit(1);
}
- return wait_child(pid) == 0;
+ int return_code = wait_child_with_timeout(pid, kTimeoutMs);
+ if (!WIFEXITED(return_code)) {
+ LOG(WARNING) << "viewcompiler failed for " << package_name << ":" << apk_path;
+ if (WTERMSIG(return_code) == SIGKILL) {
+ // If the subprocess is killed while it's writing to the file, the file is likely
+ // corrupted, so we should remove it.
+ remove_file_at_fd(outfd.get());
+ }
+ return false;
+ }
+ return WEXITSTATUS(return_code) == 0;
}
} // namespace installd
diff --git a/libs/binder/ProcessState.cpp b/libs/binder/ProcessState.cpp
index 1821729..baa817c 100644
--- a/libs/binder/ProcessState.cpp
+++ b/libs/binder/ProcessState.cpp
@@ -19,6 +19,7 @@
#include <binder/ProcessState.h>
#include <android-base/result.h>
+#include <android-base/strings.h>
#include <binder/BpBinder.h>
#include <binder/IPCThreadState.h>
#include <binder/IServiceManager.h>
@@ -367,8 +368,13 @@
String8 ProcessState::makeBinderThreadName() {
int32_t s = android_atomic_add(1, &mThreadPoolSeq);
pid_t pid = getpid();
+
+ std::string_view driverName = mDriverName.c_str();
+ android::base::ConsumePrefix(&driverName, "/dev/");
+
String8 name;
- name.appendFormat("%d_%X:%s", pid, s, mDriverName.c_str());
+ name.appendFormat("%.*s:%d_%X", static_cast<int>(driverName.length()), driverName.data(), pid,
+ s);
return name;
}
diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp
index 75c5e26..5ab0abc 100644
--- a/libs/gui/ISurfaceComposer.cpp
+++ b/libs/gui/ISurfaceComposer.cpp
@@ -46,9 +46,11 @@
namespace android {
+using gui::DisplayCaptureArgs;
using gui::IDisplayEventConnection;
using gui::IRegionSamplingListener;
using gui::IWindowInfosListener;
+using gui::LayerCaptureArgs;
using ui::ColorMode;
class BpSurfaceComposer : public BpInterface<ISurfaceComposer>
@@ -118,36 +120,6 @@
remote()->transact(BnSurfaceComposer::BOOT_FINISHED, data, &reply);
}
- status_t captureDisplay(const DisplayCaptureArgs& args,
- const sp<IScreenCaptureListener>& captureListener) override {
- Parcel data, reply;
- data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor());
- SAFE_PARCEL(args.write, data);
- SAFE_PARCEL(data.writeStrongBinder, IInterface::asBinder(captureListener));
-
- return remote()->transact(BnSurfaceComposer::CAPTURE_DISPLAY, data, &reply);
- }
-
- status_t captureDisplay(DisplayId displayId,
- const sp<IScreenCaptureListener>& captureListener) override {
- Parcel data, reply;
- data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor());
- SAFE_PARCEL(data.writeUint64, displayId.value);
- SAFE_PARCEL(data.writeStrongBinder, IInterface::asBinder(captureListener));
-
- return remote()->transact(BnSurfaceComposer::CAPTURE_DISPLAY_BY_ID, data, &reply);
- }
-
- status_t captureLayers(const LayerCaptureArgs& args,
- const sp<IScreenCaptureListener>& captureListener) override {
- Parcel data, reply;
- data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor());
- SAFE_PARCEL(args.write, data);
- SAFE_PARCEL(data.writeStrongBinder, IInterface::asBinder(captureListener));
-
- return remote()->transact(BnSurfaceComposer::CAPTURE_LAYERS, data, &reply);
- }
-
bool authenticateSurfaceTexture(
const sp<IGraphicBufferProducer>& bufferProducer) const override {
Parcel data, reply;
@@ -1451,36 +1423,6 @@
bootFinished();
return NO_ERROR;
}
- case CAPTURE_DISPLAY: {
- CHECK_INTERFACE(ISurfaceComposer, data, reply);
- DisplayCaptureArgs args;
- sp<IScreenCaptureListener> captureListener;
- SAFE_PARCEL(args.read, data);
- SAFE_PARCEL(data.readStrongBinder, &captureListener);
-
- return captureDisplay(args, captureListener);
- }
- case CAPTURE_DISPLAY_BY_ID: {
- CHECK_INTERFACE(ISurfaceComposer, data, reply);
- uint64_t value;
- SAFE_PARCEL(data.readUint64, &value);
- const auto id = DisplayId::fromValue(value);
- if (!id) return BAD_VALUE;
-
- sp<IScreenCaptureListener> captureListener;
- SAFE_PARCEL(data.readStrongBinder, &captureListener);
-
- return captureDisplay(*id, captureListener);
- }
- case CAPTURE_LAYERS: {
- CHECK_INTERFACE(ISurfaceComposer, data, reply);
- LayerCaptureArgs args;
- sp<IScreenCaptureListener> captureListener;
- SAFE_PARCEL(args.read, data);
- SAFE_PARCEL(data.readStrongBinder, &captureListener);
-
- return captureLayers(args, captureListener);
- }
case AUTHENTICATE_SURFACE: {
CHECK_INTERFACE(ISurfaceComposer, data, reply);
sp<IGraphicBufferProducer> bufferProducer =
diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index 9022e7d..6944d38 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -686,85 +686,89 @@
// ----------------------------------------------------------------------------
-status_t CaptureArgs::write(Parcel& output) const {
- SAFE_PARCEL(output.writeInt32, static_cast<int32_t>(pixelFormat));
- SAFE_PARCEL(output.write, sourceCrop);
- SAFE_PARCEL(output.writeFloat, frameScaleX);
- SAFE_PARCEL(output.writeFloat, frameScaleY);
- SAFE_PARCEL(output.writeBool, captureSecureLayers);
- SAFE_PARCEL(output.writeInt32, uid);
- SAFE_PARCEL(output.writeInt32, static_cast<int32_t>(dataspace));
- SAFE_PARCEL(output.writeBool, allowProtected);
- SAFE_PARCEL(output.writeBool, grayscale);
+namespace gui {
+
+status_t CaptureArgs::writeToParcel(Parcel* output) const {
+ SAFE_PARCEL(output->writeInt32, static_cast<int32_t>(pixelFormat));
+ SAFE_PARCEL(output->write, sourceCrop);
+ SAFE_PARCEL(output->writeFloat, frameScaleX);
+ SAFE_PARCEL(output->writeFloat, frameScaleY);
+ SAFE_PARCEL(output->writeBool, captureSecureLayers);
+ SAFE_PARCEL(output->writeInt32, uid);
+ SAFE_PARCEL(output->writeInt32, static_cast<int32_t>(dataspace));
+ SAFE_PARCEL(output->writeBool, allowProtected);
+ SAFE_PARCEL(output->writeBool, grayscale);
return NO_ERROR;
}
-status_t CaptureArgs::read(const Parcel& input) {
+status_t CaptureArgs::readFromParcel(const Parcel* input) {
int32_t value = 0;
- SAFE_PARCEL(input.readInt32, &value);
+ SAFE_PARCEL(input->readInt32, &value);
pixelFormat = static_cast<ui::PixelFormat>(value);
- SAFE_PARCEL(input.read, sourceCrop);
- SAFE_PARCEL(input.readFloat, &frameScaleX);
- SAFE_PARCEL(input.readFloat, &frameScaleY);
- SAFE_PARCEL(input.readBool, &captureSecureLayers);
- SAFE_PARCEL(input.readInt32, &uid);
- SAFE_PARCEL(input.readInt32, &value);
+ SAFE_PARCEL(input->read, sourceCrop);
+ SAFE_PARCEL(input->readFloat, &frameScaleX);
+ SAFE_PARCEL(input->readFloat, &frameScaleY);
+ SAFE_PARCEL(input->readBool, &captureSecureLayers);
+ SAFE_PARCEL(input->readInt32, &uid);
+ SAFE_PARCEL(input->readInt32, &value);
dataspace = static_cast<ui::Dataspace>(value);
- SAFE_PARCEL(input.readBool, &allowProtected);
- SAFE_PARCEL(input.readBool, &grayscale);
+ SAFE_PARCEL(input->readBool, &allowProtected);
+ SAFE_PARCEL(input->readBool, &grayscale);
return NO_ERROR;
}
-status_t DisplayCaptureArgs::write(Parcel& output) const {
- SAFE_PARCEL(CaptureArgs::write, output);
+status_t DisplayCaptureArgs::writeToParcel(Parcel* output) const {
+ SAFE_PARCEL(CaptureArgs::writeToParcel, output);
- SAFE_PARCEL(output.writeStrongBinder, displayToken);
- SAFE_PARCEL(output.writeUint32, width);
- SAFE_PARCEL(output.writeUint32, height);
- SAFE_PARCEL(output.writeBool, useIdentityTransform);
+ SAFE_PARCEL(output->writeStrongBinder, displayToken);
+ SAFE_PARCEL(output->writeUint32, width);
+ SAFE_PARCEL(output->writeUint32, height);
+ SAFE_PARCEL(output->writeBool, useIdentityTransform);
return NO_ERROR;
}
-status_t DisplayCaptureArgs::read(const Parcel& input) {
- SAFE_PARCEL(CaptureArgs::read, input);
+status_t DisplayCaptureArgs::readFromParcel(const Parcel* input) {
+ SAFE_PARCEL(CaptureArgs::readFromParcel, input);
- SAFE_PARCEL(input.readStrongBinder, &displayToken);
- SAFE_PARCEL(input.readUint32, &width);
- SAFE_PARCEL(input.readUint32, &height);
- SAFE_PARCEL(input.readBool, &useIdentityTransform);
+ SAFE_PARCEL(input->readStrongBinder, &displayToken);
+ SAFE_PARCEL(input->readUint32, &width);
+ SAFE_PARCEL(input->readUint32, &height);
+ SAFE_PARCEL(input->readBool, &useIdentityTransform);
return NO_ERROR;
}
-status_t LayerCaptureArgs::write(Parcel& output) const {
- SAFE_PARCEL(CaptureArgs::write, output);
+status_t LayerCaptureArgs::writeToParcel(Parcel* output) const {
+ SAFE_PARCEL(CaptureArgs::writeToParcel, output);
- SAFE_PARCEL(output.writeStrongBinder, layerHandle);
- SAFE_PARCEL(output.writeInt32, excludeHandles.size());
+ SAFE_PARCEL(output->writeStrongBinder, layerHandle);
+ SAFE_PARCEL(output->writeInt32, excludeHandles.size());
for (auto el : excludeHandles) {
- SAFE_PARCEL(output.writeStrongBinder, el);
+ SAFE_PARCEL(output->writeStrongBinder, el);
}
- SAFE_PARCEL(output.writeBool, childrenOnly);
+ SAFE_PARCEL(output->writeBool, childrenOnly);
return NO_ERROR;
}
-status_t LayerCaptureArgs::read(const Parcel& input) {
- SAFE_PARCEL(CaptureArgs::read, input);
+status_t LayerCaptureArgs::readFromParcel(const Parcel* input) {
+ SAFE_PARCEL(CaptureArgs::readFromParcel, input);
- SAFE_PARCEL(input.readStrongBinder, &layerHandle);
+ SAFE_PARCEL(input->readStrongBinder, &layerHandle);
int32_t numExcludeHandles = 0;
- SAFE_PARCEL_READ_SIZE(input.readInt32, &numExcludeHandles, input.dataSize());
+ SAFE_PARCEL_READ_SIZE(input->readInt32, &numExcludeHandles, input->dataSize());
excludeHandles.reserve(numExcludeHandles);
for (int i = 0; i < numExcludeHandles; i++) {
sp<IBinder> binder;
- SAFE_PARCEL(input.readStrongBinder, &binder);
+ SAFE_PARCEL(input->readStrongBinder, &binder);
excludeHandles.emplace(binder);
}
- SAFE_PARCEL(input.readBool, &childrenOnly);
+ SAFE_PARCEL(input->readBool, &childrenOnly);
return NO_ERROR;
}
+}; // namespace gui
+
ReleaseCallbackId BufferData::generateReleaseCallbackId() const {
return {buffer->getId(), frameNumber};
}
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 9269c3e..26ccda5 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -46,6 +46,7 @@
#include <ui/DynamicDisplayInfo.h>
#include <private/gui/ComposerService.h>
+#include <private/gui/ComposerServiceAIDL.h>
// This server size should always be smaller than the server cache size
#define BUFFER_CACHE_MAX_SIZE 64
@@ -62,6 +63,7 @@
// ---------------------------------------------------------------------------
ANDROID_SINGLETON_STATIC_INSTANCE(ComposerService);
+ANDROID_SINGLETON_STATIC_INSTANCE(ComposerServiceAIDL);
namespace {
// Initialize transaction id counter used to generate transaction ids
@@ -120,6 +122,52 @@
mDeathObserver = nullptr;
}
+ComposerServiceAIDL::ComposerServiceAIDL() : Singleton<ComposerServiceAIDL>() {
+ std::scoped_lock lock(mMutex);
+ connectLocked();
+}
+
+bool ComposerServiceAIDL::connectLocked() {
+ const String16 name("SurfaceFlingerAIDL");
+ mComposerService = waitForService<gui::ISurfaceComposer>(name);
+ if (mComposerService == nullptr) {
+ return false; // fatal error or permission problem
+ }
+
+ // Create the death listener.
+ class DeathObserver : public IBinder::DeathRecipient {
+ ComposerServiceAIDL& mComposerService;
+ virtual void binderDied(const wp<IBinder>& who) {
+ ALOGW("ComposerService aidl remote (surfaceflinger) died [%p]", who.unsafe_get());
+ mComposerService.composerServiceDied();
+ }
+
+ public:
+ explicit DeathObserver(ComposerServiceAIDL& mgr) : mComposerService(mgr) {}
+ };
+
+ mDeathObserver = new DeathObserver(*const_cast<ComposerServiceAIDL*>(this));
+ IInterface::asBinder(mComposerService)->linkToDeath(mDeathObserver);
+ return true;
+}
+
+/*static*/ sp<gui::ISurfaceComposer> ComposerServiceAIDL::getComposerService() {
+ ComposerServiceAIDL& instance = ComposerServiceAIDL::getInstance();
+ std::scoped_lock lock(instance.mMutex);
+ if (instance.mComposerService == nullptr) {
+ if (ComposerServiceAIDL::getInstance().connectLocked()) {
+ ALOGD("ComposerServiceAIDL reconnected");
+ }
+ }
+ return instance.mComposerService;
+}
+
+void ComposerServiceAIDL::composerServiceDied() {
+ std::scoped_lock lock(mMutex);
+ mComposerService = nullptr;
+ mDeathObserver = nullptr;
+}
+
class DefaultComposerClient: public Singleton<DefaultComposerClient> {
Mutex mLock;
sp<SurfaceComposerClient> mClient;
@@ -2267,26 +2315,29 @@
status_t ScreenshotClient::captureDisplay(const DisplayCaptureArgs& captureArgs,
const sp<IScreenCaptureListener>& captureListener) {
- sp<ISurfaceComposer> s(ComposerService::getComposerService());
+ sp<gui::ISurfaceComposer> s(ComposerServiceAIDL::getComposerService());
if (s == nullptr) return NO_INIT;
- return s->captureDisplay(captureArgs, captureListener);
+ binder::Status status = s->captureDisplay(captureArgs, captureListener);
+ return status.transactionError();
}
status_t ScreenshotClient::captureDisplay(DisplayId displayId,
const sp<IScreenCaptureListener>& captureListener) {
- sp<ISurfaceComposer> s(ComposerService::getComposerService());
+ sp<gui::ISurfaceComposer> s(ComposerServiceAIDL::getComposerService());
if (s == nullptr) return NO_INIT;
- return s->captureDisplay(displayId, captureListener);
+ binder::Status status = s->captureDisplayById(displayId.value, captureListener);
+ return status.transactionError();
}
status_t ScreenshotClient::captureLayers(const LayerCaptureArgs& captureArgs,
const sp<IScreenCaptureListener>& captureListener) {
- sp<ISurfaceComposer> s(ComposerService::getComposerService());
+ sp<gui::ISurfaceComposer> s(ComposerServiceAIDL::getComposerService());
if (s == nullptr) return NO_INIT;
- return s->captureLayers(captureArgs, captureListener);
+ binder::Status status = s->captureLayers(captureArgs, captureListener);
+ return status.transactionError();
}
// ---------------------------------------------------------------------------------
diff --git a/libs/gui/aidl/android/gui/DisplayCaptureArgs.aidl b/libs/gui/aidl/android/gui/DisplayCaptureArgs.aidl
new file mode 100644
index 0000000..2caa2b9
--- /dev/null
+++ b/libs/gui/aidl/android/gui/DisplayCaptureArgs.aidl
@@ -0,0 +1,19 @@
+/*
+ * Copyright 2021 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.
+ */
+
+package android.gui;
+
+parcelable DisplayCaptureArgs cpp_header "gui/DisplayCaptureArgs.h";
diff --git a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl
new file mode 100644
index 0000000..07921a5
--- /dev/null
+++ b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl
@@ -0,0 +1,42 @@
+/*
+ * Copyright 2021 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.
+ */
+
+package android.gui;
+
+import android.gui.DisplayCaptureArgs;
+import android.gui.LayerCaptureArgs;
+import android.gui.IScreenCaptureListener;
+
+/** @hide */
+interface ISurfaceComposer {
+ /**
+ * Capture the specified screen. This requires READ_FRAME_BUFFER
+ * permission. This function will fail if there is a secure window on
+ * screen and DisplayCaptureArgs.captureSecureLayers is false.
+ *
+ * This function can capture a subregion (the source crop) of the screen.
+ * The subregion can be optionally rotated. It will also be scaled to
+ * match the size of the output buffer.
+ */
+ void captureDisplay(in DisplayCaptureArgs args, IScreenCaptureListener listener);
+ void captureDisplayById(long displayId, IScreenCaptureListener listener);
+ /**
+ * Capture a subtree of the layer hierarchy, potentially ignoring the root node.
+ * This requires READ_FRAME_BUFFER permission. This function will fail if there
+ * is a secure window on screen
+ */
+ void captureLayers(in LayerCaptureArgs args, IScreenCaptureListener listener);
+}
diff --git a/libs/gui/aidl/android/gui/LayerCaptureArgs.aidl b/libs/gui/aidl/android/gui/LayerCaptureArgs.aidl
new file mode 100644
index 0000000..f0def50
--- /dev/null
+++ b/libs/gui/aidl/android/gui/LayerCaptureArgs.aidl
@@ -0,0 +1,19 @@
+/*
+ * Copyright 2021 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.
+ */
+
+package android.gui;
+
+parcelable LayerCaptureArgs cpp_header "gui/LayerCaptureArgs.h";
diff --git a/libs/gui/include/gui/DisplayCaptureArgs.h b/libs/gui/include/gui/DisplayCaptureArgs.h
new file mode 100644
index 0000000..ec884cf
--- /dev/null
+++ b/libs/gui/include/gui/DisplayCaptureArgs.h
@@ -0,0 +1,71 @@
+/*
+ * Copyright (C) 2022 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 <stdint.h>
+#include <sys/types.h>
+
+#include <binder/IBinder.h>
+#include <binder/Parcel.h>
+#include <binder/Parcelable.h>
+#include <ui/GraphicTypes.h>
+#include <ui/PixelFormat.h>
+
+namespace android::gui {
+
+struct CaptureArgs : public Parcelable {
+ const static int32_t UNSET_UID = -1;
+ virtual ~CaptureArgs() = default;
+
+ ui::PixelFormat pixelFormat{ui::PixelFormat::RGBA_8888};
+ Rect sourceCrop;
+ float frameScaleX{1};
+ float frameScaleY{1};
+ bool captureSecureLayers{false};
+ int32_t uid{UNSET_UID};
+ // Force capture to be in a color space. If the value is ui::Dataspace::UNKNOWN, the captured
+ // result will be in the display's colorspace.
+ // The display may use non-RGB dataspace (ex. displayP3) that could cause pixel data could be
+ // different from SRGB (byte per color), and failed when checking colors in tests.
+ // NOTE: In normal cases, we want the screen to be captured in display's colorspace.
+ ui::Dataspace dataspace = ui::Dataspace::UNKNOWN;
+
+ // The receiver of the capture can handle protected buffer. A protected buffer has
+ // GRALLOC_USAGE_PROTECTED usage bit and must not be accessed unprotected behaviour.
+ // Any read/write access from unprotected context will result in undefined behaviour.
+ // Protected contents are typically DRM contents. This has no direct implication to the
+ // secure property of the surface, which is specified by the application explicitly to avoid
+ // the contents being accessed/captured by screenshot or unsecure display.
+ bool allowProtected = false;
+
+ bool grayscale = false;
+
+ virtual status_t writeToParcel(Parcel* output) const;
+ virtual status_t readFromParcel(const Parcel* input);
+};
+
+struct DisplayCaptureArgs : CaptureArgs {
+ sp<IBinder> displayToken;
+ uint32_t width{0};
+ uint32_t height{0};
+ bool useIdentityTransform{false};
+
+ status_t writeToParcel(Parcel* output) const override;
+ status_t readFromParcel(const Parcel* input) override;
+};
+
+}; // namespace android::gui
diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h
index 0a59f52..4dfc383 100644
--- a/libs/gui/include/gui/ISurfaceComposer.h
+++ b/libs/gui/include/gui/ISurfaceComposer.h
@@ -58,11 +58,9 @@
struct client_cache_t;
struct ComposerState;
-struct DisplayCaptureArgs;
struct DisplayStatInfo;
struct DisplayState;
struct InputWindowCommands;
-struct LayerCaptureArgs;
class LayerDebugInfo;
class HdrCapabilities;
class IGraphicBufferProducer;
@@ -75,6 +73,13 @@
using gui::IScreenCaptureListener;
using gui::SpHash;
+namespace gui {
+
+struct DisplayCaptureArgs;
+struct LayerCaptureArgs;
+
+} // namespace gui
+
namespace ui {
struct DisplayMode;
@@ -261,27 +266,6 @@
*/
virtual void setGameContentType(const sp<IBinder>& display, bool on) = 0;
- /**
- * Capture the specified screen. This requires READ_FRAME_BUFFER
- * permission. This function will fail if there is a secure window on
- * screen and DisplayCaptureArgs.captureSecureLayers is false.
- *
- * This function can capture a subregion (the source crop) of the screen.
- * The subregion can be optionally rotated. It will also be scaled to
- * match the size of the output buffer.
- */
- virtual status_t captureDisplay(const DisplayCaptureArgs&,
- const sp<IScreenCaptureListener>&) = 0;
-
- virtual status_t captureDisplay(DisplayId, const sp<IScreenCaptureListener>&) = 0;
-
- /**
- * Capture a subtree of the layer hierarchy, potentially ignoring the root node.
- * This requires READ_FRAME_BUFFER permission. This function will fail if there
- * is a secure window on screen
- */
- virtual status_t captureLayers(const LayerCaptureArgs&, const sp<IScreenCaptureListener>&) = 0;
-
/* Clears the frame statistics for animations.
*
* Requires the ACCESS_SURFACE_FLINGER permission.
@@ -621,8 +605,8 @@
GET_DISPLAY_MODES, // Deprecated. Use GET_DYNAMIC_DISPLAY_INFO instead.
GET_ACTIVE_DISPLAY_MODE, // Deprecated. Use GET_DYNAMIC_DISPLAY_INFO instead.
GET_DISPLAY_STATE,
- CAPTURE_DISPLAY,
- CAPTURE_LAYERS,
+ CAPTURE_DISPLAY, // Deprecated. Autogenerated by .aidl now.
+ CAPTURE_LAYERS, // Deprecated. Autogenerated by .aidl now.
CLEAR_ANIMATION_FRAME_STATS,
GET_ANIMATION_FRAME_STATS,
SET_POWER_MODE,
@@ -649,7 +633,7 @@
GET_DESIRED_DISPLAY_MODE_SPECS,
GET_DISPLAY_BRIGHTNESS_SUPPORT,
SET_DISPLAY_BRIGHTNESS,
- CAPTURE_DISPLAY_BY_ID,
+ CAPTURE_DISPLAY_BY_ID, // Deprecated. Autogenerated by .aidl now.
NOTIFY_POWER_BOOST,
SET_GLOBAL_SHADOW_SETTINGS,
GET_AUTO_LOW_LATENCY_MODE_SUPPORT, // Deprecated. Use GET_DYNAMIC_DISPLAY_INFO instead.
diff --git a/libs/gui/include/gui/LayerCaptureArgs.h b/libs/gui/include/gui/LayerCaptureArgs.h
new file mode 100644
index 0000000..05ff9d5
--- /dev/null
+++ b/libs/gui/include/gui/LayerCaptureArgs.h
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2022 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 <stdint.h>
+#include <sys/types.h>
+
+#include <gui/DisplayCaptureArgs.h>
+#include <gui/SpHash.h>
+#include <unordered_set>
+
+namespace android::gui {
+
+struct LayerCaptureArgs : CaptureArgs {
+ sp<IBinder> layerHandle;
+ std::unordered_set<sp<IBinder>, SpHash<IBinder>> excludeHandles;
+ bool childrenOnly{false};
+
+ status_t writeToParcel(Parcel* output) const override;
+ status_t readFromParcel(const Parcel* input) override;
+};
+
+}; // namespace android::gui
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index ac9e428..885b4ae 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -29,7 +29,9 @@
#include <android/gui/DropInputMode.h>
#include <android/gui/FocusRequest.h>
+#include <gui/DisplayCaptureArgs.h>
#include <gui/ISurfaceComposer.h>
+#include <gui/LayerCaptureArgs.h>
#include <gui/LayerMetadata.h>
#include <gui/SpHash.h>
#include <gui/SurfaceControl.h>
@@ -375,56 +377,6 @@
bool ValidateFrameRate(float frameRate, int8_t compatibility, int8_t changeFrameRateStrategy,
const char* functionName, bool privileged = false);
-struct CaptureArgs {
- const static int32_t UNSET_UID = -1;
- virtual ~CaptureArgs() = default;
-
- ui::PixelFormat pixelFormat{ui::PixelFormat::RGBA_8888};
- Rect sourceCrop;
- float frameScaleX{1};
- float frameScaleY{1};
- bool captureSecureLayers{false};
- int32_t uid{UNSET_UID};
- // Force capture to be in a color space. If the value is ui::Dataspace::UNKNOWN, the captured
- // result will be in the display's colorspace.
- // The display may use non-RGB dataspace (ex. displayP3) that could cause pixel data could be
- // different from SRGB (byte per color), and failed when checking colors in tests.
- // NOTE: In normal cases, we want the screen to be captured in display's colorspace.
- ui::Dataspace dataspace = ui::Dataspace::UNKNOWN;
-
- // The receiver of the capture can handle protected buffer. A protected buffer has
- // GRALLOC_USAGE_PROTECTED usage bit and must not be accessed unprotected behaviour.
- // Any read/write access from unprotected context will result in undefined behaviour.
- // Protected contents are typically DRM contents. This has no direct implication to the
- // secure property of the surface, which is specified by the application explicitly to avoid
- // the contents being accessed/captured by screenshot or unsecure display.
- bool allowProtected = false;
-
- bool grayscale = false;
-
- virtual status_t write(Parcel& output) const;
- virtual status_t read(const Parcel& input);
-};
-
-struct DisplayCaptureArgs : CaptureArgs {
- sp<IBinder> displayToken;
- uint32_t width{0};
- uint32_t height{0};
- bool useIdentityTransform{false};
-
- status_t write(Parcel& output) const override;
- status_t read(const Parcel& input) override;
-};
-
-struct LayerCaptureArgs : CaptureArgs {
- sp<IBinder> layerHandle;
- std::unordered_set<sp<IBinder>, SpHash<IBinder>> excludeHandles;
- bool childrenOnly{false};
-
- status_t write(Parcel& output) const override;
- status_t read(const Parcel& input) override;
-};
-
}; // namespace android
#endif // ANDROID_SF_LAYER_STATE_H
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 25637ef..6c79b5b 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -57,7 +57,9 @@
class ITunnelModeEnabledListener;
class Region;
+using gui::DisplayCaptureArgs;
using gui::IRegionSamplingListener;
+using gui::LayerCaptureArgs;
struct SurfaceControlStats {
SurfaceControlStats(const sp<SurfaceControl>& sc, nsecs_t latchTime,
diff --git a/libs/gui/include/private/gui/ComposerService.h b/libs/gui/include/private/gui/ComposerService.h
index fa1071a..05ed0a0 100644
--- a/libs/gui/include/private/gui/ComposerService.h
+++ b/libs/gui/include/private/gui/ComposerService.h
@@ -37,7 +37,7 @@
// Users of this class should not retain the value from
// getComposerService() for an extended period.
//
-// (It's not clear that using Singleton is useful here anymore.)
+// (TODO: b/219785927, It's not clear that using Singleton is useful here anymore.)
class ComposerService : public Singleton<ComposerService>
{
sp<ISurfaceComposer> mComposerService;
diff --git a/libs/gui/include/private/gui/ComposerServiceAIDL.h b/libs/gui/include/private/gui/ComposerServiceAIDL.h
new file mode 100644
index 0000000..fee37ee
--- /dev/null
+++ b/libs/gui/include/private/gui/ComposerServiceAIDL.h
@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2021 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 <stdint.h>
+#include <sys/types.h>
+
+#include <android/gui/ISurfaceComposer.h>
+
+#include <utils/Singleton.h>
+#include <utils/StrongPointer.h>
+
+namespace android {
+
+// ---------------------------------------------------------------------------
+
+// ---------------------------------------------------------------------------
+
+// This holds our connection to the composer service (i.e. SurfaceFlinger).
+// If the remote side goes away, we will re-establish the connection.
+// Users of this class should not retain the value from
+// getComposerService() for an extended period.
+//
+// (TODO: b/219785927, It's not clear that using Singleton is useful here anymore.)
+class ComposerServiceAIDL : public Singleton<ComposerServiceAIDL> {
+ sp<gui::ISurfaceComposer> mComposerService;
+ sp<IBinder::DeathRecipient> mDeathObserver;
+ mutable std::mutex mMutex;
+
+ ComposerServiceAIDL();
+ bool connectLocked();
+ void composerServiceDied();
+ friend class Singleton<ComposerServiceAIDL>;
+
+public:
+ // Get a connection to the Composer Service. This will block until
+ // a connection is established. Returns null if permission is denied.
+ static sp<gui::ISurfaceComposer> getComposerService();
+};
+
+// ---------------------------------------------------------------------------
+}; // namespace android
diff --git a/libs/gui/tests/BLASTBufferQueue_test.cpp b/libs/gui/tests/BLASTBufferQueue_test.cpp
index 42a32f3..179bdd7 100644
--- a/libs/gui/tests/BLASTBufferQueue_test.cpp
+++ b/libs/gui/tests/BLASTBufferQueue_test.cpp
@@ -29,6 +29,7 @@
#include <gui/SyncScreenCaptureListener.h>
#include <gui/test/CallbackUtils.h>
#include <private/gui/ComposerService.h>
+#include <private/gui/ComposerServiceAIDL.h>
#include <ui/DisplayMode.h>
#include <ui/GraphicBuffer.h>
#include <ui/GraphicTypes.h>
@@ -283,13 +284,13 @@
static status_t captureDisplay(DisplayCaptureArgs& captureArgs,
ScreenCaptureResults& captureResults) {
- const auto sf = ComposerService::getComposerService();
+ const auto sf = ComposerServiceAIDL::getComposerService();
SurfaceComposerClient::Transaction().apply(true);
const sp<SyncScreenCaptureListener> captureListener = new SyncScreenCaptureListener();
- status_t status = sf->captureDisplay(captureArgs, captureListener);
- if (status != NO_ERROR) {
- return status;
+ binder::Status status = sf->captureDisplay(captureArgs, captureListener);
+ if (status.transactionError() != NO_ERROR) {
+ return status.transactionError();
}
captureResults = captureListener->waitForResults();
return captureResults.result;
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index 6056280..a885e92 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -31,6 +31,7 @@
#include <gui/SyncScreenCaptureListener.h>
#include <inttypes.h>
#include <private/gui/ComposerService.h>
+#include <private/gui/ComposerServiceAIDL.h>
#include <sys/types.h>
#include <ui/BufferQueueDefs.h>
#include <ui/DisplayMode.h>
@@ -205,13 +206,13 @@
static status_t captureDisplay(DisplayCaptureArgs& captureArgs,
ScreenCaptureResults& captureResults) {
- const auto sf = ComposerService::getComposerService();
+ const auto sf = ComposerServiceAIDL::getComposerService();
SurfaceComposerClient::Transaction().apply(true);
const sp<SyncScreenCaptureListener> captureListener = new SyncScreenCaptureListener();
- status_t status = sf->captureDisplay(captureArgs, captureListener);
- if (status != NO_ERROR) {
- return status;
+ binder::Status status = sf->captureDisplay(captureArgs, captureListener);
+ if (status.transactionError() != NO_ERROR) {
+ return status.transactionError();
}
captureResults = captureListener->waitForResults();
return captureResults.result;
@@ -766,16 +767,6 @@
void setAutoLowLatencyMode(const sp<IBinder>& /*display*/, bool /*on*/) override {}
void setGameContentType(const sp<IBinder>& /*display*/, bool /*on*/) override {}
- status_t captureDisplay(const DisplayCaptureArgs&, const sp<IScreenCaptureListener>&) override {
- return NO_ERROR;
- }
- status_t captureDisplay(DisplayId, const sp<IScreenCaptureListener>&) override {
- return NO_ERROR;
- }
- status_t captureLayers(const LayerCaptureArgs&, const sp<IScreenCaptureListener>&) override {
- return NO_ERROR;
- }
-
status_t clearAnimationFrameStats() override { return NO_ERROR; }
status_t getAnimationFrameStats(FrameStats* /*outStats*/) const override {
return NO_ERROR;
diff --git a/services/surfaceflinger/Android.bp b/services/surfaceflinger/Android.bp
index d9958f3..000a2cb 100644
--- a/services/surfaceflinger/Android.bp
+++ b/services/surfaceflinger/Android.bp
@@ -76,6 +76,7 @@
"libaidlcommonsupport",
"libcompositionengine",
"libframetimeline",
+ "libgui_aidl_static",
"libperfetto_client_experimental",
"librenderengine",
"libscheduler",
diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h
index 9c32b1f..f6c81c0 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.h
+++ b/services/surfaceflinger/Scheduler/Scheduler.h
@@ -169,6 +169,7 @@
// The period is the vsync period from the current display configuration.
void resyncToHardwareVsync(bool makeAvailable, nsecs_t period);
void resync() EXCLUDES(mRefreshRateConfigsLock);
+ void forceNextResync() { mLastResyncTime = 0; }
// Passes a vsync sample to VsyncController. periodFlushed will be true if
// VsyncController detected that the vsync period changed, and false otherwise.
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index d5904e8..91b479b 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1957,8 +1957,8 @@
}
void SurfaceFlinger::onComposerHalVsyncIdle(hal::HWDisplayId) {
- // TODO(b/198106220): force enable HWVsync to avoid drift problem during
- // idle.
+ ATRACE_CALL();
+ mScheduler->forceNextResync();
}
void SurfaceFlinger::setVsyncEnabled(bool enabled) {
@@ -5489,9 +5489,6 @@
case SET_FRAME_RATE:
case GET_DISPLAY_BRIGHTNESS_SUPPORT:
case GET_DISPLAY_DECORATION_SUPPORT:
- // captureLayers and captureDisplay will handle the permission check in the function
- case CAPTURE_LAYERS:
- case CAPTURE_DISPLAY:
case SET_FRAME_TIMELINE_INFO:
case GET_GPU_CONTEXT_PRIORITY:
case GET_MAX_ACQUIRED_BUFFER_COUNT: {
@@ -5526,8 +5523,7 @@
}
return OK;
}
- case ADD_TRANSACTION_TRACE_LISTENER:
- case CAPTURE_DISPLAY_BY_ID: {
+ case ADD_TRANSACTION_TRACE_LISTENER: {
IPCThreadState* ipc = IPCThreadState::self();
const int uid = ipc->getCallingUid();
if (uid == AID_ROOT || uid == AID_GRAPHICS || uid == AID_SYSTEM || uid == AID_SHELL) {
@@ -5557,6 +5553,11 @@
}
return PERMISSION_DENIED;
}
+ case CAPTURE_LAYERS:
+ case CAPTURE_DISPLAY:
+ case CAPTURE_DISPLAY_BY_ID:
+ LOG_FATAL("Deprecated opcode: %d", code);
+ return PERMISSION_DENIED;
}
// These codes are used for the IBinder protocol to either interrogate the recipient
@@ -7205,6 +7206,34 @@
mLayersAdded = true;
return true;
}
+
+// gui::ISurfaceComposer
+binder::Status SurfaceComposerAIDL::captureDisplay(
+ const DisplayCaptureArgs& args, const sp<IScreenCaptureListener>& captureListener) {
+ status_t status = mFlinger->captureDisplay(args, captureListener);
+ return binder::Status::fromStatusT(status);
+}
+
+binder::Status SurfaceComposerAIDL::captureDisplayById(
+ int64_t displayId, const sp<IScreenCaptureListener>& captureListener) {
+ status_t status;
+ IPCThreadState* ipc = IPCThreadState::self();
+ const int uid = ipc->getCallingUid();
+ if (uid == AID_ROOT || uid == AID_GRAPHICS || uid == AID_SYSTEM || uid == AID_SHELL) {
+ std::optional<DisplayId> id = DisplayId::fromValue(static_cast<uint64_t>(displayId));
+ status = mFlinger->captureDisplay(*id, captureListener);
+ } else {
+ status = PERMISSION_DENIED;
+ }
+ return binder::Status::fromStatusT(status);
+}
+
+binder::Status SurfaceComposerAIDL::captureLayers(
+ const LayerCaptureArgs& args, const sp<IScreenCaptureListener>& captureListener) {
+ status_t status = mFlinger->captureLayers(args, captureListener);
+ return binder::Status::fromStatusT(status);
+}
+
} // namespace android
#if defined(__gl_h_)
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 570108b..f49bf5d 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -23,6 +23,7 @@
*/
#include <android-base/thread_annotations.h>
+#include <android/gui/BnSurfaceComposer.h>
#include <cutils/atomic.h>
#include <cutils/compiler.h>
#include <gui/BufferQueue.h>
@@ -106,9 +107,13 @@
class RenderArea;
class TimeStats;
class FrameTracer;
+class ScreenCapturer;
class WindowInfosListenerInvoker;
+using gui::CaptureArgs;
+using gui::DisplayCaptureArgs;
using gui::IRegionSamplingListener;
+using gui::LayerCaptureArgs;
using gui::ScreenCaptureResults;
namespace frametimeline {
@@ -540,9 +545,9 @@
ISurfaceComposer::VsyncSource vsyncSource = eVsyncSourceApp,
ISurfaceComposer::EventRegistrationFlags eventRegistration = {}) override;
- status_t captureDisplay(const DisplayCaptureArgs&, const sp<IScreenCaptureListener>&) override;
- status_t captureDisplay(DisplayId, const sp<IScreenCaptureListener>&) override;
- status_t captureLayers(const LayerCaptureArgs&, const sp<IScreenCaptureListener>&) override;
+ status_t captureDisplay(const DisplayCaptureArgs&, const sp<IScreenCaptureListener>&);
+ status_t captureDisplay(DisplayId, const sp<IScreenCaptureListener>&);
+ status_t captureLayers(const LayerCaptureArgs&, const sp<IScreenCaptureListener>&);
status_t getDisplayStats(const sp<IBinder>& displayToken, DisplayStatInfo* stats) override;
status_t getDisplayState(const sp<IBinder>& displayToken, ui::DisplayState*)
@@ -1407,6 +1412,22 @@
} mPowerHintSessionData GUARDED_BY(SF_MAIN_THREAD);
nsecs_t mAnimationTransactionTimeout = s2ns(5);
+
+ friend class SurfaceComposerAIDL;
+};
+
+class SurfaceComposerAIDL : public gui::BnSurfaceComposer {
+public:
+ SurfaceComposerAIDL(sp<SurfaceFlinger> sf) { mFlinger = sf; }
+
+ binder::Status captureDisplay(const DisplayCaptureArgs&,
+ const sp<IScreenCaptureListener>&) override;
+ binder::Status captureDisplayById(int64_t, const sp<IScreenCaptureListener>&) override;
+ binder::Status captureLayers(const LayerCaptureArgs&,
+ const sp<IScreenCaptureListener>&) override;
+
+private:
+ sp<SurfaceFlinger> mFlinger;
};
} // namespace android
diff --git a/services/surfaceflinger/main_surfaceflinger.cpp b/services/surfaceflinger/main_surfaceflinger.cpp
index caeff4a..ec18054 100644
--- a/services/surfaceflinger/main_surfaceflinger.cpp
+++ b/services/surfaceflinger/main_surfaceflinger.cpp
@@ -152,6 +152,11 @@
sm->addService(String16(SurfaceFlinger::getServiceName()), flinger, false,
IServiceManager::DUMP_FLAG_PRIORITY_CRITICAL | IServiceManager::DUMP_FLAG_PROTO);
+ // publish gui::ISurfaceComposer, the new AIDL interface
+ sp<SurfaceComposerAIDL> composerAIDL = new SurfaceComposerAIDL(flinger);
+ sm->addService(String16("SurfaceFlingerAIDL"), composerAIDL, false,
+ IServiceManager::DUMP_FLAG_PRIORITY_CRITICAL | IServiceManager::DUMP_FLAG_PROTO);
+
startDisplayService(); // dependency on SF getting registered above
if (SurfaceFlinger::setSchedFifo(true) != NO_ERROR) {
diff --git a/services/surfaceflinger/tests/LayerState_test.cpp b/services/surfaceflinger/tests/LayerState_test.cpp
index fa1a5ed..094b0ff 100644
--- a/services/surfaceflinger/tests/LayerState_test.cpp
+++ b/services/surfaceflinger/tests/LayerState_test.cpp
@@ -22,6 +22,8 @@
#include <gui/LayerState.h>
namespace android {
+using gui::DisplayCaptureArgs;
+using gui::LayerCaptureArgs;
using gui::ScreenCaptureResults;
namespace test {
@@ -40,11 +42,11 @@
args.grayscale = true;
Parcel p;
- args.write(p);
+ args.writeToParcel(&p);
p.setDataPosition(0);
DisplayCaptureArgs args2;
- args2.read(p);
+ args2.readFromParcel(&p);
ASSERT_EQ(args.pixelFormat, args2.pixelFormat);
ASSERT_EQ(args.sourceCrop, args2.sourceCrop);
@@ -71,11 +73,11 @@
args.grayscale = true;
Parcel p;
- args.write(p);
+ args.writeToParcel(&p);
p.setDataPosition(0);
LayerCaptureArgs args2;
- args2.read(p);
+ args2.readFromParcel(&p);
ASSERT_EQ(args.pixelFormat, args2.pixelFormat);
ASSERT_EQ(args.sourceCrop, args2.sourceCrop);
diff --git a/services/surfaceflinger/tests/utils/ScreenshotUtils.h b/services/surfaceflinger/tests/utils/ScreenshotUtils.h
index cae7684..ee7e92c 100644
--- a/services/surfaceflinger/tests/utils/ScreenshotUtils.h
+++ b/services/surfaceflinger/tests/utils/ScreenshotUtils.h
@@ -16,6 +16,7 @@
#pragma once
#include <gui/SyncScreenCaptureListener.h>
+#include <private/gui/ComposerServiceAIDL.h>
#include <ui/Rect.h>
#include <utils/String8.h>
#include <functional>
@@ -31,15 +32,15 @@
public:
static status_t captureDisplay(DisplayCaptureArgs& captureArgs,
ScreenCaptureResults& captureResults) {
- const auto sf = ComposerService::getComposerService();
+ const auto sf = ComposerServiceAIDL::getComposerService();
SurfaceComposerClient::Transaction().apply(true);
captureArgs.dataspace = ui::Dataspace::V0_SRGB;
const sp<SyncScreenCaptureListener> captureListener = new SyncScreenCaptureListener();
- status_t status = sf->captureDisplay(captureArgs, captureListener);
+ binder::Status status = sf->captureDisplay(captureArgs, captureListener);
- if (status != NO_ERROR) {
- return status;
+ if (status.transactionError() != NO_ERROR) {
+ return status.transactionError();
}
captureResults = captureListener->waitForResults();
return captureResults.result;
@@ -64,14 +65,14 @@
static status_t captureLayers(LayerCaptureArgs& captureArgs,
ScreenCaptureResults& captureResults) {
- const auto sf = ComposerService::getComposerService();
+ const auto sf = ComposerServiceAIDL::getComposerService();
SurfaceComposerClient::Transaction().apply(true);
captureArgs.dataspace = ui::Dataspace::V0_SRGB;
const sp<SyncScreenCaptureListener> captureListener = new SyncScreenCaptureListener();
- status_t status = sf->captureLayers(captureArgs, captureListener);
- if (status != NO_ERROR) {
- return status;
+ binder::Status status = sf->captureLayers(captureArgs, captureListener);
+ if (status.transactionError() != NO_ERROR) {
+ return status.transactionError();
}
captureResults = captureListener->waitForResults();
return captureResults.result;