Adds an iOS implementation of base::SystemMonitor.

Adds an iOS implementation of base::SystemMonitor.

BUG=None
TEST=None


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=145929

[rohitrao (ping after 24h), 2012-07-10 15:41:46]

This is mostly the same as the CL I asked you about yesterday.

[Mark Mentovai, 2012-07-10 16:06:03]

http://codereview.chromium.org/10703120/diff/1/base/system_monitor/system_mon...
File base/system_monitor/system_monitor_ios.mm (right):

http://codereview.chromium.org/10703120/diff/1/base/system_monitor/system_mon...
base/system_monitor/system_monitor_ios.mm:6: //
http://developer.apple.com/library/mac/#qa/qa1340/_index.html.
I don’t see anything like this code at this URL.

http://codereview.chromium.org/10703120/diff/1/base/system_monitor/system_mon...
base/system_monitor/system_monitor_ios.mm:10: #include <UIKit/UIKit.h>
#import me.

http://codereview.chromium.org/10703120/diff/1/base/system_monitor/system_mon...
base/system_monitor/system_monitor_ios.mm:15: // No need to allocate any IO
ports on iOS.
This comment is unnecessary. There’s obviously no need to do so because the
function named “allocate system IO ports” is empty.

But your code’d be slimmer if you took this out of here and inlined an empty
definition of this function for iOS in the header file.

Your call.

http://codereview.chromium.org/10703120/diff/1/base/system_monitor/system_mon...
base/system_monitor/system_monitor_ios.mm:25:
this->ProcessPowerMessage(RESUME_EVENT);
Is the “this” really necessary?

http://codereview.chromium.org/10703120/diff/1/base/system_monitor/system_mon...
base/system_monitor/system_monitor_ios.mm:25:
this->ProcessPowerMessage(RESUME_EVENT);
I do love how clean this is.

[rohitrao (ping after 24h), 2012-07-10 16:26:43]

https://chromiumcodereview.appspot.com/10703120/diff/1/base/system_monitor/sy...
File base/system_monitor/system_monitor_ios.mm (right):

https://chromiumcodereview.appspot.com/10703120/diff/1/base/system_monitor/sy...
base/system_monitor/system_monitor_ios.mm:6: //
http://developer.apple.com/library/mac/#qa/qa1340/_index.html.
On 2012/07/10 16:06:03, Mark Mentovai wrote:
> I don’t see anything like this code at this URL.

Removed the comment.

https://chromiumcodereview.appspot.com/10703120/diff/1/base/system_monitor/sy...
base/system_monitor/system_monitor_ios.mm:10: #include <UIKit/UIKit.h>
On 2012/07/10 16:06:03, Mark Mentovai wrote:
> #import me.

Done.

https://chromiumcodereview.appspot.com/10703120/diff/1/base/system_monitor/sy...
base/system_monitor/system_monitor_ios.mm:15: // No need to allocate any IO
ports on iOS.
On 2012/07/10 16:06:03, Mark Mentovai wrote:
> This comment is unnecessary. There’s obviously no need to do so because the
> function named “allocate system IO ports” is empty.
> 
> But your code’d be slimmer if you took this out of here and inlined an empty
> definition of this function for iOS in the header file.
> 
> Your call.

Removed the comment.  I thought about making the entire method mac-but-not-ios,
but there are a half dozen callers and I think changing them all will be uglier.

https://chromiumcodereview.appspot.com/10703120/diff/1/base/system_monitor/sy...
base/system_monitor/system_monitor_ios.mm:25:
this->ProcessPowerMessage(RESUME_EVENT);
On 2012/07/10 16:06:03, Mark Mentovai wrote:
> Is the “this” really necessary?

Looks like it's implicit if I leave it out.  From the docs:

"In general you can use C++ objects within a block. Within a member function,
references to member variables and functions are via an implicitly imported this
pointer and thus appear mutable."

Do you see any danger in an implicit this->?  Do I need to do any of the crazy
shenanigans like saving "this" into another variable for use in the block,
similar to the method you would use to break retain cycles when using "self?"

[Mark Mentovai, 2012-07-10 17:06:05]

https://chromiumcodereview.appspot.com/10703120/diff/1/base/system_monitor/sy...
File base/system_monitor/system_monitor_ios.mm (right):

https://chromiumcodereview.appspot.com/10703120/diff/1/base/system_monitor/sy...
base/system_monitor/system_monitor_ios.mm:15: // No need to allocate any IO
ports on iOS.
rohitrao wrote:
> On 2012/07/10 16:06:03, Mark Mentovai wrote:
> > This comment is unnecessary. There’s obviously no need to do so because the
> > function named “allocate system IO ports” is empty.
> > 
> > But your code’d be slimmer if you took this out of here and inlined an empty
> > definition of this function for iOS in the header file.
> > 
> > Your call.
> 
> Removed the comment.  I thought about making the entire method
mac-but-not-ios,
> but there are a half dozen callers and I think changing them all will be
uglier.

