Improve error message when invalid display id is passed to screencap with -d argument.
Change-Id: Ifd38b347b64d1f6fc28f99c47adcab974e8e2467
Test: adb shell screencap -d 0 -p /tmp/test.png
Bug: 306733214
Fix: 306733214
Flag: EXEMPT bugfix
diff --git a/cmds/screencap/Android.bp b/cmds/screencap/Android.bp
index 16026ec..9f350b1 100644
--- a/cmds/screencap/Android.bp
+++ b/cmds/screencap/Android.bp
@@ -7,25 +7,66 @@
default_applicable_licenses: ["frameworks_base_license"],
}
-cc_binary {
- name: "screencap",
-
- srcs: ["screencap.cpp"],
-
- shared_libs: [
- "libcutils",
- "libutils",
- "libbinder",
- "libjnigraphics",
- "libhwui",
- "libui",
- "libgui",
- ],
+cc_defaults {
+ name: "screencap_defaults",
cflags: [
"-Wall",
"-Werror",
- "-Wunused",
"-Wunreachable-code",
+ "-Wunused",
+ ],
+
+ shared_libs: [
+ "libbinder",
+ "libcutils",
+ "libgui",
+ "libhwui",
+ "libjnigraphics",
+ "libui",
+ "libutils",
+ ],
+}
+
+cc_library {
+ name: "libscreencap",
+
+ defaults: [
+ "screencap_defaults",
+ ],
+
+ srcs: ["screencap_utils.cpp"],
+}
+
+cc_binary {
+ name: "screencap",
+
+ defaults: [
+ "screencap_defaults",
+ ],
+
+ srcs: ["screencap.cpp"],
+
+ static_libs: [
+ "libscreencap",
+ ],
+}
+
+cc_test {
+ name: "libscreencap_test",
+
+ defaults: [
+ "screencap_defaults",
+ ],
+
+ test_suites: ["device-tests"],
+
+ srcs: [
+ "tests/screencap_test.cpp",
+ ],
+
+ static_libs: [
+ "libgmock",
+ "libscreencap",
],
}
diff --git a/cmds/screencap/TEST_MAPPING b/cmds/screencap/TEST_MAPPING
new file mode 100644
index 0000000..05c598e
--- /dev/null
+++ b/cmds/screencap/TEST_MAPPING
@@ -0,0 +1,12 @@
+{
+ "presubmit": [
+ {
+ "name": "libscreencap_test"
+ }
+ ],
+ "hwasan-presubmit": [
+ {
+ "name": "libscreencap_test"
+ }
+ ]
+}
\ No newline at end of file
diff --git a/cmds/screencap/screencap.cpp b/cmds/screencap/screencap.cpp
index d563ad3..1626410 100644
--- a/cmds/screencap/screencap.cpp
+++ b/cmds/screencap/screencap.cpp
@@ -37,6 +37,9 @@
#include <ui/GraphicTypes.h>
#include <ui/PixelFormat.h>
+#include "utils/Errors.h"
+#include "screencap_utils.h"
+
using namespace android;
#define COLORSPACE_UNKNOWN 0
@@ -145,24 +148,6 @@
return NO_ERROR;
}
-status_t capture(const DisplayId displayId,
- const gui::CaptureArgs& captureArgs,
- ScreenCaptureResults& outResult) {
- sp<SyncScreenCaptureListener> captureListener = new SyncScreenCaptureListener();
- ScreenshotClient::captureDisplay(displayId, captureArgs, captureListener);
-
- ScreenCaptureResults captureResults = captureListener->waitForResults();
- if (!captureResults.fenceResult.ok()) {
- fprintf(stderr, "Failed to take screenshot. Status: %d\n",
- fenceStatus(captureResults.fenceResult));
- return 1;
- }
-
- outResult = captureResults;
-
- return 0;
-}
-
status_t saveImage(const char* fn, std::optional<AndroidBitmapCompressFormat> format,
const ScreenCaptureResults& captureResults) {
void* base = nullptr;
@@ -427,15 +412,12 @@
std::vector<ScreenCaptureResults> results;
const size_t numDisplays = displaysToCapture.size();
- for (int i=0; i<numDisplays; i++) {
- ScreenCaptureResults result;
-
+ for (int i = 0; i < numDisplays; i++) {
// 1. Capture the screen
- if (const status_t captureStatus =
- capture(displaysToCapture[i], captureArgs, result) != 0) {
-
- fprintf(stderr, "Capturing failed.\n");
- return captureStatus;
+ auto captureResult = screencap::capture(displaysToCapture[i], captureArgs);
+ if (captureResult.error().code() != OK) {
+ fprintf(stderr, "%sCapturing failed.\n", captureResult.error().message().c_str());
+ return 1;
}
// 2. Save the capture result as an image.
@@ -453,7 +435,7 @@
if (!filename.empty()) {
fn = filename.c_str();
}
- if (const status_t saveImageStatus = saveImage(fn, format, result) != 0) {
+ if (const status_t saveImageStatus = saveImage(fn, format, captureResult.value()) != 0) {
fprintf(stderr, "Saving image failed.\n");
return saveImageStatus;
}
diff --git a/cmds/screencap/screencap_utils.cpp b/cmds/screencap/screencap_utils.cpp
new file mode 100644
index 0000000..03ade73
--- /dev/null
+++ b/cmds/screencap/screencap_utils.cpp
@@ -0,0 +1,43 @@
+/*
+ * Copyright (C) 2025 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 "screencap_utils.h"
+
+#include "gui/SyncScreenCaptureListener.h"
+
+namespace android::screencap {
+
+base::Result<gui::ScreenCaptureResults> capture(const DisplayId displayId,
+ const gui::CaptureArgs& captureArgs) {
+ sp<SyncScreenCaptureListener> captureListener = new SyncScreenCaptureListener();
+ auto captureDisplayStatus =
+ ScreenshotClient::captureDisplay(displayId, captureArgs, captureListener);
+
+ gui::ScreenCaptureResults captureResults = captureListener->waitForResults();
+ if (!captureResults.fenceResult.ok()) {
+ status_t captureStatus = fenceStatus(captureResults.fenceResult);
+ std::stringstream errorMsg;
+ errorMsg << "Failed to take take screenshot. ";
+ if (captureStatus == NAME_NOT_FOUND) {
+ errorMsg << "Display Id '" << displayId.value << "' is not valid.\n";
+ }
+ return base::ResultError(errorMsg.str(), captureStatus);
+ }
+
+ return captureResults;
+}
+
+} // namespace android::screencap
\ No newline at end of file
diff --git a/cmds/screencap/screencap_utils.h b/cmds/screencap/screencap_utils.h
new file mode 100644
index 0000000..6580e3f
--- /dev/null
+++ b/cmds/screencap/screencap_utils.h
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2025 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 <android-base/result.h>
+#include <android/gui/DisplayCaptureArgs.h>
+
+#include "gui/ScreenCaptureResults.h"
+#include "ui/DisplayId.h"
+
+#pragma once
+
+namespace android::screencap {
+base::Result<gui::ScreenCaptureResults> capture(const DisplayId displayId,
+ const gui::CaptureArgs& captureArgs);
+} // namespace android::screencap
diff --git a/cmds/screencap/tests/screencap_test.cpp b/cmds/screencap/tests/screencap_test.cpp
new file mode 100644
index 0000000..cbee0ee
--- /dev/null
+++ b/cmds/screencap/tests/screencap_test.cpp
@@ -0,0 +1,51 @@
+// Copyright (C) 2025 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 <binder/ProcessState.h>
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "android/gui/CaptureArgs.h"
+#include "gmock/gmock.h"
+#include "gui/ScreenCaptureResults.h"
+#include "screencap_utils.h"
+#include "ui/DisplayId.h"
+
+using ::android::DisplayId;
+using ::android::OK;
+using ::android::ProcessState;
+using ::android::gui::CaptureArgs;
+using ::android::gui::ScreenCaptureResults;
+using ::testing::AllOf;
+using ::testing::HasSubstr;
+
+class ScreenCapTest : public ::testing::Test {
+protected:
+ static void SetUpTestSuite() {
+ // These lines are copied from screecap.cpp. They are necessary to call binder.
+ ProcessState::self()->setThreadPoolMaxThreadCount(0);
+ ProcessState::self()->startThreadPool();
+ }
+};
+
+TEST_F(ScreenCapTest, Capture_InvalidDisplayNumber) {
+ DisplayId display;
+ display.value = -1;
+
+ CaptureArgs args;
+ auto result = ::android::screencap::capture(display, args);
+ EXPECT_NE(OK, result.error().code());
+ EXPECT_THAT(result.error().message(),
+ AllOf(HasSubstr("Display Id"), HasSubstr("is not valid.")));
+}
\ No newline at end of file