[PM] Add a one-shot equivalent to V8PerFrameMemoryRequest

This object will create a memory request for a single renderer process
and send it immediately. The renderer will perform the request at the
next GC as specified by the measurement mode.

This implementation actually sends memory requests to all renderers and
ignores responses from the unininteresting ones. A followup will only
send the request to the given renderer process.

Also updates ExpectBindAndRespondToQuery to test the process ID with
gmock matchers instead of DCHECK, so that it can be called multiple
times with different process ID's in the same test.

R=chrisha, ulan

Bug: 1080672
Change-Id: Icba4b66f60bd32b86ea7786441aa84559395b570
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2430063
Commit-Queue: Joe Mason <joenotcharles@chromium.org>
Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813872}

components/performance_manager/public/v8_memory/v8_per_frame_memory_decorator.h

@@ -238,7 +238,9 @@ namespace v8_memory {
 //     SEQUENCE_CHECKER(sequence_checker_);
 //   };
 
+class V8DetailedMemoryRequestOneShot;
 class V8PerFrameMemoryObserver;
+class V8PerFrameMemoryProcessData;
 class V8PerFrameMemoryRequest;
 class V8PerFrameMemoryRequestAnySeq;
 
@@ -314,6 +316,80 @@ class V8PerFrameMemoryDecorator
   SEQUENCE_CHECKER(sequence_checker_);
 };
 
+//////////////////////////////////////////////////////////////////////////////
+// The following classes report results from memory measurements.
+
+class V8PerFrameMemoryFrameData {
+ public:
+  V8PerFrameMemoryFrameData() = default;
+  virtual ~V8PerFrameMemoryFrameData() = default;
+
+  bool operator==(const V8PerFrameMemoryFrameData& other) const {
+    return v8_bytes_used_ == other.v8_bytes_used_;
+  }
+
+  // Returns the number of bytes used by V8 for this frame at the last
+  // measurement.
+  uint64_t v8_bytes_used() const { return v8_bytes_used_; }
+
+  void set_v8_bytes_used(uint64_t v8_bytes_used) {
+    v8_bytes_used_ = v8_bytes_used;
+  }
+
+  // Returns frame data for the given node, or nullptr if no measurement has
+  // been taken. The returned pointer must only be accessed on the graph
+  // sequence and may go invalid at any time after leaving the calling scope.
+  static const V8PerFrameMemoryFrameData* ForFrameNode(const FrameNode* node);
+
+ private:
+  uint64_t v8_bytes_used_ = 0;
+};
+
+class V8PerFrameMemoryProcessData {
+ public:
+  V8PerFrameMemoryProcessData() = default;
+  virtual ~V8PerFrameMemoryProcessData() = default;
+
+  bool operator==(const V8PerFrameMemoryProcessData& other) const {
+    return unassociated_v8_bytes_used_ == other.unassociated_v8_bytes_used_;
+  }
+
+  // Returns the number of bytes used by V8 at the last measurement in this
+  // process that could not be attributed to a frame.
+  uint64_t unassociated_v8_bytes_used() const {
+    return unassociated_v8_bytes_used_;
+  }
+
+  void set_unassociated_v8_bytes_used(uint64_t unassociated_v8_bytes_used) {
+    unassociated_v8_bytes_used_ = unassociated_v8_bytes_used;
+  }
+
+  // Returns process data for the given node, or nullptr if no measurement has
+  // been taken. The returned pointer must only be accessed on the graph
+  // sequence and may go invalid at any time after leaving the calling scope.
+  static const V8PerFrameMemoryProcessData* ForProcessNode(
+      const ProcessNode* node);
+
+ private:
+  uint64_t unassociated_v8_bytes_used_ = 0;
+};
+
+class V8PerFrameMemoryObserver : public base::CheckedObserver {
+ public:
+  // Called on the PM sequence when a measurement is available for
+  // |process_node|. |process_data| contains the process-level measurements for
+  // the process, and can go invalid at any time after returning from this
+  // method. Per-frame measurements can be read by walking the graph from
+  // |process_node| to find frame nodes, and calling
+  // V8PerFrameMemoryFrameData::ForFrameNode to retrieve the measurement data.
+  virtual void OnV8MemoryMeasurementAvailable(
+      const ProcessNode* process_node,
+      const V8PerFrameMemoryProcessData* process_data) = 0;
+};
+
+//////////////////////////////////////////////////////////////////////////////
+// The following classes create requests for memory measurements.
+
 class V8PerFrameMemoryRequest {
  public:
   enum class MeasurementMode {
@@ -394,6 +470,12 @@ class V8PerFrameMemoryRequest {
       MeasurementMode mode,
       base::WeakPtr<V8PerFrameMemoryRequestAnySeq> off_sequence_request);
 
+  // Private constructor for V8DetailedMemoryRequestOneShot. Sets
+  // min_time_between_requests_ to 0, which is not allowed for repeating
+  // requests.
+  V8PerFrameMemoryRequest(util::PassKey<V8DetailedMemoryRequestOneShot>,
+                          MeasurementMode mode);
+
   // V8PerFrameMemoryDecorator::MeasurementRequestQueue calls
   // OnOwnerUnregistered for all requests in the queue when the owning
   // decorator or process node is removed from the graph.
@@ -424,73 +506,60 @@ class V8PerFrameMemoryRequest {
   SEQUENCE_CHECKER(sequence_checker_);
 };
 
-class V8PerFrameMemoryFrameData {
+// TODO(joenotcharles): Rename the other V8PerFrameMemory* classes to
+// V8DetailedMemory*.
+class V8DetailedMemoryRequestOneShot : public V8PerFrameMemoryObserver {
  public:
-  V8PerFrameMemoryFrameData() = default;
-  virtual ~V8PerFrameMemoryFrameData() = default;
+  // A callback that will be passed the results of the measurement. |process|
+  // will always match the value passed to the V8DetailedMemoryRequestOneShot
+  // constructor.
+  using MeasurementCallback =
+      base::OnceCallback<void(const ProcessNode* process,
+                              const V8PerFrameMemoryProcessData* process_data)>;
 
-  bool operator==(const V8PerFrameMemoryFrameData& other) const {
-    return v8_bytes_used_ == other.v8_bytes_used_;
-  }
+  using MeasurementMode = V8PerFrameMemoryRequest::MeasurementMode;
 
-  // Returns the number of bytes used by V8 for this frame at the last
-  // measurement.
-  uint64_t v8_bytes_used() const { return v8_bytes_used_; }
+  // Creates a one-shot memory measurement request that will immediately be
+  // sent to |process| (which must be a renderer process). The process will
+  // perform the measurement during a GC as determined by |mode|, and
+  // |callback| will be called with the results.
+  V8DetailedMemoryRequestOneShot(
+      const ProcessNode* process,
+      MeasurementCallback callback,
+      MeasurementMode mode = MeasurementMode::kDefault);
 
-  void set_v8_bytes_used(uint64_t v8_bytes_used) {
-    v8_bytes_used_ = v8_bytes_used;
-  }
+  ~V8DetailedMemoryRequestOneShot() final;
 
-  // Returns frame data for the given node, or nullptr if no measurement has
-  // been taken. The returned pointer must only be accessed on the graph
-  // sequence and may go invalid at any time after leaving the calling scope.
-  static const V8PerFrameMemoryFrameData* ForFrameNode(const FrameNode* node);
+  V8DetailedMemoryRequestOneShot(const V8DetailedMemoryRequestOneShot&) =
+      delete;
+  V8DetailedMemoryRequestOneShot& operator=(
+      const V8DetailedMemoryRequestOneShot&) = delete;
 
- private:
-  uint64_t v8_bytes_used_ = 0;
-};
+  MeasurementMode mode() const { return mode_; }
 
-class V8PerFrameMemoryProcessData {
- public:
-  V8PerFrameMemoryProcessData() = default;
-  virtual ~V8PerFrameMemoryProcessData() = default;
+  // V8PerFrameMemoryObserver implementation.
 
-  bool operator==(const V8PerFrameMemoryProcessData& other) const {
-    return unassociated_v8_bytes_used_ == other.unassociated_v8_bytes_used_;
-  }
+  void OnV8MemoryMeasurementAvailable(
+      const ProcessNode* process_node,
+      const V8PerFrameMemoryProcessData* process_data) final;
 
-  // Returns the number of bytes used by V8 at the last measurement in this
-  // process that could not be attributed to a frame.
-  uint64_t unassociated_v8_bytes_used() const {
-    return unassociated_v8_bytes_used_;
-  }
+ private:
+  void DeleteRequest();
 
-  void set_unassociated_v8_bytes_used(uint64_t unassociated_v8_bytes_used) {
-    unassociated_v8_bytes_used_ = unassociated_v8_bytes_used;
-  }
+#if DCHECK_IS_ON()
+  const ProcessNode* process_;
+#endif
 
-  // Returns process data for the given node, or nullptr if no measurement has
-  // been taken. The returned pointer must only be accessed on the graph
-  // sequence and may go invalid at any time after leaving the calling scope.
-  static const V8PerFrameMemoryProcessData* ForProcessNode(
-      const ProcessNode* node);
+  MeasurementCallback callback_;
+  MeasurementMode mode_;
+  std::unique_ptr<V8PerFrameMemoryRequest> request_;
 
- private:
-  uint64_t unassociated_v8_bytes_used_ = 0;
+  SEQUENCE_CHECKER(sequence_checker_);
 };
 
-class V8PerFrameMemoryObserver : public base::CheckedObserver {
- public:
-  // Called on the PM sequence when a measurement is available for
-  // |process_node|. |process_data| contains the process-level measurements for
-  // the process, and can go invalid at any time after returning from this
-  // method. Per-frame measurements can be read by walking the graph from
-  // |process_node| to find frame nodes, and calling
-  // V8PerFrameMemoryFrameData::ForFrameNode to retrieve the measurement data.
-  virtual void OnV8MemoryMeasurementAvailable(
-      const ProcessNode* process_node,
-      const V8PerFrameMemoryProcessData* process_data) = 0;
-};
+//////////////////////////////////////////////////////////////////////////////
+// The following classes are wrappers that can be called from outside the PM
+// sequence.
 
 // Observer that can be created on any sequence, and will be notified on that
 // sequence when measurements are available. Register the observer through
@@ -560,6 +629,9 @@ class V8PerFrameMemoryRequestAnySeq {
   base::WeakPtrFactory<V8PerFrameMemoryRequestAnySeq> weak_factory_{this};
 };
 
+//////////////////////////////////////////////////////////////////////////////
+// The following internal functions are exposed in the header for testing.
+
 namespace internal {
 
 // A callback that will bind a V8DetailedMemoryReporter interface to

components/performance_manager/v8_memory/v8_per_frame_memory_decorator.cc

@@ -603,6 +603,18 @@ V8PerFrameMemoryRequest::V8PerFrameMemoryRequest(
                                 base::Unretained(this)));
 }
 
+V8PerFrameMemoryRequest::V8PerFrameMemoryRequest(
+    util::PassKey<V8DetailedMemoryRequestOneShot>,
+    MeasurementMode mode)
+    : min_time_between_requests_(base::TimeDelta()), mode_(mode) {
+  // Do not forward to the standard constructor because it disallows the empty
+  // TimeDelta.
+#if DCHECK_IS_ON()
+  DCHECK(mode != MeasurementMode::kEagerForTesting ||
+         g_test_eager_measurement_requests_enabled);
+#endif
+}
+
 V8PerFrameMemoryRequest::~V8PerFrameMemoryRequest() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (decorator_)
@@ -651,8 +663,6 @@ void V8PerFrameMemoryRequest::NotifyObserversOnMeasurementAvailable(
   const auto* process_data =
       V8PerFrameMemoryProcessData::ForProcessNode(process_node);
   DCHECK(process_data);
-  for (V8PerFrameMemoryObserver& observer : observers_)
-    observer.OnV8MemoryMeasurementAvailable(process_node, process_data);
 
   // If this request was made from off-sequence, notify its off-sequence
   // observers with a copy of the process and frame data.
@@ -683,6 +693,11 @@ void V8PerFrameMemoryRequest::NotifyObserversOnMeasurementAvailable(
                        V8PerFrameMemoryObserverAnySeq::FrameDataMap(
                            std::move(all_frame_data))));
   }
+
+  // The observer could delete the request so this must be the last thing in
+  // the function.
+  for (V8PerFrameMemoryObserver& observer : observers_)
+    observer.OnV8MemoryMeasurementAvailable(process_node, process_data);
 }
 
 void V8PerFrameMemoryRequest::StartMeasurementImpl(
@@ -703,6 +718,53 @@ void V8PerFrameMemoryRequest::StartMeasurementImpl(
                                     this, process_node);
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// V8DetailedMemoryRequestOneShot
+
+V8DetailedMemoryRequestOneShot::V8DetailedMemoryRequestOneShot(
+    const ProcessNode* process,
+    MeasurementCallback callback,
+    MeasurementMode mode)
+    : callback_(std::move(callback)), mode_(mode) {
+  DCHECK(process);
+  DCHECK_EQ(process->GetProcessType(), content::PROCESS_TYPE_RENDERER);
+  request_ = std::make_unique<V8PerFrameMemoryRequest>(
+      util::PassKey<V8DetailedMemoryRequestOneShot>(), mode);
+  request_->AddObserver(this);
+  request_->StartMeasurementForProcess(process);
+
+#if DCHECK_IS_ON()
+  process_ = process;
+#endif
+}
+
+V8DetailedMemoryRequestOneShot::~V8DetailedMemoryRequestOneShot() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DeleteRequest();
+}
+
+void V8DetailedMemoryRequestOneShot::OnV8MemoryMeasurementAvailable(
+    const ProcessNode* process_node,
+    const V8PerFrameMemoryProcessData* process_data) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+
+#if DCHECK_IS_ON()
+  DCHECK_EQ(process_node, process_);
+#endif
+
+  // Don't send another request now that a response has been received.
+  DeleteRequest();
+
+  std::move(callback_).Run(process_node, process_data);
+}
+
+void V8DetailedMemoryRequestOneShot::DeleteRequest() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  if (request_)
+    request_->RemoveObserver(this);
+  request_.reset();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // V8PerFrameMemoryFrameData
 
@@ -949,13 +1011,22 @@ void V8PerFrameMemoryDecorator::MeasurementRequestQueue::
     NotifyObserversOnMeasurementAvailable(
         const ProcessNode* process_node) const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  for (const V8PerFrameMemoryRequest* request : bounded_measurement_requests_) {
-    request->NotifyObserversOnMeasurementAvailable(
-        util::PassKey<MeasurementRequestQueue>(), process_node);
-  }
-  for (const V8PerFrameMemoryRequest* request : lazy_measurement_requests_) {
+
+  // First collect all requests with observers to notify. The observer
+  // implementations may add or remove requests from the queue, invalidating
+  // iterators.
+  std::vector<V8PerFrameMemoryRequest*> requests_to_notify;
+  requests_to_notify.insert(requests_to_notify.end(),
+                            bounded_measurement_requests_.begin(),
+                            bounded_measurement_requests_.end());
+  requests_to_notify.insert(requests_to_notify.end(),
+                            lazy_measurement_requests_.begin(),
+                            lazy_measurement_requests_.end());
+  for (const V8PerFrameMemoryRequest* request : requests_to_notify) {
     request->NotifyObserversOnMeasurementAvailable(
         util::PassKey<MeasurementRequestQueue>(), process_node);
+    // The observer may have deleted |request| so it is no longer safe to
+    // reference.
   }
 }
 

