mac: Remove two unneeded ContentMainDelegate sandbox-related methods.

DelaySandboxInitialization() is not called anywhere and is dead code.

ProcessRegistersWithSystemProcess() was used under the V1 sandbox
architecture to warm-up access to various IOKit services. Under the V2
sandbox architecture, there is no warm-up phase and the sandbox profiles
simply allow access to those resources where needed. This allows
base::PowerMonitorDeviceSource to be simplified, since the IOServices
can be directly connected to rather than pre-allocated.

Change-Id: I56769c90bd7fd9d8a5c3b8d16dc8166a53b5ce03
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2039390
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738771}

base/power_monitor/power_monitor_device_source.h

@@ -15,6 +15,13 @@
 #include <windows.h>
 #endif  // !OS_WIN
 
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+#include <IOKit/IOTypes.h>
+
+#include "base/mac/scoped_cftyperef.h"
+#include "base/mac/scoped_ionotificationportref.h"
+#endif
+
 #if defined(OS_IOS)
 #include <objc/runtime.h>
 #endif  // OS_IOS
@@ -28,18 +35,6 @@ class BASE_EXPORT PowerMonitorDeviceSource : public PowerMonitorSource {
   PowerMonitorDeviceSource();
   ~PowerMonitorDeviceSource() override;
 
-#if defined(OS_MACOSX)
-  // Allocate system resources needed by the PowerMonitor class.
-  //
-  // This function must be called before instantiating an instance of the class
-  // and before the Sandbox is initialized.
-#if !defined(OS_IOS)
-  static void AllocateSystemIOPorts();
-#else
-  static void AllocateSystemIOPorts() {}
-#endif  // OS_IOS
-#endif  // OS_MACOSX
-
 #if defined(OS_CHROMEOS)
   // On Chrome OS, Chrome receives power-related events from powerd, the system
   // power daemon, via D-Bus signals received on the UI thread. base can't
@@ -74,13 +69,35 @@ class BASE_EXPORT PowerMonitorDeviceSource : public PowerMonitorSource {
 #if defined(OS_MACOSX)
   void PlatformInit();
   void PlatformDestroy();
-#endif
+
+#if !defined(OS_IOS)
+  // Callback from IORegisterForSystemPower(). |refcon| is the |this| pointer.
+  static void SystemPowerEventCallback(void* refcon,
+                                       io_service_t service,
+                                       natural_t message_type,
+                                       void* message_argument);
+#endif  // !OS_IOS
+#endif  // OS_MACOSX
 
   // Platform-specific method to check whether the system is currently
   // running on battery power.  Returns true if running on batteries,
   // false otherwise.
   bool IsOnBatteryPowerImpl() override;
 
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+  // Reference to the system IOPMrootDomain port.
+  io_connect_t power_manager_port_ = IO_OBJECT_NULL;
+
+  // Notification port that delivers power (sleep/wake) notifications.
+  mac::ScopedIONotificationPortRef notification_port_;
+
+  // Notifier reference for the |notification_port_|.
+  io_object_t notifier_ = IO_OBJECT_NULL;
+
+  // Run loop source to observe power-source-change events.
+  ScopedCFTypeRef<CFRunLoopSourceRef> power_source_run_loop_source_;
+#endif
+
 #if defined(OS_IOS)
   // Holds pointers to system event notification observers.
   std::vector<id> notification_observers_;

base/power_monitor/power_monitor_device_source_mac.mm

@@ -53,106 +53,79 @@ bool PowerMonitorDeviceSource::IsOnBatteryPowerImpl() {
 
 namespace {
 
-io_connect_t g_system_power_io_port = 0;
-IONotificationPortRef g_notification_port_ref = 0;
-io_object_t g_notifier_object = 0;
-CFRunLoopSourceRef g_battery_status_ref = 0;
-
 void BatteryEventCallback(void*) {
   ProcessPowerEventHelper(PowerMonitorSource::POWER_STATE_EVENT);
 }
 
-void SystemPowerEventCallback(void*,
-                              io_service_t service,
-                              natural_t message_type,
-                              void* message_argument) {
-  switch (message_type) {
-    // If this message is not handled the system may delay sleep for 30 seconds.
-    case kIOMessageCanSystemSleep:
-      IOAllowPowerChange(g_system_power_io_port,
-          reinterpret_cast<intptr_t>(message_argument));
-      break;
-    case kIOMessageSystemWillSleep:
-      ProcessPowerEventHelper(base::PowerMonitorSource::SUSPEND_EVENT);
-      IOAllowPowerChange(g_system_power_io_port,
-          reinterpret_cast<intptr_t>(message_argument));
-      break;
-
-    case kIOMessageSystemWillPowerOn:
-      ProcessPowerEventHelper(PowerMonitorSource::RESUME_EVENT);
-      break;
-  }
-}
-
 }  // namespace
 
-// The reason we can't include this code in the constructor is because
-// PlatformInit() requires an active runloop and the IO port needs to be
-// allocated at sandbox initialization time, before there's a runloop.
-// See crbug.com/83783 .
-
-// static
-void PowerMonitorDeviceSource::AllocateSystemIOPorts() {
-  DCHECK_EQ(g_system_power_io_port, 0u);
-
-  // Notification port allocated by IORegisterForSystemPower.
-  g_system_power_io_port = IORegisterForSystemPower(
-      NULL, &g_notification_port_ref, SystemPowerEventCallback,
-      &g_notifier_object);
-
-  DCHECK_NE(g_system_power_io_port, 0u);
-}
-
 void PowerMonitorDeviceSource::PlatformInit() {
-  // Need to call AllocateSystemIOPorts() before creating a PowerMonitor
-  // object.
-  DCHECK_NE(g_system_power_io_port, 0u);
-  if (g_system_power_io_port == 0)
-    return;
+  power_manager_port_ = IORegisterForSystemPower(
+      this,
+      mac::ScopedIONotificationPortRef::Receiver(notification_port_).get(),
+      &SystemPowerEventCallback, &notifier_);
+  DCHECK_NE(power_manager_port_, IO_OBJECT_NULL);
 
-  // Add the notification port and battery monitor to the application runloop
+  // Add the sleep/wake notification event source to the runloop.
   CFRunLoopAddSource(
       CFRunLoopGetCurrent(),
-      IONotificationPortGetRunLoopSource(g_notification_port_ref),
+      IONotificationPortGetRunLoopSource(notification_port_.get()),
       kCFRunLoopCommonModes);
 
-  base::ScopedCFTypeRef<CFRunLoopSourceRef> battery_status_ref(
-      IOPSNotificationCreateRunLoopSource(BatteryEventCallback, nullptr));
-  DCHECK(battery_status_ref);  // crbug.com/897557
+  // Create and add the power-source-change event source to the runloop.
+  power_source_run_loop_source_.reset(
+      IOPSNotificationCreateRunLoopSource(&BatteryEventCallback, nullptr));
+  // Verify that the source was created. This may fail if the sandbox does not
+  // permit the process to access the underlying system service. See
+  // https://crbug.com/897557 for an example of such a configuration bug.
+  DCHECK(power_source_run_loop_source_);
 
-  CFRunLoopAddSource(CFRunLoopGetCurrent(), battery_status_ref,
+  CFRunLoopAddSource(CFRunLoopGetCurrent(), power_source_run_loop_source_,
                      kCFRunLoopDefaultMode);
-  g_battery_status_ref = battery_status_ref.release();
 }
 
 void PowerMonitorDeviceSource::PlatformDestroy() {
-  DCHECK_NE(g_system_power_io_port, 0u);
-  if (g_system_power_io_port == 0)
-    return;
-
-  // Remove the sleep notification port from the application runloop
   CFRunLoopRemoveSource(
       CFRunLoopGetCurrent(),
-      IONotificationPortGetRunLoopSource(g_notification_port_ref),
+      IONotificationPortGetRunLoopSource(notification_port_.get()),
       kCFRunLoopCommonModes);
 
-  base::ScopedCFTypeRef<CFRunLoopSourceRef> battery_status_ref(
-      g_battery_status_ref);
-  CFRunLoopRemoveSource(CFRunLoopGetCurrent(), g_battery_status_ref,
+  CFRunLoopRemoveSource(CFRunLoopGetCurrent(),
+                        power_source_run_loop_source_.get(),
                         kCFRunLoopDefaultMode);
-  g_battery_status_ref = 0;
 
-  // Deregister for system sleep notifications
-  IODeregisterForSystemPower(&g_notifier_object);
+  // Deregister for system power notifications.
+  IODeregisterForSystemPower(&notifier_);
 
-  // IORegisterForSystemPower implicitly opens the Root Power Domain IOService,
-  // so we close it here.
-  IOServiceClose(g_system_power_io_port);
+  // Close the connection to the IOPMrootDomain that was opened in
+  // PlatformInit().
+  IOServiceClose(power_manager_port_);
+  power_manager_port_ = IO_OBJECT_NULL;
+}
 
-  g_system_power_io_port = 0;
+void PowerMonitorDeviceSource::SystemPowerEventCallback(
+    void* refcon,
+    io_service_t service,
+    natural_t message_type,
+    void* message_argument) {
+  auto* thiz = static_cast<PowerMonitorDeviceSource*>(refcon);
 
-  // Destroy the notification port allocated by IORegisterForSystemPower.
-  IONotificationPortDestroy(g_notification_port_ref);
+  switch (message_type) {
+    // If this message is not handled the system may delay sleep for 30 seconds.
+    case kIOMessageCanSystemSleep:
+      IOAllowPowerChange(thiz->power_manager_port_,
+                         reinterpret_cast<intptr_t>(message_argument));
+      break;
+    case kIOMessageSystemWillSleep:
+      ProcessPowerEventHelper(PowerMonitorSource::SUSPEND_EVENT);
+      IOAllowPowerChange(thiz->power_manager_port_,
+                         reinterpret_cast<intptr_t>(message_argument));
+      break;
+
+    case kIOMessageSystemWillPowerOn:
+      ProcessPowerEventHelper(PowerMonitorSource::RESUME_EVENT);
+      break;
+  }
 }
 
 }  // namespace base

chrome/app/chrome_main_delegate.cc

@@ -1133,27 +1133,7 @@ void ChromeMainDelegate::ProcessExiting(const std::string& process_type) {
 #endif  // !defined(OS_ANDROID)
 }
 
-#if defined(OS_MACOSX)
-bool ChromeMainDelegate::ProcessRegistersWithSystemProcess(
-    const std::string& process_type) {
-#if !BUILDFLAG(ENABLE_NACL)
-  return false;
-#else
-  return process_type == switches::kNaClLoaderProcess;
-#endif
-}
-
-bool ChromeMainDelegate::DelaySandboxInitialization(
-    const std::string& process_type) {
-#if BUILDFLAG(ENABLE_NACL)
-  // NaClLoader does this in NaClMainPlatformDelegate::EnableSandbox().
-  // No sandbox needed for relauncher.
-  if (process_type == switches::kNaClLoaderProcess)
-    return true;
-#endif
-  return process_type == switches::kRelauncherProcess;
-}
-#elif defined(OS_LINUX)
+#if defined(OS_LINUX)
 void ChromeMainDelegate::ZygoteStarting(
     std::vector<std::unique_ptr<service_manager::ZygoteForkDelegate>>*
         delegates) {

chrome/app/chrome_main_delegate.h

@@ -51,11 +51,7 @@ class ChromeMainDelegate : public content::ContentMainDelegate {
       const std::string& process_type,
       const content::MainFunctionParams& main_function_params) override;
   void ProcessExiting(const std::string& process_type) override;
-#if defined(OS_MACOSX)
-  bool ProcessRegistersWithSystemProcess(
-      const std::string& process_type) override;
-  bool DelaySandboxInitialization(const std::string& process_type) override;
-#elif defined(OS_LINUX)
+#if defined(OS_LINUX)
   void ZygoteStarting(
       std::vector<std::unique_ptr<service_manager::ZygoteForkDelegate>>*
           delegates) override;

chrome/browser/extensions/event_router_forwarder_unittest.cc

@@ -89,9 +89,6 @@ class EventRouterForwarderTest : public testing::Test {
   EventRouterForwarderTest()
       : task_environment_(content::BrowserTaskEnvironment::REAL_IO_THREAD),
         profile_manager_(TestingBrowserProcess::GetGlobal()) {
-#if defined(OS_MACOSX)
-    base::PowerMonitorDeviceSource::AllocateSystemIOPorts();
-#endif
     std::unique_ptr<base::PowerMonitorSource> power_monitor_source(
         new base::PowerMonitorDeviceSource());
     base::PowerMonitor::Initialize(std::move(power_monitor_source));

content/app/content_main_runner_impl.cc

@@ -693,17 +693,6 @@ int ContentMainRunnerImpl::Initialize(const ContentMainParams& params) {
     TRACE_EVENT0("startup,benchmark,rail", "ContentMainRunnerImpl::Initialize");
 #endif  // !OS_ANDROID
 
-#if defined(OS_MACOSX)
-    // We need to allocate the IO Ports before the Sandbox is initialized or
-    // the first instance of PowerMonitor is created.
-    // It's important not to allocate the ports for processes which don't
-    // register with the power monitor - see https://crbug.com/88867.
-    if (process_type.empty() ||
-        delegate_->ProcessRegistersWithSystemProcess(process_type)) {
-      base::PowerMonitorDeviceSource::AllocateSystemIOPorts();
-    }
-#endif
-
     // If we are on a platform where the default allocator is overridden (shim
     // layer on windows, tcmalloc on Linux Desktop) smoke-tests that the
     // overriding logic is working correctly. If not causes a hard crash, as its

content/public/app/content_main_delegate.cc

@@ -27,19 +27,7 @@ int ContentMainDelegate::RunProcess(
   return -1;
 }
 
-#if defined(OS_MACOSX)
-
-bool ContentMainDelegate::ProcessRegistersWithSystemProcess(
-    const std::string& process_type) {
-  return false;
-}
-
-bool ContentMainDelegate::DelaySandboxInitialization(
-    const std::string& process_type) {
-  return false;
-}
-
-#elif defined(OS_LINUX)
+#if defined(OS_LINUX)
 
 void ContentMainDelegate::ZygoteStarting(
     std::vector<std::unique_ptr<service_manager::ZygoteForkDelegate>>*

content/public/app/content_main_delegate.h

@@ -60,19 +60,7 @@ class CONTENT_EXPORT ContentMainDelegate {
   // Called right before the process exits.
   virtual void ProcessExiting(const std::string& process_type) {}
 
-#if defined(OS_MACOSX)
-  // Returns true if the process registers with the system monitor, so that we
-  // can allocate an IO port for it before the sandbox is initialized. Embedders
-  // are called only for process types that content doesn't know about.
-  virtual bool ProcessRegistersWithSystemProcess(
-      const std::string& process_type);
-
-  // Allows the embedder to override initializing the sandbox. This is needed
-  // because some processes might not want to enable it right away or might not
-  // want it at all.
-  virtual bool DelaySandboxInitialization(const std::string& process_type);
-
-#elif defined(OS_LINUX)
+#if defined(OS_LINUX)
   // Tells the embedder that the zygote process is starting, and allows it to
   // specify one or more zygote delegates if it wishes by storing them in
   // |*delegates|.