commit e71aa499d637c48b159dc4ee7638f0003789e409
author Wade Guthrie <wdg@chromium.org> Mon Sep 23 13:44:19 2013 -0700
committer chrome-internal-fetch <chrome-internal-fetch@google.com> Tue Sep 24 01:32:40 2013 +0000
tree 40fccb4958f237179ba3e50afe0be1023aa7409d
parent 39a7beb18a0c24c9b73c3cc49008ccdca19f9ac2

shill: Removes unused code in metrics.cc, adds comment to wifi.

While toiling in metrics.cc (NotifyDeviceConnectFinished, to be
specific), I spied a couple lines that were unused.  In a fit of
efficiency, I removed them.

At a nearby time, I noticed that, while background scan times were not
recorded, it's ok that we leave them unrecorded.  I thought a comment
explaining why might reduce future consternation.  I added such a
comment.

Lint also pointed out a missing space in existing code (in metrics.cc).
Upon reflection, I attended to this oversight.

BUG=None
TEST=unittest

Change-Id: I996909a1d8d8e126db3c6197c517b7b6521f9d6a
Reviewed-on: https://chromium-review.googlesource.com/170309
Reviewed-by: Paul Stewart <pstew@chromium.org>
Reviewed-by: Wade Guthrie <wdg@chromium.org>
Commit-Queue: Wade Guthrie <wdg@chromium.org>
Tested-by: Wade Guthrie <wdg@chromium.org>

metrics.cc

diff --git a/metrics.cc b/metrics.cc
index 869ed19..75e795b 100644
--- a/metrics.cc
+++ b/metrics.cc
@@ -923,8 +923,6 @@
 
   if (!device_metrics->scan_connect_timer->Stop())
     return;
-  base::TimeDelta elapsed_time;
-  device_metrics->scan_connect_timer->GetElapsedTime(&elapsed_time);
   device_metrics->scan_connect_timer->ReportMilliseconds();
 }
 
@@ -1050,7 +1048,7 @@
   }
 
   TimerReportersList &stop_timers = service_metrics->stop_on_state[new_state];
-  for (auto &stop_timer: stop_timers) {
+  for (auto &stop_timer : stop_timers) {
     SLOG(Metrics, 5) << "Stopping timer for " << stop_timer->histogram_name()
                      << " due to new state " << state_string << ".";
     if (stop_timer->Stop())

wifi.cc

diff --git a/wifi.cc b/wifi.cc
index 319fdc8..dd52c17 100644
--- a/wifi.cc
+++ b/wifi.cc
@@ -1167,6 +1167,11 @@
     manager()->OnDeviceGeolocationInfoUpdated(this);
   }
   if (scan_state_ == kScanBackgroundScanning) {
+    // Going directly to kScanIdle (instead of to kScanFoundNothing) inhibits
+    // some UMA reporting in SetScanState.  That's desired -- we don't want
+    // to report background scan results to UMA since the drivers may play
+    // background scans over a longer period in order to not interfere with
+    // traffic.
     SetScanState(kScanIdle, kScanMethodNone, __func__);
   } else if (scan_state_ != kScanIdle && IsIdle()) {
     SetScanState(kScanFoundNothing, scan_method_, __func__);
@@ -1971,6 +1976,9 @@
   } else if (new_state == kScanConnected || new_state == kScanFoundNothing) {
     // These 'terminal' states are slightly more interesting than the
     // intermediate states.
+    // NOTE: Since background scan goes directly to kScanIdle (skipping over
+    // the states required to set |is_terminal_state|), ReportScanResultToUma,
+    // below, doesn't get called.  That's intentional.
     log_level = 5;
     is_terminal_state = true;
   }