[Metrics] Move non-idle notifications from MetricsService to ChromeMetricsServiceClient

BUG=374207
TEST=none
R=asvitkine@chromium.org

Review URL: https://codereview.chromium.org/287193005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272083 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 92 insertions, 27 deletions

diff --git a/chrome/browser/metrics/chrome_metrics_service_client.cc b/chrome/browser/metrics/chrome_metrics_service_client.cc
index 7599f4cd..ae8a235 100644
--- a/chrome/browser/metrics/chrome_metrics_service_client.cc
+++ b/chrome/browser/metrics/chrome_metrics_service_client.cc
@@ -6,10 +6,13 @@
 
 #include "base/logging.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/google/google_util.h"
+#include "chrome/browser/metrics/metrics_service.h"
 #include "chrome/browser/ui/browser_otr_state.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/crash_keys.h"
+#include "content/public/browser/notification_service.h"
 
 namespace {
 
@@ -33,7 +36,8 @@ metrics::SystemProfileProto::Channel AsProtobufChannel(
 
 }  // namespace
 
-ChromeMetricsServiceClient::ChromeMetricsServiceClient() {
+ChromeMetricsServiceClient::ChromeMetricsServiceClient() : service_(NULL) {
+  RegisterForNotifications();
 }
 
 ChromeMetricsServiceClient::~ChromeMetricsServiceClient() {
@@ -63,3 +67,50 @@ std::string ChromeMetricsServiceClient::GetVersionString() {
   // TODO(asvitkine): Move over from metrics_log.cc
   return std::string();
 }
+
+void ChromeMetricsServiceClient::RegisterForNotifications() {
+  registrar_.Add(this, chrome::NOTIFICATION_BROWSER_OPENED,
+                 content::NotificationService::AllBrowserContextsAndSources());
+  registrar_.Add(this, chrome::NOTIFICATION_BROWSER_CLOSED,
+                 content::NotificationService::AllSources());
+  registrar_.Add(this, chrome::NOTIFICATION_TAB_PARENTED,
+                 content::NotificationService::AllSources());
+  registrar_.Add(this, chrome::NOTIFICATION_TAB_CLOSING,
+                 content::NotificationService::AllSources());
+  registrar_.Add(this, content::NOTIFICATION_LOAD_START,
+                 content::NotificationService::AllSources());
+  registrar_.Add(this, content::NOTIFICATION_LOAD_STOP,
+                 content::NotificationService::AllSources());
+  registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
+                 content::NotificationService::AllSources());
+  registrar_.Add(this, content::NOTIFICATION_RENDER_WIDGET_HOST_HANG,
+                 content::NotificationService::AllSources());
+  registrar_.Add(this, chrome::NOTIFICATION_OMNIBOX_OPENED_URL,
+                 content::NotificationService::AllSources());
+}
+
+void ChromeMetricsServiceClient::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  switch (type) {
+    case chrome::NOTIFICATION_BROWSER_OPENED:
+    case chrome::NOTIFICATION_BROWSER_CLOSED:
+    case chrome::NOTIFICATION_OMNIBOX_OPENED_URL:
+    case chrome::NOTIFICATION_TAB_PARENTED:
+    case chrome::NOTIFICATION_TAB_CLOSING:
+    case content::NOTIFICATION_LOAD_STOP:
+    case content::NOTIFICATION_LOAD_START:
+    case content::NOTIFICATION_RENDERER_PROCESS_CLOSED:
+    case content::NOTIFICATION_RENDER_WIDGET_HOST_HANG:
+      // TODO(isherman): Remove this NULL check: http://crbug.com/375248
+      if (service_)
+        service_->OnApplicationNotIdle();
+      break;
+
+    default:
+      NOTREACHED();
+  }
+}
diff --git a/chrome/browser/metrics/chrome_metrics_service_client.h b/chrome/browser/metrics/chrome_metrics_service_client.h
index 4e11165..9b0c868 100644
--- a/chrome/browser/metrics/chrome_metrics_service_client.h
+++ b/chrome/browser/metrics/chrome_metrics_service_client.h
@@ -8,11 +8,17 @@
 #include <string>
 
 #include "base/basictypes.h"
+#include "base/threading/thread_checker.h"
 #include "components/metrics/metrics_service_client.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+
+class MetricsService;
 
 // ChromeMetricsServiceClient provides an implementation of MetricsServiceClient
 // that depends on chrome/.
-class ChromeMetricsServiceClient : public metrics::MetricsServiceClient {
+class ChromeMetricsServiceClient : public metrics::MetricsServiceClient,
+                                   public content::NotificationObserver {
  public:
   ChromeMetricsServiceClient();
   virtual ~ChromeMetricsServiceClient();
@@ -25,7 +31,29 @@ class ChromeMetricsServiceClient : public metrics::MetricsServiceClient {
   virtual metrics::SystemProfileProto::Channel GetChannel() OVERRIDE;
   virtual std::string GetVersionString() OVERRIDE;
 
+  // Stores a weak pointer to the given |service|.
+  // TODO(isherman): Fix the memory ownership model so that this method is not
+  // needed: http://crbug.com/375248
+  void set_service(MetricsService* service) { service_ = service; }
+
  private:
+  // Registers |this| as an observer for notifications which indicate that a
+  // user is performing work. This is useful to allow some features to sleep,
+  // until the machine becomes active, such as precluding UMA uploads unless
+  // there was recent activity.
+  void RegisterForNotifications();
+
+  // content::NotificationObserver:
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+  // The MetricsService that |this| is a client of. Weak pointer.
+  MetricsService* service_;
+
+  content::NotificationRegistrar registrar_;
+  base::ThreadChecker thread_checker_;
+
   DISALLOW_COPY_AND_ASSIGN(ChromeMetricsServiceClient);
 };
 
diff --git a/chrome/browser/metrics/metrics_service.cc b/chrome/browser/metrics/metrics_service.cc
index 67eb8ab..734d984 100644
--- a/chrome/browser/metrics/metrics_service.cc
+++ b/chrome/browser/metrics/metrics_service.cc
@@ -601,24 +601,12 @@ bool MetricsService::reporting_active() const {
 void MetricsService::SetUpNotifications(
     content::NotificationRegistrar* registrar,
     content::NotificationObserver* observer) {
-  registrar->Add(observer, chrome::NOTIFICATION_BROWSER_OPENED,
-                 content::NotificationService::AllBrowserContextsAndSources());
-  registrar->Add(observer, chrome::NOTIFICATION_BROWSER_CLOSED,
-                 content::NotificationService::AllSources());
-  registrar->Add(observer, chrome::NOTIFICATION_TAB_PARENTED,
-                 content::NotificationService::AllSources());
-  registrar->Add(observer, chrome::NOTIFICATION_TAB_CLOSING,
-                 content::NotificationService::AllSources());
   registrar->Add(observer, content::NOTIFICATION_LOAD_START,
                  content::NotificationService::AllSources());
-  registrar->Add(observer, content::NOTIFICATION_LOAD_STOP,
-                 content::NotificationService::AllSources());
   registrar->Add(observer, content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
                  content::NotificationService::AllSources());
   registrar->Add(observer, content::NOTIFICATION_RENDER_WIDGET_HOST_HANG,
                  content::NotificationService::AllSources());
-  registrar->Add(observer, chrome::NOTIFICATION_OMNIBOX_OPENED_URL,
-                 content::NotificationService::AllSources());
 }
 
 void MetricsService::BrowserChildProcessHostConnected(
@@ -647,15 +635,6 @@ void MetricsService::Observe(int type,
   DCHECK(IsSingleThreaded());
 
   switch (type) {
-    case chrome::NOTIFICATION_BROWSER_OPENED:
-    case chrome::NOTIFICATION_BROWSER_CLOSED:
-    case chrome::NOTIFICATION_OMNIBOX_OPENED_URL:
-    case chrome::NOTIFICATION_TAB_PARENTED:
-    case chrome::NOTIFICATION_TAB_CLOSING:
-    case content::NOTIFICATION_LOAD_STOP:
-      // These notifications are used only to break out of idle mode.
-      break;
-
     case content::NOTIFICATION_LOAD_START: {
       content::NavigationController* controller =
           content::Source<content::NavigationController>(source).ptr();
@@ -682,10 +661,7 @@ void MetricsService::Observe(int type,
 
     default:
       NOTREACHED();
-      break;
   }
-
-  HandleIdleSinceLastTransmission(false);
 }
 
 void MetricsService::HandleIdleSinceLastTransmission(bool in_idle) {
@@ -697,6 +673,11 @@ void MetricsService::HandleIdleSinceLastTransmission(bool in_idle) {
   idle_since_last_transmission_ = in_idle;
 }
 
+void MetricsService::OnApplicationNotIdle() {
+  if (recording_active_)
+    HandleIdleSinceLastTransmission(false);
+}
+
 void MetricsService::RecordStartOfSessionEnd() {
   LogCleanShutdown();
   RecordBooleanPrefValue(prefs::kStabilitySessionEndCompleted, false);
@@ -1216,7 +1197,7 @@ void MetricsService::StartScheduledUpload() {
   // If recording has been turned off, the scheduler doesn't need to run.
   // If reporting is off, proceed if the initial log hasn't been created, since
   // that has to happen in order for logs to be cut and stored when persisting.
-  // TODO(stuartmorgan): Call Stop() on the schedule when reporting and/or
+  // TODO(stuartmorgan): Call Stop() on the scheduler when reporting and/or
   // recording are turned off instead of letting it fire and then aborting.
   if (idle_since_last_transmission_ ||
       !recording_active() ||
diff --git a/chrome/browser/metrics/metrics_service.h b/chrome/browser/metrics/metrics_service.h
index 858ec4f..91f8b1b 100644
--- a/chrome/browser/metrics/metrics_service.h
+++ b/chrome/browser/metrics/metrics_service.h
@@ -211,6 +211,10 @@ class MetricsService
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
+  // This should be called when the application is not idle, i.e. the user seems
+  // to be interacting with the application.
+  void OnApplicationNotIdle();
+
   // Invoked when we get a WM_SESSIONEND. This places a value in prefs that is
   // reset when RecordCompletedSessionEnd is invoked.
   void RecordStartOfSessionEnd();
diff --git a/chrome/browser/metrics/metrics_services_manager.cc b/chrome/browser/metrics/metrics_services_manager.cc
index dad0fb5..da9d3e6 100644
--- a/chrome/browser/metrics/metrics_services_manager.cc
+++ b/chrome/browser/metrics/metrics_services_manager.cc
@@ -30,6 +30,7 @@ MetricsService* MetricsServicesManager::GetMetricsService() {
   if (!metrics_service_) {
     metrics_service_.reset(
       new MetricsService(GetMetricsStateManager(), &metrics_service_client_));
+    metrics_service_client_.set_service(metrics_service_.get());
   }
   return metrics_service_.get();
 }