diff --git a/patches/chromium/feat_enable_passing_exit_code_on_service_process_crash.patch b/patches/chromium/feat_enable_passing_exit_code_on_service_process_crash.patch index 4d7eb0de3f..f9b2591d1e 100644 --- a/patches/chromium/feat_enable_passing_exit_code_on_service_process_crash.patch +++ b/patches/chromium/feat_enable_passing_exit_code_on_service_process_crash.patch @@ -11,7 +11,7 @@ ServiceProcessHost::Observer functions, but we need to pass the exit code to the observer. 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 +++ b/content/browser/service_process_host_impl.cc @@ -73,12 +73,15 @@ class ServiceProcessTracker { @@ -33,19 +33,7 @@ index f6082bada22c5f4e70af60ea6f555b0f363919c5..228f947edbe04bce242df62080052d9c processes_.erase(iter); } -@@ -87,6 +90,11 @@ class ServiceProcessTracker { - 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 { +@@ -154,7 +157,7 @@ class UtilityProcessClient : public UtilityProcessHost::Client { process_info_->service_process_id()); } @@ -54,7 +42,7 @@ index f6082bada22c5f4e70af60ea6f555b0f363919c5..228f947edbe04bce242df62080052d9c // TODO(crbug.com/40654042): It is unclear how we can observe // |OnProcessCrashed()| without observing |OnProcessLaunched()| first, but // 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; GetServiceProcessTracker().NotifyCrashed( @@ -63,18 +51,6 @@ index f6082bada22c5f4e70af60ea6f555b0f363919c5..228f947edbe04bce242df62080052d9c } 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 index cdc2046f465110b60285da81fb0db7cdce064a59..8feca9f1c294b3de15d74dfc94508ee2a43e5fc3 100644 --- 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 -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 -index 1a8656aef341cd3b23af588fb00569b79d6cd100..f904af7ee6bbacf4474e0939855ecf9f2c9a5eaa 100644 +index 1a8656aef341cd3b23af588fb00569b79d6cd100..6af523eb27a8c1e5529721c029e5b3ba0708b9fc 100644 --- a/content/public/browser/service_process_info.h +++ b/content/public/browser/service_process_info.h @@ -64,7 +64,13 @@ class CONTENT_EXPORT ServiceProcessInfo { @@ -129,7 +90,7 @@ index 1a8656aef341cd3b23af588fb00569b79d6cd100..f904af7ee6bbacf4474e0939855ecf9f + private: + // 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. std::string service_interface_name_; diff --git a/shell/browser/api/electron_api_utility_process.cc b/shell/browser/api/electron_api_utility_process.cc index 5c1fd14be0..de128ddd52 100644 --- a/shell/browser/api/electron_api_utility_process.cc +++ b/shell/browser/api/electron_api_utility_process.cc @@ -145,8 +145,8 @@ UtilityProcessWrapper::UtilityProcessWrapper( } } - if (!content::ServiceProcessHost::HasObserver(this)) - content::ServiceProcessHost::AddObserver(this); + // Watch for service process termination events. + content::ServiceProcessHost::AddObserver(this); mojo::PendingReceiver receiver = node_service_remote_.BindNewPipeAndPassReceiver(); @@ -258,6 +258,23 @@ void UtilityProcessWrapper::HandleTermination(uint64_t exit_code) { pid_ = base::kNullProcessId; CloseConnectorPort(); + if (killed_) { +#if BUILDFLAG(IS_POSIX) + // UtilityProcessWrapper::Kill relies on base::Process::Terminate + // to gracefully shutdown the process which is performed by sending + // SIGTERM signal. When listening for exit events via ServiceProcessHost + // observers, the exit code on posix is obtained via + // BrowserChildProcessHostImpl::GetTerminationInfo which inturn relies + // on waitpid to extract the exit signal. If the process is unavailable, + // then the exit_code will be set to 0, otherwise we get the signal that + // was sent during the base::Process::Terminate call. For a user, this is + // still a graceful shutdown case so lets' convert the exit code to the + // expected value. + if (exit_code == SIGTERM || exit_code == SIGKILL) { + exit_code = 0; + } +#endif + } EmitWithoutEvent("exit", exit_code); Unpin(); } @@ -337,7 +354,7 @@ void UtilityProcessWrapper::PostMessage(gin::Arguments* args) { connector_->Accept(&mojo_message); } -bool UtilityProcessWrapper::Kill() const { +bool UtilityProcessWrapper::Kill() { if (pid_ == base::kNullProcessId) return false; base::Process process = base::Process::Open(pid_); @@ -350,6 +367,7 @@ bool UtilityProcessWrapper::Kill() const { // process reap should be signaled through the zygote via // content::ZygoteCommunication::EnsureProcessTerminated. base::EnsureProcessTerminated(std::move(process)); + killed_ = result; return result; } diff --git a/shell/browser/api/electron_api_utility_process.h b/shell/browser/api/electron_api_utility_process.h index 7d5eae1710..c7fb48b0dd 100644 --- a/shell/browser/api/electron_api_utility_process.h +++ b/shell/browser/api/electron_api_utility_process.h @@ -76,7 +76,7 @@ class UtilityProcessWrapper final void HandleTermination(uint64_t exit_code); void PostMessage(gin::Arguments* args); - bool Kill() const; + bool Kill(); v8::Local GetOSProcessId(v8::Isolate* isolate) const; // mojo::MessageReceiver @@ -106,6 +106,7 @@ class UtilityProcessWrapper final int stderr_read_fd_ = -1; bool connector_closed_ = false; bool terminated_ = false; + bool killed_ = false; std::unique_ptr connector_; blink::MessagePortDescriptor host_port_; mojo::Receiver receiver_{this}; diff --git a/spec/api-utility-process-spec.ts b/spec/api-utility-process-spec.ts index 3fbc67b7df..89bb3c1c9a 100644 --- a/spec/api-utility-process-spec.ts +++ b/spec/api-utility-process-spec.ts @@ -200,7 +200,8 @@ describe('utilityProcess module', () => { }); await once(child, 'spawn'); expect(child.kill()).to.be.true(); - await once(child, 'exit'); + const [code] = await once(child, 'exit'); + expect(code).to.equal(0); }); });