From 75876a2c0649b8cde36329ca0a1dc6e349af6493 Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Thu, 27 Oct 2016 16:31:27 -0700 Subject: [PATCH] Created a dumpstate service. For now this is still a limited service: - It's only created when running an interactive bugreport. - It only provides a listener to get updates. - It will be just used by Shell to get updates. Test: dumpstate_test passes BUG: 31636879 Change-Id: Iae820261d220523c979bf905030456fcf0b2b618 --- cmds/dumpstate/Android.mk | 28 +++- cmds/dumpstate/DumpstateService.cpp | 92 ++++++++++++ cmds/dumpstate/DumpstateService.h | 50 +++++++ cmds/dumpstate/binder/android/os/IDumpstate.aidl | 33 +++++ .../binder/android/os/IDumpstateListener.aidl | 27 ++++ cmds/dumpstate/dumpstate.cpp | 32 +++-- cmds/dumpstate/dumpstate.h | 9 ++ cmds/dumpstate/tests/dumpstate_test.cpp | 154 +++++++++++++++++++-- cmds/dumpstate/utils.cpp | 74 +++++++--- 9 files changed, 448 insertions(+), 51 deletions(-) create mode 100644 cmds/dumpstate/DumpstateService.cpp create mode 100644 cmds/dumpstate/DumpstateService.h create mode 100644 cmds/dumpstate/binder/android/os/IDumpstate.aidl create mode 100644 cmds/dumpstate/binder/android/os/IDumpstateListener.aidl diff --git a/cmds/dumpstate/Android.mk b/cmds/dumpstate/Android.mk index da39d3af93..3987fcec6e 100644 --- a/cmds/dumpstate/Android.mk +++ b/cmds/dumpstate/Android.mk @@ -15,10 +15,13 @@ COMMON_SRC_FILES := \ utils.cpp COMMON_SHARED_LIBRARIES := \ libbase \ + libbinder \ libcutils \ + libdumpstateaidl \ libhardware_legacy \ liblog \ - libselinux + libselinux \ + libutils # ====================# # libdumpstateheaders # @@ -39,6 +42,27 @@ LOCAL_STATIC_LIBRARIES := $(LOCAL_EXPORT_STATIC_LIBRARY_HEADERS) include $(BUILD_STATIC_LIBRARY) +# ================ # +# libdumpstateaidl # +# =================# +include $(CLEAR_VARS) + +LOCAL_MODULE := libdumpstateaidl + +LOCAL_CFLAGS := $(COMMON_LOCAL_CFLAGS) + +LOCAL_SHARED_LIBRARIES := \ + libbinder \ + libutils +LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)/binder +LOCAL_AIDL_INCLUDES := $(LOCAL_PATH)/binder +LOCAL_C_INCLUDES := $(LOCAL_PATH)/binder +LOCAL_SRC_FILES := \ + binder/android/os/IDumpstateListener.aidl \ + binder/android/os/IDumpstate.aidl + +include $(BUILD_SHARED_LIBRARY) + # ==========# # dumpstate # # ==========# @@ -49,6 +73,7 @@ LOCAL_CFLAGS := -DFWDUMP_$(BOARD_WLAN_DEVICE) endif LOCAL_SRC_FILES := $(COMMON_SRC_FILES) \ + DumpstateService.cpp \ dumpstate.cpp LOCAL_MODULE := dumpstate @@ -77,6 +102,7 @@ LOCAL_MODULE_TAGS := tests LOCAL_CFLAGS := $(COMMON_LOCAL_CFLAGS) LOCAL_SRC_FILES := $(COMMON_SRC_FILES) \ + DumpstateService.cpp \ tests/dumpstate_test.cpp LOCAL_STATIC_LIBRARIES := $(COMMON_ZIP_LIBRARIES) \ diff --git a/cmds/dumpstate/DumpstateService.cpp b/cmds/dumpstate/DumpstateService.cpp new file mode 100644 index 0000000000..ce78ec6f60 --- /dev/null +++ b/cmds/dumpstate/DumpstateService.cpp @@ -0,0 +1,92 @@ +/** + * Copyright (c) 2016, 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. + */ + +#define LOG_TAG "dumpstate" + +#include "DumpstateService.h" + +#include + +#include "android/os/BnDumpstate.h" + +namespace android { +namespace os { + +DumpstateService::DumpstateService() : ds_(Dumpstate::GetInstance()) { +} + +char const* DumpstateService::getServiceName() { + return "dumpstate"; +} + +status_t DumpstateService::Start() { + IPCThreadState::self()->disableBackgroundScheduling(true); + status_t ret = BinderService::publish(); + if (ret != android::OK) { + return ret; + } + sp ps(ProcessState::self()); + ps->startThreadPool(); + ps->giveThreadPoolName(); + return android::OK; +} + +binder::Status DumpstateService::setListener(const std::string& name, + const sp& listener, bool* set) { + if (name.empty()) { + MYLOGE("setListener(): name not set\n"); + *set = false; + return binder::Status::ok(); + } + if (listener == nullptr) { + MYLOGE("setListener(): listener not set\n"); + *set = false; + return binder::Status::ok(); + } + std::lock_guard lock(lock_); + if (ds_.listener_ != nullptr) { + MYLOGE("setListener(%s): already set (%s)\n", name.c_str(), ds_.listener_name_.c_str()); + *set = false; + return binder::Status::ok(); + } + ds_.listener_name_ = name; + ds_.listener_ = listener; + *set = true; + return binder::Status::ok(); +} + +status_t DumpstateService::dump(int fd, const Vector&) { + dprintf(fd, "id: %lu\n", ds_.id_); + dprintf(fd, "pid: %d\n", ds_.pid_); + dprintf(fd, "progress: %d / %d\n", ds_.progress_, ds_.weight_total_); + dprintf(fd, "args: %s\n", ds_.args_.c_str()); + dprintf(fd, "extra_options: %s\n", ds_.extra_options_.c_str()); + dprintf(fd, "version: %s\n", ds_.version_.c_str()); + dprintf(fd, "bugreport_dir: %s\n", ds_.bugreport_dir_.c_str()); + dprintf(fd, "screenshot_path: %s\n", ds_.screenshot_path_.c_str()); + dprintf(fd, "log_path: %s\n", ds_.log_path_.c_str()); + dprintf(fd, "tmp_path: %s\n", ds_.tmp_path_.c_str()); + dprintf(fd, "path: %s\n", ds_.extra_options_.c_str()); + dprintf(fd, "base_name: %s\n", ds_.base_name_.c_str()); + dprintf(fd, "name: %s\n", ds_.name_.c_str()); + dprintf(fd, "now: %ld\n", ds_.now_); + dprintf(fd, "is_zipping: %s\n", ds_.IsZipping() ? "true" : "false"); + dprintf(fd, "listener: %s\n", ds_.listener_name_.c_str()); + + return NO_ERROR; +} +} // namespace os +} // namespace android diff --git a/cmds/dumpstate/DumpstateService.h b/cmds/dumpstate/DumpstateService.h new file mode 100644 index 0000000000..544a7b6d36 --- /dev/null +++ b/cmds/dumpstate/DumpstateService.h @@ -0,0 +1,50 @@ +/** + * Copyright (c) 2016, 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. + */ + +#ifndef ANDROID_OS_DUMPSTATE_H_ +#define ANDROID_OS_DUMPSTATE_H_ + +#include +#include + +#include + +#include "android/os/BnDumpstate.h" +#include "dumpstate.h" + +namespace android { +namespace os { + +class DumpstateService : public BinderService, public BnDumpstate { + public: + DumpstateService(); + + static status_t Start(); + static char const* getServiceName(); + + status_t dump(int fd, const Vector& args) override; + binder::Status setListener(const std::string& name, const sp& listener, + bool* set) override; + + private: + Dumpstate& ds_; + std::mutex lock_; +}; + +} // namespace os +} // namespace android + +#endif // ANDROID_OS_DUMPSTATE_H_ diff --git a/cmds/dumpstate/binder/android/os/IDumpstate.aidl b/cmds/dumpstate/binder/android/os/IDumpstate.aidl new file mode 100644 index 0000000000..e585a0e637 --- /dev/null +++ b/cmds/dumpstate/binder/android/os/IDumpstate.aidl @@ -0,0 +1,33 @@ +/** + * Copyright (c) 2016, 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.os; + +import android.os.IDumpstateListener; + +/** + * Binder interface for the currently running dumpstate process. + * {@hide} + */ +interface IDumpstate { + + /* + * Sets the listener for dumpstate progress. + * + * Returns true if the listener was set (it's like a Highlander: There Can be Only One). + */ + boolean setListener(@utf8InCpp String name, IDumpstateListener listener); +} diff --git a/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl b/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl new file mode 100644 index 0000000000..32717f4f87 --- /dev/null +++ b/cmds/dumpstate/binder/android/os/IDumpstateListener.aidl @@ -0,0 +1,27 @@ +/** + * Copyright (c) 2016, 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.os; + +/** + * Listener for dumpstate events. + * + * {@hide} + */ +interface IDumpstateListener { + void onProgressUpdated(int progress); + void onMaxProgressUpdated(int maxProgress); +} diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp index 740eb19075..80a1b07987 100644 --- a/cmds/dumpstate/dumpstate.cpp +++ b/cmds/dumpstate/dumpstate.cpp @@ -43,14 +43,13 @@ #include #include #include - +#include #include #include +#include "DumpstateService.h" #include "dumpstate.h" -#include - /* read before root is shed */ static char cmdline_buf[16384] = "(unknown)"; static const char *dump_traces_path = NULL; @@ -684,15 +683,11 @@ void Dumpstate::PrintHeader() const { JustDumpFile("", "/proc/version"); printf("Command line: %s\n", strtok(cmdline_buf, "\n")); printf("Bugreport format version: %s\n", version_.c_str()); - printf("Dumpstate info: id=%lu pid=%d dry_run=%d args=%s extra_options=%s\n", id_, getpid(), + printf("Dumpstate info: id=%lu pid=%d dry_run=%d args=%s extra_options=%s\n", id_, pid_, dry_run_, args_.c_str(), extra_options_.c_str()); printf("\n"); } -bool Dumpstate::IsZipping() const { - return zip_writer_ != nullptr; -} - // List of file extensions that can cause a zip file attachment to be rejected by some email // service providers. static const std::set PROBLEMATIC_FILE_EXTENSIONS = { @@ -1159,7 +1154,7 @@ static void dumpstate() { DumpModemLogs(); printf("========================================================\n"); - printf("== Final progress (pid %d): %d/%d (originally %d)\n", getpid(), ds.progress_, + printf("== Final progress (pid %d): %d/%d (originally %d)\n", ds.pid_, ds.progress_, ds.weight_total_, WEIGHT_TOTAL); printf("========================================================\n"); printf("== dumpstate: done (id %lu)\n", ds.id_); @@ -1316,6 +1311,7 @@ int main(int argc, char *argv[]) { int do_broadcast = 0; int is_remote_mode = 0; bool show_header_only = false; + bool do_start_service = false; /* set as high priority, and protect from OOM killer */ setpriority(PRIO_PROCESS, 0, -20); @@ -1373,6 +1369,8 @@ int main(int argc, char *argv[]) { // Framework uses a system property to override some command-line args. // Currently, it contains the type of the requested bugreport. if (ds.extra_options_ == "bugreportplus") { + // Currently, the dumpstate binder is only used by Shell to update progress. + do_start_service = true; ds.update_progress_ = true; do_fb = 0; } else if (ds.extra_options_ == "bugreportremote") { @@ -1435,6 +1433,14 @@ int main(int argc, char *argv[]) { register_sig_handler(); } + if (do_start_service) { + MYLOGI("Starting 'dumpstate' service\n"); + android::status_t ret; + if ((ret = android::os::DumpstateService::Start()) != android::OK) { + MYLOGE("Unable to start DumpstateService: %d\n", ret); + } + } + if (ds.IsDryRun()) { MYLOGI("Running on dry-run mode (to disable it, call 'setprop dumpstate.dry_run false')\n"); } @@ -1477,7 +1483,7 @@ int main(int argc, char *argv[]) { ds.screenshot_path_ = ds.GetPath(".png"); } ds.tmp_path_ = ds.GetPath(".tmp"); - ds.log_path_ = ds.GetPath("-dumpstate_log-" + std::to_string(getpid()) + ".txt"); + ds.log_path_ = ds.GetPath("-dumpstate_log-" + std::to_string(ds.pid_) + ".txt"); MYLOGD( "Bugreport dir: %s\n" @@ -1510,7 +1516,7 @@ int main(int argc, char *argv[]) { "--receiver-permission", "android.permission.DUMP", "--receiver-foreground", "--es", "android.intent.extra.NAME", ds.name_, "--ei", "android.intent.extra.ID", std::to_string(ds.id_), - "--ei", "android.intent.extra.PID", std::to_string(getpid()), + "--ei", "android.intent.extra.PID", std::to_string(ds.pid_), "--ei", "android.intent.extra.MAX", std::to_string(WEIGHT_TOTAL), }; // clang-format on @@ -1630,7 +1636,7 @@ int main(int argc, char *argv[]) { /* check if user changed the suffix using system properties */ std::string name = android::base::GetProperty( - android::base::StringPrintf("dumpstate.%d.name", getpid()), ""); + android::base::StringPrintf("dumpstate.%d.name", ds.pid_), ""); bool change_suffix= false; if (!name.empty()) { /* must whitelist which characters are allowed, otherwise it could cross directories */ @@ -1713,7 +1719,7 @@ int main(int argc, char *argv[]) { std::vector am_args = { "--receiver-permission", "android.permission.DUMP", "--receiver-foreground", "--ei", "android.intent.extra.ID", std::to_string(ds.id_), - "--ei", "android.intent.extra.PID", std::to_string(getpid()), + "--ei", "android.intent.extra.PID", std::to_string(ds.pid_), "--ei", "android.intent.extra.MAX", std::to_string(ds.weight_total_), "--es", "android.intent.extra.BUGREPORT", ds.path_, "--es", "android.intent.extra.DUMPSTATE_LOG", ds.log_path_ diff --git a/cmds/dumpstate/dumpstate.h b/cmds/dumpstate/dumpstate.h index 950f18586d..0680db2af2 100644 --- a/cmds/dumpstate/dumpstate.h +++ b/cmds/dumpstate/dumpstate.h @@ -40,6 +40,8 @@ #include #include +#include "android/os/BnDumpstate.h" + // Workaround for const char *args[MAX_ARGS_ARRAY_SIZE] variables until they're converted to // std::vector // TODO: remove once not used @@ -332,6 +334,9 @@ class Dumpstate { // dumpstate id - unique after each device reboot. unsigned long id_; + // dumpstate pid + pid_t pid_; + // Whether progress updates should be published. bool update_progress_ = false; @@ -387,6 +392,10 @@ class Dumpstate { // Pointer to the zip structure. std::unique_ptr zip_writer_; + // Binder object listing to progress. + android::sp listener_; + std::string listener_name_; + private: // Used by GetInstance() only. Dumpstate(const std::string& version = VERSION_CURRENT, bool dry_run = false, diff --git a/cmds/dumpstate/tests/dumpstate_test.cpp b/cmds/dumpstate/tests/dumpstate_test.cpp index 4073c3c3a1..d692fda80f 100644 --- a/cmds/dumpstate/tests/dumpstate_test.cpp +++ b/cmds/dumpstate/tests/dumpstate_test.cpp @@ -14,6 +14,11 @@ * limitations under the License. */ +#define LOG_TAG "dumpstate" +#include + +#include "DumpstateService.h" +#include "android/os/BnDumpstate.h" #include "dumpstate.h" #include @@ -30,8 +35,7 @@ #include #include -#define LOG_TAG "dumpstate" -#include +using namespace android; using ::testing::EndsWith; using ::testing::IsEmpty; @@ -43,10 +47,22 @@ using ::testing::internal::CaptureStdout; using ::testing::internal::GetCapturedStderr; using ::testing::internal::GetCapturedStdout; +using os::DumpstateService; +using os::IDumpstateListener; + // Not used on test cases yet... void dumpstate_board(void) { } +class DumpstateListenerMock : public IDumpstateListener { + public: + MOCK_METHOD1(onProgressUpdated, binder::Status(int32_t progress)); + MOCK_METHOD1(onMaxProgressUpdated, binder::Status(int32_t max_progress)); + + protected: + MOCK_METHOD0(onAsBinder, IBinder*()); +}; + class DumpstateTest : public Test { public: void SetUp() { @@ -102,26 +118,46 @@ class DumpstateTest : public Test { EXPECT_THAT(expected_value, StrEq(actualValue)) << "invalid value for property " << key; } - std::string GetProgressMessage(int progress, int weigh_total, int old_weigh_total = 0) { + // TODO: remove when progress is set by Binder callbacks. + std::string GetProgressMessageAndAssertSystemProperties(int progress, int weight_total, + int old_weight_total = 0) { EXPECT_EQ(progress, ds.progress_) << "invalid progress"; - EXPECT_EQ(weigh_total, ds.weight_total_) << "invalid weigh_total"; + EXPECT_EQ(weight_total, ds.weight_total_) << "invalid weight_total"; AssertSystemProperty(android::base::StringPrintf("dumpstate.%d.progress", getpid()), std::to_string(progress)); - bool max_increased = old_weigh_total > 0; + bool max_increased = old_weight_total > 0; std::string adjustment_message = ""; if (max_increased) { AssertSystemProperty(android::base::StringPrintf("dumpstate.%d.max", getpid()), - std::to_string(weigh_total)); + std::to_string(weight_total)); adjustment_message = android::base::StringPrintf( - "Adjusting total weight from %d to %d\n", old_weigh_total, weigh_total); + "Adjusting total weight from %d to %d\n", old_weight_total, weight_total); } return android::base::StringPrintf("%sSetting progress (dumpstate.%d.progress): %d/%d\n", adjustment_message.c_str(), getpid(), progress, - weigh_total); + weight_total); + } + + std::string GetProgressMessage(const std::string& listener_name, int progress, int weight_total, + int old_weight_total = 0) { + EXPECT_EQ(progress, ds.progress_) << "invalid progress"; + EXPECT_EQ(weight_total, ds.weight_total_) << "invalid weight_total"; + + bool max_increased = old_weight_total > 0; + + std::string adjustment_message = ""; + if (max_increased) { + adjustment_message = android::base::StringPrintf( + "Adjusting total weight from %d to %d\n", old_weight_total, weight_total); + } + + return android::base::StringPrintf("%sSetting progress (%s): %d/%d\n", + adjustment_message.c_str(), listener_name.c_str(), + progress, weight_total); } // `stdout` and `stderr` from the last command ran. @@ -276,33 +312,73 @@ TEST_F(DumpstateTest, RunCommandIsKilled) { " --pid --sleep 20' failed: killed by signal 15\n")); } +TEST_F(DumpstateTest, RunCommandProgressNoListener) { + ds.update_progress_ = true; + ds.progress_ = 0; + ds.weight_total_ = 30; + + EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(20).Build())); + std::string progress_message = GetProgressMessageAndAssertSystemProperties(20, 30); + EXPECT_THAT(out, StrEq("stdout\n")); + EXPECT_THAT(err, StrEq("stderr\n" + progress_message)); + + EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(10).Build())); + progress_message = GetProgressMessageAndAssertSystemProperties(30, 30); + EXPECT_THAT(out, StrEq("stdout\n")); + EXPECT_THAT(err, StrEq("stderr\n" + progress_message)); + + // Run a command that will increase maximum timeout. + EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(1).Build())); + progress_message = GetProgressMessageAndAssertSystemProperties(31, 36, 30); // 20% increase + EXPECT_THAT(out, StrEq("stdout\n")); + EXPECT_THAT(err, StrEq("stderr\n" + progress_message)); + + // Make sure command ran while in dry_run is counted. + SetDryRun(true); + EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(4).Build())); + progress_message = GetProgressMessageAndAssertSystemProperties(35, 36); + EXPECT_THAT(out, IsEmpty()); + EXPECT_THAT(err, StrEq(progress_message)); +} + TEST_F(DumpstateTest, RunCommandProgress) { + sp listener(new DumpstateListenerMock()); + ds.listener_ = listener; + ds.listener_name_ = "FoxMulder"; + ds.update_progress_ = true; ds.progress_ = 0; ds.weight_total_ = 30; + EXPECT_CALL(*listener, onProgressUpdated(20)); EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(20).Build())); - std::string progress_message = GetProgressMessage(20, 30); + std::string progress_message = GetProgressMessage(ds.listener_name_, 20, 30); EXPECT_THAT(out, StrEq("stdout\n")); EXPECT_THAT(err, StrEq("stderr\n" + progress_message)); + EXPECT_CALL(*listener, onProgressUpdated(30)); EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(10).Build())); - progress_message = GetProgressMessage(30, 30); + progress_message = GetProgressMessage(ds.listener_name_, 30, 30); EXPECT_THAT(out, StrEq("stdout\n")); EXPECT_THAT(err, StrEq("stderr\n" + progress_message)); // Run a command that will increase maximum timeout. + EXPECT_CALL(*listener, onProgressUpdated(31)); + EXPECT_CALL(*listener, onMaxProgressUpdated(36)); EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(1).Build())); - progress_message = GetProgressMessage(31, 36, 30); // 20% increase + progress_message = GetProgressMessage(ds.listener_name_, 31, 36, 30); // 20% increase EXPECT_THAT(out, StrEq("stdout\n")); EXPECT_THAT(err, StrEq("stderr\n" + progress_message)); // Make sure command ran while in dry_run is counted. SetDryRun(true); + EXPECT_CALL(*listener, onProgressUpdated(35)); EXPECT_EQ(0, RunCommand("", {simple_command}, CommandOptions::WithTimeout(4).Build())); - progress_message = GetProgressMessage(35, 36); + progress_message = GetProgressMessage(ds.listener_name_, 35, 36); EXPECT_THAT(out, IsEmpty()); EXPECT_THAT(err, StrEq(progress_message)); + + ds.listener_.clear(); } TEST_F(DumpstateTest, RunCommandDropRoot) { @@ -410,15 +486,65 @@ TEST_F(DumpstateTest, DumpFileOnDryRun) { EXPECT_THAT(err, IsEmpty()); } -TEST_F(DumpstateTest, DumpFileUpdateProgress) { +TEST_F(DumpstateTest, DumpFileUpdateProgressNoListener) { ds.update_progress_ = true; ds.progress_ = 0; ds.weight_total_ = 30; EXPECT_EQ(0, DumpFile("", test_data_path + "single-line.txt")); - std::string progress_message = GetProgressMessage(5, 30); // TODO: unhardcode WEIGHT_FILE (5)? + std::string progress_message = + GetProgressMessageAndAssertSystemProperties(5, 30); // TODO: unhardcode WEIGHT_FILE (5)? EXPECT_THAT(err, StrEq(progress_message)); EXPECT_THAT(out, StrEq("I AM LINE1\n")); // dumpstate adds missing newline } + +TEST_F(DumpstateTest, DumpFileUpdateProgress) { + sp listener(new DumpstateListenerMock()); + ds.listener_ = listener; + ds.listener_name_ = "FoxMulder"; + ds.update_progress_ = true; + ds.progress_ = 0; + ds.weight_total_ = 30; + + EXPECT_CALL(*listener, onProgressUpdated(5)); + EXPECT_EQ(0, DumpFile("", test_data_path + "single-line.txt")); + + std::string progress_message = + GetProgressMessage(ds.listener_name_, 5, 30); // TODO: unhardcode WEIGHT_FILE (5)? + EXPECT_THAT(err, StrEq(progress_message)); + EXPECT_THAT(out, StrEq("I AM LINE1\n")); // dumpstate adds missing newline + + ds.listener_.clear(); +} + +class DumpstateServiceTest : public Test { + public: + DumpstateService dss; +}; + +TEST_F(DumpstateServiceTest, SetListenerNoName) { + sp listener(new DumpstateListenerMock()); + bool result; + EXPECT_TRUE(dss.setListener("", listener, &result).isOk()); + EXPECT_FALSE(result); +} + +TEST_F(DumpstateServiceTest, SetListenerNoPointer) { + bool result; + EXPECT_TRUE(dss.setListener("whatever", nullptr, &result).isOk()); + EXPECT_FALSE(result); +} + +TEST_F(DumpstateServiceTest, SetListenerTwice) { + sp listener(new DumpstateListenerMock()); + bool result; + EXPECT_TRUE(dss.setListener("whatever", listener, &result).isOk()); + EXPECT_TRUE(result); + + EXPECT_THAT(Dumpstate::GetInstance().listener_name_, StrEq("whatever")); + + EXPECT_TRUE(dss.setListener("whatever", listener, &result).isOk()); + EXPECT_FALSE(result); +} diff --git a/cmds/dumpstate/utils.cpp b/cmds/dumpstate/utils.cpp index c41cca415f..34e09d71b5 100644 --- a/cmds/dumpstate/utils.cpp +++ b/cmds/dumpstate/utils.cpp @@ -165,7 +165,11 @@ CommandOptions::CommandOptionsBuilder CommandOptions::WithTimeout(long timeout) } Dumpstate::Dumpstate(const std::string& version, bool dry_run, const std::string& build_type) - : version_(version), now_(time(nullptr)), dry_run_(dry_run), build_type_(build_type) { + : pid_(getpid()), + version_(version), + now_(time(nullptr)), + dry_run_(dry_run), + build_type_(build_type) { } Dumpstate& Dumpstate::GetInstance() { @@ -211,6 +215,10 @@ bool Dumpstate::IsUserBuild() const { return "user" == build_type_; } +bool Dumpstate::IsZipping() const { + return zip_writer_ != nullptr; +} + std::string Dumpstate::GetPath(const std::string& suffix) const { return android::base::StringPrintf("%s/%s-%s%s", bugreport_dir_.c_str(), base_name_.c_str(), name_.c_str(), suffix.c_str()); @@ -1317,6 +1325,7 @@ void Dumpstate::UpdateProgress(int delta) { progress_ += delta; + // TODO: remove property support once Shell uses IDumpstateListener char key[PROPERTY_KEY_MAX]; char value[PROPERTY_VALUE_MAX]; @@ -1325,25 +1334,18 @@ void Dumpstate::UpdateProgress(int delta) { int new_total = weight_total_ * 1.2; MYLOGD("Adjusting total weight from %d to %d\n", weight_total_, new_total); weight_total_ = new_total; - snprintf(key, sizeof(key), "dumpstate.%d.max", getpid()); - snprintf(value, sizeof(value), "%d", weight_total_); - int status = property_set(key, value); - if (status != 0) { - MYLOGE("Could not update max weight by setting system property %s to %s: %d\n", - key, value, status); - } - } - snprintf(key, sizeof(key), "dumpstate.%d.progress", getpid()); - snprintf(value, sizeof(value), "%d", progress_); - - if (progress_ % 100 == 0) { - // We don't want to spam logcat, so only log multiples of 100. - MYLOGD("Setting progress (%s): %s/%d\n", key, value, weight_total_); - } else { - // stderr is ignored on normal invocations, but useful when calling /system/bin/dumpstate - // directly for debuggging. - fprintf(stderr, "Setting progress (%s): %s/%d\n", key, value, weight_total_); + if (listener_ != nullptr) { + listener_->onMaxProgressUpdated(weight_total_); + } else { + snprintf(key, sizeof(key), "dumpstate.%d.max", pid_); + snprintf(value, sizeof(value), "%d", weight_total_); + int status = property_set(key, value); + if (status != 0) { + MYLOGE("Could not update max weight by setting system property %s to %s: %d\n", key, + value, status); + } + } } if (control_socket_fd_ >= 0) { @@ -1351,10 +1353,36 @@ void Dumpstate::UpdateProgress(int delta) { fsync(control_socket_fd_); } - int status = property_set(key, value); - if (status) { - MYLOGE("Could not update progress by setting system property %s to %s: %d\n", - key, value, status); + if (listener_ != nullptr) { + if (progress_ % 100 == 0) { + // We don't want to spam logcat, so only log multiples of 100. + MYLOGD("Setting progress (%s): %d/%d\n", listener_name_.c_str(), progress_, + weight_total_); + } else { + // stderr is ignored on normal invocations, but useful when calling + // /system/bin/dumpstate directly for debuggging. + fprintf(stderr, "Setting progress (%s): %d/%d\n", listener_name_.c_str(), progress_, + weight_total_); + } + listener_->onProgressUpdated(progress_); + } else { + snprintf(key, sizeof(key), "dumpstate.%d.progress", pid_); + snprintf(value, sizeof(value), "%d", progress_); + + if (progress_ % 100 == 0) { + // We don't want to spam logcat, so only log multiples of 100. + MYLOGD("Setting progress (%s): %s/%d\n", key, value, weight_total_); + } else { + // stderr is ignored on normal invocations, but useful when calling + // /system/bin/dumpstate directly for debuggging. + fprintf(stderr, "Setting progress (%s): %s/%d\n", key, value, weight_total_); + } + + int status = property_set(key, value); + if (status) { + MYLOGE("Could not update progress by setting system property %s to %s: %d\n", key, + value, status); + } } } -- 2.11.0