Last Comment Bug 435764 - crash [@ cairo_draw_with_xlib] painting windowless plugins when MIME type is obtained from data/src
: crash [@ cairo_draw_with_xlib] painting windowless plugins when MIME type is ...
Status: RESOLVED FIXED
: crash, fixed1.9.0.2, fixed1.9.1
Product: Core
Classification: Components
Component: Plug-ins (show other bugs)
: Trunk
: x86 Linux
P1 critical (vote)
: mozilla1.9.1a1
Assigned To: Karl Tomlinson (:karlt)
:
: Benjamin Smedberg [:bsmedberg]
Mentors:
http://www.leparisien.fr
: 435838 442115 445043 445202 (view as bug list)
Depends on:
Blocks: 384845
  Show dependency treegraph
 
Reported: 2008-05-26 08:12 PDT by Sylvain BERTRAND
Modified: 2011-06-09 14:58 PDT (History)
18 users (show)
jonas: blocking1.9.1+
samuel.sidler+old: blocking1.9.0.2-
samuel.sidler+old: wanted1.9.0.x+
samuel.sidler+old: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
gdb bt full with a debug build of firefox 3.0 RC1 (16.83 KB, text/plain)
2008-05-26 08:14 PDT, Sylvain BERTRAND
no flags Details
move ws_info set up from nsObjectFrame::CallSetWindow() to nsPluginInstanceOwner::CreateWidget() (3.51 KB, patch)
2008-06-12 01:57 PDT, Karl Tomlinson (:karlt)
no flags Details | Diff | Splinter Review
move ws_info set up from nsObjectFrame::CallSetWindow() (28.77 KB, patch)
2008-07-01 21:42 PDT, Karl Tomlinson (:karlt)
no flags Details | Diff | Splinter Review
move ws_info set up from nsObjectFrame::CallSetWindow() v4.1 (29.11 KB, patch)
2008-07-06 19:10 PDT, Karl Tomlinson (:karlt)
roc: review+
roc: superreview+
Details | Diff | Splinter Review
move ws_info set up from nsObjectFrame::CallSetWindow() v4.1.1 (29.11 KB, patch)
2008-07-07 14:15 PDT, Karl Tomlinson (:karlt)
samuel.sidler+old: approval1.9.0.2+
Details | Diff | Splinter Review
include gdk/gdkscreen.h in system-headers (404 bytes, patch)
2008-07-07 20:35 PDT, Karl Tomlinson (:karlt)
roc: review+
samuel.sidler+old: approval1.9.0.2+
Details | Diff | Splinter Review

Description User image Sylvain BERTRAND 2008-05-26 08:12:46 PDT
User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; fr; rv:1.9) Gecko/2008052615 (Gentoo) Firefox/3.0
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; fr; rv:1.9) Gecko/2008052615 (Gentoo) Firefox/3.0

As of today (26052008), with the swfdec(git) plugin enabled or disabled, if you try to view/scroll to the bottom of the provided page, firefox 3.0 RC1 crashed (trying to display a plugin managed area).

Reproducible: Always
Comment 1 User image Sylvain BERTRAND 2008-05-26 08:14:31 PDT
Created attachment 322523 [details]
gdb bt full with a debug build of firefox 3.0 RC1
Comment 2 User image Benjamin Otte 2008-05-27 02:19:21 PDT
*** Bug 435838 has been marked as a duplicate of this bug. ***
Comment 3 User image Sylvain BERTRAND 2008-05-30 06:40:12 PDT
Crash with same BT:
http://www.universfreebox.com/article5360.html
Comment 4 User image Sylvain BERTRAND 2008-06-06 02:10:30 PDT
Made a full debug build of firefox 3 RC2 with today git swfdec:
 - http://www.leparisien.fr seems not to crash anymore.
 - http://www.universfreebox.com/article5360.html hangs the whole firefox process.
Comment 5 User image Karl Tomlinson (:karlt) 2008-06-10 21:14:47 PDT
I'm keen to try out swfdec, but I haven't found a revision of
git://swfdec.freedesktop.org/git/swfdec/swfdec in the last few days that compiles.

Can someone recommend a revision/tag that I should use, please?

BTW, the current issue is:
cc1: warnings being treated as errors
swfdec_init.c: In function 'swfdec_init':
swfdec_init.c:68: warning: implicit declaration of function 'strtoul'
swfdec_init.c:68: warning: nested extern declaration of 'strtoul'
Comment 6 User image Benjamin Otte 2008-06-11 11:59:55 PDT
Uh yeah, you're in a kind of peculiar situation here as you need to run a development version, and those set -Werror (and no, I'm not going into a discussion of why this is the best thing people should do). In theory, git master should work (and definitely compile) fine, as there's enough developers running it every day. If there's issues, poke me on IRC and I'll fix them immediately.

That said, if you still want the easy path, export CFLAGS=-Wno-error when building swfdec and swfdec-mozilla. That gets rid of the "warnings being treated as errors" thing.
Comment 7 User image Karl Tomlinson (:karlt) 2008-06-12 01:52:06 PDT
Thanks to Benjamin, I now have swfdec-mozilla compiled (and producing some
nice effects when Mozilla doesn't crash).

The crash is happening because sometimes ws_info is not set up (in
nsObjectFrame::CallSetWindow()) and so it contains zeros when used in
nsPluginInstanceOwner::Paint().

The crash happens for plugins that are instantiated through
nsObjectFrame::Instantiate(nsIChannel* aChannel, nsIStreamListener** aStreamListener)
rather than
nsObjectFrame::Instantiate(const char* aMimeType, nsIURI* aURI)
as only the later does CallSetWindow() (and I don't know why that is).

With attachment 324576 [details] [diff] [review], gfxXlibNativeRenderer::Draw almost never uses its dpy
argument and so the crash reduces to

###!!! ASSERTION: Visual changed: colormap may not match: 'ws_info->visual == visual', file /home/karl/moz/mozilla/layout/generic/nsObjectFrame.cpp, line 4207

but still no useful info is available in ws_info for the plugin.

I can reproduce the assertion when scrolling to the first windowless plugin on 
http://www.universfreebox.com/article5360.html, or on http://movies.yahoo.com/.
Comment 8 User image Karl Tomlinson (:karlt) 2008-06-12 01:57:07 PDT
Created attachment 324752 [details] [diff] [review]
move ws_info set up from nsObjectFrame::CallSetWindow() to nsPluginInstanceOwner::CreateWidget()

This is one way to solve the problem.
Perhaps another is to set up in nsObjectFrame::FixupWindow, or perhaps both
nsObjectFrame::Instantiate methods (or neither) should do CallSetWindow();

The build here has this patch and attachment 324576 [details] [diff] [review], for those who would like something that works:

https://build.mozilla.org/tryserver-builds/2008-06-11_23:42-ktomlinson@mozilla.com-lines-null-wsinfo/

(I'll see if I can work out why the Instantiate methods differ.)
Comment 9 User image Jonas Sicking (:sicking) No longer reading bugmail consistently 2008-06-12 15:14:49 PDT
Is this a common way to view flash on linux? Or are people just using the official macromedia one?
Comment 10 User image Karl Tomlinson (:karlt) 2008-06-12 15:30:33 PDT
(In reply to comment #9)

The bug is being hit because newer versions of swfdec have windowless support.

https://bugs.launchpad.net/ubuntu/+source/firefox-3.0/+bug/239182

The macromedia plugin will hit the same bug when it has wmode support.

(I don't know how many users use use swfdec or gnash or the macromedia plugin [with nspluginwrapper for 64-bit].)
Comment 12 User image Mike Melanson 2008-06-26 10:54:28 PDT
Similar problem seen in Adobe Flash Player. Most notable with http://movies.yahoo.com/ . Tracked by Adobe internal issue 228012.
Comment 13 User image Karl Tomlinson (:karlt) 2008-07-01 21:24:40 PDT
This will happen with any windowless plugin when the mime type is determined from using the URL in the data attribute rather than from the
type attribute, which now happens for object elements (bug 95549).
I guess there would be the same problem if the type attribute were not included in an embed element.
Comment 14 User image Karl Tomlinson (:karlt) 2008-07-01 21:42:46 PDT
Created attachment 327742 [details] [diff] [review]
move ws_info set up from nsObjectFrame::CallSetWindow()

nsPluginInstanceOwner::CreateWidget() is a good place to set up the display because that is where the plugin "becomes" windowless, and the display should not change.

The right place to set up the Colormap is in nsPluginInstanceOwner::Renderer::NativeDraw() where the Visual is known.

To avoid the need to hunt for the Visual on the Display to find the
Screen for selecting a Colormap, this changes the Display* argument of gfxXlibNativeRenderer::NativeDraw to a Screen*.

This also makes the treatment of CAIRO_XLIB_DRAWING_SUPPORTS_NONDEFAULT_VISUAL in _create_temp_xlib_surface consistent with that in _draw_with_xlib_direct.
i.e. when not set the visual must be the default visual of the screen used,
but need not be the default visual of the fallback dpy specified.
Comment 15 User image Mike Melanson 2008-07-02 14:42:07 PDT
Reported by Adobe Flash Player users:
http://nvidia.com/
http://www.bbc.co.uk/
I reproduced these crashes by loading the pages and then maximizing the browser frame. The crash appears to come from libxul.so.
Comment 16 User image Stephanie Daugherty [:sdaugherty] 2008-07-05 06:57:52 PDT
I'm seeing an almost identical crash with latest Adobe Flash Player on my own personal blog, which is fixed by Karl's test build. I can reproduce 100% on latest nightly by scrolling down the page.
http://crash-stats.mozilla.com/report/index/ec6a48d1-4a4b-11dd-b681-001cc45a2ce4 and http://crash-stats.mozilla.com/report/index/d95b1702-4a4c-11dd-a703-001cc45a2ce4 are two of my crashes from a nightly.
Comment 17 User image Geoff Norton 2008-07-05 18:44:16 PDT
I think my bug #442115 is a duplicate of this with some additional information as to what is causing this.  
Comment 18 User image Karl Tomlinson (:karlt) 2008-07-06 01:20:56 PDT
*** Bug 442115 has been marked as a duplicate of this bug. ***
Comment 19 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2008-07-06 16:56:44 PDT
FindColormapForVisual and the other colormap-getting machinery should move into gfxNativeRenderer as discussed. Otherwise looks great. (X suuuuucks!)
Comment 20 User image Karl Tomlinson (:karlt) 2008-07-06 19:10:27 PDT
Created attachment 328287 [details] [diff] [review]
move ws_info set up from nsObjectFrame::CallSetWindow() v4.1

(In reply to comment #19)
> FindColormapForVisual and the other colormap-getting machinery should move into
> gfxNativeRenderer as discussed.

Done.
Comment 21 User image Karl Tomlinson (:karlt) 2008-07-06 22:52:04 PDT
Requesting blocking 1.9.0.2 because both swfdec and Adobe now have
windowless-capable Flash plugins available (though Adobe's at least is still in
beta), and moonlight also has a windowless plugin.

The number of affected sites is hard to estimate as there is a race condition between reflow and plugin instantiation, but definitely high enough that we can expect plenty of users to hit this when they upgrade their plugin versions.
Comment 22 User image Geoff Norton 2008-07-07 11:44:29 PDT
Confirmed the latest patch fixes this issue with the moonlight plugin and our testcases.

@karlt, The patch doesn't appear to apply cleanly with any version of patch tho, dunno whats going on with that.
Comment 23 User image Karl Tomlinson (:karlt) 2008-07-07 14:15:21 PDT
Created attachment 328388 [details] [diff] [review]
move ws_info set up from nsObjectFrame::CallSetWindow() v4.1.1

Correct patch line counts.  (Same code.)

(In reply to comment #22)
> The patch doesn't appear to apply cleanly with any version of patch
> tho, dunno whats going on with that.

Thanks for testing, Geoff.  Sorry about the patch.  Emacs diff-mode sometimes (but I don't know exactly when) doesn't update line counts and I forgot to check that.
Comment 24 User image Karl Tomlinson (:karlt) 2008-07-07 20:35:22 PDT
Created attachment 328437 [details] [diff] [review]
include gdk/gdkscreen.h in system-headers

for:

/tools/gcc/bin/g++  -fno-rtti -fno-exceptions -Wall -Wconversion -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-long-long -pedantic -gstabs+ -fno-strict-aliasing -fshort-wchar -pthread -pipe  -DNDEBUG -DTRIMMED -Os -freorder-blocks -fno-reorder-functions -finline-limit=50 -fPIC -shared -Wl,-z,defs -Wl,-h,libxul.so -o libxul.so  nsStaticXULComponents.o nsUnicharUtils.o nsCompressedCharMap.o nsBidiUtils.o nsRDFResource.o     -lpthread   -Wl,-rpath-link,../../dist/bin  -Wl,--whole-archive ../../embedding/browser/gtk/src/libgtkembedmoz.a ../../toolkit/xre/libxulapp_s.a  ../../staticlib/components/libxpconnect.a ../../staticlib/components/libnecko.a ../../staticlib/components/libuconv.a ../../staticlib/components/libi18n.a ../../staticlib/components/libchardet.a ../../staticlib/components/libjar50.a ../../staticlib/components/libpref.a ../../staticlib/components/libcaps.a ../../staticlib/components/libhtmlpars.a ../../staticlib/components/libimglib2.a ../../staticlib/components/libgklayout.a ../../staticlib/components/libdocshell.a ../../staticlib/components/libembedcomponents.a ../../staticlib/components/libwebbrwsr.a ../../staticlib/components/libnsappshell.a ../../staticlib/components/libtxmgr.a ../../staticlib/components/libchrome.a ../../staticlib/components/libcommandlines.a ../../staticlib/components/libtoolkitcomps.a ../../staticlib/components/libpipboot.a ../../staticlib/components/libpipnss.a ../../staticlib/components/libgkplugin.a ../../staticlib/components/libmozfind.a ../../staticlib/components/libappcomps.a ../../staticlib/components/libunixproxy.a ../../staticlib/components/libxpinstall.a ../../staticlib/components/libjsd.a ../../staticlib/components/libautoconfig.a ../../staticlib/components/libauth.a ../../staticlib/components/libcookie.a ../../staticlib/components/libpermissions.a ../../staticlib/components/libuniversalchardet.a ../../staticlib/components/libcomposer.a ../../staticlib/components/librdf.a ../../staticlib/components/libwindowds.a ../../staticlib/components/libintlapp.a ../../staticlib/components/libfileview.a ../../staticlib/components/libstoragecomps.a ../../staticlib/components/libplaces.a ../../staticlib/components/libtkautocomplete.a ../../staticlib/components/libsatchel.a ../../staticlib/components/libpippki.a ../../staticlib/components/libucvmath.a ../../staticlib/components/libwidget_gtk2.a ../../staticlib/components/libsystem-pref.a ../../staticlib/components/libgkgfxthebes.a ../../staticlib/components/libaccessibility.a ../../staticlib/components/libremoteservice.a ../../staticlib/components/libspellchecker.a ../../staticlib/components/libzipwriter.a ../../staticlib/libxpcom_core.a ../../staticlib/libucvutil_s.a ../../staticlib/libgkgfx.a ../../staticlib/libgfxshared_s.a ../../staticlib/libmozreg_s.a ../../staticlib/libmorkreader_s.a ../../staticlib/libgtkxtbin.a ../../staticlib/libgfxpsshar.a ../../staticlib/libthebes.a  -Wl,--no-whole-archive -L../../dist/lib -lsqlite3 -L../../dist/bin -L../../dist/lib  -L../../dist/bin -L../../dist/lib -L../../jpeg -lmozjpeg -L../../modules/libimg/png -lmozpng -L../../dist/bin -lmozlcms -L../../dist/bin -lmozjs -L../../dist/bin -L../../dist/lib -lcrmf -lsmime3 -lssl3 -lnss3 -lnssutil3 -lsoftokn3  -L../../modules/zlib/src -lmozz -L/lib64 -lpangocairo-1.0 -lcairo -lpangoft2-1.0 -lpango-1.0 -lgobject-2.0 -lgmodule-2.0 -ldl -lglib-2.0   ../../gfx/cairo/cairo/src/libmozcairo.a ../../gfx/cairo/libpixman/src/libmozlibpixman.a   -L/usr/lib64 -lXrender -lfreetype -lfontconfig -L../../dist/lib -lplds4 -lplc4 -lnspr4 -lpthread -ldl -L/usr/lib64 -lX11  -lXft -lXrender -lfontconfig -lfreetype -lX11   -L/lib64 -lgtk-x11-2.0 -latk-1.0 -lgdk-x11-2.0 -lgdk_pixbuf-2.0 -lm -lpangocairo-1.0 -lpango-1.0 -lcairo -lgmodule-2.0 -ldl -lgobject-2.0 -lglib-2.0   -lXt -lgthread-2.0 -lfreetype -ldl -lm     
/usr/bin/ld: ../../staticlib/libthebes.a(gfxXlibNativeRenderer.o): relocation R_X86_64_PC32 against `gdk_display_get_screen' can not be used when making a shared object; recompile with -fPIC
Comment 26 User image Mike Melanson 2008-07-08 10:12:19 PDT
Another consistent crasher: http://skoda-auto.cz/
Comment 27 User image Karl Tomlinson (:karlt) 2008-07-08 16:05:26 PDT
Please use the build here:

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2008-07-08-02-mozilla-central/

It includes the fix for this bug
(and resolves the crash on http://skoda-auto.cz/).
Comment 28 User image Mike Melanson 2008-07-08 16:33:28 PDT
Awesome. Works great with all of the problem URLs. Thanks.
Comment 29 User image Stephen Moehle 2008-07-13 14:00:24 PDT
*** Bug 445043 has been marked as a duplicate of this bug. ***
Comment 30 User image Karl Tomlinson (:karlt) 2008-07-15 14:33:56 PDT
*** Bug 445202 has been marked as a duplicate of this bug. ***
Comment 31 User image Samuel Sidler (old account; do not CC) 2008-07-20 15:01:16 PDT
Can we get a test for this before landing in 1.9.0?
Comment 32 User image Karl Tomlinson (:karlt) 2008-07-20 15:59:16 PDT
(In reply to comment #31)
Automated testing depends on having a windowless plugin in our source tree
(bug 386144).

I've manually tested this patch with Swfdec on the problem sites reported
here, Geoff Norton has tested the Moonlight plugin (comment #22) on their
testcases (including attachment attachment 326988 [details]), and Mike Melanson has
tested the Adobe plugin (comment #28).
Comment 33 User image Samuel Sidler (old account; do not CC) 2008-07-20 18:41:02 PDT
Comment on attachment 328388 [details] [diff] [review]
move ws_info set up from nsObjectFrame::CallSetWindow() v4.1.1

Approved for 1.9.0.2. Please land in CVS. a=ss
Comment 34 User image Samuel Sidler (old account; do not CC) 2008-07-20 18:41:07 PDT
Comment on attachment 328437 [details] [diff] [review]
include gdk/gdkscreen.h in system-headers

Approved for 1.9.0.2. Please land in CVS. a=ss
Comment 35 User image Samuel Sidler (old account; do not CC) 2008-07-21 17:54:05 PDT
No blocking, but wanted (and the patches are already approved).

Note You need to log in before you can comment on or make changes to this bug.