Something has caused Shockwave registration to crash. Plugin saftey catches the crash on NPP_Destroy. Here's how to repro: 1. Ensure np32dsw.dll is in your plugins folder 2. Delete the registry key: HKEY_CURRENT_USER\Software\Macromedia\Shockwave 8 3. Visit simplified testcase url: http://warp.mcom.com/u/peterlubczynski/testcases/plugins/testcases/shockwave1.html This is a regression from either the subclassing code or the fix from bug 166613. If I back out the fix from bug 166613, the crash goes away. If I move the UndoSubclassing code from SetWindow to ~nsPluginNativeWindowWin then the crash goes away too.

Serge, although Peter and me found that moving UndoSubclass call fixes the crash, the regression was introduced not by subclassing code (09-26) but by the fix to bug 166613 (10-02), that's why I suggested we should probably first try to understand why it happened because it maybe a sign of something more deep.

hmm, fix for bug 166613 just restores the same code branch has. I can image adding plugin's internal channels on load group can change the code flow, e.g. delays some window destruction, which prevents the crash, but IMHO we cannot relay on this.

this looks to me like shockwave's WindowProc bug, they crash on dereferencing of null prt:( my proposal to fix this is to set |PluginWndProc| only for "application/x-shockwave-flash" mimetype, is this only plugin we suppose to subclass, right?

No the whole point of subclassing is not to handle some messages specially, but to wrap calls to the plugin winproc with our safety macros. This should supposedly increase crash tolerance.

hmm, but it regressed:(

I think we also need the subclassing code to roll up context menus and take over accelerator keys. Moving UndoSubclassing to ~nsPluginNativeWindowWin seems to do it. Can we live with that?

I think we certainly can. The reason I don't like it, is that it does not look like a cause for the problem to me. I can imagine when we crash because we resubclass too late (meaning actually not doing it at all) but doing it at any point before that per se should be perfectly safe. If this is a bug in a specific plugin I would rather made a special case for this plugin and/or this specific message. Well... unless I am terribly wrong.

Serge, do you have any ideas why it could have regressed after that streaming fix?

it has nothing to do with load group for internal (target == null) plugins stream. the problem was just hidden:(

moving |UndoSubclassAndAssociateWindow| into DTOR of |nsPluginNativeWindowWin| and setting bp at there and at CTOR of the same class shows me that DTOR for first object is never called.

on av's comment #4 here is the stack trace shows we crash in |PluginPlugin.dll| at |WinProc| but we got in there form our safety macro brackets |nsPluginHostImpl::HandleBadPlugin|

Looks like still imperfections in subclassing code. We don't want to undo subclassing if that were not us last time. Taking the bug. Patch is coming. Thanks serge who pointed me to the right direction in our private chat.

Please review.

Comment on attachment 102299 [details] [diff] [review] patch v.1 yeah, this is right, r=serge

Comment on attachment 102299 [details] [diff] [review] patch v.1 sr=beard, good find.

Comment on attachment 102299 [details] [diff] [review] patch v.1 a=asa for checkin to 1.2beta (on behalf of drivers) and noting serge's review in the patch status.

Should have marked it yesterday. Fixed.

fixed on 1011 trunk. verif.