[Mac] Allow NSExceptions in certain cases.

chrome/browser/chrome_browser_application_mac.mm

 // 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 "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"
 #import "chrome/app/breakpad_mac.h"
 #include "chrome/browser/accessibility/browser_accessibility_state.h"
 #import "chrome/browser/app_controller_mac.h"
 #include "chrome/browser/ui/browser_list.h"
 #import "chrome/browser/ui/cocoa/objc_method_swizzle.h"
 #import "chrome/browser/ui/cocoa/objc_zombie.h"
 #include "chrome/browser/ui/tab_contents/tab_contents_wrapper.h"
@@ skipping 68 matching lines @@
     if (aName == NSInvalidArgumentException) {
       fatal = YES;
     }
 
     // Dear reader: Something you just did provoked an NSException.
     // NSException is implemented in terms of setjmp()/longjmp(),
     // which does poor things when combined with C++ scoping
     // (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 {
       // Make sure that developers see when their code throws
       // exceptions.
       DLOG(ERROR) << "Someone is trying to raise an exception!  "
                   << base::SysNSStringToUTF8(value);
-      NOTREACHED();
+      DCHECK(allow);
     }
   }
 
   // Forward to the original version.
   return gOriginalInitIMP(self, _cmd, aName, aReason, someUserInfo);
 }
 @end
 
 namespace chrome_browser_application_mac {
 
@@ skipping 184 matching lines @@
       if (window == aTarget) {
         found = YES;
         break;
       }
     }
     if (!found) {
       return NO;
     }
   }
 
+  // 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.
   static NSString* const kActionKey = @"sendaction";
 
   // If the action is something generic like -commandDispatch:, then
   // the tag is essential.
   NSInteger tag = 0;
   if ([sender isKindOfClass:[NSControl class]]) {
     tag = [sender tag];
@@ skipping 28 matching lines @@
 
     // http://crbug.com/45928 is a bug about needing to double-close
     // windows sometimes.  One theory is that |-isHandlingSendEvent|
     // gets latched to always return |YES|.  Since scopers are used to
     // manipulate that value, that should not be possible.  One way to
     // sidestep scopers is setjmp/longjmp (see above).  The following
     // is to "fix" this while the more fundamental concern is
     // 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
     // is tracked because it may be the one which caused the system to
     // go off the rails.  The last exception thrown is tracked because
     // it may be the one most directly associated with the crash.
     static NSString* const kFirstExceptionKey = @"firstexception";
     static BOOL trackedFirstException = NO;
     static NSString* const kLastExceptionKey = @"lastexception";
 
@@ skipping 32 matching lines @@
         if (RenderViewHost* rvh = contents->render_view_host()) {
           rvh->EnableRendererAccessibility();
         }
       }
     }
   }
   return [super accessibilitySetValue:value forAttribute:attribute];
 }
 
 @end