22080 2008-11-04 23:49:03 -0800 CRASH at Scrollbar::invalidateRect due to null m_client 2008-11-05 10:41:41 -0800 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) All Windows XP RESOLVED FIXED http://www.megajogos.com.br/ P1 Critical --- 1 fishd fishd dave hyatt mihnea oldest_to_newest 97657 0 fishd 2008-11-04 23:49:03 -0800 CRASH at Scrollbar::invalidateRect due to null m_client I just updated Chrome to use the latest Scrollbar code, and our distributed reliability test is hitting a crash where m_client is null. The stack trace looks like so: [scrollbar.cpp:443] WebCore::Scrollbar::invalidateRect(WebCore::IntRect const &) [scrollbarthemecomposite.cpp:233] WebCore::ScrollbarThemeComposite::invalidatePart(WebCore::Scrollbar *,WebCore::ScrollbarPart) [scrollbar.cpp:292] WebCore::Scrollbar::setHoveredPart(WebCore::ScrollbarPart) [scrollbar.cpp:342] WebCore::Scrollbar::mouseExited() [eventhandler.cpp:1199] WebCore::EventHandler::handleMouseMoveEvent(WebCore::PlatformMouseEvent const &,WebCore::HitTestResult *) [eventhandler.cpp:1134] WebCore::EventHandler::mouseMoved(WebCore::PlatformMouseEvent const &) I'm guessing that there must be a code path that leads to setClient(0) being called on the same Scrollbar that EventHandler's m_lastScrollbarUnderMouse points to. I suspect that the right fix involves nulling m_lastScrollbarUnderMouse at the right time. 97658 1 fishd 2008-11-04 23:51:44 -0800 more readable stack... [scrollbar.cpp:443] Scrollbar::invalidateRect(IntRect const&) [scrollbarthemecomposite.cpp:233] ScrollbarThemeComposite::invalidatePart(Scrollbar*, ScrollbarPart) [scrollbar.cpp:292] Scrollbar::setHoveredPart(ScrollbarPart) [scrollbar.cpp:342] Scrollbar::mouseExited() [eventhandler.cpp:1199] EventHandler::handleMouseMoveEvent(PlatformMouseEvent const&, HitTestResult*) [eventhandler.cpp:1134] EventHandler::mouseMoved(PlatformMouseEvent const&) 97659 2 fishd 2008-11-05 00:22:01 -0800 Our crash data shows that this bug happens frequently on http://www.megajogos.com.br/ I could not manually repro, but one thing I noticed about that site is that it repeatedly creates and destroys a DIV with overflow. The contents of the widget titled "Megajogos Informa" appears to change over time. Sometimes the DIV requires scrollbars, and sometimes it does not. Given the way EventHandler and Scrollbar work (namely that Scrollbar is a reference counted object), I'm beginning to suspect that the right answer is for Scrollbar to null-check all accesses to its m_client. 97660 3 24906 fishd 2008-11-05 00:36:22 -0800 Created attachment 24906 v1 patch Simple null checking. 97674 4 ap 2008-11-05 04:47:38 -0800 See also: bug 22074. 97691 5 24906 darin 2008-11-05 09:19:09 -0800 Comment on attachment 24906 v1 patch + return m_client ? m_client->isActive() : false; I usually write those using && rather than ?:, but this seems fine. r=me 97701 6 fishd 2008-11-05 10:23:41 -0800 I'm happy to change to &&. That sounds better to me too. 97703 7 fishd 2008-11-05 10:25:52 -0800 *** Bug 22074 has been marked as a duplicate of this bug. *** 97709 8 fishd 2008-11-05 10:41:41 -0800 http://trac.webkit.org/changeset/38130 24906 2008-11-05 00:36:22 -0800 2008-11-05 09:19:09 -0800 v1 patch null_check_1.diff text/plain 1291 fishd SW5kZXg6IENoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBDaGFuZ2VMb2cJKHJldmlzaW9uIDM4MTE5 KQorKysgQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTQgQEAKKzIwMDgtMTEt MDUgIERhcmluIEZpc2hlciAgPGRhcmluQGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBDUkFTSCBhdCBTY3JvbGxiYXI6OmludmFs aWRhdGVSZWN0IGR1ZSB0byBudWxsIG1fY2xpZW50CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMjA4MAorCisgICAgICAgICogcGxhdGZvcm0vU2Nyb2xs YmFyLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OlNjcm9sbGJhcjo6aXNXaW5kb3dBY3RpdmUpOgor ICAgICAgICAoV2ViQ29yZTo6U2Nyb2xsYmFyOjppbnZhbGlkYXRlUmVjdCk6CisKIDIwMDgtMTEt MDQgIENhbWVyb24gWndhcmljaCAgPHp3YXJpY2hAYXBwbGUuY29tPgogCiAgICAgICAgIFJ1YmJl ci1zdGFtcGVkIGJ5IFNhbSBXZWluaWcuCkluZGV4OiBwbGF0Zm9ybS9TY3JvbGxiYXIuY3BwCj09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT0KLS0tIHBsYXRmb3JtL1Njcm9sbGJhci5jcHAJKHJldmlzaW9uIDM4MTE5KQorKysg cGxhdGZvcm0vU2Nyb2xsYmFyLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNDMzLDE0ICs0MzMsMTUg QEAgdm9pZCBTY3JvbGxiYXI6OnNldEVuYWJsZWQoYm9vbCBlKQogCiBib29sIFNjcm9sbGJhcjo6 aXNXaW5kb3dBY3RpdmUoKSBjb25zdAogewotICAgIHJldHVybiBtX2NsaWVudC0+aXNBY3RpdmUo KTsKKyAgICByZXR1cm4gbV9jbGllbnQgPyBtX2NsaWVudC0+aXNBY3RpdmUoKSA6IGZhbHNlOwog fQogIAogdm9pZCBTY3JvbGxiYXI6OmludmFsaWRhdGVSZWN0KGNvbnN0IEludFJlY3QmIHJlY3Qp CiB7CiAgICAgaWYgKHN1cHByZXNzSW52YWxpZGF0aW9uKCkpCiAgICAgICAgIHJldHVybjsKLSAg ICBtX2NsaWVudC0+aW52YWxpZGF0ZVNjcm9sbGJhclJlY3QodGhpcywgcmVjdCk7CisgICAgaWYg KG1fY2xpZW50KQorICAgICAgICBtX2NsaWVudC0+aW52YWxpZGF0ZVNjcm9sbGJhclJlY3QodGhp cywgcmVjdCk7CiB9CiAKIFBsYXRmb3JtTW91c2VFdmVudCBTY3JvbGxiYXI6OnRyYW5zZm9ybUV2 ZW50KGNvbnN0IFBsYXRmb3JtTW91c2VFdmVudCYgZXZlbnQpCg==