The default bug view has changed. See this FAQ.

Crash [@ nsViewManager::AddCoveringWidgetsToOpaqueRegion]

VERIFIED FIXED

Status

()

Core
Layout
--
critical
VERIFIED FIXED
10 years ago
6 years ago

People

(Reporter: Martijn Wargers (dead), Assigned: mats)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
x86
All
crash, fixed1.8.0.12, fixed1.8.1.4, regression
Points:
---
Dependency tree / graph
Bug Flags:
wanted1.8.1.x +
wanted1.8.0.x +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?], crash signature)

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

10 years ago
Created attachment 262340 [details]
unminised testcase

See unminimised testcase, when I reach 801 in the statusbar, current trunk builds crash. It usually happens directly, or after a reload.

Talkback ID: TB31404174E
0x80000000
nsViewManager::AddCoveringWidgetsToOpaqueRegion  [mozilla/view/src/nsviewmanager.cpp, line 619]
nsViewManager::Refresh  [mozilla/view/src/nsviewmanager.cpp, line 538]

This regressed for me between 2007-03-04 and 2007-03-05:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-03-04+04&maxdate=2007-03-05+09&cvsroot=%2Fcvsroot
I guess a regression from bug 363253, somehow.

I'm getting this crash all the time when I use the thing from bug 339128.
I suspect this is a windows only crash, somehow.

I tried to crash in a windows debug build with gdb, but there it wouldn't crash.
However, I did see some cairo rules (I guess these might indicate something):
_cairo_win32_surface_composite(BitBlt): De bewerking is voltooid.
_cairo_win3_surface_acquire_dest_image: De ingang is ongeldig.
_cairo_win32_surface_set_clip_region (reset): De ingang is ongeldig.

The testcase is totally unminimised, so this bug should be closed. When I try to minimise it, it just tends to crash less and less likely.
Flags: blocking1.9?
(Assignee)

Comment 1

10 years ago
Also crashes on Linux if I reload a few times: TB31416605Z
OS: Windows XP → All
(Assignee)

Updated

10 years ago
Assignee: nobody → mats.palmgren
(Assignee)

Comment 2

10 years ago
Created attachment 262505 [details] [diff] [review]
Patch rev. 1
(Assignee)

Comment 3

10 years ago
Created attachment 262506 [details] [diff] [review]
Patch rev. 1 (diff -w)

There are a few separate bugs here.
First there is the WillPaint() call in nsViewManager which flushes
notifications which leads up to the view being destroyed, then
calling Refresh on that view.  This affects all platforms.

The gtk2 widget code have additional bugs.
We need to hold strong refs on 'this' whenever we call DispatchEvent()
and depend on being alive after that.  Also, preventing deletion isn't
enough, we also need to test 'mIsDestroyed' if we're using any native
windows since nsWindow::Destroy makes them them invalid.  This is
necessary to avoid X Window errors like
'BadDrawable (invalid Pixmap or Window parameter)' which makes _XError
call exit().

This patch fixes the above issues.  I will review non-gtk2 widget code
for similar errors of the second type, I'm pretty sure there are some.
Attachment #262506 - Flags: superreview?(roc)
Attachment #262506 - Flags: review?(roc)
+    nsRefPtr<nsWindow> kungFuDeathGrip = this;

Can we move these out to the gtk callbacks, just before we call into the nsWindow? We should avoid deleting the nsWindow while still running methods in it.

Otherwise looks fine.
(Assignee)

Comment 5

10 years ago
Created attachment 263066 [details] [diff] [review]
Patch rev. 2
Attachment #262505 - Attachment is obsolete: true
Attachment #262506 - Attachment is obsolete: true
Attachment #262506 - Flags: superreview?(roc)
Attachment #262506 - Flags: review?(roc)
(Assignee)

Comment 6

10 years ago
Created attachment 263068 [details] [diff] [review]
Patch rev. 2 (diff -w)

Good point.  I have changed nsWindow such that all its methods now assume
the caller has a strong ref on 'this'.  I added missing refs in callers
(gtk callbacks and a few IME related calls) and removed a few AddRef's
on 'this' inside nsWindow methods.  I have added strong refs on local
nsWindow pointers inside these methods. As an example:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/gtk2/nsWindow.cpp&rev=1.215&root=/cvsroot&mark=775,852#774
Although we can assume 'this' is strongly held, we can't assume the
same for 'gFocusWindow' so I added a strong refs on such locals, to keep
the caller's promise.
Attachment #263068 - Flags: superreview?(roc)
Attachment #263068 - Flags: review?(roc)
(Assignee)

Comment 7

10 years ago
I noted that we have some long standing bugs involving nsWindow::OnDrag*
for gtk/gtk2, bug 269568, bug 300675, bug 309298, I wonder if this patch
might help.  (feel free to CC me on any bugs that have clear STR for
trunk or 1.8 and I might have a look at it)
(Assignee)

Comment 8

10 years ago
Nevermind, I can repro bug 309298 and it looks like a different problem...
Attachment #263068 - Flags: superreview?(roc)
Attachment #263068 - Flags: superreview+
Attachment #263068 - Flags: review?(roc)
Attachment #263068 - Flags: review+
(Assignee)

Comment 9

10 years ago
Checked in to trunk at 2007-04-29 17:46 PDT.

