Revert "perfetto: always use suspend aware clock for service snapshots"

This reverts commit 1fa06ac.

Reason for revert: causing crashes in Chromium

Change-Id: I3126c7ffa85f6be2822bfd77cc0089f7d627d810

protos/perfetto/config/perfetto_config.proto

@@ -3990,12 +3990,20 @@ message TraceConfig {
     // ReadBuffers() is called.
     optional uint32 snapshot_interval_ms = 6;
 
-    // Formerly controlled whether a suspend-aware clock was used (if available)
-    // for periodic snapshots of service-emitted events.
+    // Hints to the service that a suspend-aware (i.e. counting time in suspend)
+    // clock should be used for periodic snapshots of service-emitted events.
+    // This means, if a snapshot *should* have happened during suspend, it will
+    // happen immediately after the device resumes.
     //
-    // Introduced in Android S. Ignored in B+ (25Q2+) with the default being
-    // set to true.
-    optional bool prefer_suspend_clock_for_snapshot = 7 [deprecated = true];
+    // Choosing a clock like this is done on best-effort basis; not all
+    // platforms (e.g. Windows) expose a clock which can be used for periodic
+    // tasks counting suspend. If such a clock is not available, the service
+    // falls back to the best-available alternative.
+    //
+    // Introduced in Android S.
+    // TODO(lalitm): deprecate this in T and make this the default if nothing
+    // crashes in S.
+    optional bool prefer_suspend_clock_for_snapshot = 7;
 
     // Disables the reporting of per-trace-writer histograms in TraceStats.
     optional bool disable_chunk_usage_histograms = 8;

protos/perfetto/config/trace_config.proto

@@ -119,12 +119,20 @@ message TraceConfig {
     // ReadBuffers() is called.
     optional uint32 snapshot_interval_ms = 6;
 
-    // Formerly controlled whether a suspend-aware clock was used (if available)
-    // for periodic snapshots of service-emitted events.
+    // Hints to the service that a suspend-aware (i.e. counting time in suspend)
+    // clock should be used for periodic snapshots of service-emitted events.
+    // This means, if a snapshot *should* have happened during suspend, it will
+    // happen immediately after the device resumes.
     //
-    // Introduced in Android S. Ignored in B+ (25Q2+) with the default being
-    // set to true.
-    optional bool prefer_suspend_clock_for_snapshot = 7 [deprecated = true];
+    // Choosing a clock like this is done on best-effort basis; not all
+    // platforms (e.g. Windows) expose a clock which can be used for periodic
+    // tasks counting suspend. If such a clock is not available, the service
+    // falls back to the best-available alternative.
+    //
+    // Introduced in Android S.
+    // TODO(lalitm): deprecate this in T and make this the default if nothing
+    // crashes in S.
+    optional bool prefer_suspend_clock_for_snapshot = 7;
 
     // Disables the reporting of per-trace-writer histograms in TraceStats.
     optional bool disable_chunk_usage_histograms = 8;

protos/perfetto/trace/perfetto_trace.proto

@@ -3990,12 +3990,20 @@ message TraceConfig {
     // ReadBuffers() is called.
     optional uint32 snapshot_interval_ms = 6;
 
-    // Formerly controlled whether a suspend-aware clock was used (if available)
-    // for periodic snapshots of service-emitted events.
+    // Hints to the service that a suspend-aware (i.e. counting time in suspend)
+    // clock should be used for periodic snapshots of service-emitted events.
+    // This means, if a snapshot *should* have happened during suspend, it will
+    // happen immediately after the device resumes.
     //
-    // Introduced in Android S. Ignored in B+ (25Q2+) with the default being
-    // set to true.
-    optional bool prefer_suspend_clock_for_snapshot = 7 [deprecated = true];
+    // Choosing a clock like this is done on best-effort basis; not all
+    // platforms (e.g. Windows) expose a clock which can be used for periodic
+    // tasks counting suspend. If such a clock is not available, the service
+    // falls back to the best-available alternative.
+    //
+    // Introduced in Android S.
+    // TODO(lalitm): deprecate this in T and make this the default if nothing
+    // crashes in S.
+    optional bool prefer_suspend_clock_for_snapshot = 7;
 
     // Disables the reporting of per-trace-writer histograms in TraceStats.
     optional bool disable_chunk_usage_histograms = 8;

src/tracing/service/tracing_service_impl.cc

@@ -1303,7 +1303,9 @@ void TracingServiceImpl::StartTracing(TracingSessionID tsid) {
   //      events.
   base::PeriodicTask::Args snapshot_task_args;
   snapshot_task_args.start_first_task_immediately = true;
-  snapshot_task_args.use_suspend_aware_timer = true;
+  snapshot_task_args.use_suspend_aware_timer =
+      tracing_session->config.builtin_data_sources()
+          .prefer_suspend_clock_for_snapshot();
   snapshot_task_args.task = [this, tsid] { PeriodicSnapshotTask(tsid); };
   snapshot_task_args.period_ms =
       tracing_session->config.builtin_data_sources().snapshot_interval_ms();

src/tracing/service/tracing_service_impl_unittest.cc

@@ -2788,11 +2788,8 @@ TEST_F(TracingServiceImplTest, ResynchronizeTraceStreamUsingSyncMarker) {
     writer->NewTracePacket()->set_for_testing()->set_str(payload.c_str());
     if (i % (100 / kNumMarkers) == 0) {
       writer->Flush();
-      auto checkpoint_name =
-          "wait_snapshot_" + std::to_string(i / (100 / kNumMarkers));
-      auto timer_expired = task_runner.CreateCheckpoint(checkpoint_name);
-      task_runner.PostDelayedTask([timer_expired] { timer_expired(); }, 200);
-      task_runner.RunUntilCheckpoint(checkpoint_name);
+      // The snapshot will happen every 100ms
+      AdvanceTimeAndRunUntilIdle(100);
     }
   }
   writer->Flush();