tp: cleanup RPC code in prep for future changes

This CL just cleans up includes/codestyle in the httpd/rpc code to
make future CLs more consistent.

Change-Id: I44836b521ba53d0af1ec92e66d241a1f198b02e2

7 files changed, 97 insertions, 98 deletions

diff --git a/include/perfetto/ext/base/http/http_server.h b/include/perfetto/ext/base/http/http_server.h
index 2fe563bde..3e6204234 100644
--- a/include/perfetto/ext/base/http/http_server.h
+++ b/include/perfetto/ext/base/http/http_server.h
@@ -18,6 +18,8 @@
 #define INCLUDE_PERFETTO_EXT_BASE_HTTP_HTTP_SERVER_H_
 
 #include <array>
+#include <cstddef>
+#include <cstdint>
 #include <initializer_list>
 #include <list>
 #include <memory>
@@ -29,8 +31,7 @@
 #include "perfetto/ext/base/string_view.h"
 #include "perfetto/ext/base/unix_socket.h"
 
-namespace perfetto {
-namespace base {
+namespace perfetto::base {
 
 class HttpServerConnection;
 
@@ -183,7 +184,6 @@ class HttpServer : public UnixSocket::EventListener {
   bool origin_error_logged_ = false;
 };
 
-}  // namespace base
-}  // namespace perfetto
+}  // namespace perfetto::base
 
 #endif  // INCLUDE_PERFETTO_EXT_BASE_HTTP_HTTP_SERVER_H_
diff --git a/src/trace_processor/rpc/httpd.cc b/src/trace_processor/rpc/httpd.cc
index 2c44b71b5..e63bdb499 100644
--- a/src/trace_processor/rpc/httpd.cc
+++ b/src/trace_processor/rpc/httpd.cc
@@ -14,26 +14,29 @@
  * limitations under the License.
  */
 
-#include "perfetto/base/build_config.h"
-
-#if PERFETTO_BUILDFLAG(PERFETTO_TP_HTTPD)
-
-#include "src/trace_processor/rpc/httpd.h"
-
+#include <cstddef>
+#include <cstdint>
+#include <initializer_list>
+#include <memory>
+#include <optional>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "perfetto/base/logging.h"
+#include "perfetto/base/status.h"
 #include "perfetto/ext/base/http/http_server.h"
 #include "perfetto/ext/base/string_utils.h"
 #include "perfetto/ext/base/string_view.h"
 #include "perfetto/ext/base/unix_task_runner.h"
-#include "perfetto/ext/base/utils.h"
 #include "perfetto/protozero/scattered_heap_buffer.h"
 #include "perfetto/trace_processor/trace_processor.h"
+#include "src/trace_processor/rpc/httpd.h"
 #include "src/trace_processor/rpc/rpc.h"
 
 #include "protos/perfetto/trace_processor/trace_processor.pbzero.h"
 
-namespace perfetto {
-namespace trace_processor {
-
+namespace perfetto::trace_processor {
 namespace {
 
 constexpr int kBindPort = 9001;
@@ -58,9 +61,9 @@ class Httpd : public base::HttpRequestHandler {
   void OnHttpRequest(const base::HttpRequest&) override;
   void OnWebsocketMessage(const base::WebsocketMessage&) override;
 
-  void ServeHelpPage(const base::HttpRequest&);
+  static void ServeHelpPage(const base::HttpRequest&);
 
-  Rpc trace_processor_rpc_;
+  Rpc global_trace_processor_rpc_;
   base::UnixTaskRunner task_runner_;
   base::HttpServer http_srv_;
 };
@@ -68,7 +71,7 @@ class Httpd : public base::HttpRequestHandler {
 base::HttpServerConnection* g_cur_conn;
 
 base::StringView Vec2Sv(const std::vector<uint8_t>& v) {
-  return base::StringView(reinterpret_cast<const char*>(v.data()), v.size());
+  return {reinterpret_cast<const char*>(v.data()), v.size()};
 }
 
 // Used both by websockets and /rpc chunked HTTP endpoints.
@@ -91,7 +94,7 @@ void SendRpcChunk(const void* data, uint32_t len) {
 }
 
 Httpd::Httpd(std::unique_ptr<TraceProcessor> preloaded_instance)
-    : trace_processor_rpc_(std::move(preloaded_instance)),
+    : global_trace_processor_rpc_(std::move(preloaded_instance)),
       http_srv_(&task_runner_, this) {}
 Httpd::~Httpd() = default;
 
@@ -103,8 +106,9 @@ void Httpd::Run(int port) {
       "or through the Python API (see "
       "https://perfetto.dev/docs/analysis/trace-processor#python-api).");
 
-  for (size_t i = 0; i < base::ArraySize(kAllowedCORSOrigins); ++i)
-    http_srv_.AddAllowedOrigin(kAllowedCORSOrigins[i]);
+  for (const auto& kAllowedCORSOrigin : kAllowedCORSOrigins) {
+    http_srv_.AddAllowedOrigin(kAllowedCORSOrigin);
+  }
   http_srv_.Start(port);
   task_runner_.Run();
 }
@@ -139,7 +143,7 @@ void Httpd::OnHttpRequest(const base::HttpRequest& req) {
   };
 
   if (req.uri == "/status") {
-    auto status = trace_processor_rpc_.GetStatus();
+    auto status = global_trace_processor_rpc_.GetStatus();
     return conn.SendResponse("200 OK", default_headers, Vec2Sv(status));
   }
 
@@ -163,10 +167,10 @@ void Httpd::OnHttpRequest(const base::HttpRequest& req) {
                              base::HttpServerConnection::kOmitContentLength);
     PERFETTO_CHECK(g_cur_conn == nullptr);
     g_cur_conn = req.conn;
-    trace_processor_rpc_.SetRpcResponseFunction(SendRpcChunk);
+    global_trace_processor_rpc_.SetRpcResponseFunction(SendRpcChunk);
     // OnRpcRequest() will call SendRpcChunk() one or more times.
-    trace_processor_rpc_.OnRpcRequest(req.body.data(), req.body.size());
-    trace_processor_rpc_.SetRpcResponseFunction(nullptr);
+    global_trace_processor_rpc_.OnRpcRequest(req.body.data(), req.body.size());
+    global_trace_processor_rpc_.SetRpcResponseFunction(nullptr);
     g_cur_conn = nullptr;
 
     // Terminate chunked stream.
@@ -175,7 +179,7 @@ void Httpd::OnHttpRequest(const base::HttpRequest& req) {
   }
 
   if (req.uri == "/parse") {
-    base::Status status = trace_processor_rpc_.Parse(
+    base::Status status = global_trace_processor_rpc_.Parse(
         reinterpret_cast<const uint8_t*>(req.body.data()), req.body.size());
     protozero::HeapBuffered<protos::pbzero::AppendTraceDataResult> result;
     if (!status.ok()) {
@@ -186,12 +190,12 @@ void Httpd::OnHttpRequest(const base::HttpRequest& req) {
   }
 
   if (req.uri == "/notify_eof") {
-    trace_processor_rpc_.NotifyEndOfFile();
+    global_trace_processor_rpc_.NotifyEndOfFile();
     return conn.SendResponse("200 OK", default_headers);
   }
 
   if (req.uri == "/restore_initial_tables") {
-    trace_processor_rpc_.RestoreInitialTables();
+    global_trace_processor_rpc_.RestoreInitialTables();
     return conn.SendResponse("200 OK", default_headers);
   }
 
@@ -217,26 +221,27 @@ void Httpd::OnHttpRequest(const base::HttpRequest& req) {
       if (!has_more)
         conn.SendResponseBody("0\r\n\r\n", 5);
     };
-    trace_processor_rpc_.Query(
+    global_trace_processor_rpc_.Query(
         reinterpret_cast<const uint8_t*>(req.body.data()), req.body.size(),
         on_result_chunk);
     return;
   }
 
   if (req.uri == "/compute_metric") {
-    std::vector<uint8_t> res = trace_processor_rpc_.ComputeMetric(
+    std::vector<uint8_t> res = global_trace_processor_rpc_.ComputeMetric(
         reinterpret_cast<const uint8_t*>(req.body.data()), req.body.size());
     return conn.SendResponse("200 OK", default_headers, Vec2Sv(res));
   }
 
   if (req.uri == "/enable_metatrace") {
-    trace_processor_rpc_.EnableMetatrace(
+    global_trace_processor_rpc_.EnableMetatrace(
         reinterpret_cast<const uint8_t*>(req.body.data()), req.body.size());
     return conn.SendResponse("200 OK", default_headers);
   }
 
   if (req.uri == "/disable_and_read_metatrace") {
-    std::vector<uint8_t> res = trace_processor_rpc_.DisableAndReadMetatrace();
+    std::vector<uint8_t> res =
+        global_trace_processor_rpc_.DisableAndReadMetatrace();
     return conn.SendResponse("200 OK", default_headers, Vec2Sv(res));
   }
 
@@ -246,17 +251,17 @@ void Httpd::OnHttpRequest(const base::HttpRequest& req) {
 void Httpd::OnWebsocketMessage(const base::WebsocketMessage& msg) {
   PERFETTO_CHECK(g_cur_conn == nullptr);
   g_cur_conn = msg.conn;
-  trace_processor_rpc_.SetRpcResponseFunction(SendRpcChunk);
+  global_trace_processor_rpc_.SetRpcResponseFunction(SendRpcChunk);
   // OnRpcRequest() will call SendRpcChunk() one or more times.
-  trace_processor_rpc_.OnRpcRequest(msg.data.data(), msg.data.size());
-  trace_processor_rpc_.SetRpcResponseFunction(nullptr);
+  global_trace_processor_rpc_.OnRpcRequest(msg.data.data(), msg.data.size());
+  global_trace_processor_rpc_.SetRpcResponseFunction(nullptr);
   g_cur_conn = nullptr;
 }
 
 }  // namespace
 
 void RunHttpRPCServer(std::unique_ptr<TraceProcessor> preloaded_instance,
-                      std::string port_number) {
+                      const std::string& port_number) {
   Httpd srv(std::move(preloaded_instance));
   std::optional<int> port_opt = base::StringToInt32(port_number);
   int port = port_opt.has_value() ? *port_opt : kBindPort;
@@ -291,7 +296,4 @@ https://perfetto.dev/docs/contributing/getting-started#community
   req.conn->SendResponse("200 OK", headers, kPage);
 }
 
-}  // namespace trace_processor
-}  // namespace perfetto
-
-#endif  // PERFETTO_TP_HTTPD
+}  // namespace perfetto::trace_processor
diff --git a/src/trace_processor/rpc/httpd.h b/src/trace_processor/rpc/httpd.h
index 57e7ddb4b..f8d89c70f 100644
--- a/src/trace_processor/rpc/httpd.h
+++ b/src/trace_processor/rpc/httpd.h
@@ -20,8 +20,7 @@
 #include <memory>
 #include <string>
 
-namespace perfetto {
-namespace trace_processor {
+namespace perfetto::trace_processor {
 
 class TraceProcessor;
 
@@ -30,9 +29,8 @@ class TraceProcessor;
 // The unique_ptr argument is optional. If non-null, the HTTP server will adopt
 // an existing instance with a pre-loaded trace. If null, it will create a new
 // instance when pushing data into the /parse endpoint.
-void RunHttpRPCServer(std::unique_ptr<TraceProcessor>, std::string);
+void RunHttpRPCServer(std::unique_ptr<TraceProcessor>, const std::string&);
 
-}  // namespace trace_processor
-}  // namespace perfetto
+}  // namespace perfetto::trace_processor
 
 #endif  // SRC_TRACE_PROCESSOR_RPC_HTTPD_H_
diff --git a/src/trace_processor/rpc/rpc.cc b/src/trace_processor/rpc/rpc.cc
index b4552e5af..c715784f5 100644
--- a/src/trace_processor/rpc/rpc.cc
+++ b/src/trace_processor/rpc/rpc.cc
@@ -16,30 +16,36 @@
 
 #include "src/trace_processor/rpc/rpc.h"
 
-#include <string.h>
-
+#include <cinttypes>
+#include <cstdint>
+#include <cstdio>
+#include <cstring>
+#include <memory>
+#include <string>
+#include <utility>
 #include <vector>
 
 #include "perfetto/base/logging.h"
+#include "perfetto/base/status.h"
 #include "perfetto/base/time.h"
-#include "perfetto/ext/base/utils.h"
 #include "perfetto/ext/base/version.h"
 #include "perfetto/ext/protozero/proto_ring_buffer.h"
 #include "perfetto/ext/trace_processor/rpc/query_result_serializer.h"
+#include "perfetto/protozero/field.h"
 #include "perfetto/protozero/scattered_heap_buffer.h"
-#include "perfetto/protozero/scattered_stream_writer.h"
 #include "perfetto/trace_processor/basic_types.h"
+#include "perfetto/trace_processor/metatrace_config.h"
 #include "perfetto/trace_processor/trace_processor.h"
 #include "src/trace_processor/tp_metatrace.h"
 
+#include "protos/perfetto/trace_processor/metatrace_categories.pbzero.h"
 #include "protos/perfetto/trace_processor/trace_processor.pbzero.h"
 
-namespace perfetto {
-namespace trace_processor {
+namespace perfetto::trace_processor {
 
 namespace {
 // Writes a "Loading trace ..." update every N bytes.
-constexpr size_t kProgressUpdateBytes = 50 * 1000 * 1000;
+constexpr size_t kProgressUpdateBytes = 50ul * 1000 * 1000;
 using TraceProcessorRpcStream = protos::pbzero::TraceProcessorRpcStream;
 using RpcProto = protos::pbzero::TraceProcessorRpc;
 
@@ -194,7 +200,7 @@ void Rpc::ParseRpcRequest(const uint8_t* data, size_t len) {
         result->set_error(kErrFieldNotSet);
       } else {
         protozero::ConstBytes byte_range = req.append_trace_data();
-        util::Status res = Parse(byte_range.data, byte_range.size);
+        base::Status res = Parse(byte_range.data, byte_range.size);
         if (!res.ok()) {
           result->set_error(res.message());
         }
@@ -295,7 +301,7 @@ void Rpc::ParseRpcRequest(const uint8_t* data, size_t len) {
   }  // switch(req_type)
 }
 
-util::Status Rpc::Parse(const uint8_t* data, size_t len) {
+base::Status Rpc::Parse(const uint8_t* data, size_t len) {
   PERFETTO_TP_TRACE(
       metatrace::Category::API_TIMELINE, "RPC_PARSE",
       [&](metatrace::Record* r) { r->AddArg("length", std::to_string(len)); });
@@ -310,7 +316,7 @@ util::Status Rpc::Parse(const uint8_t* data, size_t len) {
   MaybePrintProgress();
 
   if (len == 0)
-    return util::OkStatus();
+    return base::OkStatus();
 
   // TraceProcessor needs take ownership of the memory chunk.
   std::unique_ptr<uint8_t[]> data_copy(new uint8_t[len]);
@@ -372,7 +378,7 @@ void Rpc::MaybePrintProgress() {
 
 void Rpc::Query(const uint8_t* args,
                 size_t len,
-                QueryResultBatchCallback result_callback) {
+                const QueryResultBatchCallback& result_callback) {
   auto it = QueryInternal(args, len);
   QueryResultSerializer serializer(std::move(it));
 
@@ -396,7 +402,7 @@ Iterator Rpc::QueryInternal(const uint8_t* args, size_t len) {
                       }
                     });
 
-  return trace_processor_->ExecuteQuery(sql.c_str());
+  return trace_processor_->ExecuteQuery(sql);
 }
 
 void Rpc::RestoreInitialTables() {
@@ -432,7 +438,7 @@ void Rpc::ComputeMetricInternal(const uint8_t* data,
   switch (args.format()) {
     case protos::pbzero::ComputeMetricArgs::BINARY_PROTOBUF: {
       std::vector<uint8_t> metrics_proto;
-      util::Status status =
+      base::Status status =
           trace_processor_->ComputeMetric(metric_names, &metrics_proto);
       if (status.ok()) {
         result->set_metrics(metrics_proto.data(), metrics_proto.size());
@@ -443,7 +449,7 @@ void Rpc::ComputeMetricInternal(const uint8_t* data,
     }
     case protos::pbzero::ComputeMetricArgs::TEXTPROTO: {
       std::string metrics_string;
-      util::Status status = trace_processor_->ComputeMetricText(
+      base::Status status = trace_processor_->ComputeMetricText(
           metric_names, TraceProcessor::MetricResultFormat::kProtoText,
           &metrics_string);
       if (status.ok()) {
@@ -455,7 +461,7 @@ void Rpc::ComputeMetricInternal(const uint8_t* data,
     }
     case protos::pbzero::ComputeMetricArgs::JSON: {
       std::string metrics_string;
-      util::Status status = trace_processor_->ComputeMetricText(
+      base::Status status = trace_processor_->ComputeMetricText(
           metric_names, TraceProcessor::MetricResultFormat::kJson,
           &metrics_string);
       if (status.ok()) {
@@ -486,7 +492,7 @@ std::vector<uint8_t> Rpc::DisableAndReadMetatrace() {
 void Rpc::DisableAndReadMetatraceInternal(
     protos::pbzero::DisableAndReadMetatraceResult* result) {
   std::vector<uint8_t> trace_proto;
-  util::Status status = trace_processor_->DisableAndReadMetatrace(&trace_proto);
+  base::Status status = trace_processor_->DisableAndReadMetatrace(&trace_proto);
   if (status.ok()) {
     result->set_metatrace(trace_proto.data(), trace_proto.size());
   } else {
@@ -506,5 +512,4 @@ std::vector<uint8_t> Rpc::GetStatus() {
   return status.SerializeAsArray();
 }
 
-}  // namespace trace_processor
-}  // namespace perfetto
+}  // namespace perfetto::trace_processor
diff --git a/src/trace_processor/rpc/rpc.h b/src/trace_processor/rpc/rpc.h
index c10d4bf8c..61cf2e636 100644
--- a/src/trace_processor/rpc/rpc.h
+++ b/src/trace_processor/rpc/rpc.h
@@ -17,25 +17,23 @@
 #ifndef SRC_TRACE_PROCESSOR_RPC_RPC_H_
 #define SRC_TRACE_PROCESSOR_RPC_RPC_H_
 
+#include <cstddef>
+#include <cstdint>
 #include <functional>
 #include <memory>
+#include <string>
 #include <vector>
 
-#include <stddef.h>
-#include <stdint.h>
-
+#include "perfetto/base/status.h"
 #include "perfetto/ext/protozero/proto_ring_buffer.h"
 #include "perfetto/trace_processor/basic_types.h"
-#include "perfetto/trace_processor/status.h"
 
 namespace perfetto {
 
-namespace protos {
-namespace pbzero {
+namespace protos::pbzero {
 class ComputeMetricResult;
 class DisableAndReadMetatraceResult;
-}  // namespace pbzero
-}  // namespace protos
+}  // namespace protos::pbzero
 
 namespace trace_processor {
 
@@ -100,12 +98,12 @@ class Rpc {
   // The methods of this class are mirrors (modulo {un,}marshalling of args) of
   // the corresponding names in trace_processor.h . See that header for docs.
 
-  util::Status Parse(const uint8_t* data, size_t len);
+  base::Status Parse(const uint8_t*, size_t);
   void NotifyEndOfFile();
-  void ResetTraceProcessor(const uint8_t* args, size_t len);
+  void ResetTraceProcessor(const uint8_t*, size_t);
   std::string GetCurrentTraceName();
-  std::vector<uint8_t> ComputeMetric(const uint8_t* data, size_t len);
-  void EnableMetatrace(const uint8_t* data, size_t len);  // EnableMetatraceArgs
+  std::vector<uint8_t> ComputeMetric(const uint8_t*, size_t);
+  void EnableMetatrace(const uint8_t*, size_t);
   std::vector<uint8_t> DisableAndReadMetatrace();
   std::vector<uint8_t> GetStatus();
 
@@ -122,22 +120,17 @@ class Rpc {
   //   ...
   //   callback(..., has_more=false)
   //   (Query() returns at this point).
-  // TODO(primiano): long-term this API should change and be turned into a
-  // bidirectional streaming api (see go/imperative-metrics). The problem with
-  // the current design is that it holds the callstack until the query is done
-  // and makes nested query hard as they cause re-entrancy. It's okay for now
-  // but will change soon.
   using QueryResultBatchCallback = std::function<
       void(const uint8_t* /*buf*/, size_t /*len*/, bool /*has_more*/)>;
-  void Query(const uint8_t* args, size_t len, QueryResultBatchCallback);
+  void Query(const uint8_t*, size_t, const QueryResultBatchCallback&);
 
  private:
-  void ParseRpcRequest(const uint8_t* data, size_t len);
-  void ResetTraceProcessorInternal(const Config& config);
+  void ParseRpcRequest(const uint8_t*, size_t);
+  void ResetTraceProcessorInternal(const Config&);
   void MaybePrintProgress();
-  Iterator QueryInternal(const uint8_t* args, size_t len);
-  void ComputeMetricInternal(const uint8_t* args,
-                             size_t len,
+  Iterator QueryInternal(const uint8_t*, size_t);
+  void ComputeMetricInternal(const uint8_t*,
+                             size_t,
                              protos::pbzero::ComputeMetricResult*);
   void DisableAndReadMetatraceInternal(
       protos::pbzero::DisableAndReadMetatraceResult*);
diff --git a/src/trace_processor/rpc/stdiod.cc b/src/trace_processor/rpc/stdiod.cc
index ae49c874f..d6cf8de42 100644
--- a/src/trace_processor/rpc/stdiod.cc
+++ b/src/trace_processor/rpc/stdiod.cc
@@ -16,10 +16,12 @@
 
 #include "src/trace_processor/rpc/stdiod.h"
 
-#if !PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
-#include <unistd.h>
-#endif
+#include <cstddef>
+#include <cstdint>
+#include <memory>
+#include <utility>
 
+#include "perfetto/base/build_config.h"
 #include "perfetto/base/logging.h"
 #include "perfetto/base/status.h"
 #include "perfetto/ext/base/file_utils.h"
@@ -27,13 +29,16 @@
 #include "perfetto/trace_processor/trace_processor.h"
 #include "src/trace_processor/rpc/rpc.h"
 
+#if !PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
+#include <unistd.h>
+#endif
+
 #if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN) && !defined(STDIN_FILENO)
 #define STDIN_FILENO 0
 #define STDOUT_FILENO 1
 #endif
 
-namespace perfetto {
-namespace trace_processor {
+namespace perfetto::trace_processor {
 
 base::Status RunStdioRpcServer(std::unique_ptr<TraceProcessor> tp) {
   Rpc rpc(std::move(tp));
@@ -57,5 +62,4 @@ base::Status RunStdioRpcServer(std::unique_ptr<TraceProcessor> tp) {
   }
 }
 
-}  // namespace trace_processor
-}  // namespace perfetto
+}  // namespace perfetto::trace_processor
diff --git a/src/trace_processor/rpc/wasm_bridge.cc b/src/trace_processor/rpc/wasm_bridge.cc
index 6e266b85d..a9ba607c5 100644
--- a/src/trace_processor/rpc/wasm_bridge.cc
+++ b/src/trace_processor/rpc/wasm_bridge.cc
@@ -15,13 +15,11 @@
  */
 
 #include <emscripten/emscripten.h>
+#include <cstdint>
 
-#include "perfetto/base/logging.h"
-#include "perfetto/trace_processor/trace_processor.h"
 #include "src/trace_processor/rpc/rpc.h"
 
-namespace perfetto {
-namespace trace_processor {
+namespace perfetto::trace_processor {
 
 namespace {
 Rpc* g_trace_processor_rpc;
@@ -60,8 +58,7 @@ void trace_processor_on_rpc_request(uint32_t size) {
 }
 
 }  // extern "C"
-}  // namespace trace_processor
-}  // namespace perfetto
+}  // namespace perfetto::trace_processor
 
 int main(int, char**) {
   // This is unused but is needed for the following reasons: