superreview granted: [Bug 410946] Crash in JS engine aborting applet making Java/JS calls : [Attachment 305554] Fix.

Boris Zbarsky (reviews very slow until May) <bzbarsky@mit.edu> has granted
Johnny Stenback (:jst) <jst@mozilla.org>'s request for superreview:
Bug 410946: Crash in JS engine aborting applet making Java/JS calls
https://bugzilla.mozilla.org/show_bug.cgi?id=3D410946

Attachment 305554: Fix.
https://bugzilla.mozilla.org/attachment.cgi?id=3D305554&action=3Dedit

------- Additional Comments from Boris Zbarsky (reviews very slow until May)
<bzbarsky@mit.edu>
Could you add documentation to nsIObjectFrame which says which of the metho=
ds
might destroy the frame?  And perhaps similar to any nsObjectLoadingContent
methods that might do it.=0D
=0D
>+++ b/modules/plugin/base/src/nsPluginHostImpl.cpp=0D
=0D
>+  NS_IMETHOD Run()=0D
>+    if (PluginDestructionGuard::DelayDestroy(mInstance)) {=0D
>+	// It's still not safe to destroy the plugin, re-dispatch this=0D
>+	// runnable.=0D
=0D
Why re-dispatch?  Why not wait for that outermost guard to go off the stack=
 and
dispatch the runnable?	Re-dispatching this one just means that either we'll
end up with two separate runnables pending or we'll end up firing this one
before the guard has gone off the stack, and thus not destroying again.=0D
=0D
Speaking of which, is this code safe when there _are_ two separate runnables
pending for the same instance?	That can happen, I think....=0D
=0D
I didn't double-check that you added this in all the places it's needed; I'm
assuming you did.=0D
=0D
r+sr=3Dbzbarsky with the above fixed.  I really like this approach!  Much l=
ess
fragile.=
0
bugzilla
2/27/2008 9:48:07 PM
mozilla.dev.super-review 29307 articles. 3 followers. Post Follow

0 Replies
509 Views

Similar Articles

[PageSpeed] 58

Reply: