Mac: Split SystemMonitor initialization so it's not blocked by the Sandbox.

In OS X 10.7 Lion, the IO port we listen on to monitor system power events is blocked by the Sandbox.

Move the allocation of the IO port so it happens early on during Chrome startup.

BUG=83783
TEST=When putting a system to sleep with Chrome running, the browser shouldn't crash.  Also, need to monitor crash logs to see that this crash goes away.

Review URL: http://codereview.chromium.org/7235023

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@91462 0039d316-1c4b-4281-b951-d872f2087c98

base/system_monitor/system_monitor.h

@@ -50,6 +50,14 @@ class BASE_API SystemMonitor {
   // Get the application-wide SystemMonitor (if not present, returns NULL).
   static SystemMonitor* Get();
 
+#if defined(OS_MACOSX)
+  // Allocate system resources needed by the SystemMonitor class.
+  //
+  // This function must be called before instantiating an instance of the class
+  // and before the Sandbox is initialized.
+  static void AllocateSystemIOPorts();
+#endif
+
   //
   // Power-related APIs
   //
@@ -130,11 +138,6 @@ class BASE_API SystemMonitor {
   base::OneShotTimer<SystemMonitor> delayed_battery_check_;
 #endif
 
-#if defined(OS_MACOSX)
-  IONotificationPortRef notification_port_ref_;
-  io_object_t notifier_object_;
-#endif
-
   DISALLOW_COPY_AND_ASSIGN(SystemMonitor);
 };
 

base/system_monitor/system_monitor_mac.mm

@@ -15,13 +15,15 @@ namespace base {
 namespace {
 
 io_connect_t g_system_power_io_port = 0;
+IONotificationPortRef g_notification_port_ref = 0;
+io_object_t g_notifier_object = 0;
 
-void SystemPowerEventCallback(void* system_monitor,
+void SystemPowerEventCallback(void*,
                               io_service_t service,
                               natural_t message_type,
                               void* message_argument) {
-  DCHECK(system_monitor);
-  SystemMonitor* sys_monitor = reinterpret_cast<SystemMonitor*>(system_monitor);
+  SystemMonitor* sys_monitor = SystemMonitor::Get();
+  DCHECK(sys_monitor);
   switch (message_type) {
     case kIOMessageSystemWillSleep:
       sys_monitor->ProcessPowerMessage(SystemMonitor::SUSPEND_EVENT);
@@ -37,22 +39,36 @@ void SystemPowerEventCallback(void* system_monitor,
 
 }  // namespace
 
-void SystemMonitor::PlatformInit() {
+// 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 SystemMonitor::AllocateSystemIOPorts() {
   DCHECK_EQ(g_system_power_io_port, 0u);
 
   // Notification port allocated by IORegisterForSystemPower.
 
   g_system_power_io_port = IORegisterForSystemPower(
-      this, &notification_port_ref_, SystemPowerEventCallback,
-      &notifier_object_);
+      NULL, &g_notification_port_ref, SystemPowerEventCallback,
+      &g_notifier_object);
+
+  DCHECK_NE(g_system_power_io_port, 0u);
+}
+
+void SystemMonitor::PlatformInit() {
+  // Need to call AllocateSystemIOPorts() before constructing a SystemMonitor
+  // object.
   DCHECK_NE(g_system_power_io_port, 0u);
   if (g_system_power_io_port == 0)
     return;
 
   // Add the notification port to the application runloop
-  CFRunLoopAddSource(CFRunLoopGetCurrent(),
-                     IONotificationPortGetRunLoopSource(notification_port_ref_),
-                     kCFRunLoopCommonModes);
+  CFRunLoopAddSource(
+      CFRunLoopGetCurrent(),
+      IONotificationPortGetRunLoopSource(g_notification_port_ref),
+      kCFRunLoopCommonModes);
 }
 
 void SystemMonitor::PlatformDestroy() {
@@ -63,11 +79,11 @@ void SystemMonitor::PlatformDestroy() {
   // Remove the sleep notification port from the application runloop
   CFRunLoopRemoveSource(
       CFRunLoopGetCurrent(),
-      IONotificationPortGetRunLoopSource(notification_port_ref_),
+      IONotificationPortGetRunLoopSource(g_notification_port_ref),
       kCFRunLoopCommonModes);
 
   // Deregister for system sleep notifications
-  IODeregisterForSystemPower(&notifier_object_);
+  IODeregisterForSystemPower(&g_notifier_object);
 
   // IORegisterForSystemPower implicitly opens the Root Power Domain IOService,
   // so we close it here.
@@ -76,7 +92,7 @@ void SystemMonitor::PlatformDestroy() {
   g_system_power_io_port = 0;
 
   // Destroy the notification port allocated by IORegisterForSystemPower.
-  IONotificationPortDestroy(notification_port_ref_);
+  IONotificationPortDestroy(g_notification_port_ref);
 }
 
 }  // namespace base

base/system_monitor/system_monitor_unittest.cc

@@ -48,6 +48,10 @@ TEST(SystemMonitor, PowerNotifications) {
   // Initialize a message loop for this to run on.
   MessageLoop loop;
 
+#if defined(OS_MACOSX)
+  SystemMonitor::AllocateSystemIOPorts();
+#endif
+
   SystemMonitor system_monitor;
   PowerTest test[kObservers];
   for (int index = 0; index < kObservers; ++index)

chrome/app/chrome_main.cc

@@ -60,6 +60,7 @@
 #include "base/mac/mac_util.h"
 #include "base/mac/os_crash_dumps.h"
 #include "base/mach_ipc_mac.h"
+#include "base/system_monitor/system_monitor.h"
 #include "chrome/app/breakpad_mac.h"
 #include "chrome/browser/mac/relauncher.h"
 #include "chrome/common/chrome_paths_internal.h"
@@ -568,6 +569,10 @@ int ChromeMain(int argc, char** argv) {
 
 #if defined(OS_MACOSX)
   chrome_main::SetUpBundleOverrides();
+
+  // We need to allocate the IO Ports before the Sandbox is initialized or
+  // the first instance of SystemMonitor is created.
+  base::SystemMonitor::AllocateSystemIOPorts();
 #endif
 
   CommandLine::Init(argc, argv);

chrome/browser/extensions/extension_event_router_forwarder_unittest.cc

@@ -37,6 +37,10 @@ class ExtensionEventRouterForwarderTest : public TestingBrowserProcessTest {
   ExtensionEventRouterForwarderTest()
       : ui_thread_(BrowserThread::UI, &message_loop_),
         io_thread_(BrowserThread::IO) {
+#if defined(OS_MACOSX)
+    base::SystemMonitor::AllocateSystemIOPorts();
+#endif
+    dummy.reset(new base::SystemMonitor);
   }
 
   ~ExtensionEventRouterForwarderTest() {
@@ -66,7 +70,7 @@ class ExtensionEventRouterForwarderTest : public TestingBrowserProcessTest {
   MessageLoopForUI message_loop_;
   BrowserThread ui_thread_;
   BrowserThread io_thread_;
-  base::SystemMonitor dummy;
+  scoped_ptr<base::SystemMonitor> dummy;
   // Profiles are weak pointers, owned by ProfileManager in |browser_process_|.
   TestingProfile* profile1_;
   TestingProfile* profile2_;

chrome/browser/profiles/profile_manager_unittest.cc

@@ -38,6 +38,10 @@ class ProfileManagerTest : public TestingBrowserProcessTest {
         file_thread_(BrowserThread::FILE, &message_loop_),
         profile_manager_(new ProfileManagerWithoutInit),
         local_state_(testing_browser_process_.get()) {
+#if defined(OS_MACOSX)
+    base::SystemMonitor::AllocateSystemIOPorts();
+#endif
+    system_monitor_dummy_.reset(new base::SystemMonitor);
   }
 
   virtual void SetUp() {
@@ -61,7 +65,7 @@ class ProfileManagerTest : public TestingBrowserProcessTest {
   BrowserThread ui_thread_;
   BrowserThread file_thread_;
 
-  base::SystemMonitor system_monitor_dummy_;
+  scoped_ptr<base::SystemMonitor> system_monitor_dummy_;
 
   // Also will test profile deletion.
   scoped_ptr<ProfileManager> profile_manager_;

chrome/browser/tabs/tab_strip_model_unittest.cc

@@ -153,7 +153,12 @@ class TabStripModelTest : public RenderViewHostTestHarness {
  public:
   TabStripModelTest()
       : RenderViewHostTestHarness(),
-        browser_thread_(BrowserThread::UI, &message_loop_) {}
+        browser_thread_(BrowserThread::UI, &message_loop_) {
+#if defined(OS_MACOSX)
+    base::SystemMonitor::AllocateSystemIOPorts();
+#endif
+    system_monitor.reset(new base::SystemMonitor);
+  }
 
   TabContentsWrapper* CreateTabContents() {
     return Browser::TabContentsFactory(profile(), NULL, 0, NULL, NULL);
@@ -270,7 +275,7 @@ class TabStripModelTest : public RenderViewHostTestHarness {
   std::map<TabContents*, int> foo_;
 
   // ProfileManager requires a base::SystemMonitor.
-  base::SystemMonitor system_monitor;
+  scoped_ptr<base::SystemMonitor> system_monitor;
 
   ProfileManager pm_;
 };

chrome/test/in_process_browser_test.cc

@@ -43,6 +43,7 @@
 
 #if defined(OS_MACOSX)
 #include "base/mac/mac_util.h"
+#include "base/system_monitor/system_monitor.h"
 #endif
 
 #if defined(OS_WIN)
@@ -82,6 +83,7 @@ InProcessBrowserTest::InProcessBrowserTest()
       tab_closeable_state_watcher_enabled_(false) {
 #if defined(OS_MACOSX)
   base::mac::SetOverrideAmIBundled(true);
+  base::SystemMonitor::AllocateSystemIOPorts();
 #endif
 
   // Before we run the browser, we have to hack the path to the exe to match