[PM] Add off-sequence wrapper for V8DetailedMemoryRequestOneShot

R=fdoray

Bug: 1080672
Change-Id: Ia7d6d6139daac23a80d824734bde7bdc5c111ae7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2459567
Commit-Queue: Joe Mason <joenotcharles@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816911}

components/performance_manager/public/v8_memory/v8_detailed_memory.h

@@ -192,6 +192,7 @@ class V8DetailedMemoryProcessData;
 class V8DetailedMemoryRequest;
 class V8DetailedMemoryRequestAnySeq;
 class V8DetailedMemoryRequestOneShot;
+class V8DetailedMemoryRequestOneShotAnySeq;
 
 class V8DetailedMemoryDecorator
     : public GraphOwned,
@@ -496,7 +497,20 @@ class V8DetailedMemoryRequestOneShot : public V8DetailedMemoryObserver {
       const ProcessNode* process_node,
       const V8DetailedMemoryProcessData* process_data) final;
 
+  // Implementation details below this point.
+
+  // Private constructor for V8DetailedMemoryRequestOneShotAnySeq. Will be
+  // called from off-sequence.
+  V8DetailedMemoryRequestOneShot(
+      util::PassKey<V8DetailedMemoryRequestOneShotAnySeq>,
+      base::WeakPtr<ProcessNode> process,
+      MeasurementCallback callback,
+      MeasurementMode mode = MeasurementMode::kDefault);
+
  private:
+  void InitializeRequest(const ProcessNode* process, MeasurementMode mode);
+  void InitializeRequestFromOffSequence(base::WeakPtr<ProcessNode> process,
+                                        MeasurementMode mode);
   void DeleteRequest();
 
 #if DCHECK_IS_ON()
@@ -594,6 +608,63 @@ class V8DetailedMemoryRequestAnySeq {
   base::WeakPtrFactory<V8DetailedMemoryRequestAnySeq> weak_factory_{this};
 };
 
+// Wrapper that can instantiate a V8DetailedMemoryRequestOneShot from any
+// sequence.
+class V8DetailedMemoryRequestOneShotAnySeq {
+ public:
+  using MeasurementMode = V8DetailedMemoryRequest::MeasurementMode;
+
+  using FrameDataMap = V8DetailedMemoryObserverAnySeq::FrameDataMap;
+
+  // A callback that will be called on the request's sequence with the results
+  // of the measurement. |process_id| will always match the value passed to
+  // the V8DetailedMemoryRequestOneShotAnySeq constructor.
+  using MeasurementCallback =
+      base::OnceCallback<void(RenderProcessHostId process_id,
+                              const V8DetailedMemoryProcessData& process_data,
+                              const FrameDataMap& frame_data)>;
+
+  V8DetailedMemoryRequestOneShotAnySeq(
+      RenderProcessHostId process_id,
+      MeasurementCallback callback,
+      MeasurementMode mode = MeasurementMode::kDefault);
+
+  ~V8DetailedMemoryRequestOneShotAnySeq();
+
+  V8DetailedMemoryRequestOneShotAnySeq(
+      const V8DetailedMemoryRequestOneShotAnySeq&) = delete;
+  V8DetailedMemoryRequestOneShotAnySeq& operator=(
+      const V8DetailedMemoryRequestOneShotAnySeq&) = delete;
+
+ private:
+  void InitializeWrappedRequest(MeasurementMode mode,
+                                base::WeakPtr<ProcessNode>);
+
+  // Called on the PM sequence when a measurement is available. It will call
+  // request->InvokeWrappedCallback on |task_runner|.
+  static void OnMeasurementAvailable(
+      scoped_refptr<base::SequencedTaskRunner> task_runner,
+      base::WeakPtr<V8DetailedMemoryRequestOneShotAnySeq> request,
+      const ProcessNode* process_node,
+      const V8DetailedMemoryProcessData* process_data);
+
+  void InvokeWrappedCallback(RenderProcessHostId process_id,
+                             const V8DetailedMemoryProcessData& process_data,
+                             const FrameDataMap& frame_data);
+
+  MeasurementCallback wrapped_callback_;
+
+  // The wrapped request. Must only be accessed from the PM sequence.
+  std::unique_ptr<V8DetailedMemoryRequestOneShot> request_;
+
+  // This object can live on any sequence but all methods and the destructor
+  // must be called from that sequence.
+  SEQUENCE_CHECKER(sequence_checker_);
+
+  base::WeakPtrFactory<V8DetailedMemoryRequestOneShotAnySeq> weak_factory_{
+      this};
+};
+
 //////////////////////////////////////////////////////////////////////////////
 // The following internal functions are exposed in the header for testing.
 

