Original chromium bug: http://code.google.com/p/chromium/issues/detail?id=13693
This won't repro in Safari on OSX because it doesn't allow custom scrollbar for the main window, but it does repro in Safari on Windows (and in Google Chrome).
1. Visit http://trac.webkit.org/export/41842/trunk/LayoutTests/scrollbars/overflow-scrollbar-combinations.html
2. Shrink the window so that a veritical scroll bar appears.
3. Refresh (or visit any other site, etc.)
(Simple) fix and layout test coming shortly.
In case anyone wants to compare this to crashes that they've seen (or search for crashes using function names), here's a stack for where the crash occurs:
[animationcontroller.cpp:449] - WebCore::AnimationController::cancelAnimations(WebCore::RenderObject *)
[renderobject.cpp:1825] - WebCore::RenderObject::destroy()
[renderbox.cpp:95] - WebCore::RenderBox::destroy()
[renderscrollbar.cpp:219] - WebCore::RenderScrollbar::updateScrollbarPart(WebCore::ScrollbarPart,bool)
[renderscrollbar.cpp:132] - WebCore::RenderScrollbar::updateScrollbarParts(bool)
[renderscrollbar.cpp:54] - WebCore::RenderScrollbar::setParent(WebCore::ScrollView *)
[scrollview.cpp:73] - WebCore::ScrollView::removeChild(WebCore::Widget *)
[scrollview.cpp:96] - WebCore::ScrollView::setHasVerticalScrollbar(bool)
[frameview.cpp:159] - WebCore::FrameView::~FrameView()
[chrome.dll+0x0007f634] - WebCore::FrameView::`vector deleting destructor'(unsigned int)
[refptr.h:122] - WTF::RefPtr<WebCore::FrameView>::operator=(WebCore::FrameView *)
[frame.cpp:237] - WebCore::Frame::setView(WebCore::FrameView *)
[webframe_impl.cc:1444] - WebFrameImpl::CreateFrameView()
[frameloader.cpp:2943] - WebCore::FrameLoader::transitionToCommitted(WTF::PassRefPtr<WebCore::CachedPage>)
[frameloader.cpp:2809] - WebCore::FrameLoader::commitProvisionalLoad(WTF::PassRefPtr<WebCore::CachedPage>)
[documentloader.cpp:339] - WebCore::DocumentLoader::commitIfReady()
A few more notes:
In safari, it appears to only repro on a refresh (not navigation) and it also repro's for iframes with custom scrollbars.
Created attachment 31224 [details]
Here's a layout test to repro the crash.
Created attachment 31225 [details]
Here's a patch to make the crash repro on OSX.
Created attachment 31226 [details]
Failed attempt at fix.
"Failed attempt at fix" was my first attempt to fix the bug.
The problem is that even if I guard the call to animation() in RenderObject, it still calls
and the renderArena (from document) has been deleted and is 0 at this point due to Document::detach() being called, so it feels like something worse is going on and perhaps more should be cleaned up in Document::detach().
At the moment, I could do some fix for this but not being familiar with the code I'm not sure what is most appropriate.
Next step: Ask dhyatt :)
I just found out about a website from http://code.google.com/p/chromium/issues/detail?id=13630 that exhibits this crash:
On Safari on Windows, visit this website and refresh.
Created attachment 31305 [details]
I have a few concerns about my patch, but I put it together as a place to start.
Here are my concerns:
1. I don't know if there is another function that I should call in FrameView. (It doesn't look like there is one.)
2. I don't know if this function should do more. For example, ~FrameView does resetScrollbars right before the calls to setHas*Scrollbar(false);
Should it do even more of what ~FrameView does?
3. onDocumentDetaching doesn't seem like a great name. detach() seemed to be typical but I wasn't sure what that function mean in general so I didn't know if this fit the pattern or not.
You don't want to always destroy the scrollbars. You only want to nuke them if the document supplied them. If they were custom basically.
onDocumentDetaching could be renamed to detachCustomScrollbars.
Oh, also if the scrollbars are supplied by the ownerElement's document rather than your own, then you don't need to destroy them either.
Comment on attachment 31305 [details]
Minusing based on above comments.
Created attachment 31397 [details]
Regarding this comment "Oh, also if the scrollbars are supplied by the ownerElement's document rather than your own, then you don't need to destroy them either"
I should have asked in irc, but I wasn't quite sure how that translated to code. I did my best guess by adding the renderBox check. Let me know if this isn't correct (and hopefully what it should be).
Comment on attachment 31397 [details]
The RenderObject.cpp change seems unintentional. Everything else is perfect.
As discussed in irc, the RenderObject.cpp change is described in the changelog and David Hyatt was fine with leaving that part of the change in.
Committed as http://trac.webkit.org/changeset/44751.
Comment on attachment 31397 [details]
Clearing the r+ since this caused a layout test on windows to start crashing.
Reverted with http://trac.webkit.org/changeset/44755 due to crashing layout test on windows.
Created attachment 31566 [details]
I had to move the call to FrameView::detachCustomScrollbars in Document::detach to ensure that the renderBox was still set on the body element when this method was called.
I've tried this out on windows and it works fine. (I thought I tested the last patch on windows but I must have missed something when doing the revisions.)
Comment on attachment 31566 [details]
Committed as http://trac.webkit.org/changeset/44940