Browser watcher end-end-to-end .

components/browser_watcher/watcher_win.h

+// Copyright (c) 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.
+#ifndef COMPONENTS_BROWSER_WATCHER_WATCHER_WIN_H_
+#define COMPONENTS_BROWSER_WATCHER_WATCHER_WIN_H_
+
+#include <windows.h>

[erikwright (departed), 2014/11/14 19:46:10]

not used

[Sigurður Ásgeirsson, 2014/11/17 15:26:58]

Done.

+
+#include "base/macros.h"
+#include "base/process/process_handle.h"
+#include "base/strings/string16.h"
+#include "base/time/time.h"
+#include "base/win/scoped_handle.h"
+
+namespace base {
+class CommandLine;
+}
+
+namespace browser_watcher {
+
+// Watches for the exit code of a process and records it in a given registry
+// location.
+class ExitCodeWatcher {
+ public:
+  // Initialize the watcher with a registry path.
+  explicit ExitCodeWatcher(const wchar_t* registry_path);

[erikwright (departed), 2014/11/14 19:46:10]

It seems relatively unusual to pass around char buffer pointers in Chromium.
While technically better for performance, I would use a base::string16 for
clarity, consistency, and safety.

At a minimum, we never use wchar_t, only base::char16

[Sigurður Ásgeirsson, 2014/11/17 15:26:58]

Done.

+

[erikwright (departed), 2014/11/14 19:46:10]

an explicit out-of-line destructor is required.

[Sigurður Ásgeirsson, 2014/11/17 15:26:58]

Sure, why?

[erikwright (departed), 2014/11/17 15:35:17]

The default destructor will be inlined, which violates Chromium coding style and
is contra-indicated (though not explicitly banned) by Google style guide.

http://www.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Stop-in...
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Inline_Functions

There is a Clang rule that enforces this, but of course it won't run on your
Windows-only code.

The gist of the Clang rule is that it calculates a "complexity" score for the
destructor and prevents inlining if it is too high. Pretty much any non-POD
member pushes the complexity score over the threshold.

+  // Initializes from arguments on |cmd_line|, returns true on success.
+  // This function expects the process handle indicated in |cmd_line| to be open
+  // with sufficient privilege to wait and retrieve the process exit code.
+  // It checks the handle for validity and takes ownership of it.
+  // The intent is for this handle to be inherited into the watcher process
+  // hosting the instance of this class.
+  bool ParseArguments(const base::CommandLine& cmd_line);
+
+  // Waits for the process to exit and records its exit code in registry.
+  // This is a blocking call.
+  void WaitForExit();
+
+  base::ProcessHandle process() const { return process_.Get(); }
+
+ private:
+  // The registry path the exit codes are written to.
+  base::string16 registry_path_;
+
+  // Handle to the process we wait for and it's creation time.

[erikwright (departed), 2014/11/14 19:46:10]

it's -> its

But this comment is unnerving in its almost, but not quite, complete description
of this list of members.

I would say, "Handle, ID, and creation time of the watched process.".

[Sigurður Ásgeirsson, 2014/11/17 15:26:58]

:)

+  base::win::ScopedHandle process_;
+  base::ProcessId process_pid_;
+  base::Time process_creation_time_;
+
+  DISALLOW_COPY_AND_ASSIGN(ExitCodeWatcher);
+};
+
+}  // namespace browser_watcher
+
+#endif  // COMPONENTS_BROWSER_WATCHER_WATCHER_WIN_H_