components/performance_manager/v8_memory/v8_detailed_memory.cc

@@ -745,16 +745,7 @@ V8DetailedMemoryRequestOneShot::V8DetailedMemoryRequestOneShot(
     MeasurementCallback callback,
     MeasurementMode mode)
     : callback_(std::move(callback)), mode_(mode) {
-  DCHECK(process);
-  DCHECK_EQ(process->GetProcessType(), content::PROCESS_TYPE_RENDERER);
-  request_ = std::make_unique<V8DetailedMemoryRequest>(
-      util::PassKey<V8DetailedMemoryRequestOneShot>(), mode);
-  request_->AddObserver(this);
-  request_->StartMeasurementForProcess(process);
-
-#if DCHECK_IS_ON()
-  process_ = process;
-#endif
+  InitializeRequest(process, mode);
 }
 
 V8DetailedMemoryRequestOneShot::~V8DetailedMemoryRequestOneShot() {
@@ -777,6 +768,48 @@ void V8DetailedMemoryRequestOneShot::OnV8MemoryMeasurementAvailable(
   std::move(callback_).Run(process_node, process_data);
 }
 
+// This constructor is called from the V8DetailedMemoryRequestOneShotAnySeq's
+// sequence.
+V8DetailedMemoryRequestOneShot::V8DetailedMemoryRequestOneShot(
+    util::PassKey<V8DetailedMemoryRequestOneShotAnySeq>,
+    base::WeakPtr<ProcessNode> process,
+    MeasurementCallback callback,
+    MeasurementMode mode)
+    : callback_(std::move(callback)), mode_(mode) {
+  DETACH_FROM_SEQUENCE(sequence_checker_);
+  // Unretained is safe since |this| will be destroyed on the graph sequence
+  // from an async task posted after this.
+  PerformanceManager::CallOnGraph(
+      FROM_HERE,
+      base::BindOnce(
+          &V8DetailedMemoryRequestOneShot::InitializeRequestFromOffSequence,
+          base::Unretained(this), std::move(process), mode));
+}
+
+void V8DetailedMemoryRequestOneShot::InitializeRequest(
+    const ProcessNode* process,
+    MeasurementMode mode) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  DCHECK(process);
+  DCHECK_EQ(process->GetProcessType(), content::PROCESS_TYPE_RENDERER);
+  request_ = std::make_unique<V8DetailedMemoryRequest>(
+      util::PassKey<V8DetailedMemoryRequestOneShot>(), mode);
+  request_->AddObserver(this);
+  request_->StartMeasurementForProcess(process);
+
+#if DCHECK_IS_ON()
+  process_ = process;
+#endif
+}
+
+void V8DetailedMemoryRequestOneShot::InitializeRequestFromOffSequence(
+    base::WeakPtr<ProcessNode> process,
+    MeasurementMode mode) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  if (process)
+    InitializeRequest(process.get(), mode);
+}
+
 void V8DetailedMemoryRequestOneShot::DeleteRequest() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (request_)
@@ -1165,6 +1198,101 @@ void V8DetailedMemoryRequestAnySeq::InitializeWrappedRequest(
       mode, std::move(process_to_measure), weak_factory_.GetWeakPtr()));
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// V8DetailedMemoryRequestOneShotAnySeq
+
+V8DetailedMemoryRequestOneShotAnySeq::V8DetailedMemoryRequestOneShotAnySeq(
+    RenderProcessHostId process_id,
+    MeasurementCallback callback,
+    MeasurementMode mode)
+    : wrapped_callback_(std::move(callback)) {
+  // GetProcessNodeForRenderProcessHostId must be called from the UI thread.
+  auto ui_task_runner = content::GetUIThreadTaskRunner({});
+  if (ui_task_runner->RunsTasksInCurrentSequence()) {
+    InitializeWrappedRequest(
+        mode,
+        PerformanceManager::GetProcessNodeForRenderProcessHostId(process_id));
+  } else {
+    ui_task_runner->PostTaskAndReplyWithResult(
+        FROM_HERE,
+        base::BindOnce(
+            &PerformanceManager::GetProcessNodeForRenderProcessHostId,
+            process_id),
+        base::BindOnce(
+            &V8DetailedMemoryRequestOneShotAnySeq::InitializeWrappedRequest,
+            weak_factory_.GetWeakPtr(), mode));
+  }
+}
+
+V8DetailedMemoryRequestOneShotAnySeq::~V8DetailedMemoryRequestOneShotAnySeq() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  PerformanceManager::CallOnGraph(
+      FROM_HERE,
+      base::BindOnce(
+          [](std::unique_ptr<V8DetailedMemoryRequestOneShot> request) {
+            request.reset();
+          },
+          std::move(request_)));
+}
+
+void V8DetailedMemoryRequestOneShotAnySeq::InitializeWrappedRequest(
+    MeasurementMode mode,
+    base::WeakPtr<ProcessNode> process_node) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  // Can't use make_unique since this calls the private any-sequence
+  // constructor. After construction the V8DetailedMemoryRequestOneShot must
+  // only be accessed on the graph sequence.
+  request_ = base::WrapUnique(new V8DetailedMemoryRequestOneShot(
+      util::PassKey<V8DetailedMemoryRequestOneShotAnySeq>(),
+      std::move(process_node),
+      base::BindOnce(
+          &V8DetailedMemoryRequestOneShotAnySeq::OnMeasurementAvailable,
+          base::SequencedTaskRunnerHandle::Get(), weak_factory_.GetWeakPtr()),
+      mode));
+}
+
+// static
+void V8DetailedMemoryRequestOneShotAnySeq::OnMeasurementAvailable(
+    scoped_refptr<base::SequencedTaskRunner> task_runner,
+    base::WeakPtr<V8DetailedMemoryRequestOneShotAnySeq> request,
+    const ProcessNode* process_node,
+    const V8DetailedMemoryProcessData* process_data) {
+  DCHECK(process_node);
+  DCHECK_ON_GRAPH_SEQUENCE(process_node->GetGraph());
+
+  using FrameAndData =
+      std::pair<content::GlobalFrameRoutingId, V8DetailedMemoryFrameData>;
+  std::vector<FrameAndData> all_frame_data;
+  process_node->VisitFrameNodes(base::BindRepeating(
+      [](std::vector<FrameAndData>* all_frame_data,
+         const FrameNode* frame_node) {
+        const auto* frame_data =
+            V8DetailedMemoryFrameData::ForFrameNode(frame_node);
+        if (frame_data) {
+          all_frame_data->push_back(std::make_pair(
+              frame_node->GetRenderFrameHostProxy().global_frame_routing_id(),
+              *frame_data));
+        }
+        return true;
+      },
+      base::Unretained(&all_frame_data)));
+
+  task_runner->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          &V8DetailedMemoryRequestOneShotAnySeq::InvokeWrappedCallback,
+          std::move(request), process_node->GetRenderProcessHostId(),
+          *process_data, std::move(all_frame_data)));
+}
+
+void V8DetailedMemoryRequestOneShotAnySeq::InvokeWrappedCallback(
+    RenderProcessHostId process_id,
+    const V8DetailedMemoryProcessData& process_data,
+    const FrameDataMap& frame_data) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  std::move(wrapped_callback_).Run(process_id, process_data, frame_data);
+}
+
 }  // namespace v8_memory
 
 }  // namespace performance_manager

components/performance_manager/v8_memory/v8_detailed_memory_unittest.cc

@@ -1829,11 +1829,104 @@ TEST_F(V8DetailedMemoryRequestAnySeqTest, SingleProcessRequest) {
   // from ObserverList.
   all_process_request.RemoveObserver(&all_process_observer);
   single_process_request.RemoveObserver(&single_process_observer);
+}
 
-  // Execute the above tasks and exit.
-  base::RunLoop run_loop;
-  PerformanceManager::CallOnGraph(FROM_HERE, run_loop.QuitClosure());
-  run_loop.Run();
+TEST_F(V8DetailedMemoryRequestAnySeqTest, OneShot) {
+  content::IsolateAllSitesForTesting(base::CommandLine::ForCurrentProcess());
+  SetContents(CreateTestWebContents());
+
+  // Set up a page at a.com with a subframe at b.com. These should be in
+  // different processes. We will create one request that measures both
+  // processes, and a one-shot request that measures only one.
+  const GURL kUrlA("http://a.com/");
+  const GURL kUrlB("http://b.com/");
+  content::RenderFrameHost* main_frame =
+      content::NavigationSimulator::NavigateAndCommitFromBrowser(
+          web_contents(), GURL("http://a.com"));
+  content::RenderFrameHost* child_frame =
+      content::RenderFrameHostTester::For(main_frame)->AppendChild("frame1");
+  child_frame = content::NavigationSimulator::NavigateAndCommitFromDocument(
+      GURL("http://b.com"), child_frame);
+
+  const RenderProcessHostId process_id1(main_frame->GetProcess()->GetID());
+  const RenderProcessHostId process_id2(child_frame->GetProcess()->GetID());
+  ASSERT_NE(process_id1, process_id2);
+
+  // Set the all process request to only send once within the test.
+  V8DetailedMemoryRequestAnySeq all_process_request(
+      V8DetailedMemoryDecoratorTest::kMinTimeBetweenRequests * 100);
+
+  // 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), process_id1);
+  }
+
+  MockV8DetailedMemoryReporter mock_reporter2;
+  {
+    auto data = NewPerProcessV8MemoryUsage(1);
+    data->isolates[0]->unassociated_bytes_used = 2ULL;
+    ExpectBindAndRespondToQuery(&mock_reporter2, std::move(data), process_id2);
+  }
+
+  task_environment()->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;
+  V8DetailedMemoryRequestOneShotAnySeq process1_request(
+      process_id1,
+      base::BindLambdaForTesting(
+          [&](RenderProcessHostId process_id,
+              const V8DetailedMemoryProcessData& process_data,
+              const V8DetailedMemoryRequestOneShotAnySeq::FrameDataMap&
+                  frame_data) {
+            EXPECT_EQ(process_id, process_id1);
+            unassociated_v8_bytes_used =
+                process_data.unassociated_v8_bytes_used();
+          }));
+  task_environment()->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<V8DetailedMemoryRequestOneShotAnySeq>(
+      process_id1,
+      base::BindOnce(
+          [](RenderProcessHostId process_id,
+             const V8DetailedMemoryProcessData& process_data,
+             const V8DetailedMemoryRequestOneShotAnySeq::FrameDataMap&
+                 frame_data) {
+            FAIL() << "Callback called after request deleted.";
+          }));
+
+  // Verify that requests are sent but reply is not received.
+  task_environment()->RunUntilIdle();
+  Mock::VerifyAndClearExpectations(&mock_reporter1);
+  Mock::VerifyAndClearExpectations(&mock_reporter2);
+
+  doomed_request.reset();
+  task_environment()->RunUntilIdle();
 }
 
 }  // namespace v8_memory