500654 - Fix OS/2 plugin code similar to bug 493601

Status: VERIFIED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Peter Weilbacher]

I can't see what bug 493601 is really about (it's still hidden) but as one can see e.g. in bonsai (for the checkin on CVS trunk)

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/modules/plugin/base/src&command=DIFF_FRAMESET&file=nsPluginNativeWindowWin.cpp&rev1=1.28&rev2=1.29&root=/cvsroot

it changes functions that until now were mostly in sync with OS/2. The checkin comment is "Fix crash involving Flash module unloading". I guess this would be relevant for mozilla-central, mozilla-1.9.1 and 1.9.0=CVS-trunk.

Maybe this can help to make us more stable even though we have this unstable flash?

Comment 2

[Rich Walsh]

Attachment: updated OS/2 plugin code

This largely a port of changes to the Win32 version of nsPluginNativeWindowXXX.cpp.  Whether it actually does anything worthwhile remains to be seen.  There are also some formatting changes to improve consistency & readability.

Comment 3

[Peter Weilbacher]

Great, looks like that way you found fixes for some other crashes that could appear like Flash problems (like bug 374229 and the ancient bug 131007). I guess even if we don't take the sWM_FLASHBOUNCEMSG stuff we should add the rest. I don't like :: if they are not required, but OK. The
   sPluginNativeWindowOS2 * win
before the NS_ASSERTION should get an #ifdef DEBUG around it, to circumvent the warning.

Because of the plugin situation on trunk I ported the patch to the 1.9.1 branch to get wider testing. The DLL in this ZIP file:
   http://temp.weilbacher.org/bug500654_gkplugin_20090726_191.zip
goes together with the nightly SeaMonkey build from here:
   http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/contrib/2009/07/2009-07-26-04-comm-1.9.1/

Comment 4

[Rich Walsh]

(In reply to comment #3)
> Great, looks like that way you found fixes for some other crashes that
> could appear

Honestly, this is just a dumb port.

> I guess even if we don't take the sWM_FLASHBOUNCEMSG stuff

Of all of it, this seems the most significant.  In the interests of "parallel-ism" with Win32, I'd leave everything in place unless it could be shown to interfere.

> I don't like :: if they are not required, but OK.

I don't either, but fewer lines were changed by adding them than would have been by deleting them.  My goal was stylistic consistency.

Comment 5

[Peter Weilbacher]

Comment on attachment 390740 [details] [diff] [review]
updated OS/2 plugin code

So, as remarked in comment 3 I'll add the #fdef DEBUG on checkin. 
Otherwise this looks OK. Nobody noticed anything bad in the last 4 days, so I'm going to push it.

Comment 6

[Peter Weilbacher]

Pushed to trunk:
http://hg.mozilla.org/mozilla-central/rev/6121c7853d5d

This should also go to the 1.9.1 branch very soon.

Comment 7

[Peter Weilbacher]

Attachment: updated OS/2 plugin code for 1.9.1 branch

This is basically the same patch that I checked in to mozilla-central, just slightly updated to fit the 1.9.1 branch.

Comment 8

[Peter Weilbacher]

Attachment: updated OS/2 plugin code for 1.9.1 branch

That patch was obviously broken. This is what will go onto the 1.9.1 branch unless someone objects.

Comment 9

[Peter Weilbacher]

Comment on attachment 392080 [details] [diff] [review]
updated OS/2 plugin code for 1.9.1 branch

Pushed to 1.9.1:
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/4da2a7ffd27f

(too late for FF 3.5.2).

Comment 10

[Peter Weilbacher]

I'll mark this verified, because the code sync has happened, and didn't seem to cause any problems on trunk or branch.