Skip to content

Commit

Permalink
Remove ExitFunnel instrumentation and clean up leaked data.
Browse files Browse the repository at this point in the history
BUG=442526

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

Cr-Commit-Position: refs/heads/master@{#352870}
  • Loading branch information
sigurasg authored and Commit bot committed Oct 7, 2015
1 parent f986f95 commit d896f9a
Show file tree
Hide file tree
Showing 12 changed files with 166 additions and 320 deletions.
10 changes: 1 addition & 9 deletions chrome/browser/chrome_browser_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@
#include "chrome/installer/util/helper.h"
#include "chrome/installer/util/install_util.h"
#include "chrome/installer/util/shell_util.h"
#include "components/browser_watcher/exit_funnel_win.h"
#include "net/base/net_util.h"
#include "ui/base/l10n/l10n_util_win.h"
#include "ui/gfx/win/dpi.h"
Expand Down Expand Up @@ -490,15 +489,8 @@ bool ProcessSingletonNotificationCallback(
const base::CommandLine& command_line,
const base::FilePath& current_directory) {
// Drop the request if the browser process is already in shutdown path.
if (!g_browser_process || g_browser_process->IsShuttingDown()) {
#if defined(OS_WIN)
browser_watcher::ExitFunnel::RecordSingleEvent(
chrome::kBrowserExitCodesRegistryPath,
L"ProcessSingletonIsShuttingDown");
#endif // defined(OS_WIN)

if (!g_browser_process || g_browser_process->IsShuttingDown())
return false;
}

if (command_line.HasSwitch(switches::kOriginalProcessStartTime)) {
std::string start_time_string =
Expand Down
17 changes: 0 additions & 17 deletions chrome/browser/lifetime/application_lifetime.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@

#if defined(OS_WIN)
#include "base/win/win_util.h"
#include "components/browser_watcher/exit_funnel_win.h"
#endif

#if defined(USE_ASH)
Expand Down Expand Up @@ -257,12 +256,6 @@ void ExitCleanly() {
#endif

void SessionEnding() {
#if defined(OS_WIN)
browser_watcher::ExitFunnel funnel;

funnel.Init(kBrowserExitCodesRegistryPath, base::GetCurrentProcessHandle());
funnel.RecordEvent(L"SessionEnding");
#endif
// This is a time-limited shutdown where we need to write as much to
// disk as we can as soon as we can, and where we must kill the
// process within a hang timeout to avoid user prompts.
Expand All @@ -288,22 +281,12 @@ void SessionEnding() {
content::NotificationService::AllSources(),
content::NotificationService::NoDetails());

#if defined(OS_WIN)
funnel.RecordEvent(L"EndSession");
#endif
// Write important data first.
g_browser_process->EndSession();

#if defined(OS_WIN)
base::win::SetShouldCrashOnProcessDetach(false);
#endif

#if defined(OS_WIN)
// KillProcess ought to terminate the process without further ado, so if
// execution gets to this point, presumably this is normal exit.
funnel.RecordEvent(L"KillProcess");
#endif

// On Windows 7 and later, the system will consider the process ripe for
// termination as soon as it hides or destroys its windows. Since any
// execution past that point will be non-deterministically cut short, we
Expand Down
18 changes: 2 additions & 16 deletions chrome/browser/metrics/chrome_metrics_service_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -348,25 +348,11 @@ void ChromeMetricsServiceClient::Initialize() {
metrics_service_->RegisterMetricsProvider(
scoped_ptr<metrics::MetricsProvider>(google_update_metrics_provider_));

// Report exit funnels for canary and dev only.
bool report_exit_funnels = false;
switch (chrome::GetChannel()) {
case version_info::Channel::CANARY:
case version_info::Channel::DEV:
report_exit_funnels = true;
break;
case version_info::Channel::UNKNOWN:
case version_info::Channel::BETA:
case version_info::Channel::STABLE:
// report_exit_funnels was initialized to the right value above.
DCHECK(!report_exit_funnels);
break;
}

metrics_service_->RegisterMetricsProvider(
scoped_ptr<metrics::MetricsProvider>(
new browser_watcher::WatcherMetricsProviderWin(
chrome::kBrowserExitCodesRegistryPath, report_exit_funnels)));
chrome::kBrowserExitCodesRegistryPath,
content::BrowserThread::GetBlockingPool())));
#endif // defined(OS_WIN)

#if defined(ENABLE_PLUGINS)
Expand Down
10 changes: 1 addition & 9 deletions chrome/browser/process_singleton_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
#include "chrome/common/chrome_switches.h"
#include "chrome/grit/chromium_strings.h"
#include "chrome/installer/util/wmi.h"
#include "components/browser_watcher/exit_funnel_win.h"
#include "content/public/common/result_codes.h"
#include "net/base/escape.h"
#include "ui/base/l10n/l10n_util.h"
Expand Down Expand Up @@ -268,9 +267,7 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() {
remote_window_ = NULL;
return PROCESS_NONE;
case chrome::NOTIFY_WINDOW_HUNG:
// Record a hung rendezvous event in this process' exit funnel.
browser_watcher::ExitFunnel::RecordSingleEvent(
chrome::kBrowserExitCodesRegistryPath, L"RendezvousToHungBrowser");
// Fall through and potentially terminate the hung browser.
break;
}

Expand All @@ -294,11 +291,6 @@ ProcessSingleton::NotifyResult ProcessSingleton::NotifyOtherProcess() {
return PROCESS_NOTIFIED;
}

// Record the termination event in the hung process' exit funnel.
browser_watcher::ExitFunnel funnel;
if (funnel.Init(chrome::kBrowserExitCodesRegistryPath, process.Handle()))
funnel.RecordEvent(L"HungBrowserTerminated");

// Time to take action. Kill the browser process.
process.Terminate(content::RESULT_CODE_HUNG, true);
remote_window_ = NULL;
Expand Down
8 changes: 1 addition & 7 deletions chrome/browser/ui/browser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,6 @@
#include "chrome/browser/task_manager/task_manager.h"
#include "chrome/browser/ui/view_ids.h"
#include "components/autofill/core/browser/autofill_ie_toolbar_import_win.h"
#include "components/browser_watcher/exit_funnel_win.h"
#include "ui/base/touch/touch_device.h"
#include "ui/base/win/shell.h"
#endif // OS_WIN
Expand Down Expand Up @@ -714,13 +713,8 @@ void Browser::OnWindowClosing() {
bool should_quit_if_last_browser =
browser_shutdown::IsTryingToQuit() || !chrome::WillKeepAlive();

if (should_quit_if_last_browser && chrome::ShouldStartShutdown(this)) {
#if defined(OS_WIN)
browser_watcher::ExitFunnel::RecordSingleEvent(
chrome::kBrowserExitCodesRegistryPath, L"LastWindowClose");
#endif
if (should_quit_if_last_browser && chrome::ShouldStartShutdown(this))
browser_shutdown::OnShutdownStarting(browser_shutdown::WINDOW_CLOSE);
}

// Don't use GetForProfileIfExisting here, we want to force creation of the
// session service so that user can restore what was open.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include "chrome/browser/ui/views/tabs/tab_strip.h"
#include "chrome/browser/ui/views/theme_image_mapper.h"
#include "chrome/common/chrome_constants.h"
#include "components/browser_watcher/exit_funnel_win.h"
#include "ui/base/theme_provider.h"
#include "ui/gfx/win/dpi.h"
#include "ui/views/controls/menu/native_menu_win.h"
Expand Down Expand Up @@ -74,31 +73,6 @@ class DesktopThemeProvider : public ui::ThemeProvider {
DISALLOW_COPY_AND_ASSIGN(DesktopThemeProvider);
};

// See http://crbug.com/412384.
void TraceSessionEnding(LPARAM lparam) {
browser_watcher::ExitFunnel funnel;
if (!funnel.Init(chrome::kBrowserExitCodesRegistryPath,
base::GetCurrentProcessHandle())) {
return;
}

// This exit path is the prime suspect for most our unclean shutdowns.
// Trace all the possible options to WM_ENDSESSION. This may result in
// multiple events for a single shutdown, but that's fine.
funnel.RecordEvent(L"WM_ENDSESSION");

if (lparam & ENDSESSION_CLOSEAPP)
funnel.RecordEvent(L"ES_CloseApp");
if (lparam & ENDSESSION_CRITICAL)
funnel.RecordEvent(L"ES_Critical");
if (lparam & ENDSESSION_LOGOFF)
funnel.RecordEvent(L"ES_Logoff");
const LPARAM kKnownBits =
ENDSESSION_CLOSEAPP | ENDSESSION_CRITICAL | ENDSESSION_LOGOFF;
if (lparam & ~kKnownBits)
funnel.RecordEvent(L"ES_Other");
}

} // namespace

////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -211,7 +185,6 @@ bool BrowserDesktopWindowTreeHostWin::PreHandleMSG(UINT message,
minimize_button_metrics_.OnHWNDActivated();
return false;
case WM_ENDSESSION:
TraceSessionEnding(l_param);
chrome::SessionEnding();
return true;
case WM_INITMENUPOPUP:
Expand Down
27 changes: 0 additions & 27 deletions chrome/browser/ui/views/status_icons/status_tray_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "chrome/browser/ui/views/status_icons/status_icon_win.h"
#include "chrome/browser/ui/views/status_icons/status_tray_state_changer_win.h"
#include "chrome/common/chrome_constants.h"
#include "components/browser_watcher/exit_funnel_win.h"
#include "ui/gfx/screen.h"
#include "ui/gfx/win/hwnd_util.h"

Expand All @@ -30,31 +29,6 @@ UINT ReservedIconId(StatusTray::StatusIconType type) {
return kBaseIconId + static_cast<UINT>(type);
}

// See http://crbug.com/412384.
void TraceSessionEnding(LPARAM lparam) {
browser_watcher::ExitFunnel funnel;
if (!funnel.Init(chrome::kBrowserExitCodesRegistryPath,
base::GetCurrentProcessHandle())) {
return;
}

// This exit path is the prime suspect for most our unclean shutdowns.
// Trace all the possible options to WM_ENDSESSION. This may result in
// multiple events for a single shutdown, but that's fine.
funnel.RecordEvent(L"TraybarEndSession");

if (lparam & ENDSESSION_CLOSEAPP)
funnel.RecordEvent(L"ES_CloseApp");
if (lparam & ENDSESSION_CRITICAL)
funnel.RecordEvent(L"ES_Critical");
if (lparam & ENDSESSION_LOGOFF)
funnel.RecordEvent(L"ES_Logoff");
const LPARAM kKnownBits =
ENDSESSION_CLOSEAPP | ENDSESSION_CRITICAL | ENDSESSION_LOGOFF;
if (lparam & ~kKnownBits)
funnel.RecordEvent(L"ES_Other");
}

} // namespace

// Default implementation for StatusTrayStateChanger that communicates to
Expand Down Expand Up @@ -226,7 +200,6 @@ LRESULT CALLBACK StatusTrayWin::WndProc(HWND hwnd,
// If Chrome is in background-only mode, this is the only notification
// it gets that Windows is exiting. Make sure we shutdown in an orderly
// fashion.
TraceSessionEnding(lparam);
chrome::SessionEnding();
}
return ::DefWindowProc(hwnd, message, wparam, lparam);
Expand Down
47 changes: 3 additions & 44 deletions chrome/chrome_watcher/chrome_watcher_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
#include "chrome/installer/util/util_constants.h"
#include "components/browser_watcher/endsession_watcher_window_win.h"
#include "components/browser_watcher/exit_code_watcher_win.h"
#include "components/browser_watcher/exit_funnel_win.h"
#include "components/browser_watcher/window_hang_monitor_win.h"

#ifdef KASKO
Expand Down Expand Up @@ -79,9 +78,6 @@ class BrowserMonitor {
// Posted to main thread from Watch when browser exits.
void BrowserExited();

// The funnel used to record events for this browser.
browser_watcher::ExitFunnel exit_funnel_;

browser_watcher::ExitCodeWatcher exit_code_watcher_;
browser_watcher::EndSessionWatcherWindow end_session_watcher_window_;

Expand Down Expand Up @@ -121,11 +117,6 @@ bool BrowserMonitor::StartWatching(
if (!exit_code_watcher_.Initialize(process.Pass()))
return false;

if (!exit_funnel_.Init(registry_path,
exit_code_watcher_.process().Handle())) {
return false;
}

if (!background_thread_.StartWithOptions(
base::Thread::Options(base::MessageLoop::TYPE_IO, 0))) {
return false;
Expand All @@ -144,22 +135,6 @@ bool BrowserMonitor::StartWatching(
void BrowserMonitor::OnEndSessionMessage(UINT message, LPARAM lparam) {
DCHECK_EQ(main_thread_, base::ThreadTaskRunnerHandle::Get());

if (message == WM_QUERYENDSESSION) {
exit_funnel_.RecordEvent(L"WatcherQueryEndSession");
} else if (message == WM_ENDSESSION) {
exit_funnel_.RecordEvent(L"WatcherEndSession");
}
if (lparam & ENDSESSION_CLOSEAPP)
exit_funnel_.RecordEvent(L"ES_CloseApp");
if (lparam & ENDSESSION_CRITICAL)
exit_funnel_.RecordEvent(L"ES_Critical");
if (lparam & ENDSESSION_LOGOFF)
exit_funnel_.RecordEvent(L"ES_Logoff");
const LPARAM kKnownBits =
ENDSESSION_CLOSEAPP | ENDSESSION_CRITICAL | ENDSESSION_LOGOFF;
if (lparam & ~kKnownBits)
exit_funnel_.RecordEvent(L"ES_Other");

// If the browser hasn't exited yet, dally for a bit to try and stretch this
// process' lifetime to give it some more time to capture the browser exit.
browser_exited_.TimedWait(base::TimeDelta::FromSeconds(kDelayTimeSeconds));
Expand All @@ -177,7 +152,6 @@ void BrowserMonitor::Watch(base::win::ScopedHandle on_initialized_event) {
on_initialized_event.Close();

exit_code_watcher_.WaitForExit();
exit_funnel_.RecordEvent(L"BrowserExit");

// Note that the browser has exited.
browser_exited_.Signal();
Expand Down Expand Up @@ -213,24 +187,9 @@ void OnWindowEvent(
base::Process process,
const base::Callback<void(const base::Process&)>& on_hung_callback,
browser_watcher::WindowHangMonitor::WindowEvent window_event) {
browser_watcher::ExitFunnel exit_funnel;
if (exit_funnel.Init(registry_path.c_str(), process.Handle())) {
switch (window_event) {
case browser_watcher::WindowHangMonitor::WINDOW_NOT_FOUND:
exit_funnel.RecordEvent(L"MessageWindowNotFound");
break;
case browser_watcher::WindowHangMonitor::WINDOW_HUNG:
exit_funnel.RecordEvent(L"MessageWindowHung");
if (!on_hung_callback.is_null())
on_hung_callback.Run(process);
break;
case browser_watcher::WindowHangMonitor::WINDOW_VANISHED:
exit_funnel.RecordEvent(L"MessageWindowVanished");
break;
default:
NOTREACHED();
break;
}
if (window_event == browser_watcher::WindowHangMonitor::WINDOW_HUNG &&
!on_hung_callback.is_null()) {
on_hung_callback.Run(process);
}
}

Expand Down
3 changes: 3 additions & 0 deletions components/browser_watcher/exit_funnel_win.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ namespace browser_watcher {
// The event trace is stored in registry under a new key named
// <pid>-<uniquifier>, where each event is a value named after the event, with
// an associated QWORD value noting the event time.
// Note: this is deprecated, and is only kept around for testing the cleanup
// in the WatcherMetricsProvider.
// TODO(siggi): Kill this class - see http://crbug.com/442526.
class ExitFunnel {
public:
ExitFunnel();
Expand Down
Loading

0 comments on commit d896f9a

Please sign in to comment.