[Remoting Android] Refactor ClientTelemetryLogger

remoting/client/client_telemetry_logger.cc

Index: remoting/client/client_telemetry_logger.cc
diff --git a/remoting/client/client_telemetry_logger.cc b/remoting/client/client_telemetry_logger.cc
index 7d47d10f5b28681b424d524c8cd9cd15e52a3eff..54fa1577dc2d773d093b3ca6a9e9b138fde1358c 100644
--- a/remoting/client/client_telemetry_logger.cc
+++ b/remoting/client/client_telemetry_logger.cc
@@ -13,7 +13,6 @@ namespace {
 const char kSessionIdAlphabet[] =
     "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890";
 const int kSessionIdLength = 20;
-
 const int kMaxSessionIdAgeDays = 1;
 
 }  // namespace
@@ -26,8 +25,10 @@ struct ClientTelemetryLogger::HostInfo {
   const std::string host_os_version;
 };
 
-ClientTelemetryLogger::ClientTelemetryLogger(ChromotingEvent::Mode mode)
-    : mode_(mode) {}
+ClientTelemetryLogger::ClientTelemetryLogger(
+    base::WeakPtr<ChromotingEventLogWriter> log_writer,

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

ChromotingJniRuntime is guaranteed to outlive ChromotingJniInstance, so
log_writer will always outlive ClientTelemetryLogger. So do we really need to
use weak pointers here instead of raw pointers?

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

Done. You are right. It should be safe enough to use raw pointer.

+    ChromotingEvent::Mode mode)
+    : mode_(mode), log_writer_(log_writer) {}
 
 ClientTelemetryLogger::~ClientTelemetryLogger() {}
 
@@ -37,25 +38,11 @@ void ClientTelemetryLogger::SetHostInfo(const std::string& host_version,
   host_info_.reset(new HostInfo{host_version, host_os, host_os_version});
 }
 
-void ClientTelemetryLogger::Start(
-    std::unique_ptr<UrlRequestFactory> request_factory,
-    const std::string& telemetry_base_url) {
-  DCHECK(!log_writer_);
-  DCHECK(thread_checker_.CalledOnValidThread());
-  log_writer_.reset(
-      new TelemetryLogWriter(telemetry_base_url, std::move(request_factory)));
-}
-
 void ClientTelemetryLogger::LogSessionStateChange(
     ChromotingEvent::SessionState state,
     ChromotingEvent::ConnectionError error) {
-  DCHECK(log_writer_) << "Please call Start before posting logs.";
   DCHECK(thread_checker_.CalledOnValidThread());
-  ExpireSessionIdIfOutdated();
-
-  if (session_id_.empty()) {
-    GenerateSessionId();
-  }
+  RefreshSessionIdIfOutdated();
   if (session_start_time_.is_null()) {
     session_start_time_ = base::TimeTicks::Now();
   }
@@ -72,37 +59,18 @@ void ClientTelemetryLogger::LogSessionStateChange(
 
 void ClientTelemetryLogger::LogStatistics(
     protocol::PerformanceTracker* perf_tracker) {
-  DCHECK(log_writer_) << "Please call Start before posting logs.";
   DCHECK(thread_checker_.CalledOnValidThread());
-  ExpireSessionIdIfOutdated();
+  RefreshSessionIdIfOutdated();
 
   ChromotingEvent event = MakeStatsEvent(perf_tracker);
   log_writer_->Log(event);
 }
 
-void ClientTelemetryLogger::SetAuthToken(const std::string& token) {
-  DCHECK(log_writer_) << "Please call Start before setting the token.";
-  DCHECK(thread_checker_.CalledOnValidThread());
-  log_writer_->SetAuthToken(token);
-}
-
-void ClientTelemetryLogger::SetAuthClosure(const base::Closure& closure) {
-  DCHECK(log_writer_) << "Please call Start before setting the closure.";
-  DCHECK(thread_checker_.CalledOnValidThread());
-  log_writer_->SetAuthClosure(closure);
-}
-
 void ClientTelemetryLogger::SetSessionIdGenerationTimeForTest(
     base::TimeTicks gen_time) {
   session_id_generation_time_ = gen_time;
 }
 
-void ClientTelemetryLogger::StartForTest(
-    std::unique_ptr<ChromotingEventLogWriter> writer) {
-  DCHECK(!log_writer_);
-  log_writer_ = std::move(writer);
-}
-
 // static
 ChromotingEvent::SessionState ClientTelemetryLogger::TranslateState(
     protocol::ConnectionToHost::State state) {
@@ -191,8 +159,9 @@ void ClientTelemetryLogger::GenerateSessionId() {
   session_id_generation_time_ = base::TimeTicks::Now();
 }
 
-void ClientTelemetryLogger::ExpireSessionIdIfOutdated() {
+void ClientTelemetryLogger::RefreshSessionIdIfOutdated() {
   if (session_id_.empty()) {
+    GenerateSessionId();
     return;
   }