Revert "Reland "[tracing] Remove special handling of task execution a…

…nd log events""

This reverts commit 3b99bb2.

Reason for revert: Suspected of causing TraceEventDataSourceTest.StartupTracingTimeout failure in services_unittests on Builder Linux CFI.

Original change's description:
> Reland "[tracing] Remove special handling of task execution and log events"
>
> This reverts commit 989724c.
>
> Reason for revert:
> Fixing test failures. The msan failures were due to hash operator trying
> to hash uninitialized padded bytes of the struct. Fix the struct to not
> have any padding.
>
> Original change's description:
> > Revert "[tracing] Remove special handling of task execution and log events"
> >
> > This reverts commit f79b151.
> >
> > Reason for revert: Breaks WebKit Linux MSAN
> >
> > The following tests started failing since this landed:
> >
> > * http/tests/devtools/tracing.js
> > * http/tests/devtools/tracing/decode-resize.js
> > * http/tests/devtools/tracing/timeline-paint/update-layer-tree.js
> > * http/tests/devtools/tracing/timeline-style/parse-author-style-sheet.js
> > * http/tests/devtools/tracing/tracing-record-input-events.js
> > * http/tests/devtools/tracing/user-timing.js
> > * http/tests/devtools/tracing/worker-events.js
> > * http/tests/devtools/tracing/worker-js-frames.js
> > * inspector-protocol/sessions/tracing-start.js
> > * inspector-protocol/timeline/tracing-proto-format.js
> >
> > First run that failed, also the run where the CL landed:
> > https://ci.chromium.org/p/chromium/builders/ci/WebKit%20Linux%20MSAN/7830
> >
> > Original change's description:
> > > [tracing] Remove special handling of task execution and log events
> > >
> > > The task execution and log events are handled specially for writing
> > > proto arguments because we did not have the support for typed events in
> > > base and interning support for the event macros. Now these events can
> > > use the new client library API.
> > > The events have to be migrated together because the source location
> > > interning index is common for all events in the API and does not
> > > account for the special handling in event sink.
> > >
> > > BUG=1136635
> > > TBR=chirantan@chromium.org
> > >
> > > Change-Id: I381e8c90e49c3fbd9ce4d8fb2e9db3d166b9b06d
> > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462636
> > > Reviewed-by: ssid <ssid@chromium.org>
> > > Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
> > > Reviewed-by: danakj <danakj@chromium.org>
> > > Reviewed-by: Erik Chen <erikchen@chromium.org>
> > > Reviewed-by: Eric Seckler <eseckler@chromium.org>
> > > Commit-Queue: ssid <ssid@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#815911}
> >
> > TBR=danakj@chromium.org,chirantan@chromium.org,erikchen@chromium.org,skyostil@chromium.org,ssid@chromium.org,eseckler@chromium.org
> >
> > # Not skipping CQ checks because original CL landed > 1 day ago.
> >
> > Bug: 1136635
> > Change-Id: I7a84356400bbdcae3144fd65afdcab7a4407dcd9
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462895
> > Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
> > Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#816069}
>
> TBR=danakj@chromium.org,chirantan@chromium.org,erikchen@chromium.org,skyostil@chromium.org,ssid@chromium.org,ortuno@chromium.org,eseckler@chromium.org
>
> # Not skipping CQ checks because this is a reland.
>
> Bug: 1136635
> Change-Id: Ia659185ebeb265dc47ff6c2ef47bd9d8cc43dd65
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2466507
> Reviewed-by: Eric Seckler <eseckler@chromium.org>
> Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
> Reviewed-by: danakj <danakj@chromium.org>
> Reviewed-by: ssid <ssid@chromium.org>
> Reviewed-by: Erik Chen <erikchen@chromium.org>
> Commit-Queue: ssid <ssid@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#816620}

TBR=danakj@chromium.org,chirantan@chromium.org,erikchen@chromium.org,skyostil@chromium.org,ssid@chromium.org,ortuno@chromium.org,eseckler@chromium.org

Change-Id: Ib19224f57b12b423dfbf40f51ebfcc946be061ad
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1136635
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2468503
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816764}

base/BUILD.gn

@@ -2143,7 +2143,6 @@ component("base") {
       "trace_event/optional_trace_event.h",
       "trace_event/process_memory_dump.cc",
       "trace_event/process_memory_dump.h",
-      "trace_event/task_execution_macros.h",
       "trace_event/thread_instruction_count.cc",
       "trace_event/thread_instruction_count.h",
       "trace_event/trace_arguments.cc",

base/trace_event/base_tracing.h

@@ -16,10 +16,8 @@
 // Update the check in //base/PRESUBMIT.py when adding new headers here.
 // TODO(crbug/1006541): Switch to perfetto for trace event implementation.
 #include "base/trace_event/blame_context.h"
-#include "base/trace_event/heap_profiler.h"
 #include "base/trace_event/memory_allocator_dump_guid.h"
 #include "base/trace_event/memory_dump_provider.h"
-#include "base/trace_event/task_execution_macros.h"
 #include "base/trace_event/trace_event.h"
 #include "base/trace_event/traced_value.h"
 #include "base/trace_event/typed_macros.h"

base/trace_event/common/trace_event_common.h

@@ -969,7 +969,6 @@
 #define TRACE_TASK_EXECUTION(run_function, task) \
   INTERNAL_TRACE_TASK_EXECUTION(run_function, task)
 
-// Special trace event macro to trace log messages.
 #define TRACE_LOG_MESSAGE(file, message, line) \
   INTERNAL_TRACE_LOG_MESSAGE(file, message, line)
 

base/trace_event/trace_event.h

@@ -22,6 +22,7 @@
 #include "base/time/time_override.h"
 #include "base/trace_event/builtin_categories.h"
 #include "base/trace_event/common/trace_event_common.h"
+#include "base/trace_event/heap_profiler.h"
 #include "base/trace_event/log_message.h"
 #include "base/trace_event/thread_instruction_count.h"
 #include "base/trace_event/trace_arguments.h"
@@ -389,6 +390,45 @@
     }                                                                \
   } while (0)
 
+#define INTERNAL_TRACE_LOG_MESSAGE(file, message, line)                        \
+  TRACE_EVENT_INSTANT1(                                                        \
+      "log", "LogMessage",                                                     \
+      TRACE_EVENT_FLAG_TYPED_PROTO_ARGS | TRACE_EVENT_SCOPE_THREAD, "message", \
+      std::make_unique<base::trace_event::LogMessage>(file, message, line))
+
+#if BUILDFLAG(ENABLE_LOCATION_SOURCE)
+
+// Implementation detail: internal macro to trace a task execution with the
+// location where it was posted from.
+//
+// This implementation is for when location sources are available.
+// TODO(ssid): The program counter of the current task should be added here.
+#define INTERNAL_TRACE_TASK_EXECUTION(run_function, task)                      \
+  INTERNAL_TRACE_EVENT_ADD_SCOPED_WITH_FLAGS(                                  \
+      "toplevel", run_function, TRACE_EVENT_FLAG_TYPED_PROTO_ARGS, "src_file", \
+      (task).posted_from.file_name(), "src_func",                              \
+      (task).posted_from.function_name());                                     \
+  TRACE_HEAP_PROFILER_API_SCOPED_TASK_EXECUTION INTERNAL_TRACE_EVENT_UID(      \
+      task_event)((task).posted_from.file_name());                             \
+  TRACE_HEAP_PROFILER_API_SCOPED_WITH_PROGRAM_COUNTER                          \
+  INTERNAL_TRACE_EVENT_UID(task_pc_event)((task).posted_from.program_counter());
+
+#else
+
+// TODO(http://crbug.com760702) remove file name and just pass the program
+// counter to the heap profiler macro.
+// TODO(ssid): The program counter of the current task should be added here.
+#define INTERNAL_TRACE_TASK_EXECUTION(run_function, task)                 \
+  INTERNAL_TRACE_EVENT_ADD_SCOPED_WITH_FLAGS(                             \
+      "toplevel", run_function, TRACE_EVENT_FLAG_TYPED_PROTO_ARGS, "src", \
+      (task).posted_from.ToString())                                      \
+  TRACE_HEAP_PROFILER_API_SCOPED_TASK_EXECUTION INTERNAL_TRACE_EVENT_UID( \
+      task_event)((task).posted_from.file_name());                        \
+  TRACE_HEAP_PROFILER_API_SCOPED_WITH_PROGRAM_COUNTER                     \
+  INTERNAL_TRACE_EVENT_UID(task_pc_event)((task).posted_from.program_counter());
+
+#endif
+
 namespace trace_event_internal {
 
 // Specify these values when the corresponding argument of AddTraceEvent is not

cc/raster/synchronous_task_graph_runner.cc

@@ -10,7 +10,6 @@
 #include <utility>
 
 #include "base/threading/simple_thread.h"
-#include "base/trace_event/heap_profiler.h"
 #include "base/trace_event/trace_event.h"
 
 namespace cc {

components/heap_profiling/multi_process/test_driver.cc

@@ -17,7 +17,6 @@
 #include "base/stl_util.h"
 #include "base/test/bind_test_util.h"
 #include "base/threading/platform_thread.h"
-#include "base/trace_event/heap_profiler.h"
 #include "base/trace_event/heap_profiler_event_filter.h"
 #include "base/values.h"
 #include "build/build_config.h"

components/services/heap_profiling/public/cpp/profiling_client.cc

@@ -14,7 +14,6 @@
 #include "base/sampling_heap_profiler/sampling_heap_profiler.h"
 #include "base/task/post_task.h"
 #include "base/task/thread_pool.h"
-#include "base/trace_event/heap_profiler_allocation_context_tracker.h"
 #include "base/trace_event/heap_profiler_event_filter.h"
 #include "base/trace_event/malloc_dump_provider.h"
 #include "base/trace_event/memory_dump_manager.h"

components/timers/alarm_timer_chromeos.cc

@@ -17,7 +17,6 @@
 #include "base/memory/ptr_util.h"
 #include "base/pending_task.h"
 #include "base/task/common/task_annotator.h"
-#include "base/trace_event/task_execution_macros.h"
 #include "base/trace_event/trace_event.h"
 
 namespace timers {

components/tracing/test/trace_event_perftest.cc

@@ -11,7 +11,6 @@
 #include "base/run_loop.h"
 #include "base/test/task_environment.h"
 #include "base/threading/thread.h"
-#include "base/trace_event/task_execution_macros.h"
 #include "base/trace_event/trace_event.h"
 #include "base/trace_event/traced_value.h"
 #include "perf_test_helpers.h"

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

@@ -26,7 +26,6 @@
 #include "base/threading/thread_id_name_manager.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
-#include "base/trace_event/task_execution_macros.h"
 #include "base/trace_event/thread_instruction_count.h"
 #include "base/trace_event/trace_event.h"
 #include "base/trace_event/trace_log.h"
@@ -1019,17 +1018,20 @@ TEST_F(TraceEventDataSourceTest, EventWithConvertableArgs) {
 TEST_F(TraceEventDataSourceTest, TaskExecutionEvent) {
   CreateTraceEventDataSource();
 
-  base::PendingTask task;
-  task.posted_from =
-      base::Location("my_func", "my_file", 0, /*program_counter=*/&task);
-  { TRACE_TASK_EXECUTION("ThreadControllerImpl::RunTask1", task); }
-  { TRACE_TASK_EXECUTION("ThreadControllerImpl::RunTask1", task); }
+  INTERNAL_TRACE_EVENT_ADD(
+      TRACE_EVENT_PHASE_INSTANT, "toplevel", "ThreadControllerImpl::RunTask",
+      TRACE_EVENT_SCOPE_THREAD | TRACE_EVENT_FLAG_TYPED_PROTO_ARGS, "src_file",
+      "my_file", "src_func", "my_func");
+  INTERNAL_TRACE_EVENT_ADD(
+      TRACE_EVENT_PHASE_INSTANT, "toplevel", "ThreadControllerImpl::RunTask",
+      TRACE_EVENT_SCOPE_THREAD | TRACE_EVENT_FLAG_TYPED_PROTO_ARGS, "src_file",
+      "my_file", "src_func", "my_func");
 
   size_t packet_index = ExpectStandardPreamble();
 
   auto* e_packet = producer_client()->GetFinalizedPacket(packet_index++);
   ExpectTraceEvent(e_packet, /*category_iid=*/1u, /*name_iid=*/1u,
-                   TRACE_EVENT_PHASE_BEGIN);
+                   TRACE_EVENT_PHASE_INSTANT, TRACE_EVENT_SCOPE_THREAD);
 
   const auto& annotations = e_packet->track_event().debug_annotations();
   EXPECT_EQ(annotations.size(), 0);
@@ -1041,9 +1043,9 @@ TEST_F(TraceEventDataSourceTest, TaskExecutionEvent) {
   EXPECT_EQ(locations[0].function_name(), "my_func");
 
   // Second event should refer to the same interning entries.
-  auto* e_packet2 = producer_client()->GetFinalizedPacket(++packet_index);
+  auto* e_packet2 = producer_client()->GetFinalizedPacket(packet_index++);
   ExpectTraceEvent(e_packet2, /*category_iid=*/1u, /*name_iid=*/1u,
-                   TRACE_EVENT_PHASE_BEGIN);
+                   TRACE_EVENT_PHASE_INSTANT, TRACE_EVENT_SCOPE_THREAD);
 
   EXPECT_EQ(e_packet2->track_event().task_execution().posted_from_iid(), 1u);
   EXPECT_EQ(e_packet2->interned_data().source_locations().size(), 0);
@@ -1052,16 +1054,16 @@ TEST_F(TraceEventDataSourceTest, TaskExecutionEvent) {
 TEST_F(TraceEventDataSourceTest, TaskExecutionEventWithoutFunction) {
   CreateTraceEventDataSource();
 
-  base::PendingTask task;
-  task.posted_from = base::Location(/*function_name=*/nullptr, "my_file", 0,
-                                    /*program_counter=*/&task);
-  { TRACE_TASK_EXECUTION("ThreadControllerImpl::RunTask", task); }
+  INTERNAL_TRACE_EVENT_ADD(
+      TRACE_EVENT_PHASE_INSTANT, "toplevel", "ThreadControllerImpl::RunTask",
+      TRACE_EVENT_SCOPE_THREAD | TRACE_EVENT_FLAG_TYPED_PROTO_ARGS, "src",
+      "my_file");
 
   size_t packet_index = ExpectStandardPreamble();
 
   auto* e_packet = producer_client()->GetFinalizedPacket(packet_index++);
   ExpectTraceEvent(e_packet, /*category_iid=*/1u, /*name_iid=*/1u,
-                   TRACE_EVENT_PHASE_BEGIN, TRACE_EVENT_SCOPE_THREAD);
+                   TRACE_EVENT_PHASE_INSTANT, TRACE_EVENT_SCOPE_THREAD);
 
   const auto& annotations = e_packet->track_event().debug_annotations();
   EXPECT_EQ(annotations.size(), 0);