Restructure chromoting_jni_instance handling of Java strings

remoting/client/jni/chromoting_jni_instance.h

Index: remoting/client/jni/chromoting_jni_instance.h
diff --git a/remoting/client/jni/chromoting_jni_instance.h b/remoting/client/jni/chromoting_jni_instance.h
index 75ef56bd87f10c12073bd3a77202e69422bc9ef9..d45562b29115b2b6c7962266df1d24c6ba660976 100644
--- a/remoting/client/jni/chromoting_jni_instance.h
+++ b/remoting/client/jni/chromoting_jni_instance.h
@@ -36,31 +36,36 @@ const int CHAT_PORT = 5222;
 const bool CHAT_USE_TLS = true;
 const char* const CHAT_AUTH_METHOD = "oauth2";
 
-// ClientUserInterface that makes and (indirectly) receives JNI calls.
+// ClientUserInterface that makes and (indirectly) receives JNI calls. It also
+// contains global resources (e.g. message loops and task runners) on which the

[Wez, 2013/07/12 01:25:44]

nit: Either move the bracket to the end of the line, or change the wording to
"... that the Chromoting components use."

[solb, 2013/07/12 02:31:49]

Done.

+// Chromium components run.

[Wez, 2013/07/12 01:25:44]

nit: Chromium -> Chromoting

[solb, 2013/07/12 02:31:49]

Done.

 class ChromotingJNIInstance : public ClientUserInterface {
  public:
+  // This class is instantiated at process initialization and persists until
+  // we close. It reuses many of its components between connections (i.e. when
+  // a DisconnectFromHost() call is followed by a ConnectToHost() one.
   static ChromotingJNIInstance* GetInstance();
 
-  // Call from UI thread.
+  // Should only be called while disconnected. Must be invoked on UI thread.

[Wez, 2013/07/12 01:25:44]

Please see
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...

[solb, 2013/07/12 02:31:49]

Done.

   void ConnectToHost(
-      jstring username,
-      jstring auth_token,
-      jstring host_jid,
-      jstring host_id,
-      jstring host_pubkey);
-
-  // Call from UI thread.
+      const char* username,
+      const char* auth_token,
+      const char* host_jid,
+      const char* host_id,
+      const char* host_pubkey);
+
+  // Must only be called during a successful or failed connection. Must be
+  // invoked on UI thread.

[Wez, 2013/07/12 01:25:44]

See
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...

[solb, 2013/07/12 02:31:49]

Done.

   void DisconnectFromHost();
 
-  // Call from UI thread.
-  void AuthenticateWithPin(jstring pin);
+  // To be called once the user has provided a PIN to the user interface.

[Wez, 2013/07/12 01:25:44]

See
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...

[solb, 2013/07/12 02:31:49]

Done.

+  void AuthenticateWithPin(const char* pin);
 
-  // Called by client authenticator.
-  // Gets notified if the user needs to enter a PIN, and notifies Java in turn.
+  // Called by client authenticator. Calls to Java to prompt for the user's PIN.

[Wez, 2013/07/12 01:25:44]

See
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...

[solb, 2013/07/12 02:31:49]

Done.

   void FetchSecret(bool pairable,
-                   const protocol::SecretFetchedCallback& callback_encore);
+                   const protocol::SecretFetchedCallback& callback);
 
-  // ClientUserInterface implementation:
+  // ClientUserInterface implementation.
   virtual void OnConnectionState(
       protocol::ConnectionToHost::State state,
       protocol::ErrorCode error) OVERRIDE;
@@ -75,6 +80,11 @@ class ChromotingJNIInstance : public ClientUserInterface {
 
  private:
   ChromotingJNIInstance();
+
+  // Any existing or attempted connection must have been terminated using
+  // DisconnectFromHost() before this singleton is destroyed. 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 ~ChromotingJNIInstance();
 
   void ConnectToHostOnDisplayThread();
@@ -82,41 +92,49 @@ class ChromotingJNIInstance : public ClientUserInterface {
 
   void DisconnectFromHostOnNetworkThread();
 
-  // Reusable between sessions:
-  jclass class_;  // Reference to the Java class into which we make JNI calls.
+  // The below variables are reused across consecutive sessions.
+
+  // 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_;
+
+  // Chromium code's connection to the Java message loop.
   scoped_ptr<base::MessageLoopForUI> ui_loop_;
-  scoped_refptr<AutoThreadTaskRunner> ui_runner_;
-  scoped_refptr<AutoThreadTaskRunner> net_runner_;
-  scoped_refptr<AutoThreadTaskRunner> disp_runner_;
+
+  // Runners that allow posting tasks to the various native threads.
+  scoped_refptr<AutoThreadTaskRunner> ui_task_runner_;
+  scoped_refptr<AutoThreadTaskRunner> network_task_runner_;
+  scoped_refptr<AutoThreadTaskRunner> display_task_runner_;
+
   scoped_refptr<net::URLRequestContextGetter> url_requester_;
-  scoped_refptr<FrameConsumerProxy> frames_;
+  scoped_refptr<FrameConsumerProxy> frame_consumer_;
+
+  // The below variables are specific to each connection.
+
+  // True iff ConnectToHost() has been called without a subsequent
+  // call to DisconnectFromHost() (i.e. while connecting, once connected, and
+  // between the time a connection fails and DisconnectFromHost() is called).
+  bool connected_;
 
-  // Specific to each session:
   scoped_ptr<ClientConfig> client_config_;
   scoped_ptr<ClientContext> client_context_;
   scoped_ptr<protocol::ConnectionToHost> connection_;
   scoped_ptr<ChromotingClient> client_;
-  scoped_ptr<XmppSignalStrategy::XmppServerConfig> chat_config_;
-  scoped_ptr<XmppSignalStrategy> chat_;  // must outlive client_
-  scoped_ptr<NetworkSettings> netset_;
-  protocol::SecretFetchedCallback announce_secret_;
-
-  // 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_;
+  scoped_ptr<XmppSignalStrategy::XmppServerConfig> signaling_config_;
+  scoped_ptr<XmppSignalStrategy> signaling_;  // must outlive client_
+  scoped_ptr<NetworkSettings> network_settings_;
+  protocol::SecretFetchedCallback pin_callback_;
+
+  // These strings describe the current connection, and are not reused.
+  std::string username_;
+  std::string auth_token_;
+  std::string host_jid_;
+  std::string host_id_;
+  std::string host_pubkey_;
+  std::string pin_;
 
   friend struct DefaultSingletonTraits<ChromotingJNIInstance>;