[Remoting Android] Refactor ClientTelemetryLogger

remoting/client/jni/chromoting_jni_instance.cc

Index: remoting/client/jni/chromoting_jni_instance.cc
diff --git a/remoting/client/jni/chromoting_jni_instance.cc b/remoting/client/jni/chromoting_jni_instance.cc
index c11b0775f442dc3b43f754dd7b1aa5dc8481e080..4a0b5def00ad7563c16b7534d313d16384ed99e4 100644
--- a/remoting/client/jni/chromoting_jni_instance.cc
+++ b/remoting/client/jni/chromoting_jni_instance.cc
@@ -54,29 +54,14 @@ ChromotingJniInstance::ChromotingJniInstance(
     base::WeakPtr<JniPairingSecretFetcher> secret_fetcher,
     std::unique_ptr<protocol::CursorShapeStub> cursor_shape_stub,
     std::unique_ptr<protocol::VideoRenderer> video_renderer,
-    const std::string& username,
-    const std::string& auth_token,
-    const std::string& host_jid,
-    const std::string& host_id,
-    const std::string& host_pubkey,
-    const std::string& pairing_id,
-    const std::string& pairing_secret,
-    const std::string& capabilities,
-    const std::string& flags,
-    const std::string& host_version,
-    const std::string& host_os,
-    const std::string& host_os_version)
+    const ConnectToHostInfo& info)
     : jni_runtime_(jni_runtime),
       jni_client_(jni_client),
       secret_fetcher_(secret_fetcher),
-      host_jid_(host_jid),
-      flags_(flags),
+      connection_info_(info),
       cursor_shape_stub_(std::move(cursor_shape_stub)),
       video_renderer_(std::move(video_renderer)),
-      capabilities_(capabilities),
-      host_version_(host_version),
-      host_os_(host_os),
-      host_os_version_(host_os_version),
+      capabilities_(info.capabilities),
       weak_factory_(this) {
   DCHECK(jni_runtime_->ui_task_runner()->BelongsToCurrentThread());
   weak_ptr_ = weak_factory_.GetWeakPtr();
@@ -85,16 +70,17 @@ ChromotingJniInstance::ChromotingJniInstance(
   xmpp_config_.host = kXmppServer;
   xmpp_config_.port = kXmppPort;
   xmpp_config_.use_tls = kXmppUseTls;
-  xmpp_config_.username = username;
-  xmpp_config_.auth_token = auth_token;
+  xmpp_config_.username = info.username;
+  xmpp_config_.auth_token = info.auth_token;
 
-  client_auth_config_.host_id = host_id;
-  client_auth_config_.pairing_client_id = pairing_id;
-  client_auth_config_.pairing_secret = pairing_secret;
+  client_auth_config_.host_id = info.host_id;
+  client_auth_config_.pairing_client_id = info.pairing_id;
+  client_auth_config_.pairing_secret = info.pairing_secret;
   client_auth_config_.fetch_secret_callback =
       base::Bind(&JniPairingSecretFetcher::FetchSecret, secret_fetcher);
   client_auth_config_.fetch_third_party_token_callback = base::Bind(
-      &ChromotingJniInstance::FetchThirdPartyToken, GetWeakPtr(), host_pubkey);
+      &ChromotingJniInstance::FetchThirdPartyToken, GetWeakPtr(),
+      info.host_pubkey);
 }
 
 ChromotingJniInstance::~ChromotingJniInstance() {
@@ -129,7 +115,7 @@ void ChromotingJniInstance::Disconnect() {
   // Remote disconnection will trigger OnConnectionState(...) and later trigger
   // Disconnect().
   if (connected_) {
-    jni_runtime_->logger()->LogSessionStateChange(
+    logger_->LogSessionStateChange(
         ChromotingEvent::SessionState::CLOSED,
         ChromotingEvent::ConnectionError::NONE);
     connected_ = false;
@@ -306,7 +292,7 @@ void ChromotingJniInstance::OnConnectionState(
   connected_ = state == protocol::ConnectionToHost::CONNECTED;
   EnableStatsLogging(connected_);
 
-  jni_runtime_->logger()->LogSessionStateChange(
+  logger_->LogSessionStateChange(
       ClientTelemetryLogger::TranslateState(state),
       ClientTelemetryLogger::TranslateError(error));
 
@@ -381,10 +367,6 @@ base::WeakPtr<ChromotingJniInstance> ChromotingJniInstance::GetWeakPtr() {
 void ChromotingJniInstance::ConnectToHostOnNetworkThread() {
   DCHECK(jni_runtime_->network_task_runner()->BelongsToCurrentThread());
 
-  jni_runtime_->logger()->SetHostInfo(
-      host_version_, ChromotingEvent::ParseOsFromString(host_os_),
-      host_os_version_);
-
   jingle_glue::JingleThreadWrapper::EnsureForCurrentMessageLoop();
 
   client_context_.reset(new ClientContext(jni_runtime_->network_task_runner()));
@@ -399,6 +381,13 @@ void ChromotingJniInstance::ConnectToHostOnNetworkThread() {
     audio_player_.reset(new AudioPlayerAndroid());
   }
 
+  logger_.reset(new ClientTelemetryLogger(jni_runtime_->log_writer(),
+                                          ChromotingEvent::Mode::ME2ME));
+  logger_->SetHostInfo(
+      connection_info_.host_version,
+      ChromotingEvent::ParseOsFromString(connection_info_.host_os),
+      connection_info_.host_os_version);
+
   client_.reset(new ChromotingClient(client_context_.get(), this,
                                      video_renderer_.get(),
                                      audio_player_->GetWeakPtr()));
@@ -418,7 +407,7 @@ void ChromotingJniInstance::ConnectToHostOnNetworkThread() {
           protocol::TransportRole::CLIENT);
 
 #if defined(ENABLE_WEBRTC_REMOTING_CLIENT)
-  if (flags_.find("useWebrtc") != std::string::npos) {
+  if (connection_info_.flags.find("useWebrtc") != std::string::npos) {
     VLOG(0) << "Attempting to connect using WebRTC.";
     std::unique_ptr<protocol::CandidateSessionConfig> protocol_config =
         protocol::CandidateSessionConfig::CreateEmpty();
@@ -428,7 +417,7 @@ void ChromotingJniInstance::ConnectToHostOnNetworkThread() {
   }
 #endif  // defined(ENABLE_WEBRTC_REMOTING_CLIENT)
   client_->Start(signaling_.get(), client_auth_config_, transport_context,
-                 host_jid_, capabilities_);
+                 connection_info_.host_jid, capabilities_);
 }
 
 void ChromotingJniInstance::SetDeviceName(const std::string& device_name) {
@@ -492,7 +481,7 @@ void ChromotingJniInstance::LogPerfStats() {
       perf_tracker_->round_trip_ms().Average(),
       perf_tracker_->round_trip_ms().Max());
 
-  jni_runtime_->logger()->LogStatistics(perf_tracker_.get());
+  logger_->LogStatistics(perf_tracker_.get());
 
   jni_runtime_->network_task_runner()->PostDelayedTask(
       FROM_HERE, base::Bind(&ChromotingJniInstance::LogPerfStats, GetWeakPtr()),
@@ -501,6 +490,7 @@ void ChromotingJniInstance::LogPerfStats() {
 
 void ChromotingJniInstance::ReleaseResources() {
   // |client_| must be torn down before |signaling_|.

[Sergey Ulanov, 2017/01/19 00:50:38]

best to keep this comment before client_.reset() call.

[Yuwei, 2017/01/19 23:00:00]

Done.

+  logger_.reset();
   client_.reset();
   audio_player_.reset();
   video_renderer_.reset();