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

6 files changed, 183 insertions, 2 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);
+}