Add ProcessPowerCollector to audit power information.

chrome/browser/power/process_power_collector.h

Index: chrome/browser/power/process_power_collector.h
diff --git a/chrome/browser/power/process_power_collector.h b/chrome/browser/power/process_power_collector.h
new file mode 100644
index 0000000000000000000000000000000000000000..8f887e2f54e8674edd5ffc4c9ba5f596fecdbafb
--- /dev/null
+++ b/chrome/browser/power/process_power_collector.h
@@ -0,0 +1,93 @@
+// 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.
+
+#ifndef CHROME_BROWSER_POWER_PROCESS_POWER_COLLECTOR_H_
+#define CHROME_BROWSER_POWER_PROCESS_POWER_COLLECTOR_H_
+
+#include <map>
+
+#include "base/memory/linked_ptr.h"
+#include "base/process/process_handle.h"
+#include "base/process/process_metrics.h"
+#include "base/timer/timer.h"
+#include "components/power/origin_power_map_factory.h"
+#include "url/gurl.h"
+
+class Profile;
+
+namespace content {
+class RenderProcessHost;
+}
+
+// Manages regular updates of the profile power consumption.
+class ProcessPowerCollector {
+ public:
+  ProcessPowerCollector();
+  ~ProcessPowerCollector();
+
+  // Begin periodically updating the power consumption numbers by profile.
+  void StartUpdating();
+
+ private:
+  FRIEND_TEST_ALL_PREFIXES(BrowserProcessPowerTest, NoSite);
+  FRIEND_TEST_ALL_PREFIXES(BrowserProcessPowerTest, OneSite);
+  FRIEND_TEST_ALL_PREFIXES(BrowserProcessPowerTest, MultipleSites);
+  FRIEND_TEST_ALL_PREFIXES(BrowserProcessPowerTest,
+                           IncognitoDoesntRecordPowerUsage);
+  FRIEND_TEST_ALL_PREFIXES(BrowserProcessPowerTest,
+                           MultipleProfilesRecordSeparately);
+  FRIEND_TEST_ALL_PREFIXES(BrowserProcessPowerTest, AppsRecordPowerUsage);
+
+  struct PerProcessData {
+    explicit PerProcessData(base::ProcessMetrics* metrics)

[Daniel Erat, 2014/08/18 20:43:05]

scoped_ptr

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

Done.

+        : metrics(metrics), last_cpu(-1) {}

[Daniel Erat, 2014/08/18 20:43:04]

nit: one per line if they don't all fit on the same line as the signature

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

Done.

+    PerProcessData() {}
+    ~PerProcessData();
+
+    // |metrics| is owned by PerProcessData and freed on destruction.
+    base::ProcessMetrics* metrics;

[Daniel Erat, 2014/08/18 20:43:04]

make this be a scoped_ptr since it's owned by this class (which should be a
class with underscore-suffixed members and DISALLOW_COPY_AND_ASSIGN instead;
otherwise it's easy to accidentally copy the struct and double-free the metrics)

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

Done.

+
+    // |profile| is not owned by PerProcessData.
+    Profile* profile;
+    GURL last_origin;

[Daniel Erat, 2014/08/18 20:43:04]

please add comments documenting what all of these fields represent

[Daniel Nishi, 2014/08/19 19:24:21]

Done.

+    double total_usage;

[Daniel Erat, 2014/08/18 20:43:04]

i don't see this member being used anywhere in the .cc file

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

You're right -- that functionality got moved into the OriginPowerMap.

Done.

+    int last_cpu;

[Daniel Erat, 2014/08/18 20:43:04]

last_cpu_percent?

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

Done.

+  };
+
+  // A map from all process handles to a metric.
+  typedef std::map<base::ProcessHandle, linked_ptr<PerProcessData> >
+      ProcessMetricsMap;
+
+  // Callback from the timer to update. Invokes the update power consumption
+  // procedure.
+  void UpdatePowerConsumptionCallback();
+
+  // Updates the |metrics_map_| with currently active processes.
+  void UpdateMetricsMap();
+
+  // Updates the |metrics_map_| with updated CPU usage information and
+  // removes all inactive processes from the |metrics_map_|.
+  double PopulateCpuUsageByOrigin();
+
+  // Attributes the power usage to the profiles and origins given a total amount
+  // of CPU used in this cycle, |cpu_cycle|.
+  void RecordCpuUsageByOrigin(double cpu_cycle);

[Daniel Erat, 2014/08/18 20:43:04]

i think that the difference between "Populate" and "Record" in this method and
the previous one is non-obvious. since they both work with the same data and
only get called back-to-back, would you be okay with merging them into a single
method?

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

I originally had them merged, but I split it into two functions so I could use
test values between the population and the recording.

+
+  // Iterates over all tabs to determine the power usage since the last sweep.
+  void UpdatePowerConsumption();
+
+  // Adds the information from a given RenderProcessHost to the map at the for

[Daniel Erat, 2014/08/18 20:43:04]

nit: fix grammar in this comment (should it be "... to |metrics_map_| for a
given origin"?)

[Daniel Nishi, 2014/08/19 19:24:21]

Done.

+  // a given origin.
+  void AddProcessToMap(const content::RenderProcessHost* render_process,
+                       const GURL& origin);
+
+  ProcessMetricsMap* GetMetricsMapForTesting() { return metrics_map_.get(); }

[Daniel Erat, 2014/08/18 20:43:04]

nit: make this public and rename it to metrics_map_for_testing(); can the map
also be const?

[Daniel Nishi, 2014/08/19 19:24:21]

Renamed. This also results in PerProcessData and the ProcessMetricsMap typedef
moving to public as well.

It can't be const because we modify the values inside the map to make mock CPU
usage during testing.

+
+  scoped_ptr<ProcessMetricsMap> metrics_map_;

[Daniel Erat, 2014/08/18 20:43:05]

does this really need to be a scoped_ptr? if not, just do:

  ProcessMetricsMap metrics_map_;

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

Done.

+  base::RepeatingTimer<ProcessPowerCollector> timer_;
+
+  DISALLOW_COPY_AND_ASSIGN(ProcessPowerCollector);
+};
+
+#endif  // CHROME_BROWSER_POWER_PROCESS_POWER_COLLECTOR_H_