Browse Source

refactor: simplify child process tracking for app.getAppMetrics() (#26657)

Milan Burda 4 years ago
parent
commit
4e3e3d414d

+ 10 - 11
shell/browser/api/electron_api_app.cc

@@ -595,7 +595,7 @@ App::App() {
       ->set_delegate(this);
   Browser::Get()->AddObserver(this);
 
-  base::ProcessId pid = base::GetCurrentProcId();
+  auto pid = content::ChildProcessHost::kInvalidUniqueID;
   auto process_metric = std::make_unique<electron::ProcessMetric>(
       content::PROCESS_TYPE_BROWSER, base::GetCurrentProcessHandle(),
       base::ProcessMetrics::CreateCurrentProcessMetrics());
@@ -835,26 +835,26 @@ void App::OnGpuProcessCrashed(base::TerminationStatus status) {
 
 void App::BrowserChildProcessLaunchedAndConnected(
     const content::ChildProcessData& data) {
-  ChildProcessLaunched(data.process_type, data.GetProcess().Handle(),
+  ChildProcessLaunched(data.process_type, data.id, data.GetProcess().Handle(),
                        data.metrics_name, base::UTF16ToUTF8(data.name));
 }
 
 void App::BrowserChildProcessHostDisconnected(
     const content::ChildProcessData& data) {
-  ChildProcessDisconnected(base::GetProcId(data.GetProcess().Handle()));
+  ChildProcessDisconnected(data.id);
 }
 
 void App::BrowserChildProcessCrashed(
     const content::ChildProcessData& data,
     const content::ChildProcessTerminationInfo& info) {
-  ChildProcessDisconnected(base::GetProcId(data.GetProcess().Handle()));
+  ChildProcessDisconnected(data.id);
   BrowserChildProcessCrashedOrKilled(data, info);
 }
 
 void App::BrowserChildProcessKilled(
     const content::ChildProcessData& data,
     const content::ChildProcessTerminationInfo& info) {
-  ChildProcessDisconnected(base::GetProcId(data.GetProcess().Handle()));
+  ChildProcessDisconnected(data.id);
   BrowserChildProcessCrashedOrKilled(data, info);
 }
 
@@ -875,7 +875,7 @@ void App::BrowserChildProcessCrashedOrKilled(
 }
 
 void App::RenderProcessReady(content::RenderProcessHost* host) {
-  ChildProcessLaunched(content::PROCESS_TYPE_RENDERER,
+  ChildProcessLaunched(content::PROCESS_TYPE_RENDERER, host->GetID(),
                        host->GetProcess().Handle());
 
   // TODO(jeremy): this isn't really the right place to be creating
@@ -890,16 +890,15 @@ void App::RenderProcessReady(content::RenderProcessHost* host) {
   }
 }
 
-void App::RenderProcessDisconnected(base::ProcessId host_pid) {
-  ChildProcessDisconnected(host_pid);
+void App::RenderProcessExited(content::RenderProcessHost* host) {
+  ChildProcessDisconnected(host->GetID());
 }
 
 void App::ChildProcessLaunched(int process_type,
+                               int pid,
                                base::ProcessHandle handle,
                                const std::string& service_name,
                                const std::string& name) {
-  auto pid = base::GetProcId(handle);
-
 #if defined(OS_MAC)
   auto metrics = base::ProcessMetrics::CreateProcessMetrics(
       handle, content::BrowserChildProcessHost::GetPortProvider());
@@ -910,7 +909,7 @@ void App::ChildProcessLaunched(int process_type,
       process_type, handle, std::move(metrics), service_name, name);
 }
 
-void App::ChildProcessDisconnected(base::ProcessId pid) {
+void App::ChildProcessDisconnected(int pid) {
   app_metrics_.erase(pid);
 }
 

+ 5 - 5
shell/browser/api/electron_api_app.h

@@ -5,9 +5,9 @@
 #ifndef SHELL_BROWSER_API_ELECTRON_API_APP_H_
 #define SHELL_BROWSER_API_ELECTRON_API_APP_H_
 
+#include <map>
 #include <memory>
 #include <string>
-#include <unordered_map>
 #include <utility>
 #include <vector>
 
@@ -74,7 +74,7 @@ class App : public ElectronBrowserClient::Delegate,
 
   base::FilePath GetAppPath() const;
   void RenderProcessReady(content::RenderProcessHost* host);
-  void RenderProcessDisconnected(base::ProcessId host_pid);
+  void RenderProcessExited(content::RenderProcessHost* host);
 
   App();
 
@@ -167,10 +167,11 @@ class App : public ElectronBrowserClient::Delegate,
 
   void SetAppPath(const base::FilePath& app_path);
   void ChildProcessLaunched(int process_type,
+                            int pid,
                             base::ProcessHandle handle,
                             const std::string& service_name = std::string(),
                             const std::string& name = std::string());
-  void ChildProcessDisconnected(base::ProcessId pid);
+  void ChildProcessDisconnected(int pid);
 
   void SetAppLogsPath(gin_helper::ErrorThrower thrower,
                       base::Optional<base::FilePath> custom_path);
@@ -250,8 +251,7 @@ class App : public ElectronBrowserClient::Delegate,
   base::FilePath app_path_;
 
   using ProcessMetricMap =
-      std::unordered_map<base::ProcessId,
-                         std::unique_ptr<electron::ProcessMetric>>;
+      std::map<int, std::unique_ptr<electron::ProcessMetric>>;
   ProcessMetricMap app_metrics_;
 
   bool disable_hw_acceleration_ = false;

+ 2 - 9
shell/browser/electron_browser_client.cc

@@ -1125,8 +1125,6 @@ void ElectronBrowserClient::RenderProcessHostDestroyed(
 
 void ElectronBrowserClient::RenderProcessReady(
     content::RenderProcessHost* host) {
-  render_process_host_pids_[host->GetID()] =
-      base::GetProcId(host->GetProcess().Handle());
   if (delegate_) {
     static_cast<api::App*>(delegate_)->RenderProcessReady(host);
   }
@@ -1135,13 +1133,8 @@ void ElectronBrowserClient::RenderProcessReady(
 void ElectronBrowserClient::RenderProcessExited(
     content::RenderProcessHost* host,
     const content::ChildProcessTerminationInfo& info) {
-  auto host_pid = render_process_host_pids_.find(host->GetID());
-  if (host_pid != render_process_host_pids_.end()) {
-    if (delegate_) {
-      static_cast<api::App*>(delegate_)->RenderProcessDisconnected(
-          host_pid->second);
-    }
-    render_process_host_pids_.erase(host_pid);
+  if (delegate_) {
+    static_cast<api::App*>(delegate_)->RenderProcessExited(host);
   }
 }
 

+ 0 - 2
shell/browser/electron_browser_client.h

@@ -315,8 +315,6 @@ class ElectronBrowserClient : public content::ContentBrowserClient,
   // pending_render_process => web contents.
   std::map<int, content::WebContents*> pending_processes_;
 
-  std::map<int, base::ProcessId> render_process_host_pids_;
-
   std::set<int> renderer_is_subframe_;
 
   // list of site per affinity. weak_ptr to prevent instance locking