Add ProcessPowerCollector to audit power information.

chrome/browser/power/process_power_collector.cc

Index: chrome/browser/power/process_power_collector.cc
diff --git a/chrome/browser/power/process_power_collector.cc b/chrome/browser/power/process_power_collector.cc
new file mode 100644
index 0000000000000000000000000000000000000000..bbfb5910799ef1bc80305682d82ab7258dedc352
--- /dev/null
+++ b/chrome/browser/power/process_power_collector.cc
@@ -0,0 +1,200 @@
+// 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 "chrome/browser/power/process_power_collector.h"

[sky, 2014/08/25 20:43:43]

nit: newline between 3/4.

[Daniel Nishi, 2014/08/25 21:08:33]

Done.

+
+#include "apps/app_window.h"
+#include "apps/app_window_registry.h"
+#include "base/process/process_handle.h"
+#include "base/process/process_metrics.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/tab_contents/tab_contents_iterator.h"
+#include "components/power/origin_power_map.h"
+#include "components/power/origin_power_map_factory.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/web_contents.h"
+#include "url/gurl.h"
+
+#if defined(OS_CHROMEOS)
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/power_manager/power_supply_properties.pb.h"
+#endif
+
+namespace {
+const int kSecondsPerSample = 30;
+}
+
+ProcessPowerCollector::PerProcessData::PerProcessData(
+    scoped_ptr<base::ProcessMetrics> metrics,
+    const GURL& origin,
+    Profile* profile)
+    : metrics_(metrics.Pass()),
+      profile_(profile),
+      last_origin_(origin),
+      last_cpu_percent_(0),
+      seen_this_cycle_(true) {
+}
+
+ProcessPowerCollector::PerProcessData::PerProcessData()
+    : profile_(NULL),
+      last_cpu_percent_(0.0),
+      seen_this_cycle_(false) {
+}
+
+ProcessPowerCollector::PerProcessData::~PerProcessData() {
+}
+
+ProcessPowerCollector::ProcessPowerCollector()
+    : scale_factor_(1.0),
+      is_initialized_(false)
+{
+#if defined(OS_CHROMEOS)
+  chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->AddObserver(
+      this);
+#endif
+}
+
+ProcessPowerCollector::~ProcessPowerCollector() {
+#if defined(OS_CHROMEOS)
+  chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->RemoveObserver(
+      this);
+#endif
+}
+
+#if defined(OS_CHROMEOS)
+void ProcessPowerCollector::PowerChanged(
+    const power_manager::PowerSupplyProperties& prop) {
+  if (prop.battery_state() ==
+      power_manager::PowerSupplyProperties::DISCHARGING) {
+    if (is_initialized_ && !timer_.IsRunning())
+      Initialize();

[sky, 2014/08/25 19:38:39]

Init/Initialize methods are generally one shot functions the creator calls.
Having this call Initialize(), especially when the previous line is
is_initialized_, is confusing. How about adding a StartTimer that this and
Initialize call? I would also be inclined to get rid of is_initialized_

[Daniel Nishi, 2014/08/25 20:19:24]

Now in a StartTimer().

I've changed the |is_initialized_| to a |has_started_| -- basically, I need to
guard against the possibility that a power event update in Chrome OS hits that
tries to activate the timer before it has been manually activated through
StartTimer(). This should stop the timer from being accidentally activated
during unit tests and such.

[sky, 2014/08/25 20:43:43]

This patchset had the Initialize() function. The expectation with an
init/initialize function is that it runs costly functions and therefor isn't
part of the constructor. The other expectation is that Init is invoked
immediately after the constructor.

The current patchset does away with Init() and instead expects StartTimer() to
be invoked. This is a slight different behavior. In particular StartTimer()
doesn't imply the function shouldn't be invoked more than once (it shouldn't)
and it doesn't imply that it'll be invoked immediately after the constructor (as
Init() does). The code would be clearer if you make StartTimer() private, add
back the Init() that invokes StartTimer() and do away with has_started_.
where as in the current patchset you've gotten rid of it. 

[Daniel Nishi, 2014/08/25 21:08:33]

Initialize() brought back, StartTimer() made private, |has_started_| removed.

+    scale_factor_ = prop.battery_discharge_rate();
+  } else {
+    timer_.Stop();
+  }
+}
+#endif
+
+void ProcessPowerCollector::Initialize() {
+  DCHECK(!timer_.IsRunning());
+  is_initialized_ = true;
+  timer_.Start(FROM_HERE,
+               base::TimeDelta::FromSeconds(kSecondsPerSample),
+               this,
+               &ProcessPowerCollector::HandleUpdateTimeout);
+}
+
+double ProcessPowerCollector::UpdatePowerConsumptionForTesting() {
+  return UpdatePowerConsumption();
+}
+
+double ProcessPowerCollector::UpdatePowerConsumption() {
+  double total_cpu_percent = SynchronizeProcesses();
+  RecordCpuUsageByOrigin(total_cpu_percent);
+  return total_cpu_percent;
+}
+
+void ProcessPowerCollector::HandleUpdateTimeout() {
+  UpdatePowerConsumption();
+}
+
+double ProcessPowerCollector::SynchronizeProcesses() {
+  // Update all tabs.
+  for (TabContentsIterator it; !it.done(); it.Next()) {
+    content::RenderProcessHost* render_process = it->GetRenderProcessHost();
+    // Skip incognito web contents.
+    if (render_process->GetBrowserContext()->IsOffTheRecord())
+      continue;
+    UpdateProcessInMap(render_process, it->GetLastCommittedURL().GetOrigin());
+  }
+
+  // Iterate over all profiles to find all app windows to attribute all apps.
+  ProfileManager* pm = g_browser_process->profile_manager();
+  std::vector<Profile*> open_profiles = pm->GetLoadedProfiles();
+  for (std::vector<Profile*>::const_iterator it = open_profiles.begin();
+       it != open_profiles.end();
+       ++it) {
+    const apps::AppWindowRegistry::AppWindowList& app_windows =
+        apps::AppWindowRegistry::Get(*it)->app_windows();
+    for (apps::AppWindowRegistry::AppWindowList::const_iterator itr =
+             app_windows.begin();
+         itr != app_windows.end();
+         ++itr) {
+      content::WebContents* web_contents = (*itr)->web_contents();
+
+      UpdateProcessInMap(web_contents->GetRenderProcessHost(),
+                         web_contents->GetLastCommittedURL().GetOrigin());
+    }
+  }
+
+  // Remove invalid processes and sum up the cpu cycle.
+  double total_cpu_percent = 0.0;
+  ProcessMetricsMap::iterator it = metrics_map_.begin();
+  while (it != metrics_map_.end()) {
+    if (!it->second->seen_this_cycle()) {
+      metrics_map_.erase(it++);
+      continue;
+    }
+
+    total_cpu_percent += it->second->last_cpu_percent();
+    it++;

[sky, 2014/08/25 19:38:40]

++it;

[Daniel Nishi, 2014/08/25 20:19:24]

Done.

[Daniel Nishi, 2014/08/25 20:19:24]

Done.

+  }
+
+  return total_cpu_percent;
+}
+
+void ProcessPowerCollector::RecordCpuUsageByOrigin(double total_cpu_percent) {
+  for (ProcessMetricsMap::iterator it = metrics_map_.begin();

[sky, 2014/08/25 19:38:40]

nit: It's a bit weird to do this pass in a function called RecordCpuUsageByOrig.
Maybe it should move to the call site as well as the early out if <= 0.

[Daniel Nishi, 2014/08/25 20:19:24]

Moved to call site.

+       it != metrics_map_.end();
+       ++it) {
+    // Invalidate the process for the next cycle.
+    it->second->set_seen_this_cycle(false);
+  }
+
+  if (total_cpu_percent <= 0)

[sky, 2014/08/25 19:38:39]

I could see the total being 0, but < 0? Isn't that an error?

[Daniel Nishi, 2014/08/25 20:19:24]

Changed to DCHECKing on less than, return on ==.

+    return;
+
+  for (ProcessMetricsMap::iterator it = metrics_map_.begin();
+       it != metrics_map_.end();
+       ++it) {
+    double last_process_power_usage = it->second->last_cpu_percent();
+    last_process_power_usage *= scale_factor_ / total_cpu_percent;
+
+    GURL origin = it->second->last_origin();
+    power::OriginPowerMap* origin_power_map =
+        power::OriginPowerMapFactory::GetForBrowserContext(
+            it->second->profile());
+    DCHECK(origin_power_map);
+    origin_power_map->AddPowerForOrigin(origin, last_process_power_usage);
+  }
+}
+
+void ProcessPowerCollector::UpdateProcessInMap(
+    const content::RenderProcessHost* rph,
+    const GURL& origin) {
+  base::ProcessHandle handle = rph->GetHandle();
+  if (metrics_map_.find(handle) == metrics_map_.end()) {
+    metrics_map_[handle] = linked_ptr<PerProcessData>(new PerProcessData(
+#if defined(OS_MACOSX)
+        scoped_ptr<base::ProcessMetrics>(
+            base::ProcessMetrics::CreateProcessMetrics(handle, NULL)),
+#else
+        scoped_ptr<base::ProcessMetrics>(
+            base::ProcessMetrics::CreateProcessMetrics(handle)),
+#endif
+        origin,
+        Profile::FromBrowserContext(rph->GetBrowserContext())));
+  }
+
+  linked_ptr<PerProcessData>& process_data = metrics_map_[handle];
+  process_data->set_last_cpu_percent(
+      cpu_usage_callback_.is_null() ? process_data->metrics()->GetCPUUsage()

[sky, 2014/08/25 19:38:40]

Is GetCPUUsage an expensive call? Does it do IO or anything?

[Daniel Nishi, 2014/08/25 20:19:24]

It shouldn't be that costly -- in the Linux implementation, it does a lookup on
the virtual /proc filesystem to get the CPU time, but that's a virtual
filesystem stored in RAM.

+                                    : cpu_usage_callback_.Run(handle));
+  // Skip errors.
+  if (process_data->last_cpu_percent() < 0)
+    process_data->set_last_cpu_percent(0);

[sky, 2014/08/25 19:38:40]

nit: std::max(0, ... on 193 would take care of this.

+  process_data->set_seen_this_cycle(true);
+}