[ukm] Some fixes to the metrics recording framework

Some fixes that were required to make the metric recording framework run
better:
  - Set the foreground task runner later so it can still be modified in
    test cases
  - Add Start and Stop methods to TimedScope for more control
  - Clear map of contexts explicitly to avoid it being triggered at the
    end of the destructor when counters are already destroyed and a
    SEGFAULT may occur due to histogram updates during destruction of
    the weak persistent handles.

R=rmcilroy@chromium.org

Bug: chromium:1101749
Change-Id: Ib41c7aeb1aac96f0fa102f0fceadbf7ec2dd78dc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2351668
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Emanuel Ziegler <ecmziegler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69422}

src/api/api.cc

@@ -8823,8 +8823,8 @@ void Isolate::SetAddHistogramSampleFunction(
 
 void Isolate::SetMetricsRecorder(
     const std::shared_ptr<metrics::Recorder>& metrics_recorder) {
-  reinterpret_cast<i::Isolate*>(this)->metrics_recorder()->SetRecorder(
-      metrics_recorder);
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
+  isolate->metrics_recorder()->SetRecorder(isolate, metrics_recorder);
 }
 
 void Isolate::SetAddCrashKeyCallback(AddCrashKeyCallback callback) {

src/execution/isolate.cc

@@ -2995,6 +2995,7 @@ void Isolate::Deinit() {
   }
 
   metrics_recorder_->NotifyIsolateDisposal();
+  recorder_context_id_map_.clear();
 
 #if defined(V8_OS_WIN64)
   if (win64_unwindinfo::CanRegisterUnwindInfoForNonABICompliantCodeRange() &&
@@ -3479,7 +3480,7 @@ bool Isolate::Init(ReadOnlyDeserializer* read_only_deserializer,
   // Enable logging before setting up the heap
   logger_->SetUp(this);
 
-  metrics_recorder_ = std::make_shared<metrics::Recorder>(this);
+  metrics_recorder_ = std::make_shared<metrics::Recorder>();
 
   {  // NOLINT
     // Ensure that the thread has a valid stack guard.  The v8::Locker object

src/logging/metrics.cc

@@ -31,13 +31,11 @@ class Recorder::Task : public v8::Task {
   std::shared_ptr<Recorder> recorder_;
 };
 
-Recorder::Recorder(Isolate* isolate)
-    : foreground_task_runner_(V8::GetCurrentPlatform()->GetForegroundTaskRunner(
-          reinterpret_cast<v8::Isolate*>(isolate))),
-      embedder_recorder_(nullptr) {}
-
 void Recorder::SetRecorder(
+    Isolate* isolate,
     const std::shared_ptr<v8::metrics::Recorder>& embedder_recorder) {
+  foreground_task_runner_ = V8::GetCurrentPlatform()->GetForegroundTaskRunner(
+      reinterpret_cast<v8::Isolate*>(isolate));
   CHECK_NULL(embedder_recorder_);
   embedder_recorder_ = embedder_recorder;
 }

src/logging/metrics.h

@@ -22,9 +22,8 @@ namespace metrics {
 
 class Recorder : public std::enable_shared_from_this<Recorder> {
  public:
-  explicit V8_EXPORT_PRIVATE Recorder(Isolate* isolate);
-
   V8_EXPORT_PRIVATE void SetRecorder(
+      Isolate* isolate,
       const std::shared_ptr<v8::metrics::Recorder>& embedder_recorder);
 
   V8_EXPORT_PRIVATE void NotifyIsolateDisposal();
@@ -86,11 +85,18 @@ template <class T, int64_t (base::TimeDelta::*precision)() const =
                        &base::TimeDelta::InMicroseconds>
 class TimedScope {
  public:
-  TimedScope(T* event, int64_t T::*time)
-      : event_(event), time_(time), start_time_(base::TimeTicks::Now()) {}
-  ~TimedScope() {
+  TimedScope(T* event, int64_t T::*time) : event_(event), time_(time) {
+    Start();
+  }
+  ~TimedScope() { Stop(); }
+
+  void Start() { start_time_ = base::TimeTicks::Now(); }
+
+  void Stop() {
+    if (start_time_.IsMin()) return;
     base::TimeDelta duration = base::TimeTicks::Now() - start_time_;
     event_->*time_ = (duration.*precision)();
+    start_time_ = base::TimeTicks::Min();
   }
 
  private: