PM: Add a RequestNodeDescriptions method to mojo interface.

This allows the graph view to poll for changes to node attached and node
related data for the nodes that are interesting at any time. This will
probably be done on hover and/or for nodes on explicit display.

Bug: 1068233
Change-Id: I52c6ba433784dbe8829beaa77d081f2a40d80010
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2152888Reviewed-by: Joe Mason <joenotcharles@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760169}

chrome/browser/ui/webui/discards/discards.mojom

@@ -194,4 +194,16 @@ interface GraphChangeStream {
 interface GraphDump {
   // Subscribes |change_subscriber| to a graph change stream.
   SubscribeToChanges(pending_remote<GraphChangeStream> change_subscriber);
+
+  // Requests the node descriptions for the nodes with IDs |node_ids|. If any
+  // ID in |node_ids| is invalid, e.g. is not the ID of an existent node,
+  // the result will omit that node ID. The result will also omit nodes that
+  // have no description.
+  // Each returned description is a dictionary of values, where each value is
+  // generated by a performance_manager::NodeDataDescriber implementation and
+  // keyed by the name it registered with. The intent is for each describer to
+  // describe private node-related or node-attached data in some way, to allow
+  // presenting otherwise hidden state in the chrome://discards/graph view.
+  RequestNodeDescriptions(array<int64> node_ids) =>
+      (map<int64, string> node_descriptions_json);
 };

chrome/browser/ui/webui/discards/graph_dump_impl.cc

@@ -226,6 +226,28 @@ void DiscardsGraphDumpImpl::SubscribeToChanges(
   graph_->AddWorkerNodeObserver(this);
 }
 
+void DiscardsGraphDumpImpl::RequestNodeDescriptions(
+    const std::vector<int64_t>& node_ids,
+    RequestNodeDescriptionsCallback callback) {
+  base::flat_map<int64_t, std::string> descriptions;
+  performance_manager::NodeDataDescriberRegistry* describer_registry =
+      graph_->GetNodeDataDescriberRegistry();
+  for (int64_t node_id : node_ids) {
+    auto it = nodes_by_id_.find(NodeId::FromUnsafeValue(node_id));
+    // The requested node may have been removed by the time the request arrives,
+    // in which case no description is returned for that node ID.
+    if (it != nodes_by_id_.end()) {
+      const performance_manager::Node* node = it->second;
+      base::Value description = describer_registry->DescribeNodeData(node);
+
+      if (!description.is_none())
+        descriptions[node_id] = ToJSON(description);
+    }
+  }
+
+  std::move(callback).Run(descriptions);
+}
+
 void DiscardsGraphDumpImpl::OnPassedToGraph(performance_manager::Graph* graph) {
   DCHECK(!graph_);
   graph_ = graph;
@@ -358,11 +380,17 @@ void DiscardsGraphDumpImpl::OnBeforeClientWorkerRemoved(
 
 void DiscardsGraphDumpImpl::AddNode(const performance_manager::Node* node) {
   DCHECK(node_ids_.find(node) == node_ids_.end());
-  node_ids_.insert(std::make_pair(node, node_id_generator_.GenerateNextId()));
+  NodeId new_id = node_id_generator_.GenerateNextId();
+  node_ids_.insert(std::make_pair(node, new_id));
+  nodes_by_id_.insert(std::make_pair(new_id, node));
 }
 
 void DiscardsGraphDumpImpl::RemoveNode(const performance_manager::Node* node) {
-  size_t erased = node_ids_.erase(node);
+  auto it = node_ids_.find(node);
+  DCHECK(it != node_ids_.end());
+  NodeId node_id = it->second;
+  node_ids_.erase(it);
+  size_t erased = nodes_by_id_.erase(node_id);
   DCHECK_EQ(1u, erased);
 }
 

chrome/browser/ui/webui/discards/graph_dump_impl.h

@@ -51,6 +51,9 @@ class DiscardsGraphDumpImpl : public discards::mojom::GraphDump,
   void SubscribeToChanges(
       mojo::PendingRemote<discards::mojom::GraphChangeStream> change_subscriber)
       override;
+  void RequestNodeDescriptions(
+      const std::vector<int64_t>& node_ids,
+      RequestNodeDescriptionsCallback callback) override;
 
   // GraphOwned implementation.
   void OnPassedToGraph(performance_manager::Graph* graph) override;
@@ -215,6 +218,7 @@ class DiscardsGraphDumpImpl : public discards::mojom::GraphDump,
 
   // The live nodes and their IDs.
   base::flat_map<const performance_manager::Node*, NodeId> node_ids_;
+  base::flat_map<NodeId, const performance_manager::Node*> nodes_by_id_;
   NodeId::Generator node_id_generator_;
 
   // The current change subscriber to this dumper. This instance is subscribed

chrome/browser/ui/webui/discards/graph_dump_impl_unittest.cc

@@ -23,6 +23,7 @@
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/receiver.h"
 #include "mojo/public/cpp/bindings/remote.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -286,6 +287,53 @@ TEST_F(DiscardsGraphDumpImplTest, ChangeStream) {
 
   task_environment.RunUntilIdle();
 
+  // Test RequestNodeDescriptions.
+  std::vector<int64_t> descriptions_requested;
+  for (int64_t node_id : change_stream.id_set()) {
+    descriptions_requested.push_back(node_id);
+  }
+  // Increase the last ID by one. As the entries are in increasing order, this
+  // results in a request for all but one nodes, and one non-existent node id.
+  descriptions_requested.back() += 1;
+
+  {
+    base::RunLoop run_loop;
+    base::RepeatingClosure quit_closure = run_loop.QuitClosure();
+
+    graph_dump_remote->RequestNodeDescriptions(
+        descriptions_requested,
+        base::BindLambdaForTesting(
+            [&descriptions_requested,
+             &quit_closure](const base::flat_map<int64_t, std::string>&
+                                node_descriptions_json) {
+              std::vector<int64_t> keys_received;
+              // Check that the descriptions make sense.
+              for (auto kv : node_descriptions_json) {
+                keys_received.push_back(kv.first);
+                base::Optional<base::Value> v =
+                    base::JSONReader::Read(kv.second);
+                EXPECT_TRUE(v->is_dict());
+                std::string* str = v->FindStringKey("test");
+                EXPECT_TRUE(str);
+                if (str) {
+                  EXPECT_TRUE(*str == "frame" || *str == "page" ||
+                              *str == "process" || *str == "worker");
+                }
+              }
+
+              EXPECT_THAT(keys_received,
+                          ::testing::UnorderedElementsAreArray(
+                              descriptions_requested.data(),
+                              descriptions_requested.size() - 1));
+
+              quit_closure.Run();
+            }));
+
+    run_loop.Run();
+  }
+
+  task_environment.RunUntilIdle();
+
   // Make sure the Dump impl is torn down when the proxy closes.
   graph_dump_remote.reset();
   task_environment.RunUntilIdle();