From 26b95242b8853564a5c7d406d2ba2c6d2d359096 Mon Sep 17 00:00:00 2001 From: blundell Date: Mon, 17 Aug 2015 03:29:24 -0700 Subject: [PATCH] Remove listening of OMNIBOX_OPENED_URL from metrics code I am working to eliminate this notification entirely, as its usage is impeding iOS upstreaming (see bug for details). This CL replaces its usage in //chrome/browser/metrics with usage of OmniboxEventGlobalTracker. BUG=517885 Review URL: https://codereview.chromium.org/1282473002 Cr-Commit-Position: refs/heads/master@{#343662} --- chrome/browser/metrics/chrome_metrics_service_client.cc | 12 +++++++++--- chrome/browser/metrics/chrome_metrics_service_client.h | 9 +++++++++ chrome/browser/metrics/thread_watcher.cc | 15 ++++++++++++--- chrome/browser/metrics/thread_watcher.h | 12 ++++++++++++ 4 files changed, 42 insertions(+), 6 deletions(-) diff --git a/chrome/browser/metrics/chrome_metrics_service_client.cc b/chrome/browser/metrics/chrome_metrics_service_client.cc index 3f7a88858d31..63a7cbdb2494 100644 --- a/chrome/browser/metrics/chrome_metrics_service_client.cc +++ b/chrome/browser/metrics/chrome_metrics_service_client.cc @@ -592,8 +592,11 @@ void ChromeMetricsServiceClient::RegisterForNotifications() { 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()); + + omnibox_url_opened_subscription_ = + OmniboxEventGlobalTracker::GetInstance()->RegisterCallback( + base::Bind(&ChromeMetricsServiceClient::OnURLOpenedFromOmnibox, + base::Unretained(this))); } void ChromeMetricsServiceClient::Observe( @@ -605,7 +608,6 @@ void ChromeMetricsServiceClient::Observe( 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: @@ -619,3 +621,7 @@ void ChromeMetricsServiceClient::Observe( NOTREACHED(); } } + +void ChromeMetricsServiceClient::OnURLOpenedFromOmnibox(OmniboxLog* log) { + metrics_service_->OnApplicationNotIdle(); +} diff --git a/chrome/browser/metrics/chrome_metrics_service_client.h b/chrome/browser/metrics/chrome_metrics_service_client.h index ee879a5b65ef..1500649d9317 100644 --- a/chrome/browser/metrics/chrome_metrics_service_client.h +++ b/chrome/browser/metrics/chrome_metrics_service_client.h @@ -16,6 +16,7 @@ #include "chrome/browser/metrics/metrics_memory_details.h" #include "components/metrics/metrics_service_client.h" #include "components/metrics/profiler/tracking_synchronizer_observer.h" +#include "components/omnibox/browser/omnibox_event_global_tracker.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" @@ -127,6 +128,9 @@ class ChromeMetricsServiceClient const content::NotificationSource& source, const content::NotificationDetails& details) override; + // Called when a URL is opened from the Omnibox. + void OnURLOpenedFromOmnibox(OmniboxLog* log); + #if defined(OS_WIN) // Counts (and removes) the browser crash dump attempt signals left behind by // any previous browser processes which generated a crash dump. @@ -194,6 +198,11 @@ class ChromeMetricsServiceClient base::ScopedPtrMap> host_resource_usage_map_; + // Subscription for receiving callbacks that a URL was opened from the + // omnibox. + scoped_ptr::Subscription> + omnibox_url_opened_subscription_; + base::WeakPtrFactory weak_ptr_factory_; DISALLOW_COPY_AND_ASSIGN(ChromeMetricsServiceClient); diff --git a/chrome/browser/metrics/thread_watcher.cc b/chrome/browser/metrics/thread_watcher.cc index 0f9e9acdca01..6c9a93e83394 100644 --- a/chrome/browser/metrics/thread_watcher.cc +++ b/chrome/browser/metrics/thread_watcher.cc @@ -688,9 +688,10 @@ void ThreadWatcherObserver::SetupNotifications( observer->registrar_.Add(observer, content::NOTIFICATION_RENDER_WIDGET_HOST_HANG, content::NotificationService::AllSources()); - observer->registrar_.Add(observer, - chrome::NOTIFICATION_OMNIBOX_OPENED_URL, - content::NotificationService::AllSources()); + observer->omnibox_url_opened_subscription_ = + OmniboxEventGlobalTracker::GetInstance()->RegisterCallback( + base::Bind(&ThreadWatcherObserver::OnURLOpenedFromOmnibox, + base::Unretained(observer))); } // static @@ -706,6 +707,14 @@ void ThreadWatcherObserver::Observe( int type, const content::NotificationSource& source, const content::NotificationDetails& details) { + OnUserActivityDetected(); +} + +void ThreadWatcherObserver::OnURLOpenedFromOmnibox(OmniboxLog* log) { + OnUserActivityDetected(); +} + +void ThreadWatcherObserver::OnUserActivityDetected() { // There is some user activity, see if thread watchers are to be awakened. base::TimeTicks now = base::TimeTicks::Now(); if ((now - last_wakeup_time_) < wakeup_interval_) diff --git a/chrome/browser/metrics/thread_watcher.h b/chrome/browser/metrics/thread_watcher.h index f62b04d1be45..457c2d681a6a 100644 --- a/chrome/browser/metrics/thread_watcher.h +++ b/chrome/browser/metrics/thread_watcher.h @@ -59,6 +59,7 @@ #include "base/threading/thread.h" #include "base/threading/watchdog.h" #include "base/time/time.h" +#include "components/omnibox/browser/omnibox_event_global_tracker.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/notification_observer.h" #include "content/public/browser/notification_registrar.h" @@ -532,6 +533,12 @@ class ThreadWatcherObserver : public content::NotificationObserver { const content::NotificationSource& source, const content::NotificationDetails& details) override; + // Called when a URL is opened from the Omnibox. + void OnURLOpenedFromOmnibox(OmniboxLog* log); + + // Called when user activity is detected. + void OnUserActivityDetected(); + // The singleton of this class. static ThreadWatcherObserver* g_thread_watcher_observer_; @@ -544,6 +551,11 @@ class ThreadWatcherObserver : public content::NotificationObserver { // It is the time interval between wake up calls to thread watchers. const base::TimeDelta wakeup_interval_; + // Subscription for receiving callbacks that a URL was opened from the + // omnibox. + scoped_ptr::Subscription> + omnibox_url_opened_subscription_; + DISALLOW_COPY_AND_ASSIGN(ThreadWatcherObserver); }; -- 2.11.4.GIT