PM: Rearrange cleanup and removal.

This CL rearranges cleanup and removal notification such that removal
notification is always the last notification issued for or by any node.

Change-Id: Ia83c58a3995e226c4860640f7096cae6e0aed77b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2231265Reviewed-by: Chris Hamilton <chrisha@chromium.org>
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#775314}

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

@@ -295,11 +295,7 @@ void DiscardsGraphDumpImpl::OnOpenerFrameNodeChanged(
     const performance_manager::PageNode* page_node,
     const performance_manager::FrameNode*,
     OpenedType) {
-  // This notification can arrive for a page node that has already been
-  // removed, because it fires as part of the |page_node| destructor. If that's
-  // the case then ignore this silently.
-  if (!HasNode(page_node))
-    return;
+  DCHECK(HasNode(page_node));
   SendPageNotification(page_node, false);
 }
 

components/performance_manager/graph/graph_impl.cc

@@ -375,6 +375,10 @@ void GraphImpl::OnNodeAdded(NodeBase* node) {
 void GraphImpl::OnBeforeNodeRemoved(NodeBase* node) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  // Clear any node-specific state and issue the relevant notifications before
+  // sending the last-gasp removal notification for this node.
+  node->OnBeforeLeavingGraph();
+
   // This handles the strongly typed observer notifications.
   switch (node->type()) {
     case NodeTypeEnum::kFrame: {
@@ -503,8 +507,9 @@ void GraphImpl::AddNewNode(NodeBase* new_node) {
   auto it = nodes_.insert(new_node);
   DCHECK(it.second);  // Inserted successfully
 
-  // Allow the node to initialize itself now that it's been added.
+  // Add the node to the graph and allow it to initialize itself.
   new_node->JoinGraph(this);
+  new_node->OnJoiningGraph();
 
   // Then notify observers.
   OnNodeAdded(new_node);

components/performance_manager/graph/node_base.cc

@@ -40,8 +40,6 @@ void NodeBase::JoinGraph(GraphImpl* graph) {
   DCHECK(graph->NodeInGraph(this));
 
   graph_ = graph;
-
-  OnJoiningGraph();
 }
 
 void NodeBase::LeaveGraph() {
@@ -49,8 +47,6 @@ void NodeBase::LeaveGraph() {
   DCHECK(graph_);
   DCHECK(graph_->NodeInGraph(this));
 
-  OnBeforeLeavingGraph();
-
   graph_ = nullptr;
 }
 

components/performance_manager/graph/node_base.h

@@ -73,13 +73,10 @@ class NodeBase {
     return graph->GetObservers<Observer>();
   }
 
-  // Joins the |graph|. Assigns |graph_| and invokes OnJoiningGraph() to allow
-  // subclasses to initialize.
+  // Joins the |graph|.
   void JoinGraph(GraphImpl* graph);
 
-  // Leaves the graph that this node is a part of. Invokes
-  // OnBeforeLeavingGraph() to allow subclasses to uninitialize then clears
-  // |graph_|.
+  // Leaves the graph that this node is a part of.
   void LeaveGraph();
 
   // Called as this node is joining |graph_|, a good opportunity to initialize