From 39e3b9538deaedaaa8104f1cc67bcddaacad8269 Mon Sep 17 00:00:00 2001 From: "shess@chromium.org" Date: Tue, 24 May 2011 22:07:05 +0000 Subject: [Mac] Allow NSExceptions in certain cases. Thirdy-party print drivers seem to be a source of NSExceptions which Chromium will never be able to fix. ScopedNSExceptionEnabler causes the code which makes throwing an NSException fatal to allow throws. The flag will be reset in -reportException: in most cases. For now, allow exceptions to be thrown for -selectPDE: (bug 80686) and PrintingContextMac::AskUserForSettings() (bug 82589). BUG=80686, 82589 TEST=Monitor crash server. Review URL: http://codereview.chromium.org/7038010 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@86503 0039d316-1c4b-4281-b951-d872f2087c98 --- base/base.gypi | 2 ++ base/mac/scoped_nsexception_enabler.h | 46 ++++++++++++++++++++++++ base/mac/scoped_nsexception_enabler.mm | 44 +++++++++++++++++++++++ chrome/browser/chrome_browser_application_mac.mm | 26 ++++++++++++-- printing/printing_context_mac.mm | 11 ++++++ 5 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 base/mac/scoped_nsexception_enabler.h create mode 100644 base/mac/scoped_nsexception_enabler.mm diff --git a/base/base.gypi b/base/base.gypi index c3bcd00..22d66f8 100644 --- a/base/base.gypi +++ b/base/base.gypi @@ -129,6 +129,8 @@ 'mac/scoped_cftyperef.h', 'mac/scoped_nsautorelease_pool.h', 'mac/scoped_nsautorelease_pool.mm', + 'mac/scoped_nsexception_enabler.h', + 'mac/scoped_nsexception_enabler.mm', 'mach_ipc_mac.h', 'mach_ipc_mac.mm', 'memory/linked_ptr.h', diff --git a/base/mac/scoped_nsexception_enabler.h b/base/mac/scoped_nsexception_enabler.h new file mode 100644 index 0000000..a10120b --- /dev/null +++ b/base/mac/scoped_nsexception_enabler.h @@ -0,0 +1,46 @@ +// 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. + +#ifndef BASE_MAC_SCOPED_NSEXCEPTION_ENABLER_H_ +#define BASE_MAC_SCOPED_NSEXCEPTION_ENABLER_H_ +#pragma once + +#include "base/basictypes.h" + +namespace base { +namespace mac { + +// BrowserCrApplication attempts to restrict throwing of NSExceptions +// because they interact badly with C++ scoping rules. Unfortunately, +// there are some cases where exceptions must be supported, such as +// when third-party printer drivers are used. These helpers can be +// used to enable exceptions for narrow windows. + +// Make it easy to safely allow NSException to be thrown in a limited +// scope. Note that if an exception is thrown, then this object will +// not be appropriately destructed! If the exception ends up in the +// top-level event loop, things are cleared in -reportException:. If +// the exception is caught at a lower level, a higher level scoper +// should eventually reset things. +class ScopedNSExceptionEnabler { + public: + ScopedNSExceptionEnabler(); + ~ScopedNSExceptionEnabler(); + + private: + bool was_enabled_; + + DISALLOW_COPY_AND_ASSIGN(ScopedNSExceptionEnabler); +}; + +// Access the exception setting for the current thread. This is for +// the support code in BrowserCrApplication, other code should use +// the scoper. +bool GetNSExceptionsAllowed(); +void SetNSExceptionsAllowed(bool allowed); + +} // namespace mac +} // namespace base + +#endif // BASE_MAC_SCOPED_NSEXCEPTION_ENABLER_H_ diff --git a/base/mac/scoped_nsexception_enabler.mm b/base/mac/scoped_nsexception_enabler.mm new file mode 100644 index 0000000..6815de3 --- /dev/null +++ b/base/mac/scoped_nsexception_enabler.mm @@ -0,0 +1,44 @@ +// 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. + +#import "base/mac/scoped_nsexception_enabler.h" + +#import "base/lazy_instance.h" +#import "base/threading/thread_local.h" + +// To make the |g_exceptionsAllowed| declaration readable. +using base::LazyInstance; +using base::LeakyLazyInstanceTraits; +using base::ThreadLocalBoolean; + +namespace { + +// Whether to allow NSExceptions to be raised on the current thread. +LazyInstance > + g_exceptionsAllowed(base::LINKER_INITIALIZED); + +} // namespace + +namespace base { +namespace mac { + +bool GetNSExceptionsAllowed() { + return g_exceptionsAllowed.Get().Get(); +} + +void SetNSExceptionsAllowed(bool allowed) { + return g_exceptionsAllowed.Get().Set(allowed); +} + +ScopedNSExceptionEnabler::ScopedNSExceptionEnabler() { + was_enabled_ = GetNSExceptionsAllowed(); + SetNSExceptionsAllowed(true); +} + +ScopedNSExceptionEnabler::~ScopedNSExceptionEnabler() { + SetNSExceptionsAllowed(was_enabled_); +} + +} // namespace mac +} // namespace base diff --git a/chrome/browser/chrome_browser_application_mac.mm b/chrome/browser/chrome_browser_application_mac.mm index 745e0ab..d407dde 100644 --- a/chrome/browser/chrome_browser_application_mac.mm +++ b/chrome/browser/chrome_browser_application_mac.mm @@ -5,6 +5,7 @@ #import "chrome/browser/chrome_browser_application_mac.h" #import "base/logging.h" +#import "base/mac/scoped_nsexception_enabler.h" #import "base/metrics/histogram.h" #import "base/memory/scoped_nsobject.h" #import "base/sys_string_conversions.h" @@ -93,7 +94,8 @@ static IMP gOriginalInitIMP = NULL; // (destructors are skipped). Chrome should be NSException-free, // please check your backtrace and see if you can't file a bug // with a repro case. - if (fatal) { + const bool allow = base::mac::GetNSExceptionsAllowed(); + if (fatal && !allow) { LOG(FATAL) << "Someone is trying to raise an exception! " << base::SysNSStringToUTF8(value); } else { @@ -101,7 +103,7 @@ static IMP gOriginalInitIMP = NULL; // exceptions. DLOG(ERROR) << "Someone is trying to raise an exception! " << base::SysNSStringToUTF8(value); - NOTREACHED(); + DCHECK(allow); } } @@ -329,6 +331,21 @@ BOOL SwizzleNSExceptionInit() { [sender className], tag, actionString, aTarget]; ScopedCrashKey key(kActionKey, value); + + // Certain third-party code, such as print drivers, can still throw + // exceptions and Chromium cannot fix them. This provides a way to + // work around those on a spot basis. + bool enableNSExceptions = false; + + // http://crbug.com/80686 , an Epson printer driver. + if (anAction == @selector(selectPDE:)) { + enableNSExceptions = true; + } + + // Minimize the window by keeping this close to the super call. + scoped_ptr enabler(NULL); + if (enableNSExceptions) + enabler.reset(new base::mac::ScopedNSExceptionEnabler()); return [super sendAction:anAction to:aTarget from:sender]; } @@ -354,6 +371,11 @@ BOOL SwizzleNSExceptionInit() { // addressed elsewhere. [self clearIsHandlingSendEvent]; + // If |ScopedNSExceptionEnabler| is used to allow exceptions, and an + // uncaught exception is thrown, it will throw past all of the scopers. + // Reset the flag so that future exceptions are not masked. + base::mac::SetNSExceptionsAllowed(false); + // Store some human-readable information in breakpad keys in case // there is a crash. Since breakpad does not provide infinite // storage, we track two exceptions. The first exception thrown diff --git a/printing/printing_context_mac.mm b/printing/printing_context_mac.mm index a8e4fa0..599fa78 100644 --- a/printing/printing_context_mac.mm +++ b/printing/printing_context_mac.mm @@ -9,6 +9,8 @@ #include "base/logging.h" #include "base/mac/scoped_cftyperef.h" +#include "base/mac/scoped_nsautorelease_pool.h" +#include "base/mac/scoped_nsexception_enabler.h" #include "base/sys_string_conversions.h" #include "base/values.h" #include "printing/print_settings_initializer_mac.h" @@ -36,6 +38,15 @@ void PrintingContextMac::AskUserForSettings(gfx::NativeView parent_view, int max_pages, bool has_selection, PrintSettingsCallback* callback) { + // Third-party print drivers seem to be an area prone to raising exceptions. + // This will allow exceptions to be raised, but does not handle them. The + // NSPrintPanel appears to have appropriate NSException handlers. + base::mac::ScopedNSExceptionEnabler enabler; + + // Exceptions can also happen when the NSPrintPanel is being + // deallocated, so it must be autoreleased within this scope. + base::mac::ScopedNSAutoreleasePool pool; + DCHECK([NSThread isMainThread]); // We deliberately don't feed max_pages into the dialog, because setting -- cgit v1.1