Fix the INTERNAL_UID macro so it actually appends the value of __LINE__ rather...

Fix the INTERNAL_UID macro so it actually appends the value of __LINE__ rather then the string "__LINE__"

Add a test to catch it. And also switch to the same implementation as
the client library with the in parameter.

Change-Id: I94b775e6692fe88a14efc7c6030c280057012935
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1939971
Commit-Queue: Stephen Nusko <nuskos@chromium.org>
Commit-Queue: Sami Kyöstilä <skyostil@chromium.org>
Auto-Submit: Stephen Nusko <nuskos@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719563}

services/tracing/public/cpp/perfetto/macros_internal.h

@@ -45,7 +45,8 @@ static inline base::trace_event::TraceEventHandle AddTraceEvent(
 // These macros should not be called directly. They are intended to be used by
 // macros in //services/tracing/perfetto/macros.h only.
 
-#define TRACING_INTERNAL_CONCAT(a, b) a##b
+#define TRACING_INTERNAL_CONCAT2(a, b) a##b
+#define TRACING_INTERNAL_CONCAT(a, b) TRACING_INTERNAL_CONCAT2(a, b)
 #define TRACING_INTERNAL_UID(prefix) TRACING_INTERNAL_CONCAT(prefix, __LINE__)
 
 #define TRACING_INTERNAL_ADD_TRACE_EVENT(phase, category, name, ...)     \
@@ -61,6 +62,13 @@ static inline base::trace_event::TraceEventHandle AddTraceEvent(
 #define TRACING_INTERNAL_SCOPED_ADD_TRACE_EVENT(category, name, ...)          \
   struct {                                                                    \
     struct ScopedTraceEvent {                                                 \
+      /* The int parameter is an implementation detail. It allows the     */  \
+      /* anonymous struct to use aggregate initialization to invoke the   */  \
+      /* lambda to emit the begin event with the proper reference capture */  \
+      /* for any TrackEventArgumentFunction in |__VA_ARGS__|. This is     */  \
+      /* required so that the scoped event is exactly ONE line and can't  */  \
+      /* escape the scope if used in a single line if statement.          */  \
+      ScopedTraceEvent(int) {}                                                \
       ~ScopedTraceEvent() {                                                   \
         /* TODO(nuskos): Remove the empty string passed as the |name|  */     \
         /* field. As described in macros.h we shouldn't need it in our */     \
@@ -68,13 +76,6 @@ static inline base::trace_event::TraceEventHandle AddTraceEvent(
         TRACING_INTERNAL_ADD_TRACE_EVENT(TRACE_EVENT_PHASE_END, category, "", \
                                          [](perfetto::EventContext) {});      \
       }                                                                       \
-      /* The variable |x| is an implementation detail. It allows the      */  \
-      /* anonymous struct to use aggregate initialization to invoke the   */  \
-      /* lambda to emit the begin event with the proper reference capture */  \
-      /* for any TrackEventArgumentFunction in |__VA_ARGS__|. This is     */  \
-      /* required so that the scoped event is exactly ONE line and can't  */  \
-      /* escape the scope if used in a single line if statement.          */  \
-      int x;                                                                  \
     } event;                                                                  \
   } TRACING_INTERNAL_UID(scoped_event){[&]() {                                \
     TRACING_INTERNAL_ADD_TRACE_EVENT(TRACE_EVENT_PHASE_BEGIN, category, name, \

services/tracing/public/cpp/perfetto/trace_event_data_source_unittest.cc

@@ -1510,6 +1510,55 @@ TEST_F(TraceEventDataSourceTest, TypedArgumentsTracingOnScopedCapture) {
   EXPECT_TRUE(called);
 }
 
+TEST_F(TraceEventDataSourceTest, TypedArgumentsTracingOnScopedMultipleEvents) {
+  CreateTraceEventDataSource();
+
+  {
+    TRACE_EVENT("browser", "bar", [&](perfetto::EventContext ctx) {
+      ctx.event()->set_log_message()->set_body_iid(42);
+    });
+    TRACE_EVENT("browser", "bar", [&](perfetto::EventContext ctx) {
+      ctx.event()->set_log_message()->set_body_iid(43);
+    });
+  }
+
+  EXPECT_EQ(producer_client()->GetFinalizedPacketCount(), 5u);
+
+  auto* td_packet = producer_client()->GetFinalizedPacket();
+  ExpectThreadDescriptor(td_packet);
+
+  // The first TRACE_EVENT begin.
+  auto* e_packet = producer_client()->GetFinalizedPacket(1);
+  ExpectTraceEvent(e_packet, /*category_iid=*/1u, /*name_iid=*/1u,
+                   TRACE_EVENT_PHASE_BEGIN);
+
+  ExpectEventCategories(e_packet, {{1u, "browser"}});
+  ExpectEventNames(e_packet, {{1u, "bar"}});
+  ASSERT_TRUE(e_packet->track_event().has_log_message());
+  EXPECT_EQ(e_packet->track_event().log_message().body_iid(), 42u);
+
+  // The second TRACE_EVENT begin.
+  e_packet = producer_client()->GetFinalizedPacket(2);
+  ExpectTraceEvent(e_packet, /*category_iid=*/1u, /*name_iid=*/1u,
+                   TRACE_EVENT_PHASE_BEGIN);
+  ASSERT_TRUE(e_packet->track_event().has_log_message());
+  EXPECT_EQ(e_packet->track_event().log_message().body_iid(), 43u);
+
+  // The second TRACE_EVENT end.
+  e_packet = producer_client()->GetFinalizedPacket(3);
+  ExpectTraceEvent(e_packet, /*category_iid=*/1u, /*name_iid=*/2u,
+                   TRACE_EVENT_PHASE_END);
+
+  ExpectEventNames(e_packet, {{2u, kTraceEventEndName}});
+  EXPECT_FALSE(e_packet->track_event().has_log_message());
+
+  // The first TRACE_EVENT end.
+  e_packet = producer_client()->GetFinalizedPacket(4);
+  ExpectTraceEvent(e_packet, /*category_iid=*/1u, /*name_iid=*/2u,
+                   TRACE_EVENT_PHASE_END);
+  EXPECT_FALSE(e_packet->track_event().has_log_message());
+}
+
 // TODO(eseckler): Add startup tracing unittests.
 
 }  // namespace