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

RESOLVED DUPLICATE of bug 435764

Status

()

Core
Layout
--
critical
RESOLVED DUPLICATE of bug 435764
9 years ago
9 years ago

People

(Reporter: Geoff Norton, Assigned: Geoff Norton)

Tracking

({crash})

Trunk
x86
Linux
crash
Points:
---
Bug Flags:
blocking1.9.0.1 -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(3 attachments)

(Assignee)

Description

9 years ago
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.
(Assignee)

Comment 1

9 years ago
Created attachment 326987 [details]
stacktrace

stacktrace attached due to line munging
(Assignee)

Comment 2

9 years ago
Created attachment 326988 [details]
testcase

testcase attached due to line munging
Component: General → GFX: Thebes
Keywords: crash
Product: Firefox → Core
QA Contact: general → thebes
(Assignee)

Comment 3

9 years ago
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?
(Assignee)

Comment 4

9 years ago
Sorry minefield is crashing, I had some local changes I forgot about.
(Assignee)

Comment 5

9 years ago
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.

(Assignee)

Updated

9 years ago
Component: GFX: Thebes → Layout
(Assignee)

Comment 6

9 years ago
Created attachment 327145 [details] [diff] [review]
Prevent flushing layout changes before a plugin has been initialized.
QA Contact: thebes → layout
Version: unspecified → Trunk

Updated

9 years ago
Flags: blocking1.9.0.1?
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?
Attachment #327145 - Flags: review?(bzbarsky)
Assignee: nobody → gnorton
Status: UNCONFIRMED → NEW
Ever confirmed: true
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 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.
Attachment #327145 - Flags: review?(bzbarsky) → review-
(Assignee)

Comment 10

9 years ago
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.
Flags: wanted1.9.0.x?
Flags: blocking1.9.0.1?
Flags: blocking1.9.0.1-
(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.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 435764
Flags: wanted1.9.0.x?
You need to log in before you can comment on or make changes to this bug.