Added daemon process NPAPI.

remoting/host/plugin/daemon_npapi.h

+// Copyright (c) 2012 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 REMOTING_DAEMON_NPAPI_H_

[Sergey Ulanov, 2012/02/02 23:41:24]

nit: REMOTING_HOST_PLUGIN_DAEMONG_NPAPI_H_

[Jamie, 2012/02/03 00:47:41]

Done.

+#define REMOTING_DAEMON_NPAPI_H_
+
+namespace remoting {
+
+class DaemonNpapi {

[Sergey Ulanov, 2012/02/02 23:41:24]

Not sure if this is a right name for this class - it doesn't seem to be NPAPI
specific. Maybe DaemonController?

[Jamie, 2012/02/03 00:47:41]

Done.

[Wez, 2012/02/03 01:25:36]

Agreed.

+ public:
+  virtual ~DaemonNpapi() {}
+
+  enum State {

[Sergey Ulanov, 2012/02/02 23:41:24]

nit: This should be declared before the constructor. Types go first.

[Jamie, 2012/02/03 00:47:41]

I think it's clearer placed next to where it's used, but I'll adhere to the
style guide.

+    // The daemon process is not installed. This is functionally equivalent
+    // to STATE_STOPPED, but the start method is expected to be significantly
+    // slower, and might involve user interaction. It might be appropriate to
+    // indicate this in the UI.
+    STATE_NOT_INSTALLED = 0,

[Wez, 2012/02/03 01:25:36]

nit: Do we need explicit values for each enum value?

[Jamie, 2012/02/03 01:40:15]

When the JS side of this is implemented, we'll need to keep enum values in sync,
and I like to specify them explicitly as a signal to that effect. Once I know
what JS file the values will be duplicated in, I'll add a comment here as well.

+    // The daemon process is installed but not running. Call Start to start it.
+    STATE_STOPPED = 1,
+    // The daemon process is running. Call Start again to change the PIN or
+    // Stop to stop it.
+    STATE_STARTED = 2,
+    // The previous Start operation failed. This is functionally equivalent
+    // to STATE_STOPPED, but the UI should report an error in this state.
+    // This state should not persist across restarts of the NPAPI process.
+    STATE_START_FAILED = 3

[Wez, 2012/02/03 01:25:36]

Perhaps have a STATE_NOT_IMPLEMENTED for now, for Linux & Mac to return, so we
can avoid presenting the UI on those platforms for now?

[Jamie, 2012/02/03 01:40:15]

Good idea. Done.

+  };
+
+  // Return the state of the daemon process.
+  virtual State GetState() = 0;
+
+  // Start the daemon process and set the PIN, which should be expressed as a
+  // UTF8-encoded string. Since this may require that the daemon be downloaded
+  // and installed, this may take a long time--poll GetState until the state is
+  // STATE_STARTED or STATE_START_FAILED.
+  //
+  // Calling Start when the daemon is already running changes the PIN.
+  virtual void Start(const char* pin) = 0;

[alexeypa (please no reviews), 2012/02/03 00:26:05]

nit: typically it is pretty bad idea to pass a string as a char*. I would wrap
it to std::string or something else.

Potentially there is also a i18n issue. If, say, in the future the PIN will not
be numeric or, for instance we decide to support arabic digits
(http://unicode.org/glossary/#arabic_indic_digits).

[Jamie, 2012/02/03 00:47:41]

Both seem reasonably common in Chrome. I prefer const char* because it avoids an
unnecessary string ctor if the caller passes in a C-style string.
The string is UTF8-encoded, so I don't think there are i18n issues.

[Sergey Ulanov, 2012/02/03 01:08:24]

Chromoting uses string class in most cases, and I think it's better to be
consistent with the rest of remoting code. Also, string is more efficient when
you copy it from one object to another, i.e. if the caller does have string, and
the method being called needs to save it, then it would not need to duplicate
the the content of the string (it just increases ref-counter), while with char*
the data would have to be duplicated.
You can also use base::StringPiece class if you worry about efficiency, but I
don't think it is really necessary here.

[Wez, 2012/02/03 01:25:36]

Why do we want to set the PIN when we start the Daemon process?  What if we find
the daemon is installed & configured but is not running?  How do we then start
it?  How do we change the PIN of an already-installed daemon?

[Wez, 2012/02/03 01:25:36]

Surely wrapping it as std::string doesn't help - it's more to do with the
character encoding convention, which in Chrome is UTF-8 for the most part, and
is stated explicitly earlier in this method's description.

I think the preference is for APIs to use const base::StringPiece& , although
most remoting/ code uses const std::string&.

[Jamie, 2012/02/03 01:40:15]

According to the latest mocks, you can't start the daemon without specifying a
PIN.
Call Start again. There's a comment to that effect.

+
+  // Stop the daemon process. It is permitted to call Stop while the daemon
+  // process is being installed, in which case the installation should be
+  // aborted if possible; if not then it is sufficient to ensure that the
+  // daemon process is not started automatically upon successful installation.
+  virtual void Stop() = 0;
+
+  static DaemonNpapi* Create();
+};
+
+}  // namespace remoting
+
+#endif

[Sergey Ulanov, 2012/02/02 23:41:24]

nit: // REMOTING_...

[Jamie, 2012/02/03 00:47:41]

Done.