Fix PerformanceTracker to use its own timer to send UMA stats.

Previously the same timer was used to update stats in UI and UMA stats.
Also now UMA is not called when the video stream is paused.

BUG=508602

Review URL: https://codereview.chromium.org/1352033003

Cr-Commit-Position: refs/heads/master@{#349973}

2 files changed, 15 insertions, 24 deletions

diff --git a/remoting/client/plugin/chromoting_instance.cc b/remoting/client/plugin/chromoting_instance.cc
index fabda64..05a1531 100644
--- a/remoting/client/plugin/chromoting_instance.cc
+++ b/remoting/client/plugin/chromoting_instance.cc
@@ -85,6 +85,9 @@ const int kConnectionDurationHistogramMinMinutes = 1;
 const int kConnectionDurationHistogramMaxMinutes = 24 * 60;
 const int kConnectionDurationHistogramBuckets = 50;
 
+// Update perf stats in the UI every second.
+const int kUIStatsUpdatePeriodSeconds = 1;
+
 // TODO(sergeyu): Ideally we should just pass ErrorCode to the webapp
 // and let it handle it, but it would be hard to fix it now because
 // client plugin and webapp versions may not be in sync. It should be
@@ -715,11 +718,10 @@ void ChromotingInstance::HandleConnect(const base::DictionaryValue& data) {
                  transport_factory.Pass(), host_jid, capabilities);
 
   // Start timer that periodically sends perf stats.
-  plugin_task_runner_->PostDelayedTask(
-      FROM_HERE, base::Bind(&ChromotingInstance::SendPerfStats,
-                            weak_factory_.GetWeakPtr()),
-      base::TimeDelta::FromSeconds(
-          protocol::PerformanceTracker::kStatsUpdateFrequencyInSeconds));
+  stats_update_timer_.Start(
+      FROM_HERE, base::TimeDelta::FromSeconds(kUIStatsUpdatePeriodSeconds),
+      base::Bind(&ChromotingInstance::UpdatePerfStatsInUI,
+                 base::Unretained(this)));
 }
 
 void ChromotingInstance::HandleDisconnect(const base::DictionaryValue& data) {
@@ -845,6 +847,7 @@ void ChromotingInstance::HandleVideoControl(const base::DictionaryValue& data) {
   bool pause_video = false;
   if (data.GetBoolean("pause", &pause_video)) {
     video_control.set_enable(!pause_video);
+    perf_tracker_.OnPauseStateChanged(pause_video);
   }
   bool lossless_encode = false;
   if (data.GetBoolean("losslessEncode", &lossless_encode)) {
@@ -988,6 +991,7 @@ void ChromotingInstance::Disconnect() {
   mouse_input_filter_.set_input_stub(nullptr);
   client_.reset();
   video_renderer_.reset();
+  stats_update_timer_.Stop();
 }
 
 void ChromotingInstance::PostChromotingMessage(const std::string& method,
@@ -1023,17 +1027,7 @@ void ChromotingInstance::SendOutgoingIq(const std::string& iq) {
   PostLegacyJsonMessage("sendOutgoingIq", data.Pass());
 }
 
-void ChromotingInstance::SendPerfStats() {
-  if (!video_renderer_.get()) {
-    return;
-  }
-
-  plugin_task_runner_->PostDelayedTask(
-      FROM_HERE, base::Bind(&ChromotingInstance::SendPerfStats,
-                            weak_factory_.GetWeakPtr()),
-      base::TimeDelta::FromSeconds(
-          protocol::PerformanceTracker::kStatsUpdateFrequencyInSeconds));
-
+void ChromotingInstance::UpdatePerfStatsInUI() {
   // Fetch performance stats from the VideoRenderer and send them to the client
   // for display to users.
   scoped_ptr<base::DictionaryValue> data(new base::DictionaryValue());
@@ -1045,11 +1039,6 @@ void ChromotingInstance::SendPerfStats() {
   data->SetDouble("renderLatency", perf_tracker_.video_paint_ms());
   data->SetDouble("roundtripLatency", perf_tracker_.round_trip_ms());
   PostLegacyJsonMessage("onPerfStats", data.Pass());
-
-  // Record the video frame-rate, packet-rate and bandwidth stats to UMA.
-  // TODO(anandc): Create a timer in PerformanceTracker to do this work.
-  // See http://crbug/508602.
-  perf_tracker_.UploadRateStatsToUma();
 }
 
 // static
diff --git a/remoting/client/plugin/chromoting_instance.h b/remoting/client/plugin/chromoting_instance.h
index 1ad4767..5f582d9 100644
--- a/remoting/client/plugin/chromoting_instance.h
+++ b/remoting/client/plugin/chromoting_instance.h
@@ -234,7 +234,7 @@ class ChromotingInstance : public ClientUserInterface,
   // Callback for DelegatingSignalStrategy.
   void SendOutgoingIq(const std::string& iq);
 
-  void SendPerfStats();
+  void UpdatePerfStatsInUI();
 
   // Returns true if there is a ConnectionToHost and it is connected.
   bool IsConnected();
@@ -287,13 +287,15 @@ class ChromotingInstance : public ClientUserInterface,
 
   base::WeakPtr<TokenFetcherProxy> token_fetcher_proxy_;
 
-  // Weak reference to this instance, used for global logging and task posting.
-  base::WeakPtrFactory<ChromotingInstance> weak_factory_;
+  base::RepeatingTimer<ChromotingInstance> stats_update_timer_;
 
   base::TimeTicks connection_started_time;
   base::TimeTicks connection_authenticated_time_;
   base::TimeTicks connection_connected_time_;
 
+  // Weak reference to this instance, used for global logging and task posting.
+  base::WeakPtrFactory<ChromotingInstance> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(ChromotingInstance);
 };