Battery impact UMA

content/browser/power_usage_monitor_impl.cc

+// Copyright 2014 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 "content/browser/power_usage_monitor_impl.h"
+
+#include "base/bind.h"
+#include "base/lazy_instance.h"
+#include "base/logging.h"
+#include "base/macros.h"
+#include "base/message_loop/message_loop.h"

[Daniel Erat, 2014/11/04 17:04:02]

don't think you're using this now

[jeremy, 2014/11/05 11:40:19]

Done.

+#include "base/metrics/histogram.h"
+#include "base/strings/stringprintf.h"
+#include "base/sys_info.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/notification_service.h"
+#include "content/public/browser/notification_source.h"
+#include "content/public/browser/notification_types.h"
+#include "content/public/browser/power_usage_monitor.h"
+#include "content/public/browser/render_process_host.h"
+
+namespace content {
+
+namespace {
+
+class PowerUsageMonitorSystemInterface
+    : public PowerUsageMonitor::SystemInterface {
+ public:
+  explicit PowerUsageMonitorSystemInterface(PowerUsageMonitor* owner)
+      : power_usage_monitor_(owner),
+        weak_ptr_factory_(this) {}
+  virtual ~PowerUsageMonitorSystemInterface() {}

[Daniel Erat, 2014/11/04 17:04:03]

override instead of virtual

[jeremy, 2014/11/05 11:40:19]

Done.

+
+  void ScheduleHistogramReport(base::TimeDelta delay) override {
+    BrowserThread::PostDelayedTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(
+            &PowerUsageMonitorSystemInterface::ReportBatteryLevelHistogram,
+            weak_ptr_factory_.GetWeakPtr(),
+            Now(),
+            delay),
+        delay);
+  }
+
+  void CancelPendingHistogramReports() override {
+    weak_ptr_factory_.InvalidateWeakPtrs();
+  }
+
+  base::Time Now() override { return base::Time::Now(); }
+
+ protected:
+  void ReportBatteryLevelHistogram(base::Time start_time,
+                                   base::TimeDelta discharge_time) {
+    // It's conceivable that the code to cancel pending histogram reports on
+    // system suspend, will only get called after the system has woken up.
+    // To mitigage this, check whether more time has passed than expected and
+    // abort histogram recording in this case.
+
+    // Delayed tasks are subject to timer coalescing and can fire anywhere from
+    // delay -> delay * 1.5) . In most cases, the OS should fire the task
+    // at the next wakeup and not as late as it can.
+    // A threshold of 2 minutes is used, since that should be large enough to
+    // take the slop factor due to coalescing into account.
+    base::TimeDelta threshold = discharge_time +
+        base::TimeDelta::FromMinutes(2);
+    if ((Now() - start_time) > threshold) {
+      UMA_HISTOGRAM_BOOLEAN("Power.BatteryDischargeReportsCancelled", true);
+      return;
+    }
+
+    const std::string histogram_name = base::StringPrintf(
+        "Power.BatteryDischarge_%d", discharge_time.InMinutes());
+    base::HistogramBase* histogram =
+        base::Histogram::FactoryGet(histogram_name,
+                                    1,
+                                    100,
+                                    101,
+                                    base::Histogram::kUmaTargetedHistogramFlag);
+    double discharge_amount = power_usage_monitor_->discharge_amount();
+    histogram->Add(discharge_amount * 100);
+  }
+
+ private:
+  PowerUsageMonitor* power_usage_monitor_;  // Not owned.
+
+  // Used to cancel in progress delayed tasks.
+  base::WeakPtrFactory<PowerUsageMonitorSystemInterface> weak_ptr_factory_;
+};
+
+}  // namespace
+
+void StartPowerUsageMonitor() {
+  static base::LazyInstance<PowerUsageMonitor>::Leaky monitor =
+     LAZY_INSTANCE_INITIALIZER;
+  monitor.Get().Start();
+}
+
+PowerUsageMonitor::PowerUsageMonitor()
+    : callback_(base::Bind(&PowerUsageMonitor::OnBatteryStatusUpdate,
+          base::Unretained(this))),
+      system_interface_(new PowerUsageMonitorSystemInterface(this)),
+      started_(false),
+      on_battery_power_(false),
+      initial_battery_level_(0),
+      current_battery_level_(0) {
+}
+
+PowerUsageMonitor::~PowerUsageMonitor() {
+  if (started_)
+    base::PowerMonitor::Get()->RemoveObserver(this);
+}
+
+void PowerUsageMonitor::Start() {
+  // Delay initialization until the system has been up for at least 30 minutes.
+  // This is to mitigate the effect of increased power draw during system start.
+  int64 uptime_in_minutes = base::SysInfo::Uptime() / (60 * 1000);

[Daniel Erat, 2014/11/04 17:04:03]

cleaner to just convert this to a TimeDelta immediately (it's ugly that Uptime()
doesn't return this to begin with) :-/

  const base::TimeDelta uptime = base::TimeDelta::FromMilliseconds(
      base::SysInfo::Uptime());
  const base::TimeDelta min_uptime = base::TimeDelta::FromMinutes(
      kMinUptimeMinutes);
  if (uptime < min_uptime) {
    BrowserThread::PostDelayedTask(
        ...,
        min_uptime - uptime);
  } else {
    StartInternal();
  }

(and define kMinUptimeMinutes in the anon namespace at the top of the file)

[jeremy, 2014/11/05 11:40:19]

Done.

+  if (uptime_in_minutes < 30) {
+    base::TimeDelta delay =
+        base::TimeDelta::FromMinutes(30 - uptime_in_minutes);
+    BrowserThread::PostDelayedTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(&PowerUsageMonitor::StartInternal, base::Unretained(this)),
+        delay);
+  } else {
+    StartInternal();
+  }
+}
+
+void PowerUsageMonitor::StartInternal() {

[Daniel Erat, 2014/11/04 17:04:03]

nit: CHECK(!started_);

[jeremy, 2014/11/05 11:40:19]

Done.

+  started_ = true;
+
+  // PowerMonitor is used to get suspend/resume notifications.
+  base::PowerMonitor::Get()->AddObserver(this);
+
+  registrar_.Add(this,
+                 NOTIFICATION_RENDERER_PROCESS_CREATED,
+                 NotificationService::AllBrowserContextsAndSources());
+  registrar_.Add(this,
+                 NOTIFICATION_RENDERER_PROCESS_TERMINATED,
+                 NotificationService::AllBrowserContextsAndSources());
+  registrar_.Add(this,
+                 NOTIFICATION_RENDERER_PROCESS_CLOSED,
+                 NotificationService::AllBrowserContextsAndSources());
+  subscription_ =
+      device::BatteryStatusService::GetInstance()->AddCallback(callback_);
+}
+
+void PowerUsageMonitor::DischargeStarted(double battery_level) {
+  // If all browser windows are closed, don't report power metrics since
+  // Chrome's power draw is likely not significant.
+  if (live_renderer_ids_.empty())
+    return;
+
+  // Cancel any in-progress ReportBatteryLevelHistogram() calls.
+  system_interface_->CancelPendingHistogramReports();
+
+  on_battery_power_ = true;
+  initial_battery_level_ = battery_level;
+  current_battery_level_ = battery_level;
+  start_discharge_time_ = system_interface_->Now();
+
+  const int kBatteryReportingIntervalMinutes[] = {5, 15, 30};
+  for (auto reporting_interval : kBatteryReportingIntervalMinutes) {
+    base::TimeDelta delay = base::TimeDelta::FromMinutes(reporting_interval);
+    system_interface_->ScheduleHistogramReport(delay);
+  }
+}
+
+void PowerUsageMonitor::WallPowerConnected(double battery_level) {
+  // Cancel any in-progress ReportBatteryLevelHistogram() calls.
+  system_interface_->CancelPendingHistogramReports();
+
+  if (!start_discharge_time_.is_null()) {
+    base::TimeDelta discharge_time =
+        system_interface_->Now() - start_discharge_time_;
+
+    if (discharge_time.InMinutes() > 30) {

[Daniel Erat, 2014/11/04 17:04:02]

move the "30" to a constant at the top of the file, e.g. kMinDischargeMinutes

[jeremy, 2014/11/05 11:40:19]

Done.

+      double discharge_percent = initial_battery_level_ - battery_level;
+      double discharge_rate = discharge_percent / discharge_time.InMinutes();
+
+        UMA_HISTOGRAM_PERCENTAGE("Power.BatteryDischargeRateWhenUnplugged",
+                                 discharge_rate * 100.0);
+    }
+  }
+
+  on_battery_power_ = false;
+  initial_battery_level_ = 0;
+  current_battery_level_ = 0;
+  start_discharge_time_ = base::Time();
+}
+
+void PowerUsageMonitor::OnBatteryStatusUpdate(
+    const device::BatteryStatus& status) {
+  bool now_on_battery_power = (status.charging == 0);
+  bool was_on_battery_power = on_battery_power_;

[Daniel Erat, 2014/11/04 17:04:02]

i've mentioned it before, but i feel pretty strongly that you should be updating
on_battery_power_ unconditionally right here. by potentially returning from
DischargeStarted() without having set it to true, you end up with a member named
on_battery_power_ that sometimes doesn't accurately reflect whether the system
is on battery power or not. this is a bug waiting to happen later when someone
changes this code and assumes (quite reasonably) that on_battery_power_ is
always kept up-to-date.

perhaps you could add an additional tracking_discharge_ or measuring_discharge_
member that gets updated in DischargeStarted() if you need to be able to defer
measurement when there aren't any live renderers.

[jeremy, 2014/11/05 11:40:19]

Done.

+  double battery_level = status.level;
+
+  if (now_on_battery_power == was_on_battery_power) {
+    if (now_on_battery_power)
+      current_battery_level_ = battery_level;
+    return;
+  } else if (now_on_battery_power) {  // Wall power disconnected.
+    DischargeStarted(battery_level);
+  } else {  // Wall power connected.
+    WallPowerConnected(battery_level);
+  }
+}
+
+void PowerUsageMonitor::SetSystemInterfaceForTest(
+    scoped_ptr<SystemInterface> interface) {
+  system_interface_ = interface.Pass();
+}
+
+void PowerUsageMonitor::SetRenderersForTest(bool has_live_renderers) {
+  live_renderer_ids_.clear();
+  if (has_live_renderers)
+    live_renderer_ids_.insert(1);
+}
+
+void PowerUsageMonitor::OnSuspend() {
+  CancelPendingHistogramReporting();
+}
+
+void PowerUsageMonitor::Observe(int type,
+                                const NotificationSource& source,
+                                const NotificationDetails& details) {
+  RenderProcessHost* rph = Source<RenderProcessHost>(source).ptr();
+
+  size_t previous_num_live_renderers = live_renderer_ids_.size();
+
+  if (type == NOTIFICATION_RENDERER_PROCESS_CREATED) {
+    live_renderer_ids_.insert(rph->GetID());
+  } else if (type == NOTIFICATION_RENDERER_PROCESS_TERMINATED ||
+             type == NOTIFICATION_RENDERER_PROCESS_CLOSED) {
+    live_renderer_ids_.erase(rph->GetID());
+  } else {
+    NOTREACHED()  << "Unexpected notification type: " << type;
+  }
+
+  if (live_renderer_ids_.empty() && previous_num_live_renderers != 0) {
+    // All render processes have died.
+    CancelPendingHistogramReporting();
+  }
+}
+
+void PowerUsageMonitor::CancelPendingHistogramReporting() {
+  // Cancel any in-progress histogram reports and reporting of discharge UMA.
+  system_interface_->CancelPendingHistogramReports();
+  start_discharge_time_ = base::Time();
+}
+
+}  // namespace content