Now that we do not support 10.2, we can use the (much nicer) @try/@catch/@finally blocks to do exception handling, instead of the very delicate NS_DURING/NS_HANDLER/NS_ENDHANDLER. We just need to add -fobjc-exceptions (or some such) to our compiler flags. I'm not sure what the optimal place to add that is, though.

Benjamin - ideas on where to put -fobjc-exceptions?

What code needs to have this flag? I'm guessing only the widget code? If so, just add CPPFLAGS += -fobjc-exceptions to the relevant makefiles, ifdefed if necessary.

Yeah, if it's needed in other modules for whatever reason (it could potentially be useful anywhere there is a .mm file), we can always just turn it on there as well.

Alright. I enabled this for widget/src/cocoa, and replaced all our uses of NS_HANDLER and friends with @try/@catch. While I was there I took the opportunity to use warnings instead of assertions (a mistake on my part initally). In nsClipboard.mm, I had to move the exception handling code below the calls to EqualsLiteral. Basically, it's necessary because of the way ObjC exceptions (setjmp/longmp based) interact with local variables (which need to be declared volatile in that case by the C ANSI standard). Thanks to dcamp for help with it. Trying to clear off some of the lower hanging fruit from my bug list.

Why nsPrintfCString? What is wrong with printf or NSLog?

I wanted it to be an NS_WARNING, because people (like Jesse) look for the syntax that NS_WARNING produces. A random NSLog or printf doesn't give line/function/file information either, and putting that information in the NSLog is just as ugly looking as what I did already, IMO.

Comment on attachment 283797 [details] [diff] [review] fix v1.0 > CXXFLAGS += \ > -DUSE_COCOA \ >+ -fobjc-exceptions \ You're mixing two spaces vs. a tab. Whichever one is right use it consistently.

Use tabs, like the rest of the file.

Comment on attachment 284211 [details] [diff] [review] fix v1.1 ", make sure to catch any exceptions (timeouts)" This part of the two comments is not necessary. I think it is pretty obvious that we're catching exceptions here. Also the handler will print out the cause so no need to copy the documentation on the method call to the comment. Fix on checkin.

Checking in widget/src/cocoa/Makefile.in; /cvsroot/mozilla/widget/src/cocoa/Makefile.in,v <-- Makefile.in new revision: 1.75; previous revision: 1.74 done Checking in widget/src/cocoa/nsClipboard.mm; /cvsroot/mozilla/widget/src/cocoa/nsClipboard.mm,v <-- nsClipboard.mm new revision: 1.19; previous revision: 1.18 done Checking in widget/src/cocoa/nsNativeThemeCocoa.mm; /cvsroot/mozilla/widget/src/cocoa/nsNativeThemeCocoa.mm,v <-- nsNativeThemeCocoa.mm new revision: 1.64; previous revision: 1.63 done

Checking in widget/src/cocoa/nsClipboard.mm; /cvsroot/mozilla/widget/src/cocoa/nsClipboard.mm,v <-- nsClipboard.mm new revision: 1.20; previous revision: 1.19 done ^ this fixed bustage.

I backed this out since it was still busted and Colin wasn't responding to hails. There was further bustage after the first bustage fix.

Colin - please post a new patch and request review again before landing.

So I'm still unable to reproduce those build errors on my machine.

I've separated the @try calls into separate functions, since AFAICT only variables local to that function get tagged with volatile after @try/@catch (setjmp/longjmp). Can't run on try server, it seems to be down :(

Just a reminder to please commit this by Monday if you want to get it in before beta. Otherwise, approval1.9+ will be revoked, and you will need to re-request it after M9 if you still want to land the patch. If you would like somebody else to commit this for you, please add the "checkin-needed" keyword.

I'll just wait until after beta on this one.

Reed, go ahead and land this since you're all ready to.

Checking in widget/src/cocoa/Makefile.in; /cvsroot/mozilla/widget/src/cocoa/Makefile.in,v <-- Makefile.in new revision: 1.78; previous revision: 1.77 done Checking in widget/src/cocoa/nsClipboard.mm; /cvsroot/mozilla/widget/src/cocoa/nsClipboard.mm,v <-- nsClipboard.mm new revision: 1.25; previous revision: 1.24 done Checking in widget/src/cocoa/nsNativeThemeCocoa.mm; /cvsroot/mozilla/widget/src/cocoa/nsNativeThemeCocoa.mm,v <-- nsNativeThemeCocoa.mm new revision: 1.66; previous revision: 1.65 done