Monitor sleep/wake on Mac. Should fix some networking issues arising after sl...

app/system_monitor_mac.mm

+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "app/system_monitor.h"
+
+#import <Cocoa/Cocoa.h>

[Mark Mentovai, 2010/11/24 18:52:44]

Use <AppKit/AppKit.h> for this one. I’m trying to keep these scoped just a
little bit more narrowly when possible.

+
+@interface SystemMonitorBridge : NSObject {
+ @private
+  SystemMonitor* systemMonitor_;  // weak
+}
+
+- (id)initWithSystemMonitor:(SystemMonitor*)monitor;
+- (void)computerDidSleep:(NSNotification*)notification;
+- (void)computerDidWake:(NSNotification*)notification;
+
+@end
+
+@implementation SystemMonitorBridge
+
+- (id)initWithSystemMonitor:(SystemMonitor*)monitor {
+  self = [super init];
+  if (self) {
+    systemMonitor_ = monitor;
+
+    // See QA1340 <http://developer.apple.com/library/mac/#qa/qa2004/qa1340.html>

[Mark Mentovai, 2010/11/24 18:52:44]

80.

[Avi (use Gerrit), 2010/11/24 19:02:15]

I thought we could let URLs slide? Or do we want to put it on its own line?

[Mark Mentovai, 2010/11/24 19:13:35]

Avi wrote:
You can let a URL slide, but if you can make it fit within 80 by putting it on
its own line, you should.

+    // for more details.
+    [[[NSWorkspace sharedWorkspace] notificationCenter]
+        addObserver:self
+           selector:@selector(computerDidSleep:)
+               name:NSWorkspaceWillSleepNotification
+             object:NULL];

[Mark Mentovai, 2010/11/24 18:52:44]

This is an |id|, use nil here and on line 38.

+    [[[NSWorkspace sharedWorkspace] notificationCenter]
+        addObserver:self
+           selector:@selector(computerDidWake:)
+               name:NSWorkspaceDidWakeNotification
+             object:NULL];
+  }
+  return self;
+}
+
+- (void)dealloc {
+  [[[NSWorkspace sharedWorkspace] notificationCenter]
+      removeObserver:self];
+  [super dealloc];
+}
+
+- (void)computerDidSleep:(NSNotification*)notification {
+  systemMonitor_->ProcessPowerMessage(SystemMonitor::SUSPEND_EVENT);
+}
+
+- (void)computerDidWake:(NSNotification*)notification {
+  systemMonitor_->ProcessPowerMessage(SystemMonitor::RESUME_EVENT);
+}
+
+@end
+
+static SystemMonitorBridge* g_system_monitor_bridge = nil;
+
+void SystemMonitor::PlatformInit() {

[Mark Mentovai, 2010/11/24 18:52:44]

PlatformInit and PlatformDestroy aren’t static methods on SystemMonitor. You
shouldn’t have g_system_monitor_bridge. You should have a member variable on
SystemMonitor instead. The Objective-C bridge object belongs to the
SystemMonitor, not to this file, and it’s someone else’s problem that the
application is limited elsewhere to one SystemMonitor instance.

[Avi (use Gerrit), 2010/11/24 19:02:15]

Something like the 

#if OBJC
@class bridge
#else
class bridge
#endif

deal? We've done that with parameters of that type before, but have we done that
with member variables? The ODR keeps dancing in my head when I think of that.

[Mark Mentovai, 2010/11/24 19:13:35]

Avi wrote:
Yup, but:

#ifdef __OBJC__

and use semicolons.
If you’re worried about ODR, parameters and member variables (and return types
and anything else) should be analyzed in exactly the same way. Yes, we’ve
already got instances where we use this trick for member variables, see
base/file_version_info_mac.h for one example. It also comes up fairly regularly
in WebKit.

The reason that this isn’t an ODR violation is that the D in the rule stands for
Definition, not Declaration. The declarations are equivalent in the sense that
they’re sufficient to deal properly with a pointer to type “bridge.” It’ll
always be an error to dereference a “bridge*” when all you’ve got is the
non-__OBJC__ class declaration, because there isn’t any matching definition. The
definition has an @ on it, there’s only one of them, and (because of the @) it
only makes sense in an .mm. C++ is restricted to shuffling pointers around,
which is safe, and is all that we need it to do. An attempt to do anything more
with it in C++-land is a compile-time error, which is exactly what we want it to
be.

Would void* and casts make you feel better? (I hope not.)

+  DCHECK(!g_system_monitor_bridge);
+  g_system_monitor_bridge =
+      [[SystemMonitorBridge alloc] initWithSystemMonitor:this];
+}
+
+void SystemMonitor::PlatformDestroy() {
+  DCHECK(g_system_monitor_bridge);
+  [g_system_monitor_bridge release];
+  g_system_monitor_bridge = nil;
+}