[Mac] Allow NSExceptions in certain cases.

chrome/browser/chrome_browser_application_mac.mm

Index: chrome/browser/chrome_browser_application_mac.mm
diff --git a/chrome/browser/chrome_browser_application_mac.mm b/chrome/browser/chrome_browser_application_mac.mm
index 745e0ab4dbb9396913f4db9a7f0da5232da12271..f070efac4d7abc1db2de239d41e434a3b005d9c8 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) {

[Mark Mentovai, 2011/05/17 19:07:34]

If the exception is “fatal” because it matched something above, but “allow” is
true because we’re inside a scoper on this thread, then…LOG(FATAL)?

Is that right?

[Scott Hess - ex-Googler, 2011/05/17 19:34:40]

Crap, did I get that backwards?  I had another CL with added code to force
exceptions at various points, and it all seemed right, but it's possible in
clean-up I tweaked something so that I wasn't testing the cases I needed to
test.

[Scott Hess - ex-Googler, 2011/05/17 20:00:44]

Indeed, my forced exceptions were all NSGenericException, which wasn't fatal. 
Sigh.

       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);
     }
   }
 
@@ -306,6 +308,18 @@ BOOL SwizzleNSExceptionInit() {
     }
   }
 
+  // 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;
+  }
+
+  base::mac::ScopedNSExceptionEnabler enabler(enableNSExceptions);

[Mark Mentovai, 2011/05/17 19:07:34]

I’d prefer to get this as close to [super sendAction:to:from:] as possible.

+
   // When a Cocoa control is wired to a freed object, we get crashers
   // in the call to |super| with no useful information in the
   // backtrace.  Attempt to add some useful information.
@@ -354,6 +368,11 @@ BOOL SwizzleNSExceptionInit() {
     // addressed elsewhere.
     [self clearIsHandlingSendEvent];
 
+    // If |ScopedNSExceptionEnabler| is used to allow exceptions, and
+    // an uncaught exception is thrown, the flag must be reset so that
+    // future exceptions are not masked.
+    base::mac::SetNSExceptionsAllowed(false);

[stuartmorgan, 2011/05/17 18:52:14]

Given that you've designed these to nest arbitrarily, how do you know this will
do the right thing? Shouldn't we instead catch (and rethrow if we want to) when
we use a C++ scoped object to manage ObjC exceptions?

[Mark Mentovai, 2011/05/17 19:07:34]

false instead of the original “was” value?

(which would be hard to access at this point?)

[Scott Hess - ex-Googler, 2011/05/17 19:34:40]

In the 32-bit runtime, you cannot use C++ scoped objects to manage ObjC
exceptions.  This specific case is when the exception was caught by the
exception handler AppKit wraps around the outermost event loop.  If the flag is
currently set, it's because the exception blew past all the scopers.  Basically
it's the same as the -clearIsHandlingSendEvent call just above.

[Scott Hess - ex-Googler, 2011/05/17 19:34:40]

This is effectively the outermost scope.

+
     // 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