Refactoring and rewriting the chromoting jni instance to be chromoting session.

remoting/client/client_telemetry_logger.cc

 // Copyright 2016 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/client_telemetry_logger.h"
 
+#if defined(OS_ANDROID)

[Lambros, 2017/04/05 01:31:19]

Move below the other #includes.

[nicholss, 2017/04/05 17:24:26]

Acknowledged.

+#include <android/log.h>
+#endif  // OS_ANDROID
+
+#include "base/format_macros.h"
 #include "base/logging.h"
 #include "base/rand_util.h"
+#include "base/strings/stringprintf.h"
 #include "remoting/base/telemetry_log_writer.h"
 
 namespace {
 
 const char kSessionIdAlphabet[] =
     "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890";
 const int kSessionIdLength = 20;
 const int kMaxSessionIdAgeDays = 1;
 
 }  // namespace
@@ skipping 36 matching lines @@
     session_id_.clear();
     session_start_time_ = base::TimeTicks();
   }
 }
 
 void ClientTelemetryLogger::LogStatistics(
     protocol::PerformanceTracker* perf_tracker) {
   DCHECK(thread_checker_.CalledOnValidThread());
   RefreshSessionIdIfOutdated();
 
+  PrintLogStatistics(perf_tracker);
+
   ChromotingEvent event = MakeStatsEvent(perf_tracker);
   log_writer_->Log(event);
 }
 
+#if defined(OS_ANDROID)
+void ClientTelemetryLogger::PrintLogStatistics(
+    protocol::PerformanceTracker* perf_tracker) {
+  __android_log_print(
+      ANDROID_LOG_INFO, "stats",
+      "Bandwidth:%.0f FrameRate:%.1f;"
+      " (Avg, Max) Capture:%.1f, %" PRId64 " Encode:%.1f, %" PRId64
+      " Decode:%.1f, %" PRId64 " Render:%.1f, %" PRId64 " RTL:%.0f, %" PRId64,
+      perf_tracker->video_bandwidth(), perf_tracker->video_frame_rate(),
+      perf_tracker->video_capture_ms().Average(),
+      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());
+}
+#else
+void ClientTelemetryLogger::PrintLogStatistics(
+    protocol::PerformanceTracker* perf_tracker) {
+  VLOG(1) << base::StringPrintf(

[Yuwei, 2017/04/03 21:40:46]

What about putting the #ifdefs inside the function body?

[nicholss, 2017/04/03 21:56:00]

I don't think that makes this more readable.

+      "Bandwidth:%.0f FrameRate:%.1f;"

[Lambros, 2017/04/05 01:31:19]

Duplicating this code makes me sad. Could you write this to a temporary string
object, then use #ifdef to log the string differently on Android?

On Android, I think we do want to see this particular log, so we should keep the
#ifdef in this case. Ideally, we would display the numbers on the screen in
real-time (and provide an option for the user to toggle this off/on). Writing to
the system log is the next-best thing.

[nicholss, 2017/04/05 17:24:25]

That type of feature is pretty easy now with the addition of drawables. Which is
why I implemented it. I will look into adding this when I get to the rendering
part of the app.

+      " (Avg, Max) Capture:%.1f, %" PRId64 " Encode:%.1f, %" PRId64
+      " Decode:%.1f, %" PRId64 " Render:%.1f, %" PRId64 " RTL:%.0f, %" PRId64,
+      perf_tracker->video_bandwidth(), perf_tracker->video_frame_rate(),
+      perf_tracker->video_capture_ms().Average(),
+      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());
+}
+#endif  // OS_ANDROID
+
 void ClientTelemetryLogger::SetSessionIdGenerationTimeForTest(
     base::TimeTicks gen_time) {
   session_id_generation_time_ = gen_time;
 }
 
 // static
 ChromotingEvent::SessionState ClientTelemetryLogger::TranslateState(
     protocol::ConnectionToHost::State state) {
   switch (state) {
     case protocol::ConnectionToHost::State::INITIALIZING:
@@ skipping 148 matching lines @@
   return event;
 }
 
 ChromotingEvent ClientTelemetryLogger::MakeSessionIdNewEvent() {
   ChromotingEvent event(ChromotingEvent::Type::SESSION_ID_NEW);
   FillEventContext(&event);
   return event;
 }
 
 }  // namespace remoting