Separate singleton out of ChromotingJNIInstance

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 05ee17f1960484c3309cff36ad84a94f9f8cfe2f..069c8bcdc0780e84d2fdbd318640afead687c65a 100644
--- a/remoting/client/jni/chromoting_jni_instance.h
+++ b/remoting/client/jni/chromoting_jni_instance.h
@@ -5,15 +5,11 @@
 #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/chromoting_client.h"
 #include "remoting/client/client_config.h"
 #include "remoting/client/client_context.h"
@@ -23,12 +19,8 @@
 #include "remoting/jingle_glue/xmpp_signal_strategy.h"
 #include "remoting/protocol/connection_to_host.h"
 
-template<typename T> struct DefaultSingletonTraits;
-
 namespace remoting {
-
-// Class and package name of the Java class supporting the methods we call.
-const char* const JAVA_CLASS = "org/chromium/chromoting/jni/JNIInterface";
+class ChromotingJni;
 
 // TODO(solb) Move into location shared with client plugin.
 const char* const CHAT_SERVER = "talk.google.com";
@@ -36,28 +28,22 @@ 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. It also
-// contains global resources on which the Chromoting components run
-// (e.g. message loops and task runners).
-class ChromotingJNIInstance : public ClientUserInterface {
+// ClientUserInterface that indirectly makes and receives JNI calls.
+class ChromotingJniInstance
+  : public ClientUserInterface,
+    public base::RefCountedThreadSafe<ChromotingJniInstance> {

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

Why does this need to be RefCountedThreadSafe?

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

In order to maintain thread safety in ChromotingJni. That class needs to create
an instance of this one on the UI thread; however, this one must be torn down
almost entirely on the network thread. If this weren't ref counted, there would
be a weird period while an instance's members were being destroyed but
ChromotingJni still held a reference to the (now terribly invalid) object. The
way it is now, the UI thread can post the cleanup procedure to the network
thread and immediately nullify the pointer to the class; thus, the singleton can
forget about the instance as soon as it becomes invalid.

  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();
-
-  // Initiates a connection with the specified host. This may only be called
-  // when |connected_| is false, and must be invoked on the UI thread.
-  void ConnectToHost(
+  // Initiates a connection with the specified host. Call from the UI thread.
+  ChromotingJniInstance(
       const char* username,
       const char* auth_token,
       const char* host_jid,
       const char* host_id,
       const char* host_pubkey);
 
-  // Terminates the current connection (if it hasn't already failed) and clean
-  // up. This may only be called when |connected_|, and only from the UI thread.
-  void DisconnectFromHost();
+  // Terminates the current connection (if it hasn't already failed) and cleans
+  // up. Must be called before destruction.
+  void Cleanup();
 
   // Provides the user's PIN and resumes the host authentication attempt. Call
   // on the UI thread once the user has finished entering this PIN into the UI,
@@ -78,53 +64,19 @@ class ChromotingJNIInstance : public ClientUserInterface {
       GetTokenFetcher(const std::string& host_public_key) OVERRIDE;
 
  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.
-  // As a singleton, this object must be destroyed on the main (UI) thread.
-  virtual ~ChromotingJNIInstance();
+  // This object is ref-counted, so it cleans itself up.
+  virtual ~ChromotingJniInstance();
 
   void ConnectToHostOnDisplayThread();
   void ConnectToHostOnNetworkThread();
 
-  void DisconnectFromHostOnNetworkThread();
-
   // Notifies the user interface that the user needs to enter a PIN. The
   // current authentication attempt is put on hold until |callback| is invoked.
   void FetchSecret(bool pairable,
                    const protocol::SecretFetchedCallback& callback);
 
-  // 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_;
-
-  // 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> frame_consumer_;
 
-  // All 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).
-  // To be used on the UI thread.
-  bool connected_;
-
   // This group of variables is to be used on the network thread.
   scoped_ptr<ClientConfig> client_config_;
   scoped_ptr<ClientContext> client_context_;
@@ -148,9 +100,9 @@ class ChromotingJNIInstance : public ClientUserInterface {
   std::string host_id_;
   std::string host_pubkey_;
 
-  friend struct DefaultSingletonTraits<ChromotingJNIInstance>;
+  friend class base::RefCountedThreadSafe<ChromotingJniInstance>;
 
-  DISALLOW_COPY_AND_ASSIGN(ChromotingJNIInstance);
+  DISALLOW_COPY_AND_ASSIGN(ChromotingJniInstance);
 };
 
 }  // namespace remoting