199607 - crash with smooth scrolling when scrolling horizonally - Trunk M140A [@ nsScrollPortView::IncrementalScroll ]

Status: RESOLVED FIXED

(Core :: Web Painting, defect)

Description

[Adam D. Moss]

When the page <http://dear_raed.blogspot.com/> first loads and I hit the
right-arrow key to scroll horizonally, Mozilla crashes immediately.

This only happens when smooth-scrolling is enabled.  This doesn't happen on
most pages.  It is totally reproducable on the above page.

The backtrace (sorry, not compiled with debug info) is:

#0  0x40bdccfe in nsScrollPortView::IncrementalScroll() ()
   from /marian/cvs/mozilla/dist/bin/components/libgklayout.so
#1  0x40bdcc40 in nsScrollPortView::SmoothScrollAnimationCallback(nsITimer*,
void*) () from /marian/cvs/mozilla/dist/bin/components/libgklayout.so
#2  0x405e26cd in nsTimerImpl::Fire() ()
   from /marian/cvs/mozilla/dist/bin/libxpcom.so
#3  0x405e2776 in handleTimerEvent(TimerEventType*) ()
   from /marian/cvs/mozilla/dist/bin/libxpcom.so
#4  0x405dd282 in PL_HandleEvent ()
   from /marian/cvs/mozilla/dist/bin/libxpcom.so
#5  0x405dd188 in PL_ProcessPendingEvents ()
   from /marian/cvs/mozilla/dist/bin/libxpcom.so
#6  0x405de55e in nsEventQueueImpl::ProcessPendingEvents() ()
   from /marian/cvs/mozilla/dist/bin/libxpcom.so
#7  0x40d99d75 in event_processor_callback(void*, int, GdkInputCondition) ()
   from /marian/cvs/mozilla/dist/bin/components/libwidget_gtk.so
#8  0x40d9990c in our_gdk_io_invoke(_GIOChannel*, GIOCondition, void*) ()
   from /marian/cvs/mozilla/dist/bin/components/libwidget_gtk.so
#9  0x4025a129 in g_io_unix_dispatch (source_data=0x82d0e38, 
    current_time=0xbfffee90, user_data=0x82d0e10) at giounix.c:135
#10 0x4025b8e6 in g_main_dispatch (dispatch_time=0xbfffee90) at gmain.c:656
#11 0x4025bf13 in g_main_iterate (block=1, dispatch=1) at gmain.c:877
#12 0x4025c0cc in g_main_run (loop=0x82d0eb0) at gmain.c:935
#13 0x4017700b in gtk_main () at gtkmain.c:524
#14 0x40d9a13f in nsAppShell::Run() ()
   from /marian/cvs/mozilla/dist/bin/components/libwidget_gtk.so
#15 0x40d7ab12 in nsAppShellService::Run() ()
   from /marian/cvs/mozilla/dist/bin/components/libnsappshell.so
#16 0x08058aee in main1(int, char**, nsISupports*) ()
#17 0x0805925e in main ()
#18 0x404231f6 in __libc_start_main () from /lib/libc.so.6

This is linux/x86.  My Mozilla window dimensions are 1180x937.
Modern theme.

Comment 1

[Adam D. Moss]

That's with CVS HEAD, btw.

Comment 2

[Olivier Cahagne]

also crashing on Win2k build 2003032704 (trunk): TB18552260M.

Comment 3

[Adam D. Moss]

Bafflingly, IFF I scroll down (and optionally back up again) before I scroll
right then it doesn't crash (though it IS very slow, and continues to scroll for
ages after I release the right-arrow key).

Comment 4

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

I've located this bug. Patch coming.

Comment 5

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

Attachment: fixes

This patch should fix the crashes and hangs people have been seeing.

It also fixes the sometimes long delay between releasing an arrow key and
scrolling actually stopping.

It also switches to a "constant velocity" strategy instead of the "constant
acceleration" strategy we were using. This simplifies the code and may make the
scrolling look smoother.

Comment 6

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

Comment on attachment 118794 [details] [diff] [review]
fixes

Going to dbaron for r/sr... I don't think anyone really knows this code other
than me (maybe kin, but he's mostly out of commission for now)

Comment 7

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

The crashes were caused by the nsGfxScrollFrame::AttributeChanged reentrancy
logic not being right, which meant an IncrementalScroll could trigger the start
of a new smoothscroll operation, which was very bad. I've elaborated the
reentrancy logic and documented it.

The hangs were due to the velocity computation being broken in a way which meant
the final loop to tweak the velocity vector was counting down numbers like
1000000000 by 1 until they reached zero. Solution: use much simpler velocity
calculation.

The delay between releasing an arrow key and scrolling finishing was because we
were forcing one immediate IncrementalScroll display update every time a smooth
scroll was initiated (e.g., on every keypress generated by autorepeat). In some
situations keypresses could arrive faster than we could do one
IncrementalScroll, so those events would back up in the input queue. Solution:
don't do the immediate IncrementalScroll, so we'll process all the key events in
the queue before the timer event does the first IncrementalScroll.

Comment 8

[Adam D. Moss]

That seems to fix it fine!  I'm don't find the new scroll motion quite as
aesthetically pleasing but it's still better than not-smooth scrolling and all
the better than the old smooth-scrolling code for not crashing. :)

Comment 9

[Adam D. Moss]

That seems to fix it fine!  I don't find the new scroll motion quite as
aesthetically pleasing but it's still better than not-smooth scrolling and all
the better than the old smooth-scrolling code for not crashing. :)

Comment 10

[Waheed Islam]

expected checkin date?

seeing as this is a topcrash, it would help if this went in asap
- if the patch has been tested and approved, of course.

Comment 11

[Adam D. Moss]

I'd like to see this in RSN too -- but as you can see, it still needs r/sr.

Comment 12

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

*** Bug 200214 has been marked as a duplicate of this bug. ***

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

*** Bug 200453 has been marked as a duplicate of this bug. ***

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 118794 [details] [diff] [review]
fixes

r+sr=dbaron (mostly rubber-stamp)

Comment 15

[Waheed Islam]

*** Bug 199107 has been marked as a duplicate of this bug. ***

Comment 16

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

Fix checked in.

Comment 17

[Olivier Cahagne]

*** Bug 200911 has been marked as a duplicate of this bug. ***

Comment 18

[Sébastien Delahaye]

*** Bug 201058 has been marked as a duplicate of this bug. ***

Comment 19

[Jay Patel [:jay]]

Adding M140A to summary for future reference since this is a topcrasher with
Mozilla 1.4 alpha.  This crash was fixed on the MozillaTrunk AFTER we released
that milestone...so we should not see this crash with Mozilla 1.4 beta.

Comment 20

[Sébastien Delahaye]

*** Bug 201771 has been marked as a duplicate of this bug. ***