Add Origin Power Map to Store Battery Auditing Data.

chrome/browser/power/origin_power_map.cc

Index: chrome/browser/power/origin_power_map.cc
diff --git a/chrome/browser/power/origin_power_map.cc b/chrome/browser/power/origin_power_map.cc
new file mode 100644
index 0000000000000000000000000000000000000000..ab931d732ca7eee658ca8ee20f520b88c6190968
--- /dev/null
+++ b/chrome/browser/power/origin_power_map.cc
@@ -0,0 +1,47 @@
+// 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/origin_power_map.h"
+
+#include "base/logging.h"
+#include "content/public/common/url_constants.h"
+#include "url/gurl.h"
+
+OriginPowerMap::OriginPowerMap() {
+}
+
+OriginPowerMap::~OriginPowerMap() {
+}
+
+int OriginPowerMap::GetPowerForOrigin(const GURL& origin) {
+  DCHECK_EQ(origin, origin.GetOrigin());
+  if (origin_map_.find(origin) == origin_map_.end())
+    return -1;

[sky, 2014/08/08 19:12:25]

Why the -1 here? I would have expected 0.

[Daniel Nishi, 2014/08/08 19:41:05]

Fair enough. If it's not in the map, it hasn't consumed any power.

+  else if (!total_consumed_)

[sky, 2014/08/08 19:12:25]

nit: no else after a return (see chromium specific style guide).

[Daniel Nishi, 2014/08/08 19:41:05]

Changed the return value if not found to 0 and merged the if and elif.

+    return 0;
+  return origin_map_[origin] * 100 / total_consumed_;
+}
+
+void OriginPowerMap::AddPowerForOrigin(const GURL& origin, double power) {
+  DCHECK_EQ(origin, origin.GetOrigin());

[sky, 2014/08/08 19:12:25]

Why does the caller have to have called GetOrigin() first? Shouldn't the exact
heuristics of what constitutes and origin be up to this class and not the
caller?

[Daniel Nishi, 2014/08/08 19:41:05]

Looking over the GetOrigin() call, I agree. For one example, a GURL could be
valid for the map even if these replacements[0] haven't been run.

I removed the DCHECKs.

[0]
https://code.google.com/p/chromium/codesearch#chromium/src/url/gurl.cc&rcl=14...

+  if (!origin.is_valid() || origin.SchemeIs(content::kChromeUIScheme))

[sky, 2014/08/08 19:12:25]

Why do you special case chrome urls?

[Daniel Nishi, 2014/08/08 19:41:05]

When they appear in the resource monitoring UI, there is nothing actionable that
can be done about the chrome URLs, whereas actions can be taken against
individual origins or Chrome apps.

It felt like the data was less polluted and gave a better idea for which
sites/apps were using your resources when the chrome URLs were excluded.

+    return;
+
+  if (origin_map_.find(origin) == origin_map_.end()) {

[sky, 2014/08/08 19:12:25]

nit: no {}

[sky, 2014/08/08 19:12:25]

And you actually shouldn't need this if at all.
origin_map_[something_you_never_added] should be 0.

[Daniel Nishi, 2014/08/08 19:41:05]

Good point. I've removed this if.

+    origin_map_[origin] = 0;
+  }
+  origin_map_[origin] += power;
+  total_consumed_ += power;
+}
+
+scoped_ptr<const OriginPowerMap::PercentOriginMap>
+OriginPowerMap::GetPercentOriginMap() {
+  OriginPowerMap::PercentOriginMap* percent_map =

[sky, 2014/08/08 19:12:25]

Create a scoped_ptr here and use .Pass() in the return statement.

[Daniel Nishi, 2014/08/08 19:41:05]

I think I'd lose the const-ness if I did this, since I populate the
PercentOriginMap in this function.

[sky, 2014/08/08 23:15:28]

There is no point in returning a const value here. The return value should be
scoped_ptr<PercentOriginMap>.

[Daniel Nishi, 2014/08/08 23:38:07]

Done.

+      new OriginPowerMap::PercentOriginMap;
+  for (OriginMap::iterator it = origin_map_.begin(); it != origin_map_.end();
+       ++it) {
+    (*percent_map)[it->first] = (it->second / total_consumed_ * 100);
+  }
+  return scoped_ptr<const OriginPowerMap::PercentOriginMap>(percent_map).Pass();
+}