Take screenshot only when screenshot is requested
- Take screenshot only when screenshot is requested for preventing from taking screenshot unnecessarily
BUG: 149525300
Test: Flash and test interactive/full bugreports generated using Shell, and Shell flow does not break during tests
Change-Id: Ie5cbe97f9e6e32fecc3d95893b9804b6e716c628
diff --git a/cmds/dumpstate/DumpstateService.cpp b/cmds/dumpstate/DumpstateService.cpp
index 466575f..a0b9cbb 100644
--- a/cmds/dumpstate/DumpstateService.cpp
+++ b/cmds/dumpstate/DumpstateService.cpp
@@ -84,7 +84,8 @@
android::base::unique_fd bugreport_fd,
android::base::unique_fd screenshot_fd,
int bugreport_mode,
- const sp<IDumpstateListener>& listener) {
+ const sp<IDumpstateListener>& listener,
+ bool is_screenshot_requested) {
MYLOGI("startBugreport() with mode: %d\n", bugreport_mode);
// Ensure there is only one bugreport in progress at a time.
@@ -118,7 +119,7 @@
std::unique_ptr<Dumpstate::DumpOptions> options = std::make_unique<Dumpstate::DumpOptions>();
options->Initialize(static_cast<Dumpstate::BugreportMode>(bugreport_mode), bugreport_fd,
- screenshot_fd);
+ screenshot_fd, is_screenshot_requested);
if (bugreport_fd.get() == -1 || (options->do_screenshot && screenshot_fd.get() == -1)) {
MYLOGE("Invalid filedescriptor");
diff --git a/cmds/dumpstate/DumpstateService.h b/cmds/dumpstate/DumpstateService.h
index 6dc0225..ac8d3ac 100644
--- a/cmds/dumpstate/DumpstateService.h
+++ b/cmds/dumpstate/DumpstateService.h
@@ -41,7 +41,8 @@
binder::Status startBugreport(int32_t calling_uid, const std::string& calling_package,
android::base::unique_fd bugreport_fd,
android::base::unique_fd screenshot_fd, int bugreport_mode,
- const sp<IDumpstateListener>& listener) override;
+ const sp<IDumpstateListener>& listener,
+ bool is_screenshot_requested) override;
// No-op
binder::Status cancelBugreport();
diff --git a/cmds/dumpstate/binder/android/os/IDumpstate.aidl b/cmds/dumpstate/binder/android/os/IDumpstate.aidl
index 3f359c8..ba008bb 100644
--- a/cmds/dumpstate/binder/android/os/IDumpstate.aidl
+++ b/cmds/dumpstate/binder/android/os/IDumpstate.aidl
@@ -64,10 +64,12 @@
* @param screenshotFd the file to which screenshot should be written
* @param bugreportMode the mode that specifies other run time options; must be one of above
* @param listener callback for updates; optional
+ * @param isScreenshotRequested indicates screenshot is requested or not
*/
void startBugreport(int callingUid, @utf8InCpp String callingPackage,
FileDescriptor bugreportFd, FileDescriptor screenshotFd,
- int bugreportMode, IDumpstateListener listener);
+ int bugreportMode, IDumpstateListener listener,
+ boolean isScreenshotRequested);
/*
* Cancels the bugreport currently in progress.
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index e7029bd..733b71e 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -2247,20 +2247,21 @@
}
}
-static void SetOptionsFromMode(Dumpstate::BugreportMode mode, Dumpstate::DumpOptions* options) {
+static void SetOptionsFromMode(Dumpstate::BugreportMode mode, Dumpstate::DumpOptions* options,
+ bool is_screenshot_requested) {
// Modify com.android.shell.BugreportProgressService#isDefaultScreenshotRequired as well for
// default system screenshots.
options->bugreport_mode = ModeToString(mode);
switch (mode) {
case Dumpstate::BugreportMode::BUGREPORT_FULL:
- options->do_screenshot = true;
+ options->do_screenshot = is_screenshot_requested;
options->dumpstate_hal_mode = DumpstateMode::FULL;
break;
case Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE:
// Currently, the dumpstate binder is only used by Shell to update progress.
options->do_start_service = true;
options->do_progress_updates = true;
- options->do_screenshot = true;
+ options->do_screenshot = is_screenshot_requested;
options->dumpstate_hal_mode = DumpstateMode::INTERACTIVE;
break;
case Dumpstate::BugreportMode::BUGREPORT_REMOTE:
@@ -2273,7 +2274,7 @@
options->do_start_service = true;
options->do_progress_updates = true;
options->do_zip_file = true;
- options->do_screenshot = true;
+ options->do_screenshot = is_screenshot_requested;
options->dumpstate_hal_mode = DumpstateMode::WEAR;
break;
// TODO(b/148168577) rename TELEPHONY everywhere to CONNECTIVITY.
@@ -2310,7 +2311,8 @@
void Dumpstate::DumpOptions::Initialize(BugreportMode bugreport_mode,
const android::base::unique_fd& bugreport_fd_in,
- const android::base::unique_fd& screenshot_fd_in) {
+ const android::base::unique_fd& screenshot_fd_in,
+ bool is_screenshot_requested) {
// In the new API world, date is always added; output is always a zip file.
// TODO(111441001): remove these options once they are obsolete.
do_add_date = true;
@@ -2320,7 +2322,7 @@
bugreport_fd.reset(dup(bugreport_fd_in.get()));
screenshot_fd.reset(dup(screenshot_fd_in.get()));
- SetOptionsFromMode(bugreport_mode, this);
+ SetOptionsFromMode(bugreport_mode, this, is_screenshot_requested);
}
Dumpstate::RunStatus Dumpstate::DumpOptions::Initialize(int argc, char* argv[]) {
diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h
index 7e27787..7b8d282 100644
--- a/cmds/dumpstate/dumpstate.h
+++ b/cmds/dumpstate/dumpstate.h
@@ -390,7 +390,8 @@
/* Initializes options from the requested mode. */
void Initialize(BugreportMode bugreport_mode, const android::base::unique_fd& bugreport_fd,
- const android::base::unique_fd& screenshot_fd);
+ const android::base::unique_fd& screenshot_fd,
+ bool is_screenshot_requested);
/* Returns true if the options set so far are consistent. */
bool ValidateOptions() const;
diff --git a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
index f26e4db..047a1c3 100644
--- a/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
+++ b/cmds/dumpstate/tests/dumpstate_smoke_test.cpp
@@ -451,7 +451,7 @@
sp<DumpstateListener> listener(new DumpstateListener(dup(fileno(stdout))));
android::binder::Status status =
ds_binder->startBugreport(123, "com.dummy.package", std::move(bugreport_fd), std::move(screenshot_fd),
- Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, listener);
+ Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, listener, true);
// startBugreport is an async call. Verify binder call succeeded first, then wait till listener
// gets expected callbacks.
EXPECT_TRUE(status.isOk());
@@ -488,7 +488,7 @@
android::binder::Status status =
ds_binder->startBugreport(123, "com.dummy.package", std::move(bugreport_fd), std::move(screenshot_fd),
2000, // invalid bugreport mode
- listener);
+ listener, false);
EXPECT_EQ(listener->getErrorCode(), IDumpstateListener::BUGREPORT_ERROR_INVALID_INPUT);
// The service should have died, freeing itself up for a new invocation.
@@ -519,13 +519,13 @@
sp<DumpstateListener> listener1(new DumpstateListener(dup(fileno(stdout))));
android::binder::Status status =
ds_binder->startBugreport(123, "com.dummy.package", std::move(bugreport_fd), std::move(screenshot_fd),
- Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, listener1);
+ Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, listener1, true);
EXPECT_TRUE(status.isOk());
// try to make another call to startBugreport. This should fail.
sp<DumpstateListener> listener2(new DumpstateListener(dup(fileno(stdout))));
status = ds_binder->startBugreport(123, "com.dummy.package", std::move(bugreport_fd2), std::move(screenshot_fd2),
- Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, listener2);
+ Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, listener2, true);
EXPECT_FALSE(status.isOk());
WaitTillExecutionComplete(listener2.get());
EXPECT_EQ(listener2->getErrorCode(),
diff --git a/cmds/dumpstate/tests/dumpstate_test.cpp b/cmds/dumpstate/tests/dumpstate_test.cpp
index 0a0c40e..2efb130 100644
--- a/cmds/dumpstate/tests/dumpstate_test.cpp
+++ b/cmds/dumpstate/tests/dumpstate_test.cpp
@@ -233,7 +233,7 @@
}
TEST_F(DumpOptionsTest, InitializeFullBugReport) {
- options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_FULL, fd, fd);
+ options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_FULL, fd, fd, true);
EXPECT_TRUE(options_.do_add_date);
EXPECT_TRUE(options_.do_screenshot);
EXPECT_TRUE(options_.do_zip_file);
@@ -250,7 +250,7 @@
}
TEST_F(DumpOptionsTest, InitializeInteractiveBugReport) {
- options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, fd, fd);
+ options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_INTERACTIVE, fd, fd, true);
EXPECT_TRUE(options_.do_add_date);
EXPECT_TRUE(options_.do_zip_file);
EXPECT_TRUE(options_.do_progress_updates);
@@ -267,7 +267,7 @@
}
TEST_F(DumpOptionsTest, InitializeRemoteBugReport) {
- options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_REMOTE, fd, fd);
+ options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_REMOTE, fd, fd, false);
EXPECT_TRUE(options_.do_add_date);
EXPECT_TRUE(options_.do_zip_file);
EXPECT_TRUE(options_.is_remote_mode);
@@ -283,7 +283,7 @@
}
TEST_F(DumpOptionsTest, InitializeWearBugReport) {
- options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_WEAR, fd, fd);
+ options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_WEAR, fd, fd, true);
EXPECT_TRUE(options_.do_add_date);
EXPECT_TRUE(options_.do_screenshot);
EXPECT_TRUE(options_.do_zip_file);
@@ -300,7 +300,7 @@
}
TEST_F(DumpOptionsTest, InitializeTelephonyBugReport) {
- options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_TELEPHONY, fd, fd);
+ options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_TELEPHONY, fd, fd, false);
EXPECT_TRUE(options_.do_add_date);
EXPECT_FALSE(options_.do_screenshot);
EXPECT_TRUE(options_.do_zip_file);
@@ -317,7 +317,7 @@
}
TEST_F(DumpOptionsTest, InitializeWifiBugReport) {
- options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_WIFI, fd, fd);
+ options_.Initialize(Dumpstate::BugreportMode::BUGREPORT_WIFI, fd, fd, false);
EXPECT_TRUE(options_.do_add_date);
EXPECT_FALSE(options_.do_screenshot);
EXPECT_TRUE(options_.do_zip_file);