Add an exception wrapper to the WindowProc functions so

that we receive crash reports when something goes wrong.

BUG=63702
TEST=base_unittests

Review URL: http://codereview.chromium.org/6697004

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

14 files changed, 230 insertions, 11 deletions

diff --git a/base/base.gyp b/base/base.gyp
index 525d316..fcf0b88 100644
--- a/base/base.gyp
+++ b/base/base.gyp
@@ -179,6 +179,7 @@
         'win/scoped_comptr_unittest.cc',
         'win/scoped_variant_unittest.cc',
         'win/win_util_unittest.cc',
+        'win/wrapped_window_proc_unittest.cc',
       ],
       'dependencies': [
         'base',
diff --git a/base/base.gypi b/base/base.gypi
index ac8d9acd..0472ae8 100644
--- a/base/base.gypi
+++ b/base/base.gypi
@@ -333,6 +333,8 @@
           'win/win_util.h',
           'win/windows_version.cc',
           'win/windows_version.h',
+          'win/wrapped_window_proc.cc',
+          'win/wrapped_window_proc.h',
           'nix/xdg_util.h',
           'nix/xdg_util.cc',
         ],
diff --git a/base/message_pump_win.cc b/base/message_pump_win.cc
index 6098a4a..778c2f5 100644
--- a/base/message_pump_win.cc
+++ b/base/message_pump_win.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -7,6 +7,7 @@
 #include <math.h>
 
 #include "base/metrics/histogram.h"
+#include "base/win/wrapped_window_proc.h"
 
 namespace base {
 
@@ -232,7 +233,7 @@ void MessagePumpForUI::InitMessageWnd() {
 
   WNDCLASSEX wc = {0};
   wc.cbSize = sizeof(wc);
-  wc.lpfnWndProc = WndProcThunk;
+  wc.lpfnWndProc = base::win::WrappedWindowProc<WndProcThunk>;
   wc.hInstance = hinst;
   wc.lpszClassName = kWndClass;
   RegisterClassEx(&wc);
diff --git a/base/win/wrapped_window_proc.cc b/base/win/wrapped_window_proc.cc
new file mode 100644
index 0000000..c94c5ae
--- /dev/null
+++ b/base/win/wrapped_window_proc.cc
@@ -0,0 +1,32 @@
+// Copyright (c) 2011 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 "base/win/wrapped_window_proc.h"
+
+#include "base/atomicops.h"
+
+namespace {
+
+base::win::WinProcExceptionFilter s_exception_filter = NULL;
+
+}  // namespace.
+
+namespace base {
+namespace win {
+
+WinProcExceptionFilter SetWinProcExceptionFilter(
+    WinProcExceptionFilter filter) {
+  subtle::AtomicWord rv = subtle::NoBarrier_AtomicExchange(
+      reinterpret_cast<subtle::AtomicWord*>(&s_exception_filter),
+      reinterpret_cast<subtle::AtomicWord>(filter));
+  return reinterpret_cast<WinProcExceptionFilter>(rv);
+}
+
+int CallExceptionFilter(EXCEPTION_POINTERS* info) {
+  return s_exception_filter ? s_exception_filter(info) :
+                              EXCEPTION_CONTINUE_SEARCH;
+}
+
+}  // namespace win
+}  // namespace base
diff --git a/base/win/wrapped_window_proc.h b/base/win/wrapped_window_proc.h
new file mode 100644
index 0000000..0515044
--- /dev/null
+++ b/base/win/wrapped_window_proc.h
@@ -0,0 +1,66 @@
+// Copyright (c) 2011 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.
+
+// Provides a way to handle exceptions that happen while a WindowProc is
+// running. The behavior of exceptions generated inside a WindowProc is OS
+// dependent, but it is possible that the OS just ignores the exception and
+// continues execution, which leads to unpredictable behavior for Chrome.
+
+#ifndef BASE_WIN_WRAPPED_WINDOW_PROC_H_
+#define BASE_WIN_WRAPPED_WINDOW_PROC_H_
+#pragma once
+
+#include <windows.h>
+
+namespace base {
+namespace win {
+
+// An exception filter for a WindowProc. The return value determines how the
+// exception should be handled, following standard SEH rules. However, the
+// expected behavior for this function is to not return, instead of returning
+// EXCEPTION_EXECUTE_HANDLER or similar, given that in general we are not
+// prepared to handle exceptions.
+typedef int (__cdecl *WinProcExceptionFilter)(EXCEPTION_POINTERS* info);
+
+// Sets the filter to deal with exceptions inside a WindowProc. Returns the old
+// exception filter, if any.
+// This function should be called before any window is created.
+WinProcExceptionFilter SetWinProcExceptionFilter(WinProcExceptionFilter filter);
+
+// Calls the registered exception filter.
+int CallExceptionFilter(EXCEPTION_POINTERS* info);
+
+// Wrapper that supplies a standard exception frame for the provided WindowProc.
+// The normal usage is something like this:
+//
+// LRESULT CALLBACK MyWinProc(HWND hwnd, UINT message,
+//                            WPARAM wparam, LPARAM lparam) {
+//   // Do Something.
+// }
+//
+// ...
+//
+//   WNDCLASSEX wc = {0};
+//   wc.lpfnWndProc = WrappedWindowProc<MyWinProc>;
+//   wc.lpszClassName = class_name;
+//   ...
+//   RegisterClassEx(&wc);
+//
+//   CreateWindowW(class_name, window_name, ...
+//
+template <WNDPROC proc>
+LRESULT CALLBACK WrappedWindowProc(HWND hwnd, UINT message,
+                                   WPARAM wparam, LPARAM lparam) {
+  LRESULT rv = 0;
+  __try {
+    rv = proc(hwnd, message, wparam, lparam);
+  } __except(CallExceptionFilter(GetExceptionInformation())) {
+  }
+  return rv;
+}
+
+}  // namespace win
+}  // namespace base
+
+#endif  // BASE_WIN_WRAPPED_WINDOW_PROC_H_
diff --git a/base/win/wrapped_window_proc_unittest.cc b/base/win/wrapped_window_proc_unittest.cc
new file mode 100644
index 0000000..ccf3c85
--- /dev/null
+++ b/base/win/wrapped_window_proc_unittest.cc
@@ -0,0 +1,79 @@
+// Copyright (c) 2011 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 "base/win/wrapped_window_proc.h"
+#include "base/message_loop.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+DWORD kExceptionCode = 12345;
+WPARAM kCrashMsg = 98765;
+
+// A trivial WindowProc that generates an exception.
+LRESULT CALLBACK TestWindowProc(HWND hwnd, UINT message,
+                                WPARAM wparam, LPARAM lparam) {
+  if (message == kCrashMsg)
+    RaiseException(kExceptionCode, 0, 0, NULL);
+  return DefWindowProc(hwnd, message, wparam, lparam);
+}
+
+// This class implements an exception filter that can be queried about a past
+// exception.
+class TestWrappedExceptionFiter {
+ public:
+  TestWrappedExceptionFiter() : called_(false) {
+    EXPECT_FALSE(s_filter_);
+    s_filter_ = this;
+  }
+
+  ~TestWrappedExceptionFiter() {
+    EXPECT_EQ(s_filter_, this);
+    s_filter_ = NULL;
+  }
+
+  bool called() {
+    return called_;
+  }
+
+  // The actual exception filter just records the exception.
+  static int Filter(EXCEPTION_POINTERS* info) {
+    EXPECT_FALSE(s_filter_->called_);
+    if (info->ExceptionRecord->ExceptionCode == kExceptionCode)
+      s_filter_->called_ = true;
+    return EXCEPTION_EXECUTE_HANDLER;
+  }
+
+ private:
+  bool called_;
+  static TestWrappedExceptionFiter* s_filter_;
+};
+TestWrappedExceptionFiter* TestWrappedExceptionFiter::s_filter_ = NULL;
+
+}  // namespace.
+
+TEST(WrappedWindowProc, CatchesExceptions) {
+  HINSTANCE hinst = GetModuleHandle(NULL);
+  std::wstring class_name(L"TestClass");
+
+  WNDCLASS wc = {0};
+  wc.lpfnWndProc = base::win::WrappedWindowProc<TestWindowProc>;
+  wc.hInstance = hinst;
+  wc.lpszClassName = class_name.c_str();
+  RegisterClass(&wc);
+
+  HWND window = CreateWindow(class_name.c_str(), 0, 0, 0, 0, 0, 0, HWND_MESSAGE,
+                             0, hinst, 0);
+  ASSERT_TRUE(window);
+
+  // Before generating the exception we make sure that the filter will see it.
+  TestWrappedExceptionFiter wrapper;
+  base::win::WinProcExceptionFilter old_filter =
+      base::win::SetWinProcExceptionFilter(TestWrappedExceptionFiter::Filter);
+
+  SendMessage(window, kCrashMsg, 0, 0);
+  EXPECT_TRUE(wrapper.called());
+
+  base::win::SetWinProcExceptionFilter(old_filter);
+}
diff --git a/chrome/app/breakpad_win.cc b/chrome/app/breakpad_win.cc
index b860bef..dfff428 100644
--- a/chrome/app/breakpad_win.cc
+++ b/chrome/app/breakpad_win.cc
@@ -438,6 +438,18 @@ bool ShowRestartDialogIfCrashed(bool* exit_now) {
                                flags, exit_now);
 }
 
+// Crashes the process after generating a dump for the provided exception. Note
+// that the crash reporter should be initialized before calling this function
+// for it to do anything.
+extern "C" int __declspec(dllexport) CrashForException(
+    EXCEPTION_POINTERS* info) {
+  if (g_breakpad) {
+    g_breakpad->WriteMinidumpForException(info);
+    ::ExitProcess(ResultCodes::KILLED);
+  }
+  return EXCEPTION_CONTINUE_SEARCH;
+}
+
 // Determine whether configuration management allows loading the crash reporter.
 // Since the configuration management infrastructure is not initialized at this
 // point, we read the corresponding registry key directly. The return status
diff --git a/chrome/browser/browser_main_win.cc b/chrome/browser/browser_main_win.cc
index 06f99f0..3d0a052 100644
--- a/chrome/browser/browser_main_win.cc
+++ b/chrome/browser/browser_main_win.cc
@@ -19,10 +19,12 @@
 #include "base/scoped_ptr.h"
 #include "base/utf_string_conversions.h"
 #include "base/win/windows_version.h"
+#include "base/win/wrapped_window_proc.h"
 #include "chrome/browser/browser_list.h"
 #include "chrome/browser/first_run/first_run.h"
 #include "chrome/browser/metrics/metrics_service.h"
 #include "chrome/browser/ui/views/uninstall_view.h"
+#include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/env_vars.h"
 #include "chrome/common/result_codes.h"
@@ -42,9 +44,22 @@
 #include "views/window/window.h"
 
 namespace {
+
 typedef HRESULT (STDAPICALLTYPE* RegisterApplicationRestartProc)(
     const wchar_t* command_line,
     DWORD flags);
+
+void InitializeWindowProcExceptions() {
+  // Get the breakpad pointer from chrome.exe
+  base::win::WinProcExceptionFilter exception_filter =
+      reinterpret_cast<base::win::WinProcExceptionFilter>(
+          ::GetProcAddress(::GetModuleHandle(
+                               chrome::kBrowserProcessExecutableName),
+                           "CrashForException"));
+  exception_filter = base::win::SetWinProcExceptionFilter(exception_filter);
+  DCHECK(!exception_filter);
+}
+
 }  // namespace
 
 void DidEndMainMessageLoop() {
@@ -264,6 +279,9 @@ class BrowserMainPartsWin : public BrowserMainParts {
     if (!parameters().ui_task) {
       // Override the configured locale with the user's preferred UI language.
       l10n_util::OverrideLocaleWithUILanguageList();
+
+      // Make sure that we know how to handle exceptions from the message loop.
+      InitializeWindowProcExceptions();
     }
   }
 
diff --git a/chrome/browser/process_singleton_win.cc b/chrome/browser/process_singleton_win.cc
index 17910dc..6682706 100644
--- a/chrome/browser/process_singleton_win.cc
+++ b/chrome/browser/process_singleton_win.cc
@@ -11,6 +11,7 @@
 #include "base/process_util.h"
 #include "base/utf_string_conversions.h"
 #include "base/win/scoped_handle.h"
+#include "base/win/wrapped_window_proc.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/extensions_startup.h"
 #include "chrome/browser/platform_util.h"
@@ -178,7 +179,8 @@ bool ProcessSingleton::Create() {
 
   WNDCLASSEX wc = {0};
   wc.cbSize = sizeof(wc);
-  wc.lpfnWndProc = ProcessSingleton::WndProcStatic;
+  wc.lpfnWndProc =
+      base::win::WrappedWindowProc<ProcessSingleton::WndProcStatic>;
   wc.hInstance = hinst;
   wc.lpszClassName = chrome::kMessageWindowClass;
   ATOM clazz = RegisterClassEx(&wc);
diff --git a/chrome/browser/renderer_host/render_widget_host_view_win.cc b/chrome/browser/renderer_host/render_widget_host_view_win.cc
index ec99486..c6179c0 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_win.cc
+++ b/chrome/browser/renderer_host/render_widget_host_view_win.cc
@@ -13,6 +13,7 @@
 #include "base/scoped_comptr_win.h"
 #include "base/threading/thread.h"
 #include "base/win/scoped_gdi_object.h"
+#include "base/win/wrapped_window_proc.h"
 #include "chrome/browser/accessibility/browser_accessibility_manager.h"
 #include "chrome/browser/accessibility/browser_accessibility_state.h"
 #include "chrome/browser/accessibility/browser_accessibility_win.h"
@@ -511,7 +512,7 @@ HWND RenderWidgetHostViewWin::ReparentWindow(HWND window) {
     WNDCLASSEX wcex;
     wcex.cbSize         = sizeof(WNDCLASSEX);
     wcex.style          = CS_DBLCLKS;
-    wcex.lpfnWndProc    = PluginWrapperWindowProc;
+    wcex.lpfnWndProc    = base::win::WrappedWindowProc<PluginWrapperWindowProc>;
     wcex.cbClsExtra     = 0;
     wcex.cbWndExtra     = 0;
     wcex.hInstance      = GetModuleHandle(NULL);
@@ -1592,7 +1593,8 @@ gfx::PluginWindowHandle RenderWidgetHostViewWin::AcquireCompositingSurface() {
     WNDCLASSEX wcex;
     wcex.cbSize         = sizeof(WNDCLASSEX);
     wcex.style          = 0;
-    wcex.lpfnWndProc    = CompositorHostWindowProc;
+    wcex.lpfnWndProc    =
+        base::win::WrappedWindowProc<CompositorHostWindowProc>;
     wcex.cbClsExtra     = 0;
     wcex.cbWndExtra     = 0;
     wcex.hInstance      = GetModuleHandle(NULL);
diff --git a/chrome/browser/ui/views/status_icons/status_tray_win.cc b/chrome/browser/ui/views/status_icons/status_tray_win.cc
index 2fa679e..5eb7e22 100644
--- a/chrome/browser/ui/views/status_icons/status_tray_win.cc
+++ b/chrome/browser/ui/views/status_icons/status_tray_win.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/ui/views/status_icons/status_tray_win.h"
 
+#include "base/win/wrapped_window_proc.h"
 #include "chrome/browser/ui/views/status_icons/status_icon_win.h"
 #include "chrome/common/chrome_constants.h"
 #include "ui/base/win/hwnd_util.h"
@@ -16,7 +17,7 @@ StatusTrayWin::StatusTrayWin()
   HINSTANCE hinst = GetModuleHandle(NULL);
   WNDCLASSEX wc = {0};
   wc.cbSize = sizeof(wc);
-  wc.lpfnWndProc = StatusTrayWin::WndProcStatic;
+  wc.lpfnWndProc = base::win::WrappedWindowProc<StatusTrayWin::WndProcStatic>;
   wc.hInstance = hinst;
   wc.lpszClassName = chrome::kStatusTrayWindowClass;
   ATOM clazz = RegisterClassEx(&wc);
diff --git a/ui/base/clipboard/clipboard_win.cc b/ui/base/clipboard/clipboard_win.cc
index 4380b00..bb50802 100644
--- a/ui/base/clipboard/clipboard_win.cc
+++ b/ui/base/clipboard/clipboard_win.cc
@@ -17,6 +17,7 @@
 #include "base/string_util.h"
 #include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
+#include "base/win/wrapped_window_proc.h"
 #include "ui/base/clipboard/clipboard_util_win.h"
 #include "ui/gfx/size.h"
 
@@ -135,7 +136,7 @@ Clipboard::Clipboard() : create_window_(false) {
     // Make a dummy HWND to be the clipboard's owner.
     WNDCLASSEX wcex = {0};
     wcex.cbSize = sizeof(WNDCLASSEX);
-    wcex.lpfnWndProc = ClipboardOwnerWndProc;
+    wcex.lpfnWndProc = base::win::WrappedWindowProc<ClipboardOwnerWndProc>;
     wcex.hInstance = GetModuleHandle(NULL);
     wcex.lpszClassName = L"ClipboardOwnerWindowClass";
     ::RegisterClassEx(&wcex);
diff --git a/ui/base/win/window_impl.cc b/ui/base/win/window_impl.cc
index 545e79c..2f6da99 100644
--- a/ui/base/win/window_impl.cc
+++ b/ui/base/win/window_impl.cc
@@ -6,9 +6,10 @@
 
 #include <list>
 
-#include "ui/base/win/hwnd_util.h"
 #include "base/singleton.h"
 #include "base/string_number_conversions.h"
+#include "base/win/wrapped_window_proc.h"
+#include "ui/base/win/hwnd_util.h"
 
 namespace ui {
 
@@ -209,7 +210,7 @@ std::wstring WindowImpl::GetWindowClassName() {
   WNDCLASSEX class_ex;
   class_ex.cbSize = sizeof(WNDCLASSEX);
   class_ex.style = class_info.style;
-  class_ex.lpfnWndProc = &WindowImpl::WndProc;
+  class_ex.lpfnWndProc = base::win::WrappedWindowProc<&WindowImpl::WndProc>;
   class_ex.cbClsExtra = 0;
   class_ex.cbWndExtra = 0;
   class_ex.hInstance = NULL;
diff --git a/views/controls/menu/native_menu_win.cc b/views/controls/menu/native_menu_win.cc
index 9eabf46..f639a2e 100644
--- a/views/controls/menu/native_menu_win.cc
+++ b/views/controls/menu/native_menu_win.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -6,6 +6,7 @@
 
 #include "base/logging.h"
 #include "base/stl_util-inl.h"
+#include "base/win/wrapped_window_proc.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/keycodes/keyboard_codes.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -76,7 +77,7 @@ class NativeMenuWin::MenuHostWindow {
     WNDCLASSEX wcex = {0};
     wcex.cbSize = sizeof(WNDCLASSEX);
     wcex.style = CS_DBLCLKS;
-    wcex.lpfnWndProc = &MenuHostWindowProc;
+    wcex.lpfnWndProc = base::win::WrappedWindowProc<&MenuHostWindowProc>;
     wcex.hbrBackground = reinterpret_cast<HBRUSH>(COLOR_WINDOW+1);
     wcex.lpszClassName = kWindowClassName;
     ATOM clazz = RegisterClassEx(&wcex);