Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1009)

Unified Diff: remoting/client/jni/chromoting_jni_instance.cc

Issue 2643483003: [Remoting Android] Refactor ClientTelemetryLogger (Closed)
Patch Set: PTAL Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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() c
Yuwei 2017/01/19 23:00:00 Done.
+ logger_.reset();
client_.reset();
audio_player_.reset();
video_renderer_.reset();

Powered by Google App Engine
This is Rietveld 408576698