[Remoting Android] Refactor ClientTelemetryLogger

remoting/client/jni/chromoting_jni_instance.cc

 // 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.
 
 #include "remoting/client/jni/chromoting_jni_instance.h"
 
 #include <android/log.h>
 #include <stdint.h>
 
 #include "base/bind.h"
@@ skipping 36 matching lines @@
 const int kPerfStatsIntervalMs = 60000;
 
 }  // namespace
 
 ChromotingJniInstance::ChromotingJniInstance(
     ChromotingJniRuntime* jni_runtime,
     base::WeakPtr<JniClient> jni_client,
     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();
 
   // Initialize XMPP config.
   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() {
   DCHECK(jni_runtime_->network_task_runner()->BelongsToCurrentThread());
   if (client_) {
     ReleaseResources();
   }
 }
 
 void ChromotingJniInstance::Connect() {
@@ skipping 14 matching lines @@
         base::Bind(&ChromotingJniInstance::Disconnect, GetWeakPtr()));
     return;
   }
 
   stats_logging_enabled_ = false;
 
   // User disconnection will not trigger OnConnectionState(Closed, OK).
   // 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;
   }
 
   ReleaseResources();
 }
 
 void ChromotingJniInstance::FetchThirdPartyToken(
     const std::string& host_public_key,
@@ skipping 156 matching lines @@
 void ChromotingJniInstance::OnConnectionState(
     protocol::ConnectionToHost::State state,
     protocol::ErrorCode error) {
   DCHECK(jni_runtime_->network_task_runner()->BelongsToCurrentThread());
 
   // This code assumes no intermediate connection state between CONNECTED and
   // CLOSED/FAILED.
   connected_ = state == protocol::ConnectionToHost::CONNECTED;
   EnableStatsLogging(connected_);
 
-  jni_runtime_->logger()->LogSessionStateChange(
+  logger_->LogSessionStateChange(
       ClientTelemetryLogger::TranslateState(state),
       ClientTelemetryLogger::TranslateError(error));
 
   if (create_pairing_ && state == protocol::ConnectionToHost::CONNECTED) {
     protocol::PairingRequest request;
     DCHECK(!device_name_.empty());
     request.set_client_name(device_name_);
     client_->host_stub()->RequestPairing(request);
   }
 
@@ skipping 54 matching lines @@
   NOTIMPLEMENTED();
 }
 
 base::WeakPtr<ChromotingJniInstance> ChromotingJniInstance::GetWeakPtr() {
   return weak_ptr_;
 }
 
 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()));
   client_context_->Start();
 
   perf_tracker_.reset(new protocol::PerformanceTracker());
 
   video_renderer_->Initialize(*client_context_,
                               perf_tracker_.get());
 
   if (!audio_player_) {
     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()));
 
   signaling_.reset(
       new XmppSignalStrategy(net::ClientSocketFactory::GetDefaultFactory(),
                              jni_runtime_->url_requester(), xmpp_config_));
 
   scoped_refptr<protocol::TransportContext> transport_context =
       new protocol::TransportContext(
           signaling_.get(),
           base::MakeUnique<protocol::ChromiumPortAllocatorFactory>(),
           base::MakeUnique<ChromiumUrlRequestFactory>(
               jni_runtime_->url_requester()),
           protocol::NetworkSettings(
               protocol::NetworkSettings::NAT_TRAVERSAL_FULL),
           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();
     protocol_config->set_webrtc_supported(true);
     protocol_config->set_ice_supported(false);
     client_->set_protocol_config(std::move(protocol_config));
   }
 #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) {
   if (!jni_runtime_->network_task_runner()->BelongsToCurrentThread()) {
     jni_runtime_->network_task_runner()->PostTask(
         FROM_HERE, base::Bind(&ChromotingJniInstance::SetDeviceName,
                               GetWeakPtr(), device_name));
     return;
   }
 
@@ skipping 43 matching lines @@
       perf_tracker_->video_capture_ms().Max(),
       perf_tracker_->video_encode_ms().Average(),
       perf_tracker_->video_encode_ms().Max(),
       perf_tracker_->video_decode_ms().Average(),
       perf_tracker_->video_decode_ms().Max(),
       perf_tracker_->video_paint_ms().Average(),
       perf_tracker_->video_paint_ms().Max(),
       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()),
       base::TimeDelta::FromMilliseconds(kPerfStatsIntervalMs));
 }
 
 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();
   signaling_.reset();
   perf_tracker_.reset();
   client_context_.reset();
   cursor_shape_stub_.reset();
 
   weak_factory_.InvalidateWeakPtrs();
 }
 
 }  // namespace remoting