Right, that’s why the suggestion (in case it wasn’t clear) was to write in the
header:

#if defined(OS_MACOSX)
#if !defined(OS_IOS)
  static void AllocateSystemIOPorts();
#else  // OS_IOS
  static void AllocateSystemIOPorts() {}
#endif  // OS_IOS
#endif  // OS_MACOSX

That way, the no-op will be inlined into the call sites at zero cost, and you
don’t need to decorate the call sites with additional ugly #ifdefs at all.

https://chromiumcodereview.appspot.com/10703120/diff/1/base/system_monitor/sy...
base/system_monitor/system_monitor_ios.mm:25:
this->ProcessPowerMessage(RESUME_EVENT);
rohitrao wrote:
> Do you see any danger in an implicit this->?

In C++ code, we never write this-> because it’s implied. Why would “this” case
be any different?

> Do I need to do any of the crazy
> shenanigans like saving "this" into another variable for use in the block,
> similar to the method you would use to break retain cycles when using "self?"

“this,” whether implicit or explicit, is just a raw pointer as far as the
language is concerned. There’s nothing like reference counting going on when you
use it, whether or not the compiler sees it. It’s just a variable that gets
bound when the block is established. It’s just a number. Simple as that.

https://chromiumcodereview.appspot.com/10703120/diff/5001/base/system_monitor...
File base/system_monitor/system_monitor.h (right):

https://chromiumcodereview.appspot.com/10703120/diff/5001/base/system_monitor...
base/system_monitor/system_monitor.h:36: #include <objc/runtime.h>  // For id.
This comment isn’t useful. We #include headers all the time to get their
definitions. Who’s to say that id will be the only thing we care about from
<objc/runtime.h> in the future? These comments tend to go stale after a short
period of time.

[rohitrao (ping after 24h), 2012-07-10 17:17:42]

https://chromiumcodereview.appspot.com/10703120/diff/1/base/system_monitor/sy...
File base/system_monitor/system_monitor_ios.mm (right):

https://chromiumcodereview.appspot.com/10703120/diff/1/base/system_monitor/sy...
base/system_monitor/system_monitor_ios.mm:15: // No need to allocate any IO
ports on iOS.
On 2012/07/10 17:06:05, Mark Mentovai wrote:
> rohitrao wrote:
> > On 2012/07/10 16:06:03, Mark Mentovai wrote:
> > > This comment is unnecessary. There’s obviously no need to do so because
the
> > > function named “allocate system IO ports” is empty.
> > > 
> > > But your code’d be slimmer if you took this out of here and inlined an
empty
> > > definition of this function for iOS in the header file.
> > > 
> > > Your call.
> > 
> > Removed the comment.  I thought about making the entire method
> mac-but-not-ios,
> > but there are a half dozen callers and I think changing them all will be
> uglier.
> 
> Right, that’s why the suggestion (in case it wasn’t clear) was to write in the
> header:
> 
> #if defined(OS_MACOSX)
> #if !defined(OS_IOS)
>   static void AllocateSystemIOPorts();
> #else  // OS_IOS
>   static void AllocateSystemIOPorts() {}
> #endif  // OS_IOS
> #endif  // OS_MACOSX
> 
> That way, the no-op will be inlined into the call sites at zero cost, and you
> don’t need to decorate the call sites with additional ugly #ifdefs at all.

Done.

https://chromiumcodereview.appspot.com/10703120/diff/1/base/system_monitor/sy...
base/system_monitor/system_monitor_ios.mm:25:
this->ProcessPowerMessage(RESUME_EVENT);
> “this,” whether implicit or explicit, is just a raw pointer as far as the
> language is concerned. There’s nothing like reference counting going on when
you
> use it, whether or not the compiler sees it. It’s just a variable that gets
> bound when the block is established. It’s just a number. Simple as that.

Ok, makes sense.  paranoia fading.

https://chromiumcodereview.appspot.com/10703120/diff/5001/base/system_monitor...
File base/system_monitor/system_monitor.h (right):

https://chromiumcodereview.appspot.com/10703120/diff/5001/base/system_monitor...
base/system_monitor/system_monitor.h:36: #include <objc/runtime.h>  // For id.
On 2012/07/10 17:06:05, Mark Mentovai wrote:
> This comment isn’t useful. We #include headers all the time to get their
> definitions. Who’s to say that id will be the only thing we care about from
> <objc/runtime.h> in the future? These comments tend to go stale after a short
> period of time.

Done.

[Mark Mentovai, 2012-07-10 17:32:15]

LGTM

[commit-bot: I haz the power, 2012-07-10 17:43:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rohitrao@chromium.org/10703120/7002

[commit-bot: I haz the power, 2012-07-10 19:18:23]

Change committed as 145929