Revert "[segmentation_platform] Fix nesting of model execution trace …

…events"

This reverts commit 5a0906c.

Reason for revert: Speculative revert for crbug.com/1401661

Original change's description:
> [segmentation_platform] Fix nesting of model execution trace events
>
> Currently the ModelExecutionImpl::ExecuteModel trace event would finish
> after its parent ModelExecutionImpl::ExecutionState event. The parent
> is closed within RunModelExecutionCallback, whereas the child is closed
> when ModelExecutionImpl::ExecuteModel ends since it is on the stack.
>
> This could lead to confusing timestamps in the trace log.
>
> This CL changes the implementation of this to ensure that the order of
> closing the events is correct. In addition, it makes a change to now
> store the perfetto::Track instead of looking it up when the trace event
> goes out of scope, which simplifies readability.
>
> Bug: 1393919
> Change-Id: Ic4e331559718b069f606de387abe3ee8fbe1bdf1
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4098382
> Reviewed-by: Eric Seckler <eseckler@chromium.org>
> Reviewed-by: Siddhartha S <ssid@chromium.org>
> Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1084043}

Bug: 1393919
Change-Id: I2092ada77fc6d794d3025acc36b25041438be0a7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4114083
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Commit-Queue: Justin Cohen <justincohen@chromium.org>
Reviewed-by: Christian Xu <christianxu@chromium.org>
Owners-Override: Christian Xu <christianxu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1084319}

components/segmentation_platform/internal/execution/model_executor_impl.cc

@@ -30,8 +30,8 @@ struct ModelExecutorImpl::ModelExecutionTraceEvent {
                            const ModelExecutorImpl::ExecutionState& state);
   ~ModelExecutionTraceEvent();
 
