PM: Add ProcessNodeObserver::OnProcessLifetimeChange.

Bug: 996273

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

chrome/browser/performance_manager/graph/process_node_impl.cc

@@ -54,7 +54,7 @@ void ProcessNodeImpl::SetProcessExitStatus(int32_t exit_status) {
   exit_status_ = exit_status;
 
   // Close the process handle to kill the zombie.
-  process_.Close();
+  process_.SetAndNotify(this, base::Process());
 
   // No more message should be received from this process.
   binding_.Close();
@@ -67,7 +67,7 @@ void ProcessNodeImpl::SetProcess(base::Process process,
   // Either this is the initial process associated with this process node,
   // or it's a subsequent process. In the latter case, there must have been
   // an exit status associated with the previous process.
-  DCHECK(!process_.IsValid() || exit_status_.has_value());
+  DCHECK(!process_.value().IsValid() || exit_status_.has_value());
 
   base::ProcessId pid = process.Pid();
   SetProcessImpl(std::move(process), pid, launch_time);
@@ -120,10 +120,6 @@ void ProcessNodeImpl::SetProcessImpl(base::Process process,
 
   graph()->BeforeProcessPidChange(this, new_pid);
 
-  process_ = std::move(process);
-  process_id_ = new_pid;
-  launch_time_ = launch_time;
-
   // Clear the exit status for the previous process (if any).
   exit_status_.reset();
 
@@ -131,6 +127,12 @@ void ProcessNodeImpl::SetProcessImpl(base::Process process,
   // process.
   private_footprint_kb_ = 0;
   cumulative_cpu_usage_ = base::TimeDelta();
+
+  process_id_ = new_pid;
+  launch_time_ = launch_time;
+
+  // Set the process variable last, as it will fire the notification.
+  process_.SetAndNotify(this, std::move(process));
 }
 
 base::ProcessId ProcessNodeImpl::GetProcessId() const {

chrome/browser/performance_manager/graph/process_node_impl.h

@@ -80,7 +80,7 @@ class ProcessNodeImpl
   // access, but will return kNullProcessId when the process is not valid. It
   // will also retain the process ID for a process that has exited.
   base::ProcessId process_id() const { return process_id_; }
-  const base::Process& process() const { return process_; }
+  const base::Process& process() const { return process_.value(); }
   base::Time launch_time() const { return launch_time_; }
   base::Optional<int32_t> exit_status() const { return exit_status_; }
 
@@ -145,7 +145,11 @@ class ProcessNodeImpl
   uint64_t private_footprint_kb_ = 0u;
 
   base::ProcessId process_id_ = base::kNullProcessId;
-  base::Process process_;
+  ObservedProperty::NotifiesAlways<
+      base::Process,
+      &ProcessNodeObserver::OnProcessLifetimeChange>
+      process_;
+
   base::Time launch_time_;
   base::Optional<int32_t> exit_status_;
 

chrome/browser/performance_manager/graph/process_node_impl_unittest.cc

@@ -118,6 +118,7 @@ class LenientMockObserver : public ProcessNodeImpl::Observer {
   ~LenientMockObserver() override {}
 
   MOCK_METHOD1(OnProcessNodeAdded, void(const ProcessNode*));
+  MOCK_METHOD1(OnProcessLifetimeChange, void(const ProcessNode*));
   MOCK_METHOD1(OnBeforeProcessNodeRemoved, void(const ProcessNode*));
   MOCK_METHOD1(OnExpectedTaskQueueingDurationSample, void(const ProcessNode*));
   MOCK_METHOD1(OnMainThreadTaskLoadIsLow, void(const ProcessNode*));
@@ -155,6 +156,12 @@ TEST_F(ProcessNodeImplTest, ObserverWorks) {
   const ProcessNode* raw_process_node = process_node.get();
   EXPECT_EQ(raw_process_node, obs.TakeNotifiedProcessNode());
 
+  // Test process creation and exit events.
+  EXPECT_CALL(obs, OnProcessLifetimeChange(_));
+  process_node->SetProcess(base::Process::Current(), base::Time::Now());
+  EXPECT_CALL(obs, OnProcessLifetimeChange(_));
+  process_node->SetProcessExitStatus(10);
+
   EXPECT_CALL(obs, OnExpectedTaskQueueingDurationSample(_))
       .WillOnce(Invoke(&obs, &MockObserver::SetNotifiedProcessNode));
   process_node->SetExpectedTaskQueueingDuration(

chrome/browser/performance_manager/graph/properties.h

@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_PERFORMANCE_MANAGER_GRAPH_PROPERTIES_H_
 #define CHROME_BROWSER_PERFORMANCE_MANAGER_GRAPH_PROPERTIES_H_
 
+#include <utility>
+
 namespace performance_manager {
 
 // TODO(chrisha): Deprecate the private observer type and have everyone use the
@@ -35,7 +37,7 @@ class ObservedPropertyImpl {
 
     // Sets the property and sends a notification.
     void SetAndNotify(NodeImplType* node, PropertyType value) {
-      value_ = value;
+      value_ = std::forward<PropertyType>(value);
       for (auto* observer : node->GetObservers())
         ((observer)->*(NotifyFunctionPtr))(node);
     }
@@ -65,7 +67,7 @@ class ObservedPropertyImpl {
     bool SetAndMaybeNotify(NodeImplType* node, PropertyType value) {
       if (value_ == value)
         return false;
-      value_ = value;
+      value_ = std::forward<PropertyType>(value);
       for (auto* observer : node->GetObservers())
         ((observer)->*(NotifyFunctionPtr))(node);
       return true;

chrome/browser/performance_manager/public/graph/process_node.h

@@ -113,6 +113,11 @@ class ProcessNodeObserver {
   // Called when a |process_node| is added to the graph.
   virtual void OnProcessNodeAdded(const ProcessNode* process_node) = 0;
 
+  // The process associated with |process_node| has been started or has exited.
+  // This implies some or all of the process, process_id, launch time and/or
+  // exit status properties have changed.
+  virtual void OnProcessLifetimeChange(const ProcessNode* process_node) = 0;
+
   // Called before a |process_node| is removed from the graph.
   virtual void OnBeforeProcessNodeRemoved(const ProcessNode* process_node) = 0;
 
@@ -144,6 +149,7 @@ class ProcessNode::ObserverDefaultImpl : public ProcessNodeObserver {
 
   // ProcessNodeObserver implementation:
   void OnProcessNodeAdded(const ProcessNode* process_node) override {}
+  void OnProcessLifetimeChange(const ProcessNode* process_node) override {}
   void OnBeforeProcessNodeRemoved(const ProcessNode* process_node) override {}
   void OnExpectedTaskQueueingDurationSample(
       const ProcessNode* process_node) override {}

chrome/browser/performance_manager/webui_graph_dump_impl.cc

@@ -208,6 +208,11 @@ void WebUIGraphDumpImpl::OnProcessNodeAdded(const ProcessNode* process_node) {
   SendProcessNotification(process_node, true);
 }
 
+void WebUIGraphDumpImpl::OnProcessLifetimeChange(
+    const ProcessNode* process_node) {
+  SendProcessNotification(process_node, false);
+}
+
 void WebUIGraphDumpImpl::OnBeforeProcessNodeRemoved(
     const ProcessNode* process_node) {
   SendDeletionNotification(process_node);

chrome/browser/performance_manager/webui_graph_dump_impl.h

@@ -71,6 +71,7 @@ class WebUIGraphDumpImpl : public mojom::WebUIGraphDump,
 
   // ProcessNodeObserver implementation:
   void OnProcessNodeAdded(const ProcessNode* process_node) override;
+  void OnProcessLifetimeChange(const ProcessNode* process_node) override;
   void OnBeforeProcessNodeRemoved(const ProcessNode* process_node) override;
   void OnExpectedTaskQueueingDurationSample(
       const ProcessNode* process_node) override {}  // Ignored.