Separate singleton out of ChromotingJNIInstance

remoting/client/jni/chromoting_jni.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_JNI_CHROMOTING_JNI_H_
+#define REMOTING_CLIENT_JNI_CHROMOTING_JNI_H_
+
+#include <jni.h>
+
+#include "base/at_exit.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "remoting/base/auto_thread.h"
+#include "remoting/protocol/connection_to_host.h"
+
+template<typename T> struct DefaultSingletonTraits;
+
+namespace remoting {
+class ChromotingJniInstance;
+
+// Class and package name of the Java class supporting the methods we call.
+const char* const JAVA_CLASS = "org/chromium/chromoting/jni/JNIInterface";

[Wez, 2013/07/16 13:46:56]

Does this need to be in the header? Where else is it used?

[solb, 2013/07/16 19:23:02]

Done.

+
+// Houses the global resources on which the Chromoting components run
+// (e.g. message loops and task runners). Proxies outgoing JNI calls from its
+// ChromotingJniInstance member to Java. All its methods should be invoked
+// exclusively from the UI thread.
+class ChromotingJni {
+ public:
+  // This class is instantiated at process initialization and persists until
+  // we close. Its components are reused across |ChromotingJniInstance|s.
+  static ChromotingJni* GetInstance();
+
+  scoped_refptr<AutoThreadTaskRunner> ui_task_runner() {
+    return ui_task_runner_;
+  }
+
+  scoped_refptr<AutoThreadTaskRunner> network_task_runner() {
+    return network_task_runner_;
+  }
+
+  scoped_refptr<AutoThreadTaskRunner> display_task_runner() {
+    return display_task_runner_;
+  }
+
+  scoped_refptr<net::URLRequestContextGetter> url_requester() {
+    return url_requester_;
+  }
+
+  // Initiates a connection with the specified host. Must only be called when
+  // |session| is null (i.e. before any other call to Connect() or following
+  // a call to Disconnect()).
+  void ConnectToHost(const char* username,
+                     const char* auth_token,
+                     const char* host_jid,
+                     const char* host_id,
+                     const char* host_pubkey);
+
+  // Terminates any ongoing connection attempt and cleans up by nullifying
+  // |session|. This is a no-op unless |session| is currently non-null.
+  void DisconnectFromHost();
+
+  // Returns the client for the currently-active session, or null if no
+  // connection attempt has been initiated.
+  scoped_refptr<ChromotingJniInstance> session() {
+    DCHECK(session_);

[Wez, 2013/07/16 13:46:56]

This will check if no connection attempt has been initiated; either remove the
DCHECK or update the comment.

[solb, 2013/07/16 19:23:02]

Done.

+    return session_;
+  }
+
+  // Notifies the user that the connection status has changed.
+  void ReportConnectionStatus(protocol::ConnectionToHost::State state,
+                              protocol::ErrorCode error);
+
+  // Pops up a dialog box asking the user to enter a PIN.
+  void DisplayAuthenticationPrompt();
+
+ private:
+  ChromotingJni();
+
+  // Forces a DisconnectFromHost() in case there is any active or failed
+  // connection, then proceeds to tear down the Chromium dependencies on which
+  // all sessions depended. Because destruction only occurs at application exit
+  // after all connections have terminated, it is safe to make unretained
+  // cross-thread calls on the class.
+  virtual ~ChromotingJni();
+
+  // Reference to the Java class into which we make JNI calls.
+  jclass class_;
+
+  // Used by the Chromium libraries to clean up the base and net libraries' JNI
+  // bindings. It must persist for the lifetime of the singleton.
+  scoped_ptr<base::AtExitManager> collector_;

[Wez, 2013/07/16 13:46:56]

nit: at_exit_manager_

[solb, 2013/07/16 19:23:02]

Done.

+
+  // Chromium code's connection to the Java message loop.
+  scoped_ptr<base::MessageLoopForUI> ui_loop_;
+
+  // Runners that allow posting tasks to the various native threads.

[Wez, 2013/07/16 13:46:56]

nit: Suggest: "References to native threads."

[solb, 2013/07/16 19:23:02]

Done.

+  scoped_refptr<AutoThreadTaskRunner> ui_task_runner_;
+  scoped_refptr<AutoThreadTaskRunner> network_task_runner_;
+  scoped_refptr<AutoThreadTaskRunner> display_task_runner_;
+
+  scoped_refptr<net::URLRequestContextGetter> url_requester_;
+
+  // Contains all connection-specific state.
+  scoped_refptr<ChromotingJniInstance> session_;

[Wez, 2013/07/16 13:46:56]

nit: session_ -> instance_

[solb, 2013/07/16 19:23:02]

I believe this would make things less clear; |instance_| could be any object,
whereas |session_| makes it clearer that this represents a specific connection.

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