fix: utility process exit code for graceful termination (#44698)

This commit is contained in:
Robo 2024-11-18 23:57:06 +09:00 committed by GitHub
parent d320840a54
commit 36e1a0bf00
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 11 additions and 49 deletions

View file

@ -11,7 +11,7 @@ ServiceProcessHost::Observer functions, but we need to pass the exit code to
the observer. the observer.
diff --git a/content/browser/service_process_host_impl.cc b/content/browser/service_process_host_impl.cc diff --git a/content/browser/service_process_host_impl.cc b/content/browser/service_process_host_impl.cc
index f6082bada22c5f4e70af60ea6f555b0f363919c5..228f947edbe04bce242df62080052d9c383f1709 100644 index f6082bada22c5f4e70af60ea6f555b0f363919c5..f691676a629bf82f81117599ae0bd0a4870c9f61 100644
--- a/content/browser/service_process_host_impl.cc --- a/content/browser/service_process_host_impl.cc
+++ b/content/browser/service_process_host_impl.cc +++ b/content/browser/service_process_host_impl.cc
@@ -73,12 +73,15 @@ class ServiceProcessTracker { @@ -73,12 +73,15 @@ class ServiceProcessTracker {
@ -33,19 +33,7 @@ index f6082bada22c5f4e70af60ea6f555b0f363919c5..228f947edbe04bce242df62080052d9c
processes_.erase(iter); processes_.erase(iter);
} }
@@ -87,6 +90,11 @@ class ServiceProcessTracker { @@ -154,7 +157,7 @@ class UtilityProcessClient : public UtilityProcessHost::Client {
observers_.AddObserver(observer);
}
+ bool HasObserver(ServiceProcessHost::Observer* observer) {
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
+ return observers_.HasObserver(observer);
+ }
+
void RemoveObserver(ServiceProcessHost::Observer* observer) {
// NOTE: Some tests may remove observers after BrowserThreads are shut down.
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
@@ -154,7 +162,7 @@ class UtilityProcessClient : public UtilityProcessHost::Client {
process_info_->service_process_id()); process_info_->service_process_id());
} }
@ -54,7 +42,7 @@ index f6082bada22c5f4e70af60ea6f555b0f363919c5..228f947edbe04bce242df62080052d9c
// TODO(crbug.com/40654042): It is unclear how we can observe // TODO(crbug.com/40654042): It is unclear how we can observe
// |OnProcessCrashed()| without observing |OnProcessLaunched()| first, but // |OnProcessCrashed()| without observing |OnProcessLaunched()| first, but
// it can happen on Android. Ignore the notification in this case. // it can happen on Android. Ignore the notification in this case.
@@ -162,7 +170,7 @@ class UtilityProcessClient : public UtilityProcessHost::Client { @@ -162,7 +165,7 @@ class UtilityProcessClient : public UtilityProcessHost::Client {
return; return;
GetServiceProcessTracker().NotifyCrashed( GetServiceProcessTracker().NotifyCrashed(
@ -63,18 +51,6 @@ index f6082bada22c5f4e70af60ea6f555b0f363919c5..228f947edbe04bce242df62080052d9c
} }
private: private:
@@ -232,6 +240,11 @@ void ServiceProcessHost::AddObserver(Observer* observer) {
GetServiceProcessTracker().AddObserver(observer);
}
+// static
+bool ServiceProcessHost::HasObserver(Observer* observer) {
+ return GetServiceProcessTracker().HasObserver(observer);
+}
+
// static
void ServiceProcessHost::RemoveObserver(Observer* observer) {
GetServiceProcessTracker().RemoveObserver(observer);
diff --git a/content/browser/utility_process_host.cc b/content/browser/utility_process_host.cc diff --git a/content/browser/utility_process_host.cc b/content/browser/utility_process_host.cc
index cdc2046f465110b60285da81fb0db7cdce064a59..8feca9f1c294b3de15d74dfc94508ee2a43e5fc3 100644 index cdc2046f465110b60285da81fb0db7cdce064a59..8feca9f1c294b3de15d74dfc94508ee2a43e5fc3 100644
--- a/content/browser/utility_process_host.cc --- a/content/browser/utility_process_host.cc
@ -101,23 +77,8 @@ index 66cbabae31236758eef35bab211d4874f8a5c699..88515741fa08176ba9e952759c3a52e1
}; };
// This class is self-owned. It must be instantiated using new, and shouldn't // This class is self-owned. It must be instantiated using new, and shouldn't
diff --git a/content/public/browser/service_process_host.h b/content/public/browser/service_process_host.h
index 611a52e908f4cb70fbe5628e220a082e45320b70..d7fefab99f86515007aff5f523a423a421850c47 100644
--- a/content/public/browser/service_process_host.h
+++ b/content/public/browser/service_process_host.h
@@ -235,6 +235,10 @@ class CONTENT_EXPORT ServiceProcessHost {
// removed before destruction. Must be called from the UI thread only.
static void AddObserver(Observer* observer);
+ // Returns true if the given observer is currently registered.
+ // Must be called from the UI thread only.
+ static bool HasObserver(Observer* observer);
+
// Removes a registered observer. This must be called some time before
// |*observer| is destroyed and must be called from the UI thread only.
static void RemoveObserver(Observer* observer);
diff --git a/content/public/browser/service_process_info.h b/content/public/browser/service_process_info.h diff --git a/content/public/browser/service_process_info.h b/content/public/browser/service_process_info.h
index 1a8656aef341cd3b23af588fb00569b79d6cd100..f904af7ee6bbacf4474e0939855ecf9f2c9a5eaa 100644 index 1a8656aef341cd3b23af588fb00569b79d6cd100..6af523eb27a8c1e5529721c029e5b3ba0708b9fc 100644
--- a/content/public/browser/service_process_info.h --- a/content/public/browser/service_process_info.h
+++ b/content/public/browser/service_process_info.h +++ b/content/public/browser/service_process_info.h
@@ -64,7 +64,13 @@ class CONTENT_EXPORT ServiceProcessInfo { @@ -64,7 +64,13 @@ class CONTENT_EXPORT ServiceProcessInfo {
@ -129,7 +90,7 @@ index 1a8656aef341cd3b23af588fb00569b79d6cd100..f904af7ee6bbacf4474e0939855ecf9f
+ +
private: private:
+ // The exit code of the process, if it has exited. + // The exit code of the process, if it has exited.
+ int exit_code_; + int exit_code_ = 0;
+ +
// The name of the service interface for which the process was launched. // The name of the service interface for which the process was launched.
std::string service_interface_name_; std::string service_interface_name_;

View file

@ -15,10 +15,10 @@ This CL removes these filters so the unresponsive event can still be
accessed from our JS event. The filtering is moved into Electron's code. accessed from our JS event. The filtering is moved into Electron's code.
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 319800cec84a968b0e442fc760c8e1d701bda2ed..459663af86272fe1e23a6a163e01c67fc5f8a66d 100644 index 0e742f196367bbbe8c4e147ef4db4f8dbbe4cbbe..7026b28c4f228971f74a706b6ad99777e4ca0773 100644
--- a/content/browser/web_contents/web_contents_impl.cc --- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc
@@ -9464,25 +9464,13 @@ void WebContentsImpl::RendererUnresponsive( @@ -9518,25 +9518,13 @@ void WebContentsImpl::RendererUnresponsive(
base::RepeatingClosure hang_monitor_restarter) { base::RepeatingClosure hang_monitor_restarter) {
OPTIONAL_TRACE_EVENT1("content", "WebContentsImpl::RendererUnresponsive", OPTIONAL_TRACE_EVENT1("content", "WebContentsImpl::RendererUnresponsive",
"render_widget_host", render_widget_host); "render_widget_host", render_widget_host);

View file

@ -145,7 +145,7 @@ UtilityProcessWrapper::UtilityProcessWrapper(
} }
} }
if (!content::ServiceProcessHost::HasObserver(this)) // Watch for service process termination events.
content::ServiceProcessHost::AddObserver(this); content::ServiceProcessHost::AddObserver(this);
mojo::PendingReceiver<node::mojom::NodeService> receiver = mojo::PendingReceiver<node::mojom::NodeService> receiver =

View file

@ -215,7 +215,8 @@ describe('utilityProcess module', () => {
}); });
await once(child, 'spawn'); await once(child, 'spawn');
expect(child.kill()).to.be.true(); expect(child.kill()).to.be.true();
await once(child, 'exit'); const [code] = await once(child, 'exit');
expect(code).to.equal(0);
}); });
}); });