RESOLVED FIXED Bug 54262
Some Scrollbar functions assume an attached ScrollableArea but can be called without one
https://bugs.webkit.org/show_bug.cgi?id=54262
Summary Some Scrollbar functions assume an attached ScrollableArea but can be called ...
Peter Kasting
Reported 2011-02-10 17:51:54 PST
EventHandler caches |m_lastScrollbarUnderMouse| while the user is interacting with a scrollbar. This can cause problems if, during this interaction, the page goes through relayout and destroys its scrollbar. Because |m_lastScrollbarUnderMouse| is a RefPtr, we don't need to worry about it being deleted. However, in RenderLayer::destroyScrollbar(), scrollbar->disconnectFromScrollableArea() is called before the RenderLayer drops its ref. As a result, the scrollbar no longer has an attached ScrollableArea. The next time EventHandler calls e.g. Scrollbar::mouseMoved(), it blindly accesses scrollableArea() and we crash. I don't know the model here well enough to know whether the right fix is for the Scrollbar to add NULL-checks in several places, or whether instead at the time the scrollbar is dropped from the RenderLayer the EventHandler should be told to drop |m_lastScrollbarUnderMouse|, or perhaps something else. I don't have a minimal testcase that reproduces this bug, but I imagine a page that either continually flips between needing a scrollbar and not on a timer, or else one that turns of its scrollbar in response to onscroll, might be able to demonstrate this. Setting up such a page and then grabbing the scroll thumb and dragging around might work.
Attachments
Patch (2.47 KB, patch)
2011-02-13 10:42 PST, Sam Weinig
mitz: review+
Peter Kasting
Comment 1 2011-02-10 17:55:59 PST
(BTW, I may not have a _minimal_ testcase, but I do have one, locally, that's Chromium-specific, so I'm happy to test out any patches to fix.)
James Robinson
Comment 2 2011-02-10 18:32:00 PST
FYI this is a leading crasher on recent Chromium builds and does not seem restricted to any particular webpages (there are crash reports from youtube.com, facebook.com, google.*, etc).
Sam Weinig
Comment 3 2011-02-11 10:57:53 PST
I will add null checks to Scrollbar. That is the correct way to fix this. I should have a patch ready later today.
Sam Weinig
Comment 4 2011-02-13 10:42:43 PST
Sam Weinig
Comment 5 2011-02-13 10:47:03 PST
Fixed in r78431.
Note You need to log in before you can comment on or make changes to this bug.