Create new remoting_client_jni target

remoting/client/jni/chromoting_jni_instance.h

+// Copyright 2013 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_CLIENT_CHROMOTING_JNI_INSTANCE_H_
+#define REMOTING_CLIENT_CHROMOTING_JNI_INSTANCE_H_
+
+#include <jni.h>
+#include <string>
+
+#include "base/at_exit.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "remoting/base/auto_thread.h"
+#include "remoting/client/client_user_interface.h"
+
+template<typename T> struct DefaultSingletonTraits;
+
+// Class and package name of the Java class supporting the methods we call.
+const char* const JAVA_CLASS="org/chromium/chromoting/jni/JNIInterface";
+
+namespace remoting {
+
+// ClientUserInterface that makes and (indirectly) receives JNI calls.
+class ChromotingJNIInstance : public ClientUserInterface {

[Wez, 2013/07/11 00:56:23]

Having ChromotingJNIInstance be a singleton, and implement ClientUserInterface
means we're intrinsically tied to supporting only one client connection per
process. Is that desirable?

[solb, 2013/07/11 04:29:05]

Yes, we can only handle a single *concurrent* connection per process.  However,
ChromotingJNIInstance tries to be reusable in that calling its
DisconnectFromHost() method restores it to a state where it can establish a
*subsequent* connection, reusing many of the components from the first. 
Supporting multiple connections concurrently would require a whole boatload of
changes across the application in any case:  To start, we'd need some mechanism
of communicating which client instance we were dealing with over JNI
(reengineering the way we do JNI calls).  Is this a goal?

+ public:
+  static ChromotingJNIInstance* GetInstance();

[Wez, 2013/07/11 00:56:23]

nit: Add a comment to explain this, e.g. why it only makes sense for there to be
one of these.

[solb, 2013/07/11 04:29:05]

Done.

+
+  // Call from UI thread.

[Wez, 2013/07/11 00:56:23]

See Google C++ style guide for comment style requirements, here and below.

[solb, 2013/07/11 04:29:05]

Done.

+  void ConnectToHost(
+      jstring username,
+      jstring auth_token,
+      jstring host_jid,
+      jstring host_id,
+      jstring host_pubkey);
+
+  // Call from UI thread.
+  void DisconnectFromHost();
+
+  // ClientUserInterface implementation:

[Wez, 2013/07/11 00:56:23]

nit: : -> .

[solb, 2013/07/11 04:29:05]

Done.

+  virtual void OnConnectionState(
+      protocol::ConnectionToHost::State state,
+      protocol::ErrorCode error) OVERRIDE;
+  virtual void OnConnectionReady(bool ready) OVERRIDE;
+  virtual void SetCapabilities(const std::string& capabilities) OVERRIDE;
+  virtual void SetPairingResponse(
+      const protocol::PairingResponse& response) OVERRIDE;
+  virtual protocol::ClipboardStub* GetClipboardStub() OVERRIDE;
+  virtual protocol::CursorShapeStub* GetCursorShapeStub() OVERRIDE;
+  virtual scoped_ptr<protocol::ThirdPartyClientAuthenticator::TokenFetcher>
+      GetTokenFetcher(const std::string& host_public_key) OVERRIDE;
+
+ private:
+  ChromotingJNIInstance();
+  virtual ~ChromotingJNIInstance();
+
+  // Reusable between sessions:

[Wez, 2013/07/11 00:56:23]

As above, : -> . and see style guide re comment style

[solb, 2013/07/11 04:29:05]

Done.

+  jclass class_;  // Reference to the Java class into which we make JNI calls.
+  scoped_ptr<base::AtExitManager> collector_;
+  scoped_ptr<base::MessageLoopForUI> ui_loop_;

[Wez, 2013/07/11 00:56:23]

Add a comment to explain what |ui_loop_| is for, i.e. that it hooks us into the
Java message loop.

[solb, 2013/07/11 04:29:05]

Done.

+  scoped_refptr<AutoThreadTaskRunner> ui_runner_;

[Wez, 2013/07/11 00:56:23]

nit: ui_task_runner_

[solb, 2013/07/11 04:29:05]

Done.

+  scoped_refptr<AutoThreadTaskRunner> net_runner_;

[Wez, 2013/07/11 00:56:23]

nit: network_task_runner_

[solb, 2013/07/11 04:29:05]

Done.

+  scoped_refptr<AutoThreadTaskRunner> disp_runner_;

[Wez, 2013/07/11 00:56:23]

nit: display_task_runner_

[solb, 2013/07/11 04:29:05]

Done.

+  scoped_refptr<net::URLRequestContextGetter> url_requester_;
+
+  // Java string handles:
+  jstring username_jstr_;
+  jstring auth_token_jstr_;
+  jstring host_jid_jstr_;
+  jstring host_id_jstr_;
+  jstring host_pubkey_jstr_;
+  jstring pin_jstr_;
+
+  // C string pointers:
+  const char* username_cstr_;
+  const char* auth_token_cstr_;
+  const char* host_jid_cstr_;
+  const char* host_id_cstr_;
+  const char* host_pubkey_cstr_;
+  const char* pin_cstr_;

[Wez, 2013/07/11 00:56:23]

Replace these and the jstring references with std::strings, and copy the
contents across at Connect() time.

[solb, 2013/07/11 04:29:05]

Done.

+
+  friend struct DefaultSingletonTraits<ChromotingJNIInstance>;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromotingJNIInstance);
+};
+
+}  // namespace remoting
+
+#endif