components/performance_manager/v8_memory/v8_per_frame_memory_decorator_unittest.cc

@@ -392,6 +392,89 @@ TEST_F(V8PerFrameMemoryDecoratorTest, OnlyMeasureRenderers) {
   }
 }
 
+TEST_F(V8PerFrameMemoryDecoratorTest, OneShot) {
+  // Create 2 renderer processes. Create one request that measures both of
+  // them, and a one-shot request that measures only one.
+  constexpr RenderProcessHostId kProcessId1 = RenderProcessHostId(0xFAB);
+  auto process1 = CreateNode<ProcessNodeImpl>(
+      content::PROCESS_TYPE_RENDERER,
+      RenderProcessHostProxy::CreateForTesting(kProcessId1));
+  constexpr RenderProcessHostId kProcessId2 = RenderProcessHostId(0xBAF);
+  auto process2 = CreateNode<ProcessNodeImpl>(
+      content::PROCESS_TYPE_RENDERER,
+      RenderProcessHostProxy::CreateForTesting(kProcessId2));
+
+  // Set the all process request to only send once within the test.
+  V8PerFrameMemoryRequest all_process_request(kMinTimeBetweenRequests * 100);
+  all_process_request.StartMeasurement(graph());
+
+  // Create a mock reporter for each process and expect a query and reply on
+  // each.
+  MockV8DetailedMemoryReporter mock_reporter1;
+  {
+    auto data = NewPerProcessV8MemoryUsage(1);
+    data->isolates[0]->unassociated_bytes_used = 1ULL;
+    ExpectBindAndRespondToQuery(&mock_reporter1, std::move(data), kProcessId1);
+  }
+  MockV8DetailedMemoryReporter mock_reporter2;
+  {
+    auto data = NewPerProcessV8MemoryUsage(1);
+    data->isolates[0]->unassociated_bytes_used = 2ULL;
+    ExpectBindAndRespondToQuery(&mock_reporter2, std::move(data), kProcessId2);
+  }
+
+  task_env().RunUntilIdle();
+  Mock::VerifyAndClearExpectations(&mock_reporter1);
+  Mock::VerifyAndClearExpectations(&mock_reporter2);
+
+  // Create a one-shot request for process1 and expect the callback to be
+  // called only for that process.
+  {
+    auto data = NewPerProcessV8MemoryUsage(1);
+    data->isolates[0]->unassociated_bytes_used = 3ULL;
+    ExpectQueryAndReply(&mock_reporter1, std::move(data));
+  }
+
+  uint64_t unassociated_v8_bytes_used = 0;
+  V8DetailedMemoryRequestOneShot process1_request(
+      process1.get(), base::BindLambdaForTesting(
+                          [&unassociated_v8_bytes_used, &process1](
+                              const ProcessNode* process_node,
+                              const V8PerFrameMemoryProcessData* process_data) {
+                            ASSERT_TRUE(process_data);
+                            EXPECT_EQ(process_node, process1.get());
+                            unassociated_v8_bytes_used =
+                                process_data->unassociated_v8_bytes_used();
+                          }));
+  task_env().RunUntilIdle();
+  Mock::VerifyAndClearExpectations(&mock_reporter1);
+  Mock::VerifyAndClearExpectations(&mock_reporter2);
+  EXPECT_EQ(unassociated_v8_bytes_used, 3ULL);
+
+  // Create another request, but delete it before the result arrives.
+  {
+    auto data = NewPerProcessV8MemoryUsage(1);
+    data->isolates[0]->unassociated_bytes_used = 4ULL;
+    ExpectQueryAndDelayReply(&mock_reporter1, base::TimeDelta::FromSeconds(10),
+                             std::move(data));
+  }
+
+  auto doomed_request = std::make_unique<V8DetailedMemoryRequestOneShot>(
+      process1.get(),
+      base::BindOnce([](const ProcessNode* process_node,
+                        const V8PerFrameMemoryProcessData* process_data) {
+        FAIL() << "Callback called after request deleted.";
+      }));
+
+  // Verify that requests are sent but reply is not yet received.
+  task_env().FastForwardBy(base::TimeDelta::FromSeconds(5));
+  Mock::VerifyAndClearExpectations(&mock_reporter1);
+  Mock::VerifyAndClearExpectations(&mock_reporter2);
+
+  doomed_request.reset();
+  task_env().RunUntilIdle();
+}
+
 TEST_F(V8PerFrameMemoryDecoratorTest, QueryRateIsLimited) {
   auto process = CreateNode<ProcessNodeImpl>(
       content::PROCESS_TYPE_RENDERER,