From cb205c15423acea1a421bd7cd33b543c9c2c39de Mon Sep 17 00:00:00 2001
From: Haik Aftandilian <haftandilian@mozilla.com>
Date: Fri, 29 Mar 2019 13:47:44 +0000
Subject: [PATCH] Bug 1537940 - [Mac] With content sandbox disabled, processes
 "Not Responding" in Activity Monitor r=Alex_Gaynor

Make sure CGSShutdownServerConnections() is called regardless of whether or not the sandbox is enabled.

Differential Revision: https://phabricator.services.mozilla.com/D24794

--HG--
extra : moz-landing-system : lando
---
 dom/ipc/ContentChild.cpp  | 32 ++++++++++++++++++++++++++------
 dom/ipc/ContentParent.cpp |  4 +++-
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/dom/ipc/ContentChild.cpp b/dom/ipc/ContentChild.cpp
index b9813494ea99..7349db717aef 100644
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -1405,6 +1405,12 @@ mozilla::ipc::IPCResult ContentChild::RecvRequestPerformanceMetrics(
   return IPC_OK();
 }
 
+#if defined(XP_MACOSX)
+extern "C" {
+void CGSShutdownServerConnections();
+};
+#endif
+
 mozilla::ipc::IPCResult ContentChild::RecvInitRendering(
     Endpoint<PCompositorManagerChild>&& aCompositor,
     Endpoint<PImageBridgeChild>&& aImageBridge,
@@ -1434,6 +1440,16 @@ mozilla::ipc::IPCResult ContentChild::RecvInitRendering(
     return GetResultForRenderingInitFailure(aVRBridge.OtherPid());
   }
   VideoDecoderManagerChild::InitForContent(std::move(aVideoManager));
+
+#if defined(XP_MACOSX) && !defined(MOZ_SANDBOX)
+  // Close all current connections to the WindowServer. This ensures that the
+  // Activity Monitor will not label the content process as "Not responding"
+  // because it's not running a native event loop. See bug 1384336. When the
+  // build is configured with sandbox support, this is called during sandbox
+  // setup.
+  CGSShutdownServerConnections();
+#endif
+
   return IPC_OK();
 }
 
@@ -1502,20 +1518,24 @@ mozilla::ipc::IPCResult ContentChild::RecvReinitRenderingForDeviceReset() {
 #if defined(XP_MACOSX) && defined(MOZ_SANDBOX)
 extern "C" {
 CGError CGSSetDenyWindowServerConnections(bool);
-void CGSShutdownServerConnections();
 };
 
 static bool StartMacOSContentSandbox() {
+  // Close all current connections to the WindowServer. This ensures that the
+  // Activity Monitor will not label the content process as "Not responding"
+  // because it's not running a native event loop. See bug 1384336.
+  // This is required with or without the sandbox enabled. Until the
+  // window server is blocked as the policy level, this should be called
+  // just before CGSSetDenyWindowServerConnections() so there are no
+  // windowserver connections active when CGSSetDenyWindowServerConnections()
+  // is called.
+  CGSShutdownServerConnections();
+
   int sandboxLevel = GetEffectiveContentSandboxLevel();
   if (sandboxLevel < 1) {
     return false;
   }
 
-  // Close all current connections to the WindowServer. This ensures that the
-  // Activity Monitor will not label the content process as "Not responding"
-  // because it's not running a native event loop. See bug 1384336.
-  CGSShutdownServerConnections();
-
   // Actual security benefits are only acheived when we additionally deny
   // future connections, however this currently breaks WebGL so it's not done
   // by default.
diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp
index 50e617cbdd9e..f6a22a8502e1 100644
--- a/dom/ipc/ContentParent.cpp
+++ b/dom/ipc/ContentParent.cpp
@@ -2588,8 +2588,10 @@ void ContentParent::InitInternal(ProcessPriority aInitialPriority) {
   // during an active session. Currently the pref is only used for testing
   // purpose. If the decision is made to permanently rely on the pref, this
   // should be changed so that it is required to restart firefox for the change
-  // of value to take effect.
+  // of value to take effect. Always send SetProcessSandbox message on macOS.
+#  if !defined(XP_MACOSX)
   shouldSandbox = IsContentSandboxEnabled();
+#  endif
 
 #  ifdef XP_LINUX
   if (shouldSandbox) {