54262 2011-02-10 17:51:54 -0800 Some Scrollbar functions assume an attached ScrollableArea but can be called without one 2011-02-13 10:47:03 -0800 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) PC All RESOLVED FIXED P2 Critical --- 1 pkasting sam jamesr oldest_to_newest 349542 0 pkasting 2011-02-10 17:51:54 -0800 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. 349547 1 pkasting 2011-02-10 17:55:59 -0800 (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.) 349554 2 jamesr 2011-02-10 18:32:00 -0800 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). 349846 3 sam 2011-02-11 10:57:53 -0800 I will add null checks to Scrollbar. That is the correct way to fix this. I should have a patch ready later today. 350409 4 82268 sam 2011-02-13 10:42:43 -0800 Created attachment 82268 Patch 350412 5 sam 2011-02-13 10:47:03 -0800 Fixed in r78431. 82268 2011-02-13 10:42:43 -0800 2011-02-13 10:44:51 -0800 Patch barFinal.diff text/plain 2532 sam SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDc4NDMwKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTEtMDItMTMgIFNhbSBXZWlu aWcgIDxzYW1Ad2Via2l0Lm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBTb21lIFNjcm9sbGJhciBmdW5jdGlvbnMgYXNzdW1lIGFuIGF0dGFjaGVk IFNjcm9sbGFibGVBcmVhIGJ1dCBjYW4gYmUgY2FsbGVkIHdpdGhvdXQgb25lCisgICAgICAgIGh0 dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD01NDI2MgorCisgICAgICAgIE1h a2Ugc3VyZSB0aGF0IGFsbCBjYWxscyB0byB0aGUgc2Nyb2xsYWJsZSBhcmVhIGFyZSBudWxsIGNo ZWNrZWQuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9TY3JvbGxiYXIuY3BwOgorICAgICAgICAoV2Vi Q29yZTo6U2Nyb2xsYmFyOjphdXRvc2Nyb2xsUHJlc3NlZFBhcnQpOgorICAgICAgICAoV2ViQ29y ZTo6U2Nyb2xsYmFyOjptb3ZlVGh1bWIpOgorICAgICAgICAoV2ViQ29yZTo6U2Nyb2xsYmFyOjpt b3VzZU1vdmVkKToKKwogMjAxMS0wMi0xMyAgU2FtIFdlaW5pZyAgPHNhbUB3ZWJraXQub3JnPgog CiAgICAgICAgIFJvbGwgb3V0IHI3ODQyNC4gSXQgYnJva2UgYSBidW5jaCBvZiB0ZXN0cy4KSW5k ZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL1Njcm9sbGJhci5jcHAKPT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0g U291cmNlL1dlYkNvcmUvcGxhdGZvcm0vU2Nyb2xsYmFyLmNwcAkocmV2aXNpb24gNzg0MjkpCisr KyBTb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9TY3JvbGxiYXIuY3BwCSh3b3JraW5nIGNvcHkpCkBA IC0xOTYsNyArMTk2LDcgQEAgdm9pZCBTY3JvbGxiYXI6OmF1dG9zY3JvbGxQcmVzc2VkUGFydChk bwogICAgIH0KIAogICAgIC8vIEhhbmRsZSB0aGUgYXJyb3dzIGFuZCB0cmFjay4KLSAgICBpZiAo c2Nyb2xsYWJsZUFyZWEoKS0+c2Nyb2xsKHByZXNzZWRQYXJ0U2Nyb2xsRGlyZWN0aW9uKCksIHBy ZXNzZWRQYXJ0U2Nyb2xsR3JhbnVsYXJpdHkoKSkpCisgICAgaWYgKG1fc2Nyb2xsYWJsZUFyZWEg JiYgbV9zY3JvbGxhYmxlQXJlYS0+c2Nyb2xsKHByZXNzZWRQYXJ0U2Nyb2xsRGlyZWN0aW9uKCks IHByZXNzZWRQYXJ0U2Nyb2xsR3JhbnVsYXJpdHkoKSkpCiAgICAgICAgIHN0YXJ0VGltZXJJZk5l ZWRlZChkZWxheSk7CiB9CiAKQEAgLTI2OCw3ICsyNjgsOCBAQCB2b2lkIFNjcm9sbGJhcjo6bW92 ZVRodW1iKGludCBwb3MpCiAgICAgCiAgICAgaWYgKGRlbHRhKSB7CiAgICAgICAgIGZsb2F0IG5l d1Bvc2l0aW9uID0gc3RhdGljX2Nhc3Q8ZmxvYXQ+KHRodW1iUG9zICsgZGVsdGEpICogbWF4aW11 bSgpIC8gKHRyYWNrTGVuIC0gdGh1bWJMZW4pOwotICAgICAgICBzY3JvbGxhYmxlQXJlYSgpLT5z Y3JvbGxUb09mZnNldFdpdGhvdXRBbmltYXRpb24obV9vcmllbnRhdGlvbiwgbmV3UG9zaXRpb24p OworICAgICAgICBpZiAobV9zY3JvbGxhYmxlQXJlYSkKKyAgICAgICAgICAgIG1fc2Nyb2xsYWJs ZUFyZWEtPnNjcm9sbFRvT2Zmc2V0V2l0aG91dEFuaW1hdGlvbihtX29yaWVudGF0aW9uLCBuZXdQ b3NpdGlvbik7CiAgICAgfQogfQogCkBAIC0zMDAsOSArMzAxLDEwIEBAIHZvaWQgU2Nyb2xsYmFy OjpzZXRQcmVzc2VkUGFydChTY3JvbGxiYXIKIGJvb2wgU2Nyb2xsYmFyOjptb3VzZU1vdmVkKGNv bnN0IFBsYXRmb3JtTW91c2VFdmVudCYgZXZ0KQogewogICAgIGlmIChtX3ByZXNzZWRQYXJ0ID09 IFRodW1iUGFydCkgewotICAgICAgICBpZiAodGhlbWUoKS0+c2hvdWxkU25hcEJhY2tUb0RyYWdP cmlnaW4odGhpcywgZXZ0KSkKLSAgICAgICAgICAgIHNjcm9sbGFibGVBcmVhKCktPnNjcm9sbFRv T2Zmc2V0V2l0aG91dEFuaW1hdGlvbihtX29yaWVudGF0aW9uLCBtX2RyYWdPcmlnaW4pOwotICAg ICAgICBlbHNlIHsKKyAgICAgICAgaWYgKHRoZW1lKCktPnNob3VsZFNuYXBCYWNrVG9EcmFnT3Jp Z2luKHRoaXMsIGV2dCkpIHsKKyAgICAgICAgICAgIGlmIChtX3Njcm9sbGFibGVBcmVhKQorICAg ICAgICAgICAgICAgIG1fc2Nyb2xsYWJsZUFyZWEtPnNjcm9sbFRvT2Zmc2V0V2l0aG91dEFuaW1h dGlvbihtX29yaWVudGF0aW9uLCBtX2RyYWdPcmlnaW4pOworICAgICAgICB9IGVsc2UgewogICAg ICAgICAgICAgbW92ZVRodW1iKG1fb3JpZW50YXRpb24gPT0gSG9yaXpvbnRhbFNjcm9sbGJhciA/ IAogICAgICAgICAgICAgICAgICAgICAgIGNvbnZlcnRGcm9tQ29udGFpbmluZ1dpbmRvdyhldnQu cG9zKCkpLngoKSA6CiAgICAgICAgICAgICAgICAgICAgICAgY29udmVydEZyb21Db250YWluaW5n V2luZG93KGV2dC5wb3MoKSkueSgpKTsK