Extract performance.measureMemory implementation to a new class

Currently performance.measureMemory invokes v8::Isolate::MeasureMemory
directly. This CL decouples that and prepares for the upcoming switch
to the implementation based on PerformanceManager.

The CL is pure refactoring without observable behavour change. It moves
the existing code from Performance::measureMemory and
MeasureMemoryDelegate into a new class called MeasureMemoryController.

The new class is responsible for starting the measurement and resolving
the JavaScript promise upon completion of the measurement.

As a drive-by change this removes the code of MeasureMemoryExperiment
that is no longer running.

Bug: 1085129
Change-Id: I83382405278f8d6013583cd0f6ad79d8e7fc8d8a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2418661
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808842}

third_party/blink/renderer/core/timing/BUILD.gn

@@ -18,6 +18,8 @@ blink_core_sources("timing") {
     "layout_shift.h",
     "layout_shift_attribution.cc",
     "layout_shift_attribution.h",
+    "measure_memory/measure_memory_controller.cc",
+    "measure_memory/measure_memory_controller.h",
     "measure_memory/measure_memory_delegate.cc",
     "measure_memory/measure_memory_delegate.h",
     "memory_info.cc",

third_party/blink/renderer/core/timing/measure_memory/measure_memory_controller.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "third_party/blink/renderer/core/timing/measure_memory/measure_memory_controller.h"
+
+#include "third_party/blink/renderer/core/timing/measure_memory/measure_memory_delegate.h"
+
+#include "third_party/blink/public/platform/platform.h"
+#include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
+#include "third_party/blink/renderer/bindings/core/v8/to_v8_for_core.h"
+#include "third_party/blink/renderer/bindings/core/v8/v8_measure_memory.h"
+#include "third_party/blink/renderer/bindings/core/v8/v8_measure_memory_breakdown.h"
+#include "third_party/blink/renderer/core/execution_context/execution_context.h"
+#include "third_party/blink/renderer/core/frame/local_dom_window.h"
+#include "third_party/blink/renderer/core/frame/local_frame.h"
+#include "third_party/blink/renderer/platform/bindings/exception_state.h"
+#include "third_party/blink/renderer/platform/bindings/script_state.h"
+#include "third_party/blink/renderer/platform/heap/heap.h"
+#include "third_party/blink/renderer/platform/heap/heap_allocator.h"
+#include "third_party/blink/renderer/platform/heap/member.h"
+#include "third_party/blink/renderer/platform/heap/persistent.h"
+#include "third_party/blink/renderer/platform/heap/trace_traits.h"
+#include "third_party/blink/renderer/platform/runtime_enabled_features.h"
+#include "third_party/blink/renderer/platform/wtf/functional.h"
+#include "v8/include/v8.h"
+
+namespace blink {
+
+MeasureMemoryController::MeasureMemoryController(
+    util::PassKey<MeasureMemoryController>,
+    v8::Isolate* isolate,
+    v8::Local<v8::Context> context,
+    v8::Local<v8::Promise::Resolver> promise_resolver)
+    : isolate_(isolate),
+      context_(isolate, context),
+      promise_resolver_(isolate, promise_resolver) {
+  context_.SetPhantom();
+  // TODO(ulan): Currently we keep a strong reference to the promise resolver.
+  // This may prolong the lifetime of the context by one more GC in the worst
+  // case as JSPromise keeps its context alive.
+  // To avoid that we should use an ephemeron context_ => promise_resolver_.
+}
+
+void MeasureMemoryController::Trace(Visitor* visitor) const {
+  visitor->Trace(promise_resolver_);
+}
+
+ScriptPromise MeasureMemoryController::StartMeasurement(
+    ScriptState* script_state,
+    ExceptionState& exception_state) {
+  if (!IsMeasureMemoryAvailable(LocalDOMWindow::From(script_state))) {
+    exception_state.ThrowSecurityError(
+        "performance.measureMemory is not available in this context");
+    return ScriptPromise();
+  }
+  v8::Isolate* isolate = script_state->GetIsolate();
+  v8::TryCatch try_catch(isolate);
+  v8::Local<v8::Context> context = script_state->GetContext();
+  v8::Local<v8::Promise::Resolver> promise_resolver;
+  if (!v8::Promise::Resolver::New(context).ToLocal(&promise_resolver)) {
+    exception_state.RethrowV8Exception(try_catch.Exception());
+    return ScriptPromise();
+  }
+  v8::MeasureMemoryExecution execution =
+      RuntimeEnabledFeatures::ForceEagerMeasureMemoryEnabled(
+          ExecutionContext::From(script_state))
+          ? v8::MeasureMemoryExecution::kEager
+          : v8::MeasureMemoryExecution::kDefault;
+
+  auto* impl = MakeGarbageCollected<MeasureMemoryController>(
+      util::PassKey<MeasureMemoryController>(), isolate, context,
+      promise_resolver);
+
+  isolate->MeasureMemory(
+      std::make_unique<MeasureMemoryDelegate>(
+          isolate, context,
+          WTF::Bind(&MeasureMemoryController::MeasurementComplete,
+                    WrapPersistent(impl))),
+      execution);
+  return ScriptPromise(script_state, promise_resolver->GetPromise());
+}
+
+bool MeasureMemoryController::IsMeasureMemoryAvailable(LocalDOMWindow* window) {
+  // TODO(ulan): We should check for window.crossOriginIsolated when it ships.
+  // Until then we enable the API only for processes locked to a site
+  // similar to the precise mode of the legacy performance.memory API.
+  if (!Platform::Current()->IsLockedToSite()) {
+    return false;
+  }
+  // The window.crossOriginIsolated will be true only for the top-level frame.
+  // Until the flag is available we check for the top-level condition manually.
+  if (!window) {
+    return false;
+  }
+  LocalFrame* local_frame = window->GetFrame();
+  if (!local_frame || !local_frame->IsMainFrame()) {
+    return false;
+  }
+  return true;
+}
+
+void MeasureMemoryController::MeasurementComplete(
+    HeapVector<Member<MeasureMemoryBreakdown>> breakdown) {
+  if (context_.IsEmpty()) {
+    // The context was garbage collected in the meantime.
+    return;
+  }
+  v8::Local<v8::Context> context = context_.NewLocal(isolate_);
+  MeasureMemory* result = MeasureMemory::Create();
+  size_t total_size = 0;
+  for (auto entry : breakdown) {
+    total_size += entry->bytes();
+  }
+  result->setBytes(total_size);
+  result->setBreakdown(breakdown);
+  v8::Local<v8::Promise::Resolver> promise_resolver =
+      promise_resolver_.NewLocal(isolate_);
+  promise_resolver->Resolve(context, ToV8(result, promise_resolver, isolate_))
+      .ToChecked();
+}
+
+}  // namespace blink

third_party/blink/renderer/core/timing/measure_memory/measure_memory_controller.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef THIRD_PARTY_BLINK_RENDERER_CORE_TIMING_MEASURE_MEMORY_MEASURE_MEMORY_CONTROLLER_H_
+#define THIRD_PARTY_BLINK_RENDERER_CORE_TIMING_MEASURE_MEMORY_MEASURE_MEMORY_CONTROLLER_H_
+
+#include "base/util/type_safety/pass_key.h"
+#include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
+#include "third_party/blink/renderer/platform/bindings/scoped_persistent.h"
+#include "v8/include/v8.h"
+
+namespace blink {
+
+class LocalDOMWindow;
+class MeasureMemoryBreakdown;
+class ScriptState;
+class ExceptionState;
+
+// The implementation of Performance.measureMemory() Web API.
+// It is responsible for:
+// 1. Starting an asynchronous memory measurement.
+// 2. Waiting for the measurement to complete.
+// 3. Constructing the result and resolving the JS promise.
+class MeasureMemoryController final
+    : public GarbageCollected<MeasureMemoryController> {
+ public:
+  // Private constructor guarded by PassKey. Use the StartMeasurement() wrapper
+  // to construct the object and start the measurement.
+  MeasureMemoryController(util::PassKey<MeasureMemoryController>,
+                          v8::Isolate*,
+                          v8::Local<v8::Context>,
+                          v8::Local<v8::Promise::Resolver>);
+
+  ~MeasureMemoryController() = default;
+
+  static ScriptPromise StartMeasurement(ScriptState*, ExceptionState&);
+
+  void Trace(Visitor* visitor) const;
+
+ private:
+  static bool IsMeasureMemoryAvailable(LocalDOMWindow* window);
+  void MeasurementComplete(HeapVector<Member<MeasureMemoryBreakdown>>);
+
+  v8::Isolate* isolate_;
+  ScopedPersistent<v8::Context> context_;
+  TraceWrapperV8Reference<v8::Promise::Resolver> promise_resolver_;
+};
+
+}  // namespace blink
+
+#endif  // THIRD_PARTY_BLINK_RENDERER_CORE_TIMING_MEASURE_MEMORY_MEASURE_MEMORY_CONTROLLER_H

third_party/blink/renderer/core/timing/measure_memory/measure_memory_delegate.cc

@@ -23,23 +23,12 @@
 
 namespace blink {
 
-MeasureMemoryDelegate::MeasureMemoryDelegate(
-    v8::Isolate* isolate,
-    v8::Local<v8::Context> context,
-    v8::Local<v8::Promise::Resolver> promise_resolver)
+MeasureMemoryDelegate::MeasureMemoryDelegate(v8::Isolate* isolate,
+                                             v8::Local<v8::Context> context,
+                                             ResultCallback callback)
     : isolate_(isolate),
       context_(isolate, context),
-      promise_resolver_(isolate, promise_resolver) {
-  context_.SetPhantom();
-  // TODO(ulan): Currently we keep a strong reference to the promise resolver.
-  // This may prolong the lifetime of the context by one more GC in the worst
-  // case as JSPromise keeps its context alive.
-  // To avoid that we should store the promise resolver in V8PerContextData.
-}
-
-MeasureMemoryDelegate::MeasureMemoryDelegate(v8::Isolate* isolate,
-                                             v8::Local<v8::Context> context)
-    : isolate_(isolate), context_(isolate, context) {
+      callback_(std::move(callback)) {
   context_.SetPhantom();
 }
 
@@ -258,8 +247,6 @@ void MeasureMemoryDelegate::MeasurementComplete(
   for (const auto& context_size : context_sizes) {
     total_size += context_size.second;
   }
-  MeasureMemory* result = MeasureMemory::Create();
-  result->setBytes(total_size + unattributed_size);
   HeapVector<Member<MeasureMemoryBreakdown>> breakdown;
   size_t detached_size;
   size_t unknown_frame_size;
@@ -293,32 +280,7 @@ void MeasureMemoryDelegate::MeasurementComplete(
     breakdown.push_back(CreateMeasureMemoryBreakdown(
         unknown_frame_size, Vector<String>{kWindow, kJS}, ""));
   }
-  result->setBreakdown(breakdown);
-  if (!promise_resolver_.IsEmpty()) {
-    v8::Local<v8::Promise::Resolver> promise_resolver =
-        promise_resolver_.NewLocal(isolate_);
-    promise_resolver->Resolve(context, ToV8(result, promise_resolver, isolate_))
-        .ToChecked();
-  }
-}
-
-bool MeasureMemoryDelegate::IsMeasureMemoryAvailable(LocalDOMWindow* window) {
-  // TODO(ulan): We should check for window.crossOriginIsolated when it ships.
-  // Until then we enable the API only for processes locked to a site
-  // similar to the precise mode of the legacy performance.memory API.
-  if (!Platform::Current()->IsLockedToSite()) {
-    return false;
-  }
-  // The window.crossOriginIsolated will be true only for the top-level frame.
-  // Until the flag is available we check for the top-level condition manually.
-  if (!window) {
-    return false;
-  }
-  LocalFrame* local_frame = window->GetFrame();
-  if (!local_frame || !local_frame->IsMainFrame()) {
-    return false;
-  }
-  return true;
+  std::move(callback_).Run(breakdown);
 }
 
 }  // namespace blink

third_party/blink/renderer/core/timing/measure_memory/measure_memory_delegate.h

@@ -6,24 +6,24 @@
 #define THIRD_PARTY_BLINK_RENDERER_CORE_TIMING_MEASURE_MEMORY_MEASURE_MEMORY_DELEGATE_H_
 
 #include "third_party/blink/renderer/platform/bindings/scoped_persistent.h"
-#include "third_party/blink/renderer/platform/wtf/allocator/allocator.h"
+#include "third_party/blink/renderer/platform/heap/heap_allocator.h"
+#include "third_party/blink/renderer/platform/heap/member.h"
 #include "v8/include/v8.h"
 
 namespace blink {
 
-class LocalDOMWindow;
+class MeasureMemoryBreakdown;
 
-// Specifies V8 contexts to be measured and resolves the promise once V8
+// Specifies V8 contexts to be measured and invokes the given callback once V8
 // completes the memory measurement.
 class MeasureMemoryDelegate : public v8::MeasureMemoryDelegate {
  public:
+  using ResultCallback =
+      base::OnceCallback<void(HeapVector<Member<MeasureMemoryBreakdown>>)>;
+
   MeasureMemoryDelegate(v8::Isolate* isolate,
                         v8::Local<v8::Context> context,
-                        v8::Local<v8::Promise::Resolver> promise_resolver);
-
-  // Constructor for a Finch experiment. It does not resolve any promise and
-  // will be removed after the experiment.
-  MeasureMemoryDelegate(v8::Isolate* isolate, v8::Local<v8::Context> context);
+                        ResultCallback callback);
 
   // v8::MeasureMemoryDelegate overrides.
   bool ShouldMeasure(v8::Local<v8::Context> context) override;
@@ -31,12 +31,10 @@ class MeasureMemoryDelegate : public v8::MeasureMemoryDelegate {
       const std::vector<std::pair<v8::Local<v8::Context>, size_t>>&
           context_sizes,
       size_t unattributed_size) override;
-  static bool IsMeasureMemoryAvailable(LocalDOMWindow* window);
-
  private:
   v8::Isolate* isolate_;
   ScopedPersistent<v8::Context> context_;
-  ScopedPersistent<v8::Promise::Resolver> promise_resolver_;
+  ResultCallback callback_;
 };
 
 }  // namespace blink

third_party/blink/renderer/core/timing/performance.cc

@@ -57,7 +57,7 @@
 #include "third_party/blink/renderer/core/loader/document_loader.h"
 #include "third_party/blink/renderer/core/timing/largest_contentful_paint.h"
 #include "third_party/blink/renderer/core/timing/layout_shift.h"
-#include "third_party/blink/renderer/core/timing/measure_memory/measure_memory_delegate.h"
+#include "third_party/blink/renderer/core/timing/measure_memory/measure_memory_controller.h"
 #include "third_party/blink/renderer/core/timing/performance_element_timing.h"
 #include "third_party/blink/renderer/core/timing/performance_event_timing.h"
 #include "third_party/blink/renderer/core/timing/performance_long_task_timing.h"
@@ -153,30 +153,8 @@ EventCounts* Performance::eventCounts() {
 ScriptPromise Performance::measureMemory(
     ScriptState* script_state,
     ExceptionState& exception_state) const {
-  if (!MeasureMemoryDelegate::IsMeasureMemoryAvailable(
-          LocalDOMWindow::From(script_state))) {
-    exception_state.ThrowSecurityError(
-        "performance.measureMemory is not available in this context");
-    return ScriptPromise();
-  }
-  v8::Isolate* isolate = script_state->GetIsolate();
-  v8::TryCatch try_catch(isolate);
-  v8::Local<v8::Context> context = script_state->GetContext();
-  v8::Local<v8::Promise::Resolver> promise_resolver;
-  if (!v8::Promise::Resolver::New(context).ToLocal(&promise_resolver)) {
-    exception_state.RethrowV8Exception(try_catch.Exception());
-    return ScriptPromise();
-  }
-  v8::MeasureMemoryExecution execution =
-      RuntimeEnabledFeatures::ForceEagerMeasureMemoryEnabled(
-          ExecutionContext::From(script_state))
-          ? v8::MeasureMemoryExecution::kEager
-          : v8::MeasureMemoryExecution::kDefault;
-
-  isolate->MeasureMemory(std::make_unique<MeasureMemoryDelegate>(
-                             isolate, context, promise_resolver),
-                         execution);
-  return ScriptPromise(script_state, promise_resolver->GetPromise());
+  return MeasureMemoryController::StartMeasurement(script_state,
+                                                   exception_state);
 }
 
 DOMHighResTimeStamp Performance::timeOrigin() const {

third_party/blink/renderer/core/timing/window_performance.cc

@@ -49,7 +49,6 @@
 #include "third_party/blink/renderer/core/page/page_hidden_state.h"
 #include "third_party/blink/renderer/core/timing/largest_contentful_paint.h"
 #include "third_party/blink/renderer/core/timing/layout_shift.h"
-#include "third_party/blink/renderer/core/timing/measure_memory/measure_memory_delegate.h"
 #include "third_party/blink/renderer/core/timing/performance_element_timing.h"
 #include "third_party/blink/renderer/core/timing/performance_event_timing.h"
 #include "third_party/blink/renderer/core/timing/performance_observer.h"
@@ -154,21 +153,11 @@ WindowPerformance::WindowPerformance(LocalDOMWindow* window)
     : Performance(ToTimeOrigin(window),
                   window->GetTaskRunner(TaskType::kPerformanceTimeline)),
       ExecutionContextClient(window),
-      PageVisibilityObserver(GetFrame()->GetPage()),
-      measure_memory_experiment_timer_(
-          task_runner_,
-          this,
-          &WindowPerformance::MeasureMemoryExperimentTimerFired) {
+      PageVisibilityObserver(GetFrame()->GetPage()) {
   DCHECK(GetFrame());
   DCHECK(GetFrame()->GetPerformanceMonitor());
   GetFrame()->GetPerformanceMonitor()->Subscribe(
       PerformanceMonitor::kLongTask, kLongTaskObserverThreshold, this);
-  if (MeasureMemoryDelegate::IsMeasureMemoryAvailable(window) &&
-      base::FeatureList::IsEnabled(blink::features::kMeasureMemoryExperiment)) {
-    int delay_in_ms = base::RandInt(0, kMaxMeasureMemoryExperimentDelayInMs);
-    measure_memory_experiment_timer_.StartOneShot(
-        base::TimeDelta::FromMilliseconds(delay_in_ms), FROM_HERE);
-  }
   if (RuntimeEnabledFeatures::VisibilityStateEntryEnabled()) {
     DCHECK(GetPage());
     AddVisibilityStateEntry(GetPage()->IsPageVisible(), base::TimeTicks());
@@ -535,20 +524,4 @@ void WindowPerformance::OnPaintFinished() {
   ++frame_index_;
 }
 
-void WindowPerformance::MeasureMemoryExperimentTimerFired(TimerBase*) {
-  if (!GetFrame() || !GetExecutionContext())
-    return;
-  v8::Isolate* isolate = GetExecutionContext()->GetIsolate();
-  v8::HandleScope handle_scope(isolate);
-  v8::Local<v8::Context> context =
-      ToV8Context(GetFrame(), DOMWrapperWorld::MainWorld());
-  if (context.IsEmpty()) {
-    // The frame has been detached in the meantime.
-    return;
-  }
-  isolate->MeasureMemory(
-      std::make_unique<MeasureMemoryDelegate>(isolate, context),
-      v8::MeasureMemoryExecution::kDefault);
-}
-
 }  // namespace blink

third_party/blink/renderer/core/timing/window_performance.h

@@ -131,8 +131,6 @@ class CORE_EXPORT WindowPerformance final : public Performance,
 
   void DispatchFirstInputTiming(PerformanceEventTiming* entry);
 
-  void MeasureMemoryExperimentTimerFired(TimerBase*);
-
   // Counter of the current frame index, based on calls to OnPaintFinished().
   uint64_t frame_index_ = 1;
   // Monotonically increasing value with the last frame index on which a swap
@@ -157,11 +155,6 @@ class CORE_EXPORT WindowPerformance final : public Performance,
   Member<EventCounts> event_counts_;
   mutable Member<PerformanceNavigation> navigation_;
   mutable Member<PerformanceTiming> timing_;
-
-  // This is used in a Finch experiment to perform a memory measurement without
-  // reporting the results to evaluate its impact on stability and performance.
-  TaskRunnerTimer<WindowPerformance> measure_memory_experiment_timer_;
-  static const int kMaxMeasureMemoryExperimentDelayInMs = 30000;
 };
 
 }  // namespace blink