https://bugs.webkit.org/show_bug.cgi?id=73348 Fixed an extremely reproducible case of this bug, but it is still possible to encounter the assertion. (Sample backtrace below.) Basically, if we call into AppKit to update overlay scrollbar information during layout, then AppKit may call back into WebKit while a layout is still happening via our delegates, and do stuff that we would rather not happen during layout. Sam, Simon, and I discussed this extensively yesterday, and we think that a good way to fix this would be to add zero-delay timer for all of the AppKit calls that can be called during layout. Specifically, that would be the calls in: ScrollAnimatorMac::notifyPositionChanged() ScrollAnimatorMac::contentsResized() …and possibly the add/remove scrollbar functions. (Those functions can definitely be called during a layout, but I don't think the AppKit-related work they do is troublesome.)
See also bug 71049.
(In reply to comment #1) > See also bug 81607.
Created attachment 144815 [details] Fix + manual test that asserts I've moved notifyPositionChanged to a zero-delay timer as Beth has suggested. This assert cannot be fired when running layout tests, however the html I added as manual test should crash at load time on a debug lion build.
Comment on attachment 144815 [details] Fix + manual test that asserts View in context: https://bugs.webkit.org/attachment.cgi?id=144815&action=review > ManualTests/scrollbar-crash-on-hide-scrolled-area.html:5 > + <meta charset="utf-8"/> > + <title>QuickStart</title> Neither seem necessary > ManualTests/scrollbar-crash-on-hide-scrolled-area.html:33 > + <br /><br /><br />Manual repro: scroll down and click on <b>Next step</b><br /><br /><br /><br /><br /><br /> > + <br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /> > + <br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /> > + <br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /> > + <br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /> > + <br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /> > + <br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /><br /> Ick. Can this testase be simplified, maybe by just adding css height to an element?
There are some layout test failures regarding scrollbars. I'll try to figure out what the problem is and I'll come up with another patch.
Created attachment 145022 [details] Fix 2 + manual test that asserts
Comment on attachment 145022 [details] Fix 2 + manual test that asserts View in context: https://bugs.webkit.org/attachment.cgi?id=145022&action=review > ManualTests/scrollbar-crash-on-hide-scrolled-area.html:33 > + <a href="#" class=â"nextStepButton" onclick="document.getElementById('toHide').style.display='none'">âNext stepâ</a>â There are still some non-ascii characters here. The testcase could be cleaned further. > Source/WebCore/platform/mac/ScrollAnimatorMac.mm:1239 > +void ScrollAnimatorMac::startNotifyContentAreaScrolledTimer() > +{ > + m_notifyContentAreaScrolledTimer.startOneShot(0); > +} It would be slightly cleaner to move the isActive check in here, and rename this method to 'sendContentAreaScrolledSoon' or something. Then you don't need the notifyContentAreaScrolledTimerIsActive() method, which has a confusing name anyway.
Created attachment 146238 [details] Fix 3 + manual test
Comment on attachment 146238 [details] Fix 3 + manual test View in context: https://bugs.webkit.org/attachment.cgi?id=146238&action=review > Source/WebCore/platform/mac/ScrollAnimatorMac.mm:1238 > + if(!m_sendContentAreaScrolledTimer.isActive()) Missing space after the if
Created attachment 146497 [details] One more space
Comment on attachment 146497 [details] One more space Clearing flags on attachment: 146497 Committed r119834: <http://trac.webkit.org/changeset/119834>
All reviewed patches have been landed. Closing bug.