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

6 files changed, 196 insertions, 39 deletions

diff --git a/cmds/screencap/Android.bp b/cmds/screencap/Android.bp
index 16026eca2980..9f350b1d6054 100644
--- a/cmds/screencap/Android.bp
+++ b/cmds/screencap/Android.bp
@@ -7,25 +7,66 @@ package {
     default_applicable_licenses: ["frameworks_base_license"],
 }
 
-cc_binary {
-    name: "screencap",
+cc_defaults {
+    name: "screencap_defaults",
 
-    srcs: ["screencap.cpp"],
+    cflags: [
+        "-Wall",
+        "-Werror",
+        "-Wunreachable-code",
+        "-Wunused",
+    ],
 
     shared_libs: [
-        "libcutils",
-        "libutils",
         "libbinder",
-        "libjnigraphics",
+        "libcutils",
+        "libgui",
         "libhwui",
+        "libjnigraphics",
         "libui",
-        "libgui",
+        "libutils",
     ],
+}
 
-    cflags: [
-        "-Wall",
-        "-Werror",
-        "-Wunused",
-        "-Wunreachable-code",
+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 000000000000..05c598e1e9cc
--- /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 d563ad3fd3db..1626410358a3 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 @@ static status_t notifyMediaScanner(const char* fileName) {
     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 @@ int main(int argc, char** argv)
 
     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 @@ int main(int argc, char** argv)
         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 000000000000..03ade73d0e30
--- /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 000000000000..6580e3fa5ff1
--- /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 000000000000..cbee0ee93624
--- /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