Created attachment 264315 [details] [review]
gtkapplication-quartz: clean up inhibit code

When testing with bloatpad, the existing inhibit code seems not to be
working at all.  Replace it with a cleaner and simpler version that
works.

This code was originally written by Danw, I believe for EggSMClient.

There was a comment on irc from Daniel Stone that 'setting an app delegate is a bad idea'.

Apple's documentation for NSApplication specifically recommend setting a delegate for it:

"""

The Delegate and Notifications

You can assign a delegate to NSApp. The delegate responds to certain messages on behalf of NSApp. Some of these messages, such as application:openFile:, ask the delegate to perform an action. Another message, applicationShouldTerminate:, lets the delegate determine whether the application should be allowed to quit. The NSApplication class sends these messages directly to its delegate. 

...

Subclassing Notes

You rarely should find a real need to create a custom NSApplication subclass. Unlike some object-oriented libraries, Cocoa does not require you to create a custom application class to customize application behavior. Instead it gives you many other ways to customize an application. This section discusses both some of the possible reasons to subclass NSApplication and some of the reasons not to subclass NSApplication.

...

Alternatives to Subclassing

NSApplication defines numerous delegate methods that offer opportunities for modifying specific aspects of application behavior. Instead of making a custom subclass of NSApplication, your application delegate may be able to implement one or more of these methods to accomplish your design goals. In general, a better design than subclassing NSApplication is to put the code that expresses your application’s special behavior into one or more custom objects called controllers. Methods defined in your controllers can be invoked from a small dispatcher object without being closely tied to the global application object.


"""

https://developer.apple.com/library/mac/documentation/cocoa/reference/applicationkit/classes/NSApplication_Class/Reference/Reference.html

Was there a particular reason to avoid using a delegate in our case?

(I am the Daniel to blame for the vague comment on IRC)

The app delegate is the right way to respond to this message. But you can only have one app delegate per program, and registering it here will prevent the application from implementing a delegate that responds to the openFile: message. Which is necessary to respond to opening documents from Finder or from files dropped on your Dock icon.

Opening files is also in the domain of GTKApplication isn't it? It should probably provide a delegate that implements those methods too so users of GTKApplication don't need to worry about Cocoa stuff.

(In reply to comment #5)
> (I am the Daniel to blame for the vague comment on IRC)
> 
> The app delegate is the right way to respond to this message. But you can only
> have one app delegate per program, and registering it here will prevent the
> application from implementing a delegate that responds to the openFile:
> message. Which is necessary to respond to opening documents from Finder or from
> files dropped on your Dock icon.
> 
> Opening files is also in the domain of GTKApplication isn't it? It should
> probably provide a delegate that implements those methods too so users of
> GTKApplication don't need to worry about Cocoa stuff.

Indeed.  I saw this when I was reading the docs and I do intend to do this in a different patch.

It is my opinion that we should do our best to shield the user from Cocoa as much as possible, so I would not expect them to use an application delegate at all.  If they need to, we've probably failed and/or they should just not be using GtkApplication at all.

so, should we get this in now then, or wait for that follow-up patch ?

Separate issues, I think.  For now I was just hoping to improved the (alleged) existing functionality.

One more note: you can always set register-session to FALSE to disable it if it's causing you problems...

This is just a note about the ref counting semantics of NSApplication's setDelegate: method:

From https://developer.apple.com/library/ios/documentation/general/conceptual/CocoaEncyclopedia/DelegatesandDataSources/DelegatesandDataSources.html

"Delegating objects do not (and should not) retain their delegates. However, clients of delegating objects (applications, usually) are responsible for ensuring that their delegates are around to receive delegation messages. To do this, they may have to retain the delegate in memory-managed code."

So the application delegate shouldn't be auto-released, but instead released when no longer needed.

I believe it's also good practice to chain up to [super init] in the initializer (even if NSObject's init does nothing...).

Created attachment 265634 [details] [review]
gtkapplication-quartz: clean up inhibit code

Updated with Will's suggestions and retested.

Attachment 265634 [details] pushed as 146f3a9 - gtkapplication-quartz: clean up inhibit code