-> FIXED
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Flags: blocking1.9? → in-testsuite?
Resolution: --- → FIXED
(Assignee)

Comment 10

10 years ago
I can't crash branch builds (Linux) on the testcases, but looking at
the view manager/pres shell code on branch it looks pretty much the
same as trunk, so there is some risk this could happen on branch too.
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/view/src/nsViewManager.cpp&rev=MOZILLA_1_8_BRANCH&root=/cvsroot&mark=1964,2035,2044#1961
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/base/nsPresShell.cpp&rev=MOZILLA_1_8_BRANCH&root=/cvsroot&mark=6554,6571#6553

I tried querying http://talkback-public.mozilla.org/ for nsViewManager
crashes in branch builds but half an hour later I still haven't got the
search result back :-(
(Reporter)

Comment 11

10 years ago
It seems to me this could be the #92 crasher (nsViewManager::Refresh) at:
http://talkback-public.mozilla.org/reports/firefox/FF2003/FF2003-topcrashers.html
right?
http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=stacksig&match=contains&searchfor=nsViewManager::Refresh&vendor=MozillaOrg&product=Firefox2&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid
(especially thanks for fixing this one, Mats)
(Reporter)

Comment 12

10 years ago
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/20070430 Minefield/3.0a5pre
Status: RESOLVED → VERIFIED
(Assignee)

Updated

10 years ago
Blocks: 376155
(Assignee)

Comment 13

10 years ago
Created attachment 263268 [details] [diff] [review]
nsViewManager 1.8 branch patch
(Assignee)

Updated

10 years ago
Attachment #263268 - Attachment description: nsViewManager branch patch → nsViewManager 1.8 branch patch
(Assignee)

Comment 14

10 years ago
The nsViewManager part of the patch is low risk, maybe we should consider
taking that for 1.8.1.4/1.8.0.12?

Andrew, since you were able to reproduce bug 376155 - could you try the
attached branch patch and see if that alone fixes it?

Comment 15

10 years ago
Yes, the branch patch fixes bug 376155 for me.

Comment 16

10 years ago
I know this is very late for 1.8.1.4, but to fix bug 376155, we, IMO, should take the branch patch. (If roc or someone first reviews it.)
Flags: blocking1.8.1.4?
Flags: blocking1.8.0.12?

Updated

10 years ago
Attachment #263268 - Flags: superreview?(roc)
Attachment #263268 - Flags: review?(roc)
Comment on attachment 263268 [details] [diff] [review]
nsViewManager 1.8 branch patch

sr=dveditz fwiw, but we need a module owner review.
Attachment #263268 - Flags: superreview?(roc)
Attachment #263268 - Flags: superreview+
Attachment #263268 - Flags: approval1.8.1.4?
Attachment #263268 - Flags: approval1.8.0.12?
Comment on attachment 263268 [details] [diff] [review]
nsViewManager 1.8 branch patch

This looks fine to me, and pretty safe.  Would probably still be good if roc takes a look if he has a chance, but r=dbaron.
Attachment #263268 - Flags: review+
Comment on attachment 263268 [details] [diff] [review]
nsViewManager 1.8 branch patch

approved for 1.8.0.12 and 1.8.1.4, a=dveditz for release-drivers
Attachment #263268 - Flags: approval1.8.1.4?
Attachment #263268 - Flags: approval1.8.1.4+
Attachment #263268 - Flags: approval1.8.0.12?
Attachment #263268 - Flags: approval1.8.0.12+
The 1.8 branch patch doesn't fix the entire bug, right? If that's the case when this is checked in put the "fixed" keywords on bug 376155 rather than this one.
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Flags: blocking1.8.1.4?
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.12?
Flags: blocking1.8.0.12+
(Assignee)

Comment 21

10 years ago
Comment on attachment 263268 [details] [diff] [review]
nsViewManager 1.8 branch patch

(In reply to comment #20)
> The 1.8 branch patch doesn't fix the entire bug, right?'

Correct.  It should fix the vast majority of crashes though
(also the "topcrash #92" that was mentioned).
The remaining part is GTK2-only and leads in most cases to a
call to exit() from XError.  (With the reservation that I haven't
investigated non-GTK2 widget code.)

> put the "fixed" keywords on bug 376155 rather than this one.

Will do.

Checked in to MOZILLA_1_8_BRANCH at 2007-04-30 18:45 PDT.
Checked in to MOZILLA_1_8_0_BRANCH at 2007-04-30 18:47 PDT.
The view manager patch landed, the rest we'll have to look at in 1.8.1.5
Flags: blocking1.8.1.5?
Flags: blocking1.8.1.4+
Flags: blocking1.8.0.13?
Flags: blocking1.8.0.12+

Updated

10 years ago
Attachment #263268 - Flags: review?(roc)
With attachment 263268 [details] [diff] [review] I no longer crash on the branches on windows, and calling exit() on GTK2 is safe enough to consider this fixed on the branches.
Keywords: fixed1.8.0.12, fixed1.8.1.4
Whiteboard: [sg:critical?]
Group: security
Flags: blocking1.8.1.5?
Flags: blocking1.8.0.13?
Crash Signature: [@ nsViewManager::AddCoveringWidgetsToOpaqueRegion]
You need to log in before you can comment on or make changes to this bug.