442115 - Plugins initialized with data="mimetype," cause a crash in the compositing code

Status: RESOLVED DUPLICATE of bug 435764

(Core :: Layout, defect)

Description

[Geoff Norton]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0) Gecko/2008061600 SUSE/3.0-8.1 Firefox/3.0
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0) Gecko/2008061600 SUSE/3.0-8.1 Firefox/3.0

We narrowed down and debuged a crash in the compositing code:

#0  cairo_draw_with_xlib (cr=0x1b7ec00, callback=0x7f59009bdfd4 <NativeRendering>, closure=0x7fff0ab87710, dpy=0x0, width=300, height=300, is_opaque=CAIRO_XLIB_DRAWING_TRANSPARENT, capabilities=27, result=0x0) at cairo-xlib-utils.c:329
#1  0x00007f59009be0e4 in gfxXlibNativeRenderer::Draw (this=0x7fff0ab87770, dpy=0x0, ctx=0x1a7fae0, width=300, height=300, flags=1, output=0x0) at gfxXlibNativeRenderer.cpp:101
#2  0x00007f59003ec8de in nsPluginInstanceOwner::Paint (this=0x1aed740, aRenderingContext=@0x1ab1d40, aDirtyRect=@0x7fff0ab87840) at nsObjectFrame.cpp:4076
#3  0x00007f59003ec92a in nsObjectFrame::PaintPlugin (this=0x1aeb028, aRenderingContext=@0x1ab1d40, aDirtyRect=@0x7fff0ab87840) at nsObjectFrame.cpp:1400
#4  0x00007f59003ec9b3 in PaintPlugin (aFrame=0x1aeb028, aCtx=0x1ab1d40, aDirtyRect=@0x7fff0ab87910, aPt={x = 179861664, y = 32767}) at nsObjectFrame.cpp:1096
#5  0x00007f59003dc52c in nsDisplayGeneric::Paint (this=0x1b65488, aBuilder=<value optimized out>, aCtx=0x1ab1d40, aDirtyRect=@0x7fff0ab87910) at ./../base/nsDisplayList.h:862
#6  0x00007f5900395f8c in nsDisplayList::Paint (this=<value optimized out>, aBuilder=0x7fff0ab87a20, aCtx=0x1ab1d40, aDirtyRect=@0x7fff0ab87910) at nsDisplayList.cpp:296
#7  0x00007f590039601a in nsDisplayClip::Paint (this=<value optimized out>, aBuilder=0x7fff0ab87a20, aCtx=0x1ab1d40, aDirtyRect=<value optimized out>) at nsDisplayList.cpp:693
#8  0x00007f5900395f8c in nsDisplayList::Paint (this=<value optimized out>, aBuilder=0x7fff0ab87a20, aCtx=0x1ab1d40, aDirtyRect=@0x7fff0ab87f60) at nsDisplayList.cpp:296
#9  0x00007f59003a4ef3 in nsLayoutUtils::PaintFrame (aRenderingContext=0x1ab1d40, aFrame=<value optimized out>, aDirtyRegion=@0x7fff0ab87f30, aBackground=4294967295) at nsLayoutUtils.cpp:988
#10 0x00007f59003ab128 in PresShell::Paint (this=<value optimized out>, aView=0x0, aRenderingContext=0x1ab1d40, aDirtyRegion=@0x7fff0ab87f30) at nsPresShell.cpp:5413
#11 0x00007f59005d4cbe in nsViewManager::RenderViews (this=0x1abe120, aView=<value optimized out>, aRC=@0x1ab1d40, aRegion=@0x7fff0ab88040) at nsViewManager.cpp:614
#12 0x00007f59005d52df in nsViewManager::Refresh (this=0x1abe120, aView=0x1ae3580, aContext=0x1ab1d40, aRegion=<value optimized out>, aUpdateFlags=1) at nsViewManager.cpp:502
#13 0x00007f59005d5c70 in nsViewManager::DispatchEvent (this=0x1abe120, aEvent=0x7fff0ab88210, aStatus=0x7fff0ab881cc) at nsViewManager.cpp:1134
#14 0x00007f59005d0b55 in HandleEvent (aEvent=0x7fff0ab88210) at nsView.cpp:168
#15 0x00007f59008cd605 in nsCommonWidget::DispatchEvent (this=0x1ae9be0, aEvent=0x7fff0ab88210, aStatus=@0x7fff0ab88394) at nsCommonWidget.cpp:158
#16 0x00007f59008c932e in nsWindow::OnExposeEvent (this=0x1ae9be0, aWidget=<value optimized out>, aEvent=0x7fff0ab88a50) at nsWindow.cpp:1763
#17 0x00007f59008c98ff in expose_event_cb (widget=0xa5dc40, event=0x7fff0ab88a50) at nsWindow.cpp:4529
#18 0x00007f58fcab3998 in ?? () from /usr/lib64/libgtk-x11-2.0.so.0
#19 0x00007f58fdb5b20d in g_closure_invoke () from /usr/lib64/libgobject-2.0.so.0
#20 0x00007f58fdb6f08c in ?? () from /usr/lib64/libgobject-2.0.so.0
#21 0x00007f58fdb70392 in g_signal_emit_valist () from /usr/lib64/libgobject-2.0.so.0
#22 0x00007f58fdb70a53 in g_signal_emit () from /usr/lib64/libgobject-2.0.so.0
#23 0x00007f58fcbc8a8e in ?? () from /usr/lib64/libgtk-x11-2.0.so.0
#24 0x00007f58fcaad3da in gtk_main_do_event () from /usr/lib64/libgtk-x11-2.0.so.0
#25 0x00007f58fc4d65c5 in ?? () from /usr/lib64/libgdk-x11-2.0.so.0
#26 0x00007f58fc4d6b51 in gdk_window_process_all_updates () from /usr/lib64/libgdk-x11-2.0.so.0
#27 0x00007f58fc4d6b79 in ?? () from /usr/lib64/libgdk-x11-2.0.so.0
#28 0x00007f58fc4bc90b in ?? () from /usr/lib64/libgdk-x11-2.0.so.0
#29 0x00007f58fd6c193a in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
#30 0x00007f58fd6c5040 in ?? () from /usr/lib64/libglib-2.0.so.0
#31 0x00007f58fd6c51dc in g_main_context_iteration () from /usr/lib64/libglib-2.0.so.0
#32 0x00007f59008dfe01 in nsBaseAppShell::DoProcessNextNativeEvent (this=0x17968e0, mayWait=179860912) at nsBaseAppShell.cpp:151
#33 0x00007f59008dff4f in nsBaseAppShell::OnProcessNextEvent (this=0x75c6c0, thr=0x6b9f10, mayWait=1, recursionDepth=<value optimized out>) at nsBaseAppShell.cpp:296
#34 0x00007f5900986a6d in nsThread::ProcessNextEvent (this=0x6b9f10, mayWait=1, result=0x7fff0ab88d4c) at nsThread.cpp:497
#35 0x00007f590095bc52 in NS_ProcessNextEvent_P (thread=0x17968e0, mayWait=1) at nsThreadUtils.cpp:227
#36 0x00007f59008e006d in nsBaseAppShell::Run (this=0x75c6c0) at nsBaseAppShell.cpp:170
#37 0x00007f59007c0319 in nsAppStartup::Run (this=0x7df110) at nsAppStartup.cpp:181
#38 0x00007f5900245b66 in XRE_main (argc=<value optimized out>, argv=<value optimized out>, aAppData=<value optimized out>) at nsAppRunner.cpp:3170
#39 0x00000000004016f4 in ?? ()
#40 0x00007f5901a9d436 in __libc_start_main () from /lib64/libc.so.6
#41 0x00000000004011f9 in ?? ()
#42 0x00007fff0ab8c778 in ?? ()
#43 0x000000000000001c in ?? ()
#44 0x0000000000000005 in ?? ()
#45 0x00007fff0ab8e030 in ?? ()
#46 0x00007fff0ab8e063 in ?? ()
#47 0x0000000000000000 in ?? ()

It appears that under certain circumstances the plugin can be initialized but SetWindow is never called, causing a crash due to a null Display being sent to cairo.

I attempted to guard against null display in the function in question, but that just caused the plugin to never draw (and never call SetWindow in our code).

Also if the data="application/x-silverlight," is removed from the instantiation, the code executes as expected.

I have included the html here for completeness;

<html>
        <head>
                <title>FF3 Windowless Crash</title>
        </head>

        <body>
                <!-- If this element is removed the crashing code below will not crash.
                     this is also true if this element is moved below the div -->
                <h1>(Re)move this and we dont crash</h1>
                <div id="player"></div>

                <script type="text/javascript">
var player = document.getElementById ('player');

// This invocation crashes (modulo the comments above)
player.innerHTML = '<object type="application/x-silverlight" data="data:application/x-silverlight," id="xamlHost2" width="300" height="300" ><param name="windowless" value="true" /><param name="source" value="none.xaml" /></object>';

// This invocation works
// player.innerHTML = '<object type="application/x-silverlight" id="xamlHost2" width="300" height="300" ><param name="windowless" value="true" /><param name="source" value="none.xaml" /></object>';
</script>
</body>
</html>

The moonlight source is all available at (svn):

http://anonsvn.mono-project.com/source/trunk/moon

