SF: Simplify dumpsys flag parsing

This CL removes sequential parsing for dumpsys flags, which was order-
dependent and boilerplate-heavy.

Note that flags cannot be combined for simplicity.

Bug: None
Test: dumpsys SurfaceFlinger [--FLAG [ARGS...]]
Change-Id: Id305ffee464c6cad4f228c7a7f603c33a2978d66
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 5df2876..1cb38c8 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -28,6 +28,7 @@
 #include <functional>
 #include <mutex>
 #include <optional>
+#include <unordered_map>
 
 #include <cutils/properties.h>
 #include <log/log.h>
@@ -4344,8 +4345,8 @@
 
 // ---------------------------------------------------------------------------
 
-status_t SurfaceFlinger::doDump(int fd, const Vector<String16>& args, bool asProto)
-        NO_THREAD_SAFETY_ANALYSIS {
+status_t SurfaceFlinger::doDump(int fd, const DumpArgs& args,
+                                bool asProto) NO_THREAD_SAFETY_ANALYSIS {
     std::string result;
 
     IPCThreadState* ipc = IPCThreadState::self();
@@ -4369,114 +4370,35 @@
                           strerror(-err), err);
         }
 
-        bool dumpAll = true;
-        size_t index = 0;
-        size_t numArgs = args.size();
+        using namespace std::string_literals;
 
-        if (numArgs) {
-            if ((index < numArgs) &&
-                    (args[index] == String16("--list"))) {
-                index++;
-                listLayersLocked(args, index, result);
-                dumpAll = false;
-            }
+        static const std::unordered_map<std::string, Dumper> dumpers = {
+                {"--clear-layer-stats"s, dumper([this](std::string&) { mLayerStats.clear(); })},
+                {"--disable-layer-stats"s, dumper([this](std::string&) { mLayerStats.disable(); })},
+                {"--display-id"s, dumper(&SurfaceFlinger::dumpDisplayIdentificationData)},
+                {"--dispsync"s, dumper([this](std::string& s) { mPrimaryDispSync->dump(s); })},
+                {"--dump-layer-stats"s, dumper([this](std::string& s) { mLayerStats.dump(s); })},
+                {"--enable-layer-stats"s, dumper([this](std::string&) { mLayerStats.enable(); })},
+                {"--frame-composition"s, dumper(&SurfaceFlinger::dumpFrameCompositionInfo)},
+                {"--frame-events"s, dumper(&SurfaceFlinger::dumpFrameEventsLocked)},
+                {"--latency"s, argsDumper(&SurfaceFlinger::dumpStatsLocked)},
+                {"--latency-clear"s, argsDumper(&SurfaceFlinger::clearStatsLocked)},
+                {"--list"s, dumper(&SurfaceFlinger::listLayersLocked)},
+                {"--static-screen"s, dumper(&SurfaceFlinger::dumpStaticScreenStats)},
+                {"--timestats"s, protoDumper(&SurfaceFlinger::dumpTimeStats)},
+                {"--wide-color"s, dumper(&SurfaceFlinger::dumpWideColorInfo)},
+        };
 
-            if ((index < numArgs) &&
-                    (args[index] == String16("--latency"))) {
-                index++;
-                dumpStatsLocked(args, index, result);
-                dumpAll = false;
-            }
+        const auto flag = args.empty() ? ""s : std::string(String8(args[0]));
 
-            if ((index < numArgs) &&
-                    (args[index] == String16("--latency-clear"))) {
-                index++;
-                clearStatsLocked(args, index, result);
-                dumpAll = false;
-            }
-
-            if ((index < numArgs) &&
-                    (args[index] == String16("--dispsync"))) {
-                index++;
-                mPrimaryDispSync->dump(result);
-                dumpAll = false;
-            }
-
-            if ((index < numArgs) &&
-                    (args[index] == String16("--static-screen"))) {
-                index++;
-                dumpStaticScreenStats(result);
-                dumpAll = false;
-            }
-
-            if ((index < numArgs) &&
-                    (args[index] == String16("--frame-events"))) {
-                index++;
-                dumpFrameEventsLocked(result);
-                dumpAll = false;
-            }
-
-            if ((index < numArgs) && (args[index] == String16("--wide-color"))) {
-                index++;
-                dumpWideColorInfo(result);
-                dumpAll = false;
-            }
-
-            if ((index < numArgs) &&
-                (args[index] == String16("--enable-layer-stats"))) {
-                index++;
-                mLayerStats.enable();
-                dumpAll = false;
-            }
-
-            if ((index < numArgs) &&
-                (args[index] == String16("--disable-layer-stats"))) {
-                index++;
-                mLayerStats.disable();
-                dumpAll = false;
-            }
-
-            if ((index < numArgs) &&
-                (args[index] == String16("--clear-layer-stats"))) {
-                index++;
-                mLayerStats.clear();
-                dumpAll = false;
-            }
-
-            if ((index < numArgs) &&
-                (args[index] == String16("--dump-layer-stats"))) {
-                index++;
-                mLayerStats.dump(result);
-                dumpAll = false;
-            }
-
-            if ((index < numArgs) &&
-                    (args[index] == String16("--frame-composition"))) {
-                index++;
-                dumpFrameCompositionInfo(result);
-                dumpAll = false;
-            }
-
-            if ((index < numArgs) &&
-                (args[index] == String16("--display-identification"))) {
-                index++;
-                dumpDisplayIdentificationData(result);
-                dumpAll = false;
-            }
-
-            if ((index < numArgs) && (args[index] == String16("--timestats"))) {
-                index++;
-                mTimeStats->parseArgs(asProto, args, index, result);
-                dumpAll = false;
-            }
-        }
-
-        if (dumpAll) {
+        if (const auto it = dumpers.find(flag); it != dumpers.end()) {
+            (it->second)(args, asProto, result);
+        } else {
             if (asProto) {
                 LayersProto layersProto = dumpProtoInfo(LayerVector::StateSet::Current);
                 result.append(layersProto.SerializeAsString().c_str(), layersProto.ByteSize());
             } else {
-                dumpAllLocked(args, index, result);
+                dumpAllLocked(args, result);
             }
         }
 
@@ -4488,43 +4410,29 @@
     return NO_ERROR;
 }
 
-void SurfaceFlinger::listLayersLocked(const Vector<String16>& /* args */, size_t& /* index */,
-                                      std::string& result) const {
+void SurfaceFlinger::listLayersLocked(std::string& result) const {
     mCurrentState.traverseInZOrder(
             [&](Layer* layer) { StringAppendF(&result, "%s\n", layer->getName().string()); });
 }
 
-void SurfaceFlinger::dumpStatsLocked(const Vector<String16>& args, size_t& index,
-                                     std::string& result) const {
-    String8 name;
-    if (index < args.size()) {
-        name = String8(args[index]);
-        index++;
-    }
-
+void SurfaceFlinger::dumpStatsLocked(const DumpArgs& args, std::string& result) const {
     StringAppendF(&result, "%" PRId64 "\n", getVsyncPeriod());
 
-    if (name.isEmpty()) {
-        mAnimFrameTracker.dumpStats(result);
-    } else {
+    if (args.size() > 1) {
+        const auto name = String8(args[1]);
         mCurrentState.traverseInZOrder([&](Layer* layer) {
             if (name == layer->getName()) {
                 layer->dumpFrameStats(result);
             }
         });
+    } else {
+        mAnimFrameTracker.dumpStats(result);
     }
 }
 
-void SurfaceFlinger::clearStatsLocked(const Vector<String16>& args, size_t& index,
-                                      std::string& /* result */) {
-    String8 name;
-    if (index < args.size()) {
-        name = String8(args[index]);
-        index++;
-    }
-
+void SurfaceFlinger::clearStatsLocked(const DumpArgs& args, std::string&) {
     mCurrentState.traverseInZOrder([&](Layer* layer) {
-        if (name.isEmpty() || (name == layer->getName())) {
+        if (args.size() < 2 || String8(args[1]) == layer->getName()) {
             layer->clearFrameStats();
         }
     });
@@ -4532,6 +4440,10 @@
     mAnimFrameTracker.clearStats();
 }
 
+void SurfaceFlinger::dumpTimeStats(const DumpArgs& args, bool asProto, std::string& result) const {
+    mTimeStats->parseArgs(asProto, args, result);
+}
+
 // This should only be called from the main thread.  Otherwise it would need
 // the lock and should use mCurrentState rather than mDrawingState.
 void SurfaceFlinger::logFrameStats() {
@@ -4760,15 +4672,8 @@
     return layersProto;
 }
 
-void SurfaceFlinger::dumpAllLocked(const Vector<String16>& args, size_t& index,
-                                   std::string& result) const {
-    bool colorize = false;
-    if (index < args.size()
-            && (args[index] == String16("--color"))) {
-        colorize = true;
-        index++;
-    }
-
+void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, std::string& result) const {
+    const bool colorize = !args.empty() && args[0] == String16("--color");
     Colorizer colorizer(colorize);
 
     // figure out if we're stuck somewhere
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 499ebcd..1a54f6c 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -68,6 +68,7 @@
 
 #include <atomic>
 #include <cstdint>
+#include <functional>
 #include <map>
 #include <memory>
 #include <mutex>
@@ -75,6 +76,7 @@
 #include <set>
 #include <string>
 #include <thread>
+#include <type_traits>
 #include <unordered_map>
 #include <utility>
 
@@ -786,19 +788,9 @@
         };
     }
 
-    /* ------------------------------------------------------------------------
-     * Debugging & dumpsys
+    /*
+     * Display identification
      */
-public:
-    status_t dumpCritical(int fd, const Vector<String16>& /*args*/, bool asProto) {
-        return doDump(fd, Vector<String16>(), asProto);
-    }
-
-    status_t dumpAll(int fd, const Vector<String16>& args, bool asProto) {
-        return doDump(fd, args, asProto);
-    }
-
-private:
     sp<IBinder> getPhysicalDisplayToken(DisplayId displayId) const {
         const auto it = mPhysicalDisplayTokens.find(displayId);
         return it != mPhysicalDisplayTokens.end() ? it->second : nullptr;
@@ -830,15 +822,45 @@
         return hwcDisplayId ? getHwComposer().toPhysicalDisplayId(*hwcDisplayId) : std::nullopt;
     }
 
-    void listLayersLocked(const Vector<String16>& args, size_t& index, std::string& result) const;
-    void dumpStatsLocked(const Vector<String16>& args, size_t& index, std::string& result) const
-            REQUIRES(mStateLock);
-    void clearStatsLocked(const Vector<String16>& args, size_t& index, std::string& result);
-    void dumpAllLocked(const Vector<String16>& args, size_t& index, std::string& result) const
-            REQUIRES(mStateLock);
+    /*
+     * Debugging & dumpsys
+     */
     bool startDdmConnection();
-    void appendSfConfigString(std::string& result) const;
 
+    using DumpArgs = Vector<String16>;
+    using Dumper = std::function<void(const DumpArgs&, bool asProto, std::string&)>;
+
+    template <typename F, std::enable_if_t<!std::is_member_function_pointer_v<F>>* = nullptr>
+    static Dumper dumper(F&& dump) {
+        using namespace std::placeholders;
+        return std::bind(std::forward<F>(dump), _3);
+    }
+
+    template <typename F, std::enable_if_t<std::is_member_function_pointer_v<F>>* = nullptr>
+    Dumper dumper(F dump) {
+        using namespace std::placeholders;
+        return std::bind(dump, this, _3);
+    }
+
+    template <typename F>
+    Dumper argsDumper(F dump) {
+        using namespace std::placeholders;
+        return std::bind(dump, this, _1, _3);
+    }
+
+    template <typename F>
+    Dumper protoDumper(F dump) {
+        using namespace std::placeholders;
+        return std::bind(dump, this, _1, _2, _3);
+    }
+
+    void dumpAllLocked(const DumpArgs& args, std::string& result) const REQUIRES(mStateLock);
+
+    void appendSfConfigString(std::string& result) const;
+    void listLayersLocked(std::string& result) const;
+    void dumpStatsLocked(const DumpArgs& args, std::string& result) const REQUIRES(mStateLock);
+    void clearStatsLocked(const DumpArgs& args, std::string& result);
+    void dumpTimeStats(const DumpArgs& args, bool asProto, std::string& result) const;
     void logFrameStats();
 
     void dumpStaticScreenStats(std::string& result) const;
@@ -857,7 +879,16 @@
     bool isLayerTripleBufferingDisabled() const {
         return this->mLayerTripleBufferingDisabled;
     }
-    status_t doDump(int fd, const Vector<String16>& args, bool asProto);
+
+    status_t doDump(int fd, const DumpArgs& args, bool asProto);
+
+    status_t dumpCritical(int fd, const DumpArgs&, bool asProto) override {
+        return doDump(fd, DumpArgs(), asProto);
+    }
+
+    status_t dumpAll(int fd, const DumpArgs& args, bool asProto) override {
+        return doDump(fd, args, asProto);
+    }
 
     /* ------------------------------------------------------------------------
      * VrFlinger
diff --git a/services/surfaceflinger/TimeStats/TimeStats.cpp b/services/surfaceflinger/TimeStats/TimeStats.cpp
index 6a5488a..8d3776b 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.cpp
+++ b/services/surfaceflinger/TimeStats/TimeStats.cpp
@@ -32,19 +32,12 @@
 
 namespace android {
 
-void TimeStats::parseArgs(bool asProto, const Vector<String16>& args, size_t& index,
-                          std::string& result) {
+void TimeStats::parseArgs(bool asProto, const Vector<String16>& args, std::string& result) {
     ATRACE_CALL();
 
-    if (args.size() > index + 10) {
-        ALOGD("Invalid args count");
-        return;
-    }
-
     std::unordered_map<std::string, int32_t> argsMap;
-    while (index < args.size()) {
+    for (size_t index = 0; index < args.size(); ++index) {
         argsMap[std::string(String8(args[index]).c_str())] = index;
-        ++index;
     }
 
     if (argsMap.count("-disable")) {
diff --git a/services/surfaceflinger/TimeStats/TimeStats.h b/services/surfaceflinger/TimeStats/TimeStats.h
index 71c3ed7..e8fbcab 100644
--- a/services/surfaceflinger/TimeStats/TimeStats.h
+++ b/services/surfaceflinger/TimeStats/TimeStats.h
@@ -77,7 +77,7 @@
     TimeStats() = default;
     ~TimeStats() = default;
 
-    void parseArgs(bool asProto, const Vector<String16>& args, size_t& index, std::string& result);
+    void parseArgs(bool asProto, const Vector<String16>& args, std::string& result);
     bool isEnabled();
 
     void incrementTotalFrames();
diff --git a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
index 86f1a39..0f95cf9 100644
--- a/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TimeStatsTest.cpp
@@ -131,7 +131,6 @@
 };
 
 std::string TimeStatsTest::inputCommand(InputCommand cmd, bool useProto) {
-    size_t index = 0;
     std::string result;
     Vector<String16> args;
 
@@ -162,7 +161,7 @@
             ALOGD("Invalid control command");
     }
 
-    EXPECT_NO_FATAL_FAILURE(mTimeStats->parseArgs(useProto, args, index, result));
+    EXPECT_NO_FATAL_FAILURE(mTimeStats->parseArgs(useProto, args, result));
     return result;
 }