67669 2011-09-06 13:46:31 -0700 Null ptr crash in RenderScrollbar::updateScrollbarParts 2011-09-09 11:15:37 -0700 1 1 1 Unclassified WebKit Layout and Rendering 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Major --- 1 inferno inferno jamesr simon.fraser webkit.review.bot oldest_to_newest 462826 0 inferno 2011-09-06 13:46:31 -0700 http://code.google.com/p/chromium/issues/detail?id=95552 Thread 0 *CRASHED* ( EXCEPTION_ACCESS_VIOLATION_READ @ 0x00000018 ) 0x584150d3 [chrome.dll - renderobject.h:960] WebCore::RenderObject::setChildNeedsLayout(bool,bool) 0x5843873f [chrome.dll - renderscrollbar.cpp:198] WebCore::RenderScrollbar::updateScrollbarParts(bool) 0x58438470 [chrome.dll - renderscrollbar.cpp:93] WebCore::RenderScrollbar::setEnabled(bool) 0x58579e8a [chrome.dll - scrollview.cpp:554] WebCore::ScrollView::updateScrollbars(WebCore::IntSize const &) 0x58579633 [chrome.dll - scrollview.cpp:296] WebCore::ScrollView::setContentsSize(WebCore::IntSize const &) 0x584a4ea9 [chrome.dll - frameview.cpp:489] WebCore::FrameView::setContentsSize(WebCore::IntSize const &) 0x584a4f77 [chrome.dll - frameview.cpp:515] WebCore::FrameView::adjustViewSize() 0x584a5899 [chrome.dll - frameview.cpp:1029] WebCore::FrameView::layout(bool) 0x5861f718 [chrome.dll - document.cpp:1614] WebCore::Document::updateLayout() 0x5861f78c [chrome.dll - document.cpp:1645] WebCore::Document::updateLayoutIgnorePendingStylesheets() 0x58627013 [chrome.dll - element.cpp:402] WebCore::Element::offsetParent() 0x586c00a3 [chrome.dll - v8element.cpp:100] WebCore::ElementInternal::offsetParentAttrGetter Basically, owning renderer can be cleared in ClearOwningRenderer. we need a null check here. unfortunately top crasher, but no repro. 462836 1 106481 inferno 2011-09-06 13:55:50 -0700 Created attachment 106481 Patch 463117 2 jamesr 2011-09-06 20:54:28 -0700 From that stack, it looks like this scrollbar is associated directly with a ScrollView instead of a scrollable RenderObject, so I'm kind of surprised that this null check fixes it since clearOwningRenderer() doesn't touch the m_owningFrame ptr, it only touches the m_owner pointer. Can you take a look at the crashing URLs and see if they involve custom styled scrollbars? 463150 3 inferno 2011-09-06 22:27:28 -0700 from scrollview.h // Overridden by FrameView to create custom CSS scrollbars if applicable. virtual PassRefPtr<Scrollbar> createScrollbar(ScrollbarOrientation); (In reply to comment #2) > From that stack, it looks like this scrollbar is associated directly with a ScrollView instead of a scrollable RenderObject, so I'm kind of surprised that this null check fixes it since clearOwningRenderer() doesn't touch the m_owningFrame ptr, it only touches the m_owner pointer. > > Can you take a look at the crashing URLs and see if they involve custom styled scrollbars? The crashing url is not important, but the chrome extension is :) - https://chrome.google.com/webstore/detail/ojmmnceaidnmminjjffpndcbdibelgam [http://crash/reportdetail?reportid=0278250f44f77565]. Yeah, it creates nice custom scrollbars. 463441 4 106481 jamesr 2011-09-07 10:46:12 -0700 Comment on attachment 106481 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=106481&action=review So I think this is at least not harmful, so R=me. I really don't understand how this could help with the crash that you cite, however. > Source/WebCore/ChangeLog:7 > + > + Reviewed by NOBODY (OOPS!). > + describe the change and why you haven't added tests > Source/WebCore/rendering/RenderScrollbar.cpp:199 > + if (owningRenderer()) > + owningRenderer()->setChildNeedsLayout(true); this should be written more like if (RenderBox* box = owningRenderer()) box->setChildNeedsLayout(true) this is more common in webkit and more efficient in the case where owningRenderer() is expensive 463464 5 106603 inferno 2011-09-07 11:06:55 -0700 Created attachment 106603 Patch 463470 6 106603 jamesr 2011-09-07 11:11:20 -0700 Comment on attachment 106603 Patch OK. keep an eye on the crash reports, please 463471 7 inferno 2011-09-07 11:11:53 -0700 (In reply to comment #6) > (From update of attachment 106603 [details]) > OK. keep an eye on the crash reports, please Sure I will. Thanks for the review. 463472 8 inferno 2011-09-07 11:14:14 -0700 Committed r94690: <http://trac.webkit.org/changeset/94690> 464923 9 inferno 2011-09-09 11:15:37 -0700 This top crasher is now gone in 14.0.835.159. 106481 2011-09-06 13:55:50 -0700 2011-09-07 11:06:51 -0700 Patch bug-67669-20110906135549.patch text/plain 1271 inferno SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDk0NTk1KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTMgQEAKKzIwMTEtMDktMDYgIEFiaGlzaGVr IEFyeWEgIDxpbmZlcm5vQGNocm9taXVtLm9yZz4KKworICAgICAgICBOdWxsIG93bmluZ1JlbmRl cmVyIGNyYXNoIGluIFJlbmRlclNjcm9sbGJhcjo6dXBkYXRlU2Nyb2xsYmFyUGFydHMuCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02NzY2OQorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogcmVuZGVyaW5nL1Jl bmRlclNjcm9sbGJhci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpSZW5kZXJTY3JvbGxiYXI6OnVw ZGF0ZVNjcm9sbGJhclBhcnRzKToKKwogMjAxMS0wOS0wNiAgRGF2aWQgSHlhdHQgIDxoeWF0dEBh cHBsZS5jb20+CiAKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTY3NjU3CkluZGV4OiBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyU2Nyb2xsYmFy LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVuZGVyU2Nyb2xs YmFyLmNwcAkocmV2aXNpb24gOTQ1ODEpCisrKyBTb3VyY2UvV2ViQ29yZS9yZW5kZXJpbmcvUmVu ZGVyU2Nyb2xsYmFyLmNwcAkod29ya2luZyBjb3B5KQpAQCAtMTk1LDcgKzE5NSw4IEBAIHZvaWQg UmVuZGVyU2Nyb2xsYmFyOjp1cGRhdGVTY3JvbGxiYXJQYXIKICAgICAKICAgICBpZiAobmV3VGhp Y2tuZXNzICE9IG9sZFRoaWNrbmVzcykgewogICAgICAgICBzZXRGcmFtZVJlY3QoSW50UmVjdCh4 KCksIHkoKSwgaXNIb3Jpem9udGFsID8gd2lkdGgoKSA6IG5ld1RoaWNrbmVzcywgaXNIb3Jpem9u dGFsID8gbmV3VGhpY2tuZXNzIDogaGVpZ2h0KCkpKTsKLSAgICAgICAgb3duaW5nUmVuZGVyZXIo KS0+c2V0Q2hpbGROZWVkc0xheW91dCh0cnVlKTsKKyAgICAgICAgaWYgKG93bmluZ1JlbmRlcmVy KCkpCisgICAgICAgICAgICBvd25pbmdSZW5kZXJlcigpLT5zZXRDaGlsZE5lZWRzTGF5b3V0KHRy dWUpOwogICAgIH0KIH0KIAo= 106603 2011-09-07 11:06:55 -0700 2011-09-07 11:11:20 -0700 Patch bug-67669-20110907110654.patch text/plain 1589 inferno SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDk0NjMwKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTkgQEAKKzIwMTEtMDktMDYgIEFiaGlzaGVr IEFyeWEgIDxpbmZlcm5vQGNocm9taXVtLm9yZz4KKworICAgICAgICBOdWxsIG93bmluZ1JlbmRl cmVyIGNyYXNoIGluIFJlbmRlclNjcm9sbGJhcjo6dXBkYXRlU2Nyb2xsYmFyUGFydHMuCisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02NzY2OQorCisgICAg ICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIE93bmluZyByZW5kZXJl ciBjYW4gYmUgY2xlYXJlZCBmb3IgY3VzdG9tIHNjcm9sbGJhcnMgaW4gY2xlYXJPd25pbmdSZW5k ZXJlcigpCisgICAgICAgIGNhbGwuIFdlIG5lZWQgYSBudWxsIGNoZWNrIGluIHVwZGF0ZVNjcm9s bGJhclBhcnRzLCBzbyB0aGF0IHdlIGRvIG5vdCBjcmFzaC4KKworICAgICAgICBObyB0ZXN0cyBz aW5jZSBpc3N1ZSBzZWVuIGluIGNyYXNoIHJlcG9ydHMgb25seSBhbmQgSSBkb25vdCBrbm93IGEg d2F5IHRvCisgICAgICAgIHJlcHJvZHVjZS4KKworICAgICAgICAqIHJlbmRlcmluZy9SZW5kZXJT Y3JvbGxiYXIuY3BwOgorICAgICAgICAoV2ViQ29yZTo6UmVuZGVyU2Nyb2xsYmFyOjp1cGRhdGVT Y3JvbGxiYXJQYXJ0cyk6CisKIDIwMTEtMDktMDYgIE1hcmsgSGFobmVuYmVyZyAgPG1oYWhuZW5i ZXJnQGFwcGxlLmNvbT4KIAogICAgICAgICBVbnppcCBpbml0aWFsaXphdGlvbiBsaXN0cyBhbmQg Y29uc3RydWN0b3JzIGluIEpTQ2VsbCBoaWVyYXJjaHkgKDUvNykKSW5kZXg6IFNvdXJjZS9XZWJD b3JlL3JlbmRlcmluZy9SZW5kZXJTY3JvbGxiYXIuY3BwCj09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9X ZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJTY3JvbGxiYXIuY3BwCShyZXZpc2lvbiA5NDYzMCkKKysr IFNvdXJjZS9XZWJDb3JlL3JlbmRlcmluZy9SZW5kZXJTY3JvbGxiYXIuY3BwCSh3b3JraW5nIGNv cHkpCkBAIC0xOTUsNyArMTk1LDggQEAgdm9pZCBSZW5kZXJTY3JvbGxiYXI6OnVwZGF0ZVNjcm9s bGJhclBhcgogICAgIAogICAgIGlmIChuZXdUaGlja25lc3MgIT0gb2xkVGhpY2tuZXNzKSB7CiAg ICAgICAgIHNldEZyYW1lUmVjdChJbnRSZWN0KHgoKSwgeSgpLCBpc0hvcml6b250YWwgPyB3aWR0 aCgpIDogbmV3VGhpY2tuZXNzLCBpc0hvcml6b250YWwgPyBuZXdUaGlja25lc3MgOiBoZWlnaHQo KSkpOwotICAgICAgICBvd25pbmdSZW5kZXJlcigpLT5zZXRDaGlsZE5lZWRzTGF5b3V0KHRydWUp OworICAgICAgICBpZiAoUmVuZGVyQm94KiBib3ggPSBvd25pbmdSZW5kZXJlcigpKQorICAgICAg ICAgICAgYm94LT5zZXRDaGlsZE5lZWRzTGF5b3V0KHRydWUpOwogICAgIH0KIH0KIAo=