Getting service process on Mac to handle having things moved/changed underneath it.

chrome/common/launchd_mac.h

+// Copyright (c) 2011 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.
+
+#ifndef CHROME_COMMON_LAUNCHD_MAC_H_
+#define CHROME_COMMON_LAUNCHD_MAC_H_
+
+#include <CoreFoundation/CoreFoundation.h>
+
+#include "base/basictypes.h"
+#include "base/singleton.h"
+
+class Launchd {

[Mark Mentovai, 2011/03/21 16:50:49]

I don’t like this as a class name, especially without a namespace (which we’re
suposed to generally be using these days).

This is more of a LaunchdInterface, perhaps?

[dmac, 2011/03/21 22:59:19]

Will fix up in a later CL.

+ public:
+
+  enum Type {
+    Agent,  // LaunchAgent
+    Daemon  // LaunchDaemon
+  };
+
+  // Domains map to NSSearchPathDomainMask without requiring Foundation
+  // to be included.

[Mark Mentovai, 2011/03/21 16:50:49]

In order to get away with this, you need to have a COMPILE_ASSERT in the .mm
file that ensures that your own constants are equal to Foundation’s.

[dmac, 2011/03/21 22:59:19]

Done.

+  enum Domain {
+    User = 1,  // ~/Library/Launch*

[Mark Mentovai, 2011/03/21 16:50:49]

To make it obvious that these are bits, I would write:

User = 1 << 0,
Local = 1 << 1,
Network = 1 << 2,
System = 1 << 3

[dmac, 2011/03/21 22:59:19]

Foundation definitions have them mapped out as 1,2,4,8 so I'm leaving them to
match up.

+    Local = 2,  // /Library/Launch*
+    Network = 4,  // /Network/Library/Launch*
+    System = 8  // /System/Library/Launch*
+  };
+
+  // TODO(dmaclach): Get rid of this pseudo singleton, and inject it
+  // appropriately wherever it is used.
+  // http://crbug.com/76925
+  static Launchd* GetInstance();
+
+  virtual ~Launchd();
+
+  // Return a dictionary with the launchd export settings.
+  virtual CFDictionaryRef CopyLaunchdExports();
+
+  // Return a dictionary with the launchd entries for job labeled |name|.
+  virtual CFDictionaryRef CopyLaunchdJobDictionary(CFStringRef label);
+
+  // Return a dictionary for launchd process.
+  virtual CFDictionaryRef CopyLaunchdDictionaryByCheckingIn(CFErrorRef* error);
+
+  // Remove a launchd process from launchd. This should not be used by a process
+  // to commit suicide. ShutdownLaunchdJob/RestartLaunchdJob should be used
+  // instead.
+  virtual bool RemoveLaunchdJob(CFStringRef label, CFErrorRef* error);
+
+  // Used by a process controlled by launchd to kill oneself.

[Mark Mentovai, 2011/03/21 16:50:49]

itself

[dmac, 2011/03/21 22:59:19]

Done.

+  // |session_type| can be "Aqua", "LoginWindow", "Background", "StandardIO" or
+  // "System".
+  // ShutdownLaunchdJob starts up a separate process to tell launchd to
+  // send us SIGTERM. This call will return, but a SIGTERM should occur shortly.

[Mark Mentovai, 2011/03/21 16:50:49]

send us SIGTERM -> send this process a SIGTERM

[Mark Mentovai, 2011/03/21 16:50:49]

occur -> be received

[dmac, 2011/03/21 22:59:19]

Done.

[dmac, 2011/03/21 22:59:19]

Done.

+  virtual bool ShutdownLaunchdJob(Domain domain,
+                                  Type type,
+                                  CFStringRef name,
+                                  CFStringRef session_type);
+
+  // Used by a process controlled by launchd to restart oneself.
+  // |session_type| can be "Aqua", "LoginWindow", "Background", "StandardIO" or
+  // "System".
+  // RestartLaunchdJob starts up a separate process to tell launchd to
+  // send us SIGTERM and then reload us. This call will return, but a SIGTERM

[Mark Mentovai, 2011/03/21 16:50:49]

Revise comment as above: itself, send this process a, be received.

[dmac, 2011/03/21 22:59:19]

Done.

+  // should occur shortly.
+  virtual bool RestartLaunchdJob(Domain domain,
+                                 Type type,
+                                 CFStringRef name,
+                                 CFStringRef session_type);
+
+  // Read a launchd plist from disk.
+  // |name| should not have an extension.
+  virtual CFMutableDictionaryRef ReadLaunchdPlist(Domain domain,

[Mark Mentovai, 2011/03/21 16:50:49]

Your CFDictionaryRef-returning functions above were named with “Copy” in the
name, so they didn’t need any special ownership documentation. There’s no Copy
or Create or Get here, and there’s no comment telling me who owns the
dictionary.

+                                                  Type type,
+                                                  CFStringRef name);
+  // Write a launchd plist to disk.
+  // |name| should not have an extension.
+  virtual bool WriteLaunchdPlist(Domain domain,
+                                 Type type,
+                                 CFStringRef name,
+                                 CFDictionaryRef dict);
+
+  // Delete a launchd plist.
+  // |name| should not have an extension.
+  virtual bool DeleteLaunchDPlist(Domain domain, Type type, CFStringRef name);

[Mark Mentovai, 2011/03/21 16:50:49]

You have ReadLaunchdPlist and WriteLaunchdPlist (lowercase D), and a lot of
other things that refer to launchd with a lowercase d that aren’t related to
plists, but then you have DeleteLaunchDPlist (capital D). Be consistent. I
prefer the lowercase D.

Given the class name, you probably don’t even need to put Launchd in each
method’s name. You’ll probably have something like LaunchdInterface*
launchd_interface = LaunchdInterface::GetInstance();
launchd_interface->DeletePlist(...), which is more than sufficient. I doubt
launchd_interface->DeleteLaunchdPlist(...) would be any better.

[dmac, 2011/03/21 22:59:19]

Done.

+
+  // TODO(dmaclach): remove this once http://crbug.com/76925 is fixed.
+  // Scaffolding for doing unittests with our singleton.
+  static void SetInstance(Launchd* instance);
+  class ScopedInstance {
+   public:
+    ScopedInstance(Launchd* instance) {

[Mark Mentovai, 2011/03/21 16:50:49]

explicit

[dmac, 2011/03/21 22:59:19]

Done.

+      Launchd::SetInstance(instance);
+    }
+    ~ScopedInstance() {
+      Launchd::SetInstance(NULL);

[Mark Mentovai, 2011/03/21 16:50:49]

This doesn’t reset the initial state. That’s probably fine for what you’re doing
here.

[dmac, 2011/03/21 22:59:19]

Actually it's exactly what I want ;-)

+    }
+  };
+
+ protected:
+  Launchd() { }
+
+ private:
+  // TODO(dmaclach): remove this once http://crbug.com/76925 is fixed.
+  // Scaffolding for doing unittests with our singleton.
+  friend struct DefaultSingletonTraits<Launchd>;
+  static Launchd* g_instance_;
+  static bool g_set_to_singleton_;
+
+  DISALLOW_COPY_AND_ASSIGN(Launchd);
+};
+
+#endif  // CHROME_COMMON_LAUNCHD_MAC_H_