In bug 628651, (which is the front-end of this issue) we need to notify users when their 64bit Mac OSX tries to load a 32bit carbon plugin. At this point our code just gives up loading a mismatched plugin: http://mxr.mozilla.org/mozilla-central/source/dom/plugins/PluginModuleChild.cpp#1880 It would be great if we can utilize the same method for dealing with crashed plugins by triggering an event in the document that attempted to load the plugin, as in these cases. see: https://bugzilla.mozilla.org/show_bug.cgi?id=628651#c7 A more simple approach may be to notify Observers of this kind of failure as in: https://bugzilla.mozilla.org/show_bug.cgi?id=628651#c10

Requesting blocking as this blocks a hardblocker: bug 628651

Josh, can you look into this? This is a hardblocker since it blocks a hardblocker.

I haven't tested this beyond compiling but it should send an event when an OOPP instance fails because it tried to negotiate the Carbon event model. In order for this to be useful we probably also want a boolean attribute on the plugin host called "i386SupportsCarbonNPAPI". We'd only put up an alert about restarting if this is true. We can then use the architecture scanning code in nsPluginsDirDarwin to find out whether or not we have an i386 child binary. If we do have an i386 child binary then we can just assume it supports Carbon NPAPI. David - can you confirm that this notification works properly for you? Once we have it working we can move onto the "i386SupportsCarbonNPAPI" attribute in a separate patch.

(In reply to comment #3) > Created attachment 508186 [details] [diff] [review] > Carbon NPAPI failure notification v1.0 > > I haven't tested this beyond compiling but it should send an event when an OOPP > instance fails because it tried to negotiate the Carbon event model. When I tried to build it I got this failure: /Users/moco/code/mozilla/dom/plugins/PluginModuleChild.cpp: In member function ‘virtual bool mozilla::plugins::PluginModuleChild::AnswerPPluginInstanceConstructor(mozilla::plugins::PPluginInstanceChild*, const nsCString&, const uint16_t&, const InfallibleTArray<nsCString>&, const InfallibleTArray<nsCString>&, NPError*)’: /Users/moco/code/mozilla/dom/plugins/PluginModuleChild.cpp:1890: error: ‘class mozilla::plugins::PluginInstanceChild’ has no member named ‘CallNegotiatedCarbon’ PluginProcessParent.cpp Is there a new method missing in this patch?

or - is CallNegotiatedCarbon some kind of getter for the NegotiatedCarbon ipdl method?

s/CallNegotiatedCarbon/SendNegotiatedCarbon/

Removing from b11 radar, but hoping for ETA of Friday.

What plugins are there that we can test this with?

I thought the test plugin was a carbon plugin

(In reply to comment #9) > I thought the test plugin was a carbon plugin Looks to be cocoa in 64-bit and carbon (maybe both?) in 32 bit. In which case it should work just fine in 64-bit mode. I tried the test plugin from the 1.9.2 branch but it seems Minefield decided that that was invalid for some reason.

Comment on attachment 508573 [details] [diff] [review] Carbon NPAPI failure notification v1.1 >diff --git a/dom/plugins/PluginModuleChild.cpp b/dom/plugins/PluginModuleChild.cpp >--- a/dom/plugins/PluginModuleChild.cpp >+++ b/dom/plugins/PluginModuleChild.cpp >@@ -1878,16 +1878,23 @@ PluginModuleChild::AnswerPPluginInstance > } > > #if defined(XP_MACOSX) && defined(__i386__) > // If an i386 Mac OS X plugin has selected the Carbon event model then > // we have to fail. We do not support putting Carbon event model plugins > // out of process. Note that Carbon is the default model so out of process > // plugins need to actively negotiate something else in order to work > // out of process. >+ >+ // Send notification that a plugin tried to negotiate Carbon NPAPI so that >+ // users can be notified that restarting the browser in i386 mode may allow >+ // them to use the plugin. >+ childInstance->SendNegotiatedCarbon(); >+ >+ // Fail to instantiate. > if (childInstance->EventModel() == NPEventModelCarbon) { > *rv = NPERR_MODULE_LOAD_FAILED_ERROR; > } > #endif > > return true; > } > Am I mistaken or should the SendNegotiatedCarbon call be inside the if block there? This does send the event that we want in frontend but it appears to do it even for some working plugins (Quicktime), I assume because they are 32-bit but cocoa.

Yeah, oops.

Comment on attachment 509119 [details] [diff] [review] Carbon NPAPI failure notification v1.2 This works well for me. Presumably we need a plugin peer to review this before it can land?

Comment on attachment 509119 [details] [diff] [review] Carbon NPAPI failure notification v1.2 If Benoit can review that is good enough.

Comment on attachment 509119 [details] [diff] [review] Carbon NPAPI failure notification v1.2 Looks good!

mossop, ddahl: Can you guys land my patch here whenever you land the code that uses the event?

Yep

Landed: http://hg.mozilla.org/mozilla-central/rev/69f3807d4bb4

This broke non-libxul builds (bug 631412). :(

Comment on attachment 509119 [details] [diff] [review] Carbon NPAPI failure notification v1.2 >+ nsString type = NS_LITERAL_STRING("npapi-carbon-event-model-failure"); Unfortunately this copies the string. Perhaps you wanted NS_NAMED_LITERAL_STRING(type, "npapi-carbon-event-model-failure");