Using PolicyServiceWatcher instead of PolicyWatcherLinux/Win/Mac.

remoting/host/policy_hack/policy_service_watcher_mac.mm

+// Copyright 2014 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 "base/files/file_path.h"
+#include "base/files/file_util.h"
+#include "base/mac/foundation_util.h"
+
+namespace remoting {
+namespace policy_hack {
+
+// This function is based on handling of DIR_MANAGED_PREFS key by
+// PathService::Get in chrome/common/chrome_paths.cc.

[Mattias Nissler (ping if slow), 2015/01/06 09:06:12]

It might be cleaner to move DIR_MANAGED_PREFS over to base_paths_mac.{h,mm} so
both chrome and chromoting can use it.

[Łukasz Anforowicz, 2015/01/07 17:54:15]

Since DIR_MANAGED_PREFS was used only GetManagedPolicyPath in
chrome_browser_policy_connector.cc, I removed it from chrome_paths.cc and simply
inlined inside policy_loader_mac.mm.  I think things are simple and easy to
follow this way.  (I don't think I would like to make things more complicated by
having DIR_MAMAGED_PREFS exposed via base_paths_mac for a hypothetical scenario
where somebody other than PolicyLoaderMac needs it).

[Mattias Nissler (ping if slow), 2015/01/08 09:58:36]

For background, a common use for using path keys is to override path locations
in browser tests. You'd first put the path overrides in place and then start the
browser. We don't have any browser tests that need this right now (and I think
that's OK), I'm fine with removing the path key.

[Łukasz Anforowicz, 2015/01/08 23:09:26]

Thanks for the explanation.  I see that the PathService lives in base/ (i.e.
outside of chrome/) - in theory I could make this work.  But I think that I'll
keep the current code to keep things simple for now.

+bool GetManagedPrefsDir(base::FilePath* result) {
+  base::FilePath cur;
+  if (!base::mac::GetLocalDirectory(NSLibraryDirectory, &cur)) {
+    return false;
+  }
+  cur = cur.Append(FILE_PATH_LITERAL("Managed Preferences"));
+  char* login = getlogin();
+  if (!login)
+    return false;
+  cur = cur.AppendASCII(login);
+  if (!base::PathExists(cur))  // We don't want to create this.

[Mattias Nissler (ping if slow), 2015/01/06 09:06:12]

nit: This comment is a copy-and-paste artifact that doesn't make sense in the
new context.

[Łukasz Anforowicz, 2015/01/07 17:54:15]

I don't understand.  This is indeed a copy&pasted comment, but I think it still
makes sense - we don't want to create the directory / we want to return only an
already existing directory.

This code is now moved to policy_loader_mac.mm - please shout at me overthere,
if you think I should still do something about the comment.

[Mattias Nissler (ping if slow), 2015/01/08 09:58:36]

IIRC, in the path resolution logic the return value is commonly used to not
indicate success or error, but to tell the caller whether the directory should
be created or not. I might misremember though.

In any case, even if we pass back the path and return true, the policy loader
code will never try to create that directory. Hence, in this case you probably
just want to return cur after line 23 and drop the existence check. This should
also handle situations better where the path doesn't originally exist but gets
created later by the OS for whatever reason.

[Łukasz Anforowicz, 2015/01/08 23:09:26]

A comment for PathService::Get from base/path_service.h says:

  // ...
  // Directories are also guaranteed to exist when this function succeeds.
  //
  // Returns true if the directory or file was successfully retrieved. On
  // failure, 'path' will not be changed.
 
Makes sense.  I've removed the directory existence check in
PolicyLoaderMac::GetManagedPolicyPath.

+    return false;
+
+  *result = cur;
+  return true;
+}
+
+}  // namespace policy_hack
+}  // namespace remoting