Commit 579b892e authored by Evan Stade's avatar Evan Stade Committed by Commit Bot

Remove use of NOTIFICATION_BROWSER_CLOSED/OPENED from //c/b/metrics

A new helper called BrowserActivityWatcher is introduced to encapsulate
BrowserList observation. This results in fewer platform macros since
Android doesn't use BrowserList (or NOTIFICATION_BROWSER_[...] before
it).

The structure of ThreadWatcherObserver, particularly startup and
shutdown, was unorthodox so that's also made more idiomatic. The class
can move entirely to the cc file.

Bug: 268984
Change-Id: I46a8948349e62993c96e85963cae3420679ae45f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1695831Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Reviewed-by: default avatarSteven Holte <holte@chromium.org>
Reviewed-by: default avatarRyan Hamilton <rch@chromium.org>
Commit-Queue: Evan Stade <estade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678366}
parent 6f901938
......@@ -3101,6 +3101,8 @@ jumbo_split_static_library("browser") {
"memory/swap_thrashing_monitor_delegate.h",
"memory/swap_thrashing_monitor_delegate_win.cc",
"memory/swap_thrashing_monitor_delegate_win.h",
"metrics/browser_activity_watcher.cc",
"metrics/browser_activity_watcher.h",
"metrics/desktop_platform_features_metrics_provider.cc",
"metrics/desktop_platform_features_metrics_provider.h",
"metrics/desktop_session_duration/audible_contents_tracker.cc",
......
// Copyright 2019 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/metrics/browser_activity_watcher.h"
#include "chrome/browser/ui/browser_list.h"
#include "chrome/browser/ui/browser_list_observer.h"
BrowserActivityWatcher::BrowserActivityWatcher(
const base::RepeatingClosure& on_browser_list_change)
: on_browser_list_change_(on_browser_list_change) {
BrowserList::AddObserver(this);
}
BrowserActivityWatcher::~BrowserActivityWatcher() {
BrowserList::RemoveObserver(this);
}
void BrowserActivityWatcher::OnBrowserAdded(Browser* browser) {
on_browser_list_change_.Run();
}
void BrowserActivityWatcher::OnBrowserRemoved(Browser* browser) {
on_browser_list_change_.Run();
}
// Copyright 2019 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_METRICS_BROWSER_ACTIVITY_WATCHER_H_
#define CHROME_BROWSER_METRICS_BROWSER_ACTIVITY_WATCHER_H_
#include "base/callback.h"
#include "base/macros.h"
#include "chrome/browser/ui/browser_list_observer.h"
// Helper that fires a callback every time a Browser object is added or removed
// from the BrowserList. This class primarily exists to encapsulate this
// behavior and reduce the number of platform-specific macros as Android doesn't
// use BrowserList.
class BrowserActivityWatcher : public BrowserListObserver {
public:
explicit BrowserActivityWatcher(
const base::RepeatingClosure& on_browser_list_change);
~BrowserActivityWatcher() override;
// BrowserListObserver:
void OnBrowserAdded(Browser* browser) override;
void OnBrowserRemoved(Browser* browser) override;
private:
base::RepeatingClosure on_browser_list_change_;
DISALLOW_COPY_AND_ASSIGN(BrowserActivityWatcher);
};
#endif // CHROME_BROWSER_METRICS_BROWSER_ACTIVITY_WATCHER_H_
......@@ -105,6 +105,8 @@
#if defined(OS_ANDROID)
#include "chrome/browser/metrics/android_metrics_provider.h"
#include "chrome/browser/metrics/page_load_metrics_provider.h"
#else
#include "chrome/browser/metrics/browser_activity_watcher.h"
#endif
#if defined(OS_POSIX)
......@@ -395,17 +397,9 @@ const char kUKMFieldTrialSuffix[] = "UKM";
ChromeMetricsServiceClient::ChromeMetricsServiceClient(
metrics::MetricsStateManager* state_manager)
: metrics_state_manager_(state_manager),
waiting_for_collect_final_metrics_step_(false),
num_async_histogram_fetches_in_progress_(0)
#if BUILDFLAG(ENABLE_PLUGINS)
,
plugin_metrics_provider_(nullptr)
#endif
{
: metrics_state_manager_(state_manager) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
RecordCommandLineMetrics();
notification_listeners_active_ = RegisterForNotifications();
incognito_observer_ = std::make_unique<IncognitoObserver>(
base::BindRepeating(&ChromeMetricsServiceClient::UpdateRunningServices,
weak_ptr_factory_.GetWeakPtr()));
......@@ -576,9 +570,10 @@ void ChromeMetricsServiceClient::Initialize() {
local_state->ClearPref(prefs::kCrashReportingEnabled);
#endif
metrics_service_.reset(
new metrics::MetricsService(metrics_state_manager_, this, local_state));
metrics_service_ = std::make_unique<metrics::MetricsService>(
metrics_state_manager_, this, local_state);
notification_listeners_active_ = RegisterForNotifications();
RegisterMetricsServiceProviders();
if (IsMetricsReportingForceEnabled() ||
......@@ -886,10 +881,6 @@ void ChromeMetricsServiceClient::RecordCommandLineMetrics() {
}
bool 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,
......@@ -902,15 +893,19 @@ bool ChromeMetricsServiceClient::RegisterForNotifications() {
content::NotificationService::AllSources());
registrar_.Add(this, content::NOTIFICATION_RENDER_WIDGET_HOST_HANG,
content::NotificationService::AllSources());
registrar_.Add(this, chrome::NOTIFICATION_PROFILE_ADDED,
content::NotificationService::AllBrowserContextsAndSources());
omnibox_url_opened_subscription_ =
OmniboxEventGlobalTracker::GetInstance()->RegisterCallback(
base::Bind(&ChromeMetricsServiceClient::OnURLOpenedFromOmnibox,
base::Unretained(this)));
// Observe history deletions for all profiles.
registrar_.Add(this, chrome::NOTIFICATION_PROFILE_ADDED,
content::NotificationService::AllBrowserContextsAndSources());
#if !defined(OS_ANDROID)
browser_activity_watcher_ = std::make_unique<BrowserActivityWatcher>(
base::BindRepeating(&metrics::MetricsService::OnApplicationNotIdle,
base::Unretained(metrics_service_.get())));
#endif
bool all_profiles_succeeded = true;
for (Profile* profile :
......@@ -964,10 +959,6 @@ void ChromeMetricsServiceClient::Observe(
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
switch (type) {
case chrome::NOTIFICATION_BROWSER_OPENED:
metrics_service_->OnApplicationNotIdle();
break;
case chrome::NOTIFICATION_BROWSER_CLOSED:
case chrome::NOTIFICATION_TAB_PARENTED:
case chrome::NOTIFICATION_TAB_CLOSING:
case content::NOTIFICATION_LOAD_STOP:
......
......@@ -31,6 +31,7 @@
#include "ppapi/buildflags/buildflags.h"
#include "third_party/metrics_proto/system_profile.pb.h"
class BrowserActivityWatcher;
class PluginMetricsProvider;
class Profile;
class PrefRegistrySimple;
......@@ -176,21 +177,21 @@ class ChromeMetricsServiceClient : public metrics::MetricsServiceClient,
std::unique_ptr<IncognitoObserver> incognito_observer_;
// Whether we registered all notification listeners successfully.
bool notification_listeners_active_;
bool notification_listeners_active_ = false;
// Saved callback received from CollectFinalMetricsForLog().
base::Closure collect_final_metrics_done_callback_;
// Indicates that collect final metrics step is running.
bool waiting_for_collect_final_metrics_step_;
bool waiting_for_collect_final_metrics_step_ = false;
// Number of async histogram fetch requests in progress.
int num_async_histogram_fetches_in_progress_;
int num_async_histogram_fetches_in_progress_ = 0;
#if BUILDFLAG(ENABLE_PLUGINS)
// The PluginMetricsProvider instance that was registered with
// MetricsService. Has the same lifetime as |metrics_service_|.
PluginMetricsProvider* plugin_metrics_provider_;
PluginMetricsProvider* plugin_metrics_provider_ = nullptr;
#endif
// Callback to determine whether or not a cellular network is currently being
......@@ -202,6 +203,10 @@ class ChromeMetricsServiceClient : public metrics::MetricsServiceClient,
std::unique_ptr<base::CallbackList<void(OmniboxLog*)>::Subscription>
omnibox_url_opened_subscription_;
#if !defined(OS_ANDROID)
std::unique_ptr<BrowserActivityWatcher> browser_activity_watcher_;
#endif
base::WeakPtrFactory<ChromeMetricsServiceClient> weak_ptr_factory_{this};
DISALLOW_COPY_AND_ASSIGN(ChromeMetricsServiceClient);
......
......@@ -12,6 +12,7 @@
#include "base/lazy_instance.h"
#include "base/location.h"
#include "base/macros.h"
#include "base/metrics/histogram.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_split.h"
#include "base/strings/string_tokenizer.h"
......@@ -26,12 +27,138 @@
#include "chrome/common/chrome_switches.h"
#include "chrome/common/logging_chrome.h"
#include "components/metrics/call_stack_profile_metrics_provider.h"
#include "components/omnibox/browser/omnibox_event_global_tracker.h"
#include "components/version_info/version_info.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
#include "content/public/browser/notification_service.h"
#if !defined(OS_ANDROID)
#include "chrome/browser/metrics/browser_activity_watcher.h"
#endif
using content::BrowserThread;
namespace {
// This class ensures that the thread watching is actively taking place. Only
// one instance of this class exists.
class ThreadWatcherObserver : public content::NotificationObserver {
public:
// |wakeup_interval| specifies how often to wake up thread watchers due to
// new user activity.
static void Start(const base::TimeDelta& wakeup_interval);
static void Stop();
private:
explicit ThreadWatcherObserver(const base::TimeDelta& wakeup_interval);
~ThreadWatcherObserver() override;
// content::NotificationObserver:
void Observe(int type,
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();
#if !defined(OS_ANDROID)
std::unique_ptr<BrowserActivityWatcher> browser_activity_watcher_;
#endif
content::NotificationRegistrar registrar_;
// This is the last time when woke all thread watchers up.
base::TimeTicks last_wakeup_time_;
// 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.
std::unique_ptr<base::CallbackList<void(OmniboxLog*)>::Subscription>
omnibox_url_opened_subscription_;
DISALLOW_COPY_AND_ASSIGN(ThreadWatcherObserver);
};
ThreadWatcherObserver* g_thread_watcher_observer_ = nullptr;
ThreadWatcherObserver::ThreadWatcherObserver(
const base::TimeDelta& wakeup_interval)
: last_wakeup_time_(base::TimeTicks::Now()),
wakeup_interval_(wakeup_interval) {
DCHECK(!g_thread_watcher_observer_);
g_thread_watcher_observer_ = this;
#if !defined(OS_ANDROID)
browser_activity_watcher_ = std::make_unique<BrowserActivityWatcher>(
base::BindRepeating(&ThreadWatcherObserver::OnUserActivityDetected,
base::Unretained(this)));
#endif
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());
omnibox_url_opened_subscription_ =
OmniboxEventGlobalTracker::GetInstance()->RegisterCallback(
base::Bind(&ThreadWatcherObserver::OnURLOpenedFromOmnibox,
base::Unretained(this)));
}
ThreadWatcherObserver::~ThreadWatcherObserver() {
DCHECK_EQ(this, g_thread_watcher_observer_);
g_thread_watcher_observer_ = nullptr;
}
// static
void ThreadWatcherObserver::Start(const base::TimeDelta& wakeup_interval) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
new ThreadWatcherObserver(wakeup_interval);
}
// static
void ThreadWatcherObserver::Stop() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
delete g_thread_watcher_observer_;
}
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_)
return;
last_wakeup_time_ = now;
WatchDogThread::PostTask(FROM_HERE,
base::Bind(&ThreadWatcherList::WakeUpAll));
}
} // namespace
// ThreadWatcher methods and members.
ThreadWatcher::ThreadWatcher(const WatchingParams& params)
: thread_id_(params.thread_id),
......@@ -354,7 +481,7 @@ void ThreadWatcherList::StartWatchingAll(
&unresponsive_threshold,
&crash_on_hang_threads);
ThreadWatcherObserver::SetupNotifications(
ThreadWatcherObserver::Start(
base::TimeDelta::FromSeconds(kSleepSeconds * unresponsive_threshold));
WatchDogThread::PostTask(
......@@ -376,7 +503,7 @@ void ThreadWatcherList::StartWatchingAll(
// static
void ThreadWatcherList::StopWatchingAll() {
// TODO(rtenneti): Enable ThreadWatcher.
ThreadWatcherObserver::RemoveNotifications();
ThreadWatcherObserver::Stop();
DeleteAll();
}
......@@ -607,92 +734,6 @@ void ThreadWatcherList::SetStopped(bool stopped) {
g_stopped_ = stopped;
}
// ThreadWatcherObserver methods and members.
//
// static
ThreadWatcherObserver*
ThreadWatcherObserver::g_thread_watcher_observer_ = nullptr;
ThreadWatcherObserver::ThreadWatcherObserver(
const base::TimeDelta& wakeup_interval)
: last_wakeup_time_(base::TimeTicks::Now()),
wakeup_interval_(wakeup_interval) {
CHECK(!g_thread_watcher_observer_);
g_thread_watcher_observer_ = this;
}
ThreadWatcherObserver::~ThreadWatcherObserver() {
DCHECK(this == g_thread_watcher_observer_);
g_thread_watcher_observer_ = nullptr;
}
// static
void ThreadWatcherObserver::SetupNotifications(
const base::TimeDelta& wakeup_interval) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
ThreadWatcherObserver* observer = new ThreadWatcherObserver(wakeup_interval);
observer->registrar_.Add(
observer,
chrome::NOTIFICATION_BROWSER_OPENED,
content::NotificationService::AllBrowserContextsAndSources());
observer->registrar_.Add(observer,
chrome::NOTIFICATION_BROWSER_CLOSED,
content::NotificationService::AllSources());
observer->registrar_.Add(observer,
chrome::NOTIFICATION_TAB_PARENTED,
content::NotificationService::AllSources());
observer->registrar_.Add(observer,
chrome::NOTIFICATION_TAB_CLOSING,
content::NotificationService::AllSources());
observer->registrar_.Add(observer,
content::NOTIFICATION_LOAD_START,
content::NotificationService::AllSources());
observer->registrar_.Add(observer,
content::NOTIFICATION_LOAD_STOP,
content::NotificationService::AllSources());
observer->registrar_.Add(observer,
content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
content::NotificationService::AllSources());
observer->registrar_.Add(observer,
content::NOTIFICATION_RENDER_WIDGET_HOST_HANG,
content::NotificationService::AllSources());
observer->omnibox_url_opened_subscription_ =
OmniboxEventGlobalTracker::GetInstance()->RegisterCallback(
base::Bind(&ThreadWatcherObserver::OnURLOpenedFromOmnibox,
base::Unretained(observer)));
}
// static
void ThreadWatcherObserver::RemoveNotifications() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
if (!g_thread_watcher_observer_)
return;
g_thread_watcher_observer_->registrar_.RemoveAll();
delete g_thread_watcher_observer_;
}
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_)
return;
last_wakeup_time_ = now;
WatchDogThread::PostTask(
FROM_HERE,
base::Bind(&ThreadWatcherList::WakeUpAll));
}
// WatchDogThread methods and members.
// This lock protects g_watchdog_thread.
......
......@@ -39,18 +39,17 @@
#ifndef CHROME_BROWSER_METRICS_THREAD_WATCHER_H_
#define CHROME_BROWSER_METRICS_THREAD_WATCHER_H_
#include <stdint.h>
#include <map>
#include <string>
#include <vector>
#include <stdint.h>
#include "base/command_line.h"
#include "base/gtest_prod_util.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
#include "base/metrics/histogram.h"
#include "base/single_thread_task_runner.h"
#include "base/synchronization/lock.h"
#include "base/threading/platform_thread.h"
......@@ -59,15 +58,15 @@
#include "base/time/time.h"
#include "build/build_config.h"
#include "components/metrics/call_stack_profile_params.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"
class CustomThreadWatcher;
class StartupTimeBomb;
class ThreadWatcherList;
class ThreadWatcherObserver;
namespace base {
class HistogramBase;
}
// This class performs health check on threads that would like to be watched.
class ThreadWatcher {
......@@ -439,61 +438,6 @@ class ThreadWatcherList {
DISALLOW_COPY_AND_ASSIGN(ThreadWatcherList);
};
// This class ensures that the thread watching is actively taking place. Only
// one instance of this class exists.
class ThreadWatcherObserver : public content::NotificationObserver {
public:
// Registers |g_thread_watcher_observer_| as the Notifications observer.
// |wakeup_interval| specifies how often to wake up thread watchers. This
// method is accessible on UI thread.
static void SetupNotifications(const base::TimeDelta& wakeup_interval);
// Removes all ints from |registrar_| and deletes
// |g_thread_watcher_observer_|. This method is accessible on UI thread.
static void RemoveNotifications();
private:
// Constructor of |g_thread_watcher_observer_| singleton.
explicit ThreadWatcherObserver(const base::TimeDelta& wakeup_interval);
// Destructor of |g_thread_watcher_observer_| singleton.
~ThreadWatcherObserver() override;
// This ensures all thread watchers are active because there is some user
// activity. It will wake up all thread watchers every |wakeup_interval_|
// seconds. This is the implementation of content::NotificationObserver. When
// a matching notification is posted to the notification service, this method
// is called.
void Observe(int type,
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_;
// The registrar that holds ints to be observed.
content::NotificationRegistrar registrar_;
// This is the last time when woke all thread watchers up.
base::TimeTicks last_wakeup_time_;
// 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.
std::unique_ptr<base::CallbackList<void(OmniboxLog*)>::Subscription>
omnibox_url_opened_subscription_;
DISALLOW_COPY_AND_ASSIGN(ThreadWatcherObserver);
};
// Class for WatchDogThread and in its Init method, we start watching UI, IO,
// DB, FILE, CACHED threads.
class WatchDogThread : public base::Thread {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment