243757 - Crash when browsing airfrance site [@ nsView::ConvertFromParentCoords ][@ nsViewManager::DispatchEvent ]

Status: RESOLVED FIXED

(Core :: Web Painting, defect)

Description

[Olivier Cahagne]

build ID: FF 20040515 & Mozilla built from CVS 20040516

Steps to reproduce:
1. Load http://www.airfrance.fr/
2. Browse site, no specific link yet, I've reproduced crash 6 times with same stack,
3. Mozilla crashes,

I'm trying to come with reproducible steps or minimized testcase but thought I'd
file the bug report as it shows up in the FF Trunk crash reports.
Some TB IDs: TB49039G, TB49036X.

Comment 1

[Olivier Cahagne]

Attachment: Stacktrace Mozilla debug build 20040516

Comment 2

[Olivier Cahagne]

more data from GDB, hope it helps, I'm not sure which variables are relevant:

(gdb) frame 0
#0  0x414319d1 in nsView::ConvertFromParentCoords(int*, int*) const
(this=Variable "this" is not available.
)
    at nsView.h:272
272       void ConvertFromParentCoords(nscoord* aX, nscoord* aY) const { *aX -=
mPosX; *aY -= mPosY; }
(gdb) frame 1
#1  0x4142ce49 in nsViewManager::DispatchEvent(nsGUIEvent*, nsEventStatus*) (
    this=0x84e9530, aEvent=Variable "aEvent" is not available.
) at nsViewManager.cpp:1914
1914                  parent->ConvertFromParentCoords(&offset.x, &offset.y);
(gdb) p offset.x
$11 = -1228336656
(gdb) p offset.y
$12 = -89295639
(gdb) frame 2
#2  0x414249fc in HandleEvent (aEvent=Variable "aEvent" is not available.
) at nsView.cpp:76
76          view->GetViewManager()->DispatchEvent(aEvent, &result);
(gdb) frame 3
#3  0x41fedd64 in nsCommonWidget::DispatchEvent(nsGUIEvent*, nsEventStatus&) (
    this=Variable "this" is not available.
) at nsCommonWidget.cpp:214
214             aStatus = (* mEventCallback)(aEvent);
(gdb) p aEvent
$13 = (nsGUIEvent *) 0xbfffed80

Comment 3

[Olivier Cahagne]

Steps to reproduce:
1. Load URL http://bv.airfrance.fr/cgi-bin/FR/frameset.jsp
2. Click on link "Consultez votre compte" under "Fréquence Plus" at the very
bottom of right frame,
3. Mozilla will crash 50% of the time, if not, go back one page, browse site and
click this link again,

Although I crash on Linux, I couldn't get same build to crash on Win2k.

Comment 4

[Boris Zbarsky [:bzbarsky]]

> 1914                  parent->ConvertFromParentCoords(&offset.x, &offset.y);

What's "parent"?

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

That site is really slow for me, and I can't seem to reproduce the crash either :-(

Comment 6

[Hermann Schwab]

(In reply to comment #3)
> Steps to reproduce:
> 1. Load URL http://bv.airfrance.fr/cgi-bin/FR/frameset.jsp
> 2. Click on link "Consultez votre compte" under "Fréquence Plus" at the very
> bottom of right frame,

I didn´t succeed crashing on Win98, but this site is http: and loads an icon via
https:

'Paiement et données sécurisés', URL of the icon: 
https://cgi.airfrance.fr/CGI/prod/bin/MODULES/FFJ/html/security2.gif

Comment 7

[Olivier Cahagne]

Boris, "parent" is not available:

(gdb)frame 1
...
(gdb)p parent
Variable "parent" is not available.

according to http://talkback-public.mozilla.org/reports/FFTrunk/smart-analysis.lin
12 unique users have experienced this since build 2004051318, I assume
regression started with this build.
Related checkins between may 12th 1am and may 13th 6pm: bug 242833, bug 107518.

Comment 8

[Olivier Cahagne]

Attachment: GDB local variable output

Comment 9

[Boris Zbarsky [:bzbarsky]]

Erm... either the stack is hopelessly trashed, or that gcc version has issues... :(

Comment 10

[Olivier Cahagne]

backing out bug 233441 (as per latest CVS update) didn't fix the crash.
backing out bug 107518 didn't cure crash either.

I'm not sure if bug 242833 is completely backed out with removal of bug 233441
(nsViewManager.cpp for e.g.), will have a look tomorrow.

To reproduce crash, when loading link on airfrance.fr, you have to move the
mouse over the other left frame to get the crash, I thought it related with bug
242833's commit message: "Make nsDOMEvent::GetClientX/Y handle cases where a
subdocument's widget is not an ancestor of the event's widget. Also forward all
mouse grabbing to the root view manager of a view manager hierarchy so that
subdocuments can grab the mouse when an event occurs in an outer document."

Comment 11

[Boris Zbarsky [:bzbarsky]]

> I'm not sure if bug 242833 is completely backed out with removal of bug 233441

It's not even partially backed out, as far as I can tell from a quick glimpse at
the checkin logs.

Comment 12

[Olivier Cahagne]

> > I'm not sure if bug 242833 is completely backed out with removal of bug 233441
> 
> It's not even partially backed out, as far as I can tell from a quick glimpse at
> the checkin logs.

ok, sorry, it's late and I ran into compilation errors in nsViewManager.cpp when
backing out bug 242833, then saw nsViewManager.cpp was at v.3.330 with bug
242833 and v.3.329 with bug 233441.
I thought backing out bug 233441 would supercede bug 242833. If not, can it
introduce some old references (which would explain why it doesn't compile) ?
Will try again tomorrow with a clear mind. :)

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The trunk compiles, so it's just a matter of getting your build back to the
state of the trunk :-).

Comment 14

[Olivier Cahagne]

> The trunk compiles, so it's just a matter of getting your build back to the
> state of the trunk :-).
back with a clear mind :)

Clean Trunk CVS from 2h ago, backed out bug 242833.
I don't crash anymore, been browsing airfrance.fr site for 30 min now (I used to
crash within 5 minutes max. before), so I assume regression comes from bug 242833.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

I hope this patch will fix the crash and other regressions related to the mouse
capture checkin. Basically, I forgot to update all accesses to the grabber ...
it's especially important to update the inline GetMouseEventGrabber because
it's called in the nsView destructor to see whether we need to release grabbing
because the view is going away. Currently a view in a subdocument might not
notice it's grabbing the mouse and the mouse grab in the parent document will
end up pointing to a deleted view.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 148803 [details] [diff] [review]
fix

Hey Boris, I know you don't normally review view stuff, but dbaron is on tour
and it would be nice to try to fix this regression.

Comment 17

[Andreas Kunz]

roc:
This patch makes my Mozilla crash on startup, right after the splash screen.
(This is on WinXP, compiled with VS.NET 7.1)

The patch did not apply cleanly on the current cvs trunk, the .h hunk failed.
However, it is only one line and I changed it manually. I might have done it
wrongly, but there are not many ways one can screw up things with a hunk that
simple.
I should also note that this was not a clobber build.

However, if it's _not_ my fault, the crash is as follows:
It happens in nsrect.h line 65:
  PRBool IsEmpty() const {
    return (PRBool) ((height <= 0) || (width <= 0));
where height and width seem not to be defined.

It's called from nsviewmanager.cpp line 2572 (in nsViewManager::ResizeView; it
is doing:
  if (oldDimensions != aRect) {
and aRect is not initialized or at least its height/width values are not.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

That's rather mystifying. The patch should have applied cleanly since I just
diffed it from the trunk. And I have no idea what could cause that crash. Are
you sure your tree doesn't have non-trunk changes?

Comment 19

[Andreas Kunz]

roc: 
I _do_ have other changes, but those can hardly be responsible. However, I just
completed a clean rebuild and it's running fine now. So I probably just
should've done a clobber build in the first place (but don't say I didn't
mention it).
Sorry for the confusion.

Bug 243720 does not seem fixed by this, though.

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

OK, but it did fix this bug, right?

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 148803 [details] [diff] [review]
fix

David, if you're around, you could do this instead. Whichever one of you gets
to it first :-)