Reproducible: Always

Steps to Reproduce:
1. Install moonlight (http://go-mono.com/moonlight should work)
2. Visit http://blog.sublimeintervention/com/ff3crash/moonlight.html
Actual Results:  
Firefox crash with aforementioned trace

Expected Results:  
Call SetWindow in our code.

Its worth noting we tried closing the 0byte stream passed to us in NewStream immediately before returning from NewStream.  This caused a 2nd call into NewStream with the mimetype (application/x-silverlight) before crashing.

Comment 1

[Geoff Norton]

Attachment: stacktrace

stacktrace attached due to line munging

Comment 2

[Geoff Norton]

Attachment: testcase

testcase attached due to line munging

Comment 3

[Geoff Norton]

Did a little more debugging.  It appears that nsObjectFrame::CallSetWindow is being called, but mInstanceOwner isn't set (our Initialize call has NOT been called at this point).  Minefield doesn't appear to crash, but our plugin still doesn't work.

How is it that the layout/compositing code is attempting to paint us before initializing us?  Do we need some special handling in NewStream for the data="anything" case?

Comment 4

[Geoff Norton]

Sorry minefield is crashing, I had some local changes I forgot about.

Comment 5

[Geoff Norton]

More details:

The (pertinent) call chain appears to be like this:

NPP_GetMimeDescription
NPP_GetValue (NPPVpluginNameString)
NPP_GetValue (NPPVpluginDescriptionString)
nsObjectFrame::CallSetWindow () (on the plugin, triggered by reflow, mInstanceOwner is null)
NPP_Initialize
NPP_New

I'm not entirely sure what set of circumstances causes this to happen (see the comments in the testcase about the <h1> causing this to trigger), but its almost certainly (?) flow layout related?

I would guess the layout engine shouldn't be attempting to reflow plugins that are not yet initialized.

Comment 6

[Geoff Norton]

Attachment: Prevent flushing layout changes before a plugin has been initialized.

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 327145 [details] [diff] [review]
Prevent flushing layout changes before a plugin has been initialized.

Boris, what's your thoughts on this fix?

Comment 8

[Boris Zbarsky [:bzbarsky]]

The first thought is that it just backs out the key part of the fix for bug 381512 and thus would regress it.  Some bonsai due diligence would have been good here....

The second thought is that we need plug-in regression tests.

The third thought is that the stack in this bug has to do with painting, not layout, so I'm not quite sure why this patch is helping.  Why is it?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 327145 [details] [diff] [review]
Prevent flushing layout changes before a plugin has been initialized.

r-, given that it breaks other things, as far as I can see.  If it somehow doesn't, I'd like to know why not.

Comment 10

[Geoff Norton]

boris,

  The bug was moved to layout because the reflow caused at the changed section calls SetWindow before the plugin has had NPP_New/NPP_Initialize called on it (see my previous comments).  Causing the reflow to not flush prevents this crash.  If you look at the testcase (.html) and the comments included it shows that its something to do with layout (I think) as the <h1 /> being removed obscures the bug again.  

  Admittedly I'm not deeply familiar with the mozilla codebase, in fact this is the first time I've dug into it in earnest, but the patch was included as something that seemed logical given the exhibited behavior.

  At the very least we should NEVER be calling SetWindow down the call chain before a plugin as had Init/New called on it.

Comment 11

[Karl Tomlinson (:karlt)]

(In reply to comment #10)
>   At the very least we should NEVER be calling SetWindow down the call chain
> before a plugin as had Init/New called on it.

I expect nsObjectFrame::CallSetWindow skips the SetWindow unless Init/New has been called, but let me know if you see otherwise.

I'm pretty sure that you are right in suggesting that this is a dupe of Bug 435764.  Thank you for all your analysis.

Please test attachment 327742 [details] [diff] [review] if you can, or at least the tryserver build at bug 435764 comment 11.  If that doesn't resolve the problem then reopen this bug.