-  // Which track to add this trace event to.
-  perfetto::Track track;
+  const raw_ref<const ModelExecutorImpl::ExecutionState, DanglingUntriaged>
+      state;
 };
 
 struct ModelExecutorImpl::ExecutionState {
@@ -71,13 +71,14 @@ struct ModelExecutorImpl::ExecutionState {
 ModelExecutorImpl::ModelExecutionTraceEvent::ModelExecutionTraceEvent(
     const char* event_name,
     const ModelExecutorImpl::ExecutionState& state)
-    : track(perfetto::Track::FromPointer(&state)) {
+    : state(state) {
   TRACE_EVENT_BEGIN("segmentation_platform", perfetto::StaticString(event_name),
-                    track);
+                    perfetto::Track::FromPointer(&state));
 }
 
 ModelExecutorImpl::ModelExecutionTraceEvent::~ModelExecutionTraceEvent() {
-  TRACE_EVENT_END("segmentation_platform", track);
+  TRACE_EVENT_END("segmentation_platform",
+                  perfetto::Track::FromPointer(&*state));
 }
 
 ModelExecutorImpl::ModelExecutorImpl(
@@ -108,7 +109,7 @@ void ModelExecutorImpl::ExecuteModel(
                                        *state);
 
   if (!state->model_provider || !state->model_provider->ModelAvailable()) {
-    RunModelExecutionCallback(*state, std::move(state->callback),
+    RunModelExecutionCallback(std::move(state),
                               std::make_unique<ModelExecutionResult>(
                                   ModelExecutionStatus::kSkippedModelNotReady));
     return;
@@ -118,9 +119,8 @@ void ModelExecutorImpl::ExecuteModel(
   if (metadata_utils::ValidateSegmentInfo(segment_info) !=
       metadata_utils::ValidationResult::kValidationSuccess) {
     RunModelExecutionCallback(
-        *state, std::move(state->callback),
-        std::make_unique<ModelExecutionResult>(
-            ModelExecutionStatus::kSkippedInvalidMetadata));
+        std::move(state), std::make_unique<ModelExecutionResult>(
+                              ModelExecutionStatus::kSkippedInvalidMetadata));
     return;
   }
 
@@ -141,9 +141,8 @@ void ModelExecutorImpl::OnProcessingFeatureListComplete(
   if (error) {
     // Validation error occurred on model's metadata.
     RunModelExecutionCallback(
-        *state, std::move(state->callback),
-        std::make_unique<ModelExecutionResult>(
-            ModelExecutionStatus::kSkippedInvalidMetadata));
+        std::move(state), std::make_unique<ModelExecutionResult>(
+                              ModelExecutionStatus::kSkippedInvalidMetadata));
     return;
   }
   state->input_tensor.insert(state->input_tensor.end(), input_tensor.begin(),
@@ -206,29 +205,28 @@ void ModelExecutorImpl::OnModelExecutionComplete(
       }
     }
     ModelProvider::Request input_tensor = state->input_tensor;
-    RunModelExecutionCallback(*state, std::move(state->callback),
+    RunModelExecutionCallback(std::move(state),
                               std::make_unique<ModelExecutionResult>(
                                   std::move(input_tensor), *result));
   } else {
     VLOG(1) << "Segmentation model returned no result for segment "
             << proto::SegmentId_Name(state->segment_info.segment_id());
-    RunModelExecutionCallback(*state, std::move(state->callback),
+    RunModelExecutionCallback(std::move(state),
                               std::make_unique<ModelExecutionResult>(
                                   ModelExecutionStatus::kExecutionError));
   }
 }
 
 void ModelExecutorImpl::RunModelExecutionCallback(
-    const ExecutionState& state,
-    ModelExecutionCallback callback,
+    std::unique_ptr<ExecutionState> state,
     std::unique_ptr<ModelExecutionResult> result) {
   stats::RecordModelExecutionDurationTotal(
-      state.segment_info.segment_id(), result->status,
-      clock_->Now() - state.total_execution_start_time);
-  stats::RecordModelExecutionStatus(state.segment_info.segment_id(),
-                                    state.record_metrics_for_default,
+      state->segment_info.segment_id(), result->status,
+      clock_->Now() - state->total_execution_start_time);
+  stats::RecordModelExecutionStatus(state->segment_info.segment_id(),
+                                    state->record_metrics_for_default,
                                     result->status);
-  std::move(callback).Run(std::move(result));
+  std::move(state->callback).Run(std::move(result));
 }
 
 }  // namespace segmentation_platform

components/segmentation_platform/internal/execution/model_executor_impl.h

@@ -67,11 +67,8 @@ class ModelExecutorImpl : public ModelExecutor {
       const absl::optional<ModelProvider::Response>& result);
 
   // Helper function for synchronously invoking the callback with the given
-  // result and status. Before invoking this, it is required to move the
-  // ExecutionState::callback out as a separate parameter, e.g.:
-  // `RunModelExecutionCallback(*state, std::move(state->callback), ...)`.
-  void RunModelExecutionCallback(const ExecutionState& state,
-                                 ModelExecutionCallback callback,
+  // result and status.
+  void RunModelExecutionCallback(std::unique_ptr<ExecutionState> state,
                                  std::unique_ptr<ModelExecutionResult> result);
 
   const raw_ptr<base::Clock> clock_;

components/segmentation_platform/internal/execution/model_executor_impl_unittest.cc

@@ -34,7 +34,6 @@
 #include "components/segmentation_platform/public/proto/types.pb.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
-#include "third_party/perfetto/include/perfetto/tracing/track.h"
 
 using ::base::test::RunOnceCallback;
 using segmentation_platform::processing::FeatureListQueryProcessor;
@@ -55,10 +54,6 @@ class ModelExecutorTest : public testing::Test {
   ~ModelExecutorTest() override = default;
 
   void SetUp() override {
-    // TODO(khokhlov): Replace with a test environment for tracing after the
-    // SDK migration
-    perfetto::internal::TrackRegistry::InitializeInstance();
-
     signal_database_ = std::make_unique<MockSignalDatabase>();
     clock_.SetNow(base::Time::Now());
   }

components/segmentation_platform/internal/segmentation_platform_service_impl_unittest.cc

@@ -30,7 +30,6 @@
 #include "components/ukm/test_ukm_recorder.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
-#include "third_party/perfetto/include/perfetto/tracing/track.h"
 
 using base::test::RunOnceCallback;
 using ::testing::_;
@@ -84,10 +83,6 @@ class SegmentationPlatformServiceImplTest
     base::SetRecordActionTaskRunner(
         task_environment_.GetMainThreadTaskRunner());
 
-    // TODO(khokhlov): Replace with a test environment for tracing after the
-    // SDK migration
-    perfetto::internal::TrackRegistry::InitializeInstance();
-
     // Setup model provider data for default model for supporting on demand
     // execution.
     model_provider_data_.segments_supporting_default_model = {

components/segmentation_platform/internal/selection/segment_result_provider_unittest.cc

@@ -24,7 +24,6 @@
 #include "components/segmentation_platform/public/segmentation_platform_service.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
-#include "third_party/perfetto/include/perfetto/tracing/track.h"
 
 namespace segmentation_platform {
 namespace {
@@ -82,10 +81,6 @@ class SegmentResultProviderTest : public testing::Test {
   ~SegmentResultProviderTest() override = default;
 
   void SetUp() override {
-    // TODO(khokhlov): Replace with a test environment for tracing after the
-    // SDK migration
-    perfetto::internal::TrackRegistry::InitializeInstance();
-
     default_manager_ = std::make_unique<DefaultModelManager>(
         &provider_factory_,
         std::vector<SegmentId>({kTestSegment, kTestSegment2}));