Comment 22

[Andreas Kunz]

roc:
> OK, but it did fix this bug, right?

Well, it's filed for Linux and comment #6 already could not reproduce on
Windows, but on the other hand talkback does list many Win crashes.

So trying the steps in comment #3 about eight times I still did not manage to crash.
I have another cvs build from 05-17 at about 4:00 PDT and this one does also not
crash although it is supposed to... so without 100% reliable steps to reproduce
I cannot confirm it...

Comment 23

[Yusuf Goolamabbas]

patch doesn't apply cleanly with a fresh cvs checkout of firefox at Thu May 20
00:26:28 UTC 2004. I'll manually fix up the reject and do a rebuild
I think I see the same issue whilst browsing http://www.scmp.com/
I am using Redhat 9. 

Comment 24

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 148803 [details] [diff] [review]
fix

Makes sense.  r+sr=bzbarsky.

Comment 25

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in. Could some kind soul verify that this bug and its dependencies are
fixced?

Comment 26

[R.K.Aa.]

built with the patch (but didn't clobber.)
I now crash at 
http://www7.nrk.no/nrkplayer/testnettleser.aspx?hovedkategori_id=2&prosjekt_id=0&kategori_id=0&klipp_id=0&indeks_id=0&oppgave_id=0&sok=&bhcp=1

To repro: load site, click middle scrollbar area, grab slider, drag, crash.
I didn't crash with a build from the 21st.

#0  0x037f6597 in nsViewManager::DispatchEvent ()
   from /home/dark/MOZ/TREE2/mozilla/dist/bin/components/libgklayout.so
#1  0x037efde0 in HandleEvent ()
   from /home/dark/MOZ/TREE2/mozilla/dist/bin/components/libgklayout.so
#2  0x00b0557e in nsCommonWidget::DispatchEvent ()
   from /home/dark/MOZ/TREE2/mozilla/dist/bin/components/libwidget_gtk2.so
#3  0x00afe123 in nsWindow::OnMotionNotifyEvent ()
   from /home/dark/MOZ/TREE2/mozilla/dist/bin/components/libwidget_gtk2.so
#4  0x00b02139 in motion_notify_event_cb ()
   from /home/dark/MOZ/TREE2/mozilla/dist/bin/components/libwidget_gtk2.so
#5  0x004ac854 in gtk_marshal_VOID__UINT_STRING () from /usr/lib/libgtk-x11-2.0.so.0
#6  0x0015b160 in g_closure_invoke () from /usr/lib/libgobject-2.0.so.0

Comment 27

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Works for me. Can you try it with a nightly (once the builds with this fix have
been pushed to the FTP site)

Comment 28

[R.K.Aa.]

nightlies aren't Gtk2

Comment 29

[Christian :Biesinger (don't email me, ping me on IRC)]

I can confirm rkaa's crash. this is a gtk2 build as well. I hadn't noticed the
previous crash; but now, mozilla is crashing all over the place (usually when
clicking "back")

Comment 30

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Okay I see the problem.

Comment 31

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: next fix

This ought to fix the crash here. Not sure about biesi's. Maybe you could test
it?

The problem is that this view maanger's mRootview is not necessarily the
ancestor of both baseView and view...

Comment 32

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 149111 [details] [diff] [review]
next fix

r+sr=bzbarsky

Comment 33

[Robert O'Callahan (:roc) (email my personal email if necessary)]

checked in.

Comment 34

[Robert O'Callahan (:roc) (email my personal email if necessary)]

marking fixed