Battery impact UMA

chrome/browser/chrome_browser_main.cc

Index: chrome/browser/chrome_browser_main.cc
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc
index b509e20670fb4fb623f76565cb0a90dbb08056be..2b49029eaee5de315a9de11f99783cffa6a17370 100644
--- a/chrome/browser/chrome_browser_main.cc
+++ b/chrome/browser/chrome_browser_main.cc
@@ -116,6 +116,7 @@
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
+#include "content/public/browser/power_usage_monitor.h"
 #include "content/public/browser/site_instance.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_switches.h"
@@ -1553,6 +1554,9 @@ int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
   browser_creator_.reset();
 #endif  // !defined(OS_ANDROID)
 
+  if (g_browser_process->metrics_service()->reporting_active())
+    content::StartPowerUsageMonitor();

[Daniel Erat, 2014/10/21 16:01:20]

is this the proper way to check whether metrics are being reported? i only see
reporting_active() called in one place, SafeBrowsingUIManager::CanReportStats(),
and it also checks that metrics_service() is non-NULL. is that check incorrect,
or does this code also need it? can reporting be toggled on or off at runtime?

[jeremy, 2014/10/23 14:32:45]

Avi: Do you know whether we need to null check metrics_service()?

Regarding the null check:
The check in SafeBrowsingUIManager::CanReportStats() was added in
https://codereview.chromium.org/6023006/.

Looking at the code it seems that in some cases we null check metrics_service()
and in some cases we don't e.g.
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...
.

Regarding the call being correct:
When I looked into this my impression was that this was the supported way to do
this.
The reason I added this is because If stats reporting is disabled then there's
no reason to instantiate the powermanager and if it's toggled at runtime then
the UMAs we report just won't get sent.  I don't think it's worth the added
complexity of listening on the pref change and shutting down/starting the
powermanager.

+
 #if !defined(OS_ANDROID)
   process_power_collector_.reset(new ProcessPowerCollector);
   process_power_collector_->Initialize();