chore: Revert "fix: utility process exit code for graceful termination" (#44717)
Revert "fix: utility process exit code for graceful termination (#44698)"
This reverts commit 36e1a0bf00.
	
	
This commit is contained in:
		
					parent
					
						
							
								10d967028a
							
						
					
				
			
			
				commit
				
					
						1cae73ba09
					
				
			
		
					 4 changed files with 49 additions and 11 deletions
				
			
		| 
						 | 
				
			
			@ -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..f691676a629bf82f81117599ae0bd0a4870c9f61 100644
 | 
			
		||||
index f6082bada22c5f4e70af60ea6f555b0f363919c5..228f947edbe04bce242df62080052d9c383f1709 100644
 | 
			
		||||
--- a/content/browser/service_process_host_impl.cc
 | 
			
		||||
+++ b/content/browser/service_process_host_impl.cc
 | 
			
		||||
@@ -73,12 +73,15 @@ class ServiceProcessTracker {
 | 
			
		||||
| 
						 | 
				
			
			@ -33,7 +33,19 @@ index f6082bada22c5f4e70af60ea6f555b0f363919c5..f691676a629bf82f81117599ae0bd0a4
 | 
			
		|||
     processes_.erase(iter);
 | 
			
		||||
   }
 | 
			
		||||
 
 | 
			
		||||
@@ -154,7 +157,7 @@ class UtilityProcessClient : public UtilityProcessHost::Client {
 | 
			
		||||
@@ -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 {
 | 
			
		||||
         process_info_->service_process_id());
 | 
			
		||||
   }
 | 
			
		||||
 
 | 
			
		||||
| 
						 | 
				
			
			@ -42,7 +54,7 @@ index f6082bada22c5f4e70af60ea6f555b0f363919c5..f691676a629bf82f81117599ae0bd0a4
 | 
			
		|||
     // 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 +165,7 @@ class UtilityProcessClient : public UtilityProcessHost::Client {
 | 
			
		||||
@@ -162,7 +170,7 @@ class UtilityProcessClient : public UtilityProcessHost::Client {
 | 
			
		||||
       return;
 | 
			
		||||
 
 | 
			
		||||
     GetServiceProcessTracker().NotifyCrashed(
 | 
			
		||||
| 
						 | 
				
			
			@ -51,6 +63,18 @@ index f6082bada22c5f4e70af60ea6f555b0f363919c5..f691676a629bf82f81117599ae0bd0a4
 | 
			
		|||
   }
 | 
			
		||||
 
 | 
			
		||||
  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
 | 
			
		||||
| 
						 | 
				
			
			@ -77,8 +101,23 @@ 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..6af523eb27a8c1e5529721c029e5b3ba0708b9fc 100644
 | 
			
		||||
index 1a8656aef341cd3b23af588fb00569b79d6cd100..f904af7ee6bbacf4474e0939855ecf9f2c9a5eaa 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 {
 | 
			
		||||
| 
						 | 
				
			
			@ -90,7 +129,7 @@ index 1a8656aef341cd3b23af588fb00569b79d6cd100..6af523eb27a8c1e5529721c029e5b3ba
 | 
			
		|||
+
 | 
			
		||||
  private:
 | 
			
		||||
+  // The exit code of the process, if it has exited.
 | 
			
		||||
+  int exit_code_ = 0;
 | 
			
		||||
+  int exit_code_;
 | 
			
		||||
+
 | 
			
		||||
   // The name of the service interface for which the process was launched.
 | 
			
		||||
   std::string service_interface_name_;
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -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.
 | 
			
		||||
 | 
			
		||||
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
 | 
			
		||||
index 0e742f196367bbbe8c4e147ef4db4f8dbbe4cbbe..7026b28c4f228971f74a706b6ad99777e4ca0773 100644
 | 
			
		||||
index 319800cec84a968b0e442fc760c8e1d701bda2ed..459663af86272fe1e23a6a163e01c67fc5f8a66d 100644
 | 
			
		||||
--- a/content/browser/web_contents/web_contents_impl.cc
 | 
			
		||||
+++ b/content/browser/web_contents/web_contents_impl.cc
 | 
			
		||||
@@ -9518,25 +9518,13 @@ void WebContentsImpl::RendererUnresponsive(
 | 
			
		||||
@@ -9464,25 +9464,13 @@ void WebContentsImpl::RendererUnresponsive(
 | 
			
		||||
     base::RepeatingClosure hang_monitor_restarter) {
 | 
			
		||||
   OPTIONAL_TRACE_EVENT1("content", "WebContentsImpl::RendererUnresponsive",
 | 
			
		||||
                         "render_widget_host", render_widget_host);
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -145,8 +145,8 @@ UtilityProcessWrapper::UtilityProcessWrapper(
 | 
			
		|||
    }
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  // Watch for service process termination events.
 | 
			
		||||
  content::ServiceProcessHost::AddObserver(this);
 | 
			
		||||
  if (!content::ServiceProcessHost::HasObserver(this))
 | 
			
		||||
    content::ServiceProcessHost::AddObserver(this);
 | 
			
		||||
 | 
			
		||||
  mojo::PendingReceiver<node::mojom::NodeService> receiver =
 | 
			
		||||
      node_service_remote_.BindNewPipeAndPassReceiver();
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -215,8 +215,7 @@ describe('utilityProcess module', () => {
 | 
			
		|||
      });
 | 
			
		||||
      await once(child, 'spawn');
 | 
			
		||||
      expect(child.kill()).to.be.true();
 | 
			
		||||
      const [code] = await once(child, 'exit');
 | 
			
		||||
      expect(code).to.equal(0);
 | 
			
		||||
      await once(child, 'exit');
 | 
			
		||||
    });
 | 
			
		||||
  });
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue