commit 6968f552f485c790e81c1ef3c0f9eff790e0bd4e
author Gavin Corkery <gavincorkery@google.com> Sun Nov 22 18:09:05 2020 +0000
committer Gavin Corkery <gavincorkery@google.com> Tue Nov 24 00:34:01 2020 +0000
tree f0af095479bd9268af78ec4b81b1bbdf51feb26d
parent 5702b485d51136d938f4e33a18db87d3cd5c0040

Reduce duplicate calls for restricted dump options

The existing logic makes several identical calls for each
restriced bugreport type (telephony, wifi and limited). Instead,
handle logic for the default case so that the critical dumpsys
can be collected before running common logic for all bugreport
types.

Test: atest BugreportManagerTest
Test: atest dumpstate_test
Bug: 1587370896
Change-Id: I1784e53a526c02e488232f43d764ec3c398a9a1a

cmds/dumpstate/dumpstate.cpp

diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index 82e3699..7ebf23f 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -2890,31 +2890,23 @@
     // duration is logged into MYLOG instead.
     PrintHeader();
 
-    // TODO(b/158737089) reduce code repetition in if branches
-    if (options_->telephony_only) {
-        MaybeTakeEarlyScreenshot();
-        onUiIntensiveBugreportDumpsFinished(calling_uid);
-        MaybeCheckUserConsent(calling_uid, calling_package);
-        DumpstateTelephonyOnly(calling_package);
-    } else if (options_->wifi_only) {
-        MaybeTakeEarlyScreenshot();
-        onUiIntensiveBugreportDumpsFinished(calling_uid);
-        MaybeCheckUserConsent(calling_uid, calling_package);
-        DumpstateWifiOnly();
-    } else if (options_->limited_only) {
-        MaybeTakeEarlyScreenshot();
-        onUiIntensiveBugreportDumpsFinished(calling_uid);
-        MaybeCheckUserConsent(calling_uid, calling_package);
-        DumpstateLimitedOnly();
-    } else {
+    bool is_dumpstate_restricted = options_->telephony_only
+                                   || options_->wifi_only
+                                   || options_->limited_only;
+    if (!is_dumpstate_restricted) {
         // Invoke critical dumpsys first to preserve system state, before doing anything else.
         RunDumpsysCritical();
-
-        // Take screenshot and get consent only after critical dumpsys has finished.
-        MaybeTakeEarlyScreenshot();
-        onUiIntensiveBugreportDumpsFinished(calling_uid);
-        MaybeCheckUserConsent(calling_uid, calling_package);
-
+    }
+    MaybeTakeEarlyScreenshot();
+    onUiIntensiveBugreportDumpsFinished(calling_uid);
+    MaybeCheckUserConsent(calling_uid, calling_package);
+    if (options_->telephony_only) {
+        DumpstateTelephonyOnly(calling_package);
+    } else if (options_->wifi_only) {
+        DumpstateWifiOnly();
+    } else if (options_->limited_only) {
+        DumpstateLimitedOnly();
+    } else {
         // Dump state for the default case. This also drops root.
         RunStatus s = DumpstateDefaultAfterCritical();
         if (s != RunStatus::OK) {