367658 - implement nsIIdleService on OS X

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

[Shawn Wilsher :sdwilsh]

It'd be nice to have an implementation for this on OS X as well.

Comment 1

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

I can't test this due to issues with building, but I want my work somewhere other than just on my computer.  Feedback welcome.

Comment 2

[Shawn Wilsher :sdwilsh]

I *think* that patch works, but I'm going to let Gijs report on it working or not since he's had much more experience with the idleService stuff already (and knows how to test the heck out of it).

Comment 3

[Shawn Wilsher :sdwilsh]

Attachment: v1.1

whoops - Gijs pointed out that I was doing my conversion wrong.  With that fixed, this works.

Comment 4

[Josh Aas]

Comment on attachment 256556 [details] [diff] [review]
v1.1

+		nsIdleServiceMac.mm \

The file names should just be "nsIdleService.mm", "nsIdleService.h". This is how we want all of our files named, though there are legacy exceptions. The directory they live in tells you the OS.

+#ifndef __nsIdleServiceMac_h__

The include guard name should be "nsIdleService_h_" in line with the rest of cocoa widgets. Double underscores are reserved and should not be used. Please change this in the top and bottom of the header.

+  if (rval != KERN_SUCCESS) {
+    return NS_ERROR_FAILURE;
+  }

cocoa widgets style nit - Remove braces on these one line blocks so there is a little less clutter. There are one-line if blocks all over. If any block in an if-elsif-else statement is > 1 line, then all should have braces.

Please post another patch with these changes. Otherwise looks good! Thanks.

Comment 5

[Shawn Wilsher :sdwilsh]

Attachment: v1.2

Alright, this addresses your comments with the exception of the file name issue.  I thought we could change it, but then when I did a test compile I realized that we #include "nsIdleService.h" - which is defined elsewhere and is what allows us to not implement all of the interface for just our widget.  So, to go along with other files in the widget dir that seem to be in a similar situation, I tossed on a Cocoa to the end.  Hopefully this works.

Pending r+, I need to know who's a good candidate for requesting sr from.

Comment 6

[Shawn Wilsher :sdwilsh]

Attachment: v1.3

We can all agree on file names now!

Comment 7

[Josh Aas]

Comment on attachment 256797 [details] [diff] [review]
v1.3

Thanks!

Comment 8

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 256797 [details] [diff] [review]
v1.3

+  NS_IMETHOD GetIdleTime(PRUint32* idleTime);

isn't there a macro for this in nsIIdleService?

sr=pink

Comment 9

[Shawn Wilsher :sdwilsh]

(In reply to comment #8)
> +  NS_IMETHOD GetIdleTime(PRUint32* idleTime);
> 
> isn't there a macro for this in nsIIdleService?

No, this is just one method - everything else is defined in nsIdleService and we inherit from that.

Comment 10

[Phil Ringnalda (:philor)]

widget/src/cocoa/Makefile.in: 1.65
widget/src/cocoa/nsIdleServiceX.h: 1.1
widget/src/cocoa/nsIdleServiceX.mm: 1.1
widget/src/cocoa/nsWidgetFactory.mm: 1.31