Bug 111902

Summary: ASSERT d->m_defersLoading != defers on detik.com and drive.google.com
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: Page LoadingAssignee: Nate Chapin <japhet>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, beidson, japhet, jochen, koivisto, psolanki, simon.fraser, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch none

Alexey Proskuryakov
Reported 2013-03-08 16:51:42 PST
I can fairly reliably reproduce this assertion failing on detik.com with just a few reloads. I didn't investigate in a lot of detail, but looks like the page has multiple iframes with the same URL. Their MainResourceLoaders all appear to share the same ResourceHandle. As every iframe begins to load, Safari briefly defers loads for the MainResourceLoader. If another load starts and gets deferred in the meanwhile, then we are deferring the same ResourceHandle again.
Attachments
patch (3.92 KB, patch)
2013-03-26 14:30 PDT, Nate Chapin 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2013-03-08 16:51:52 PST
<rdar://problem/13267540>
Alexey Proskuryakov
Comment 2 2013-03-15 15:21:57 PDT
I'm also constantly hitting this on drive.google.com. Nate, this sounds like a regression from caching main resources.
Nate Chapin
Comment 3 2013-03-15 15:26:14 PDT
(In reply to comment #2) > I'm also constantly hitting this on drive.google.com. > > Nate, this sounds like a regression from caching main resources. (In reply to comment #2) > I'm also constantly hitting this on drive.google.com. > > Nate, this sounds like a regression from caching main resources. Hmm....that's odd. I suppose we could always just check to see if we're already deferred in MainResourceLoader::setDefersLoading() and ignore if we're trying to set to the same value. I'm not sure if that's too hackish though.
Alexey Proskuryakov
Comment 4 2013-03-15 16:05:37 PDT
What make this difficult for me is that I don't understand all use cases for deferring. A safe browsing check doesn't seem to require deferring an ongoing load - it can happen either from willSendRequest, or before transitioning to committed state. Do we ever actually need to defer an ongoing load?
Alexey Proskuryakov
Comment 5 2013-03-16 22:15:00 PDT
Or perhaps we should just count deferrals. I don't know why they aren't counted.
Simon Fraser (smfr)
Comment 6 2013-03-17 13:32:39 PDT
fast/frames/sandboxed-iframe-navigation-allowed.html hits this sometimes: http://build.webkit.org/results/Apple%20Lion%20Debug%20WK1%20(Tests)/r146019%20(7503)/fast/frames/sandboxed-iframe-navigation-allowed-crash-log.txt ASSERTION FAILED: d->m_defersLoading != defers /Volumes/Data/slave/lion-debug/build/Source/WebCore/platform/network/ResourceHandle.cpp(202) : void WebCore::ResourceHandle::setDefersLoading(bool) 1 0x10bafe9c5 WebCore::ResourceHandle::setDefersLoading(bool) 2 0x10bb0ac21 WebCore::ResourceLoader::setDefersLoading(bool) 3 0x10b65a6c6 WebCore::MainResourceLoader::setDefersLoading(bool) 4 0x10a8fe80d WebCore::DocumentLoader::setDefersLoading(bool) 5 0x10abd96fd WebCore::FrameLoader::setDefersLoading(bool) 6 0x10b74b9ea WebCore::Page::setDefersLoading(bool) 7 0x109dfcdd0 -[WebBaseNetscapePluginView start] 8 0x109dfd726 -[WebBaseNetscapePluginView viewDidMoveToWindow] 9 0x7fff878923a5 -[NSView _setWindow:] 10 0x7fff878905bd -[NSView addSubview:] 11 0x109e86a79 -[WebHTMLView addSubview:] 12 0x10bbb36b4 WebCore::ScrollView::platformAddChild(WebCore::Widget*) 13 0x10bba9f23 WebCore::ScrollView::addChild(WTF::PassRefPtr<WebCore::Widget>) 14 0x10bae9bd6 _ZN7WebCoreL22moveWidgetToParentSoonEPNS_6WidgetEPNS_9FrameViewE 15 0x10bae9b58 WebCore::RenderWidget::setWidget(WTF::PassRefPtr<WebCore::Widget>) 16 0x10b9c1893 WebCore::RenderPart::setWidget(WTF::PassRefPtr<WebCore::Widget>) 17 0x10bd26127 WebCore::SubframeLoader::loadPlugin(WebCore::HTMLPlugInImageElement*, WebCore::KURL const&, WTF::String const&, WTF::Vector<WTF::String, 0ul> const&, WTF::Vector<WTF::String, 0ul> const&, bool) 18 0x10bd25d79 WebCore::SubframeLoader::requestPlugin(WebCore::HTMLPlugInImageElement*, WebCore::KURL const&, WTF::String const&, WTF::Vector<WTF::String, 0ul> const&, WTF::Vector<WTF::String, 0ul> const&, bool) 19 0x10bd26349 WebCore::SubframeLoader::requestObject(WebCore::HTMLPlugInImageElement*, WTF::String const&, WTF::AtomicString const&, WTF::String const&, WTF::Vector<WTF::String, 0ul> const&, WTF::Vector<WTF::String, 0ul> const&) 20 0x10adebc98 WebCore::HTMLObjectElement::updateWidget(WebCore::PluginCreationOption) 21 0x10ac0e90c WebCore::FrameView::updateWidget(WebCore::RenderObject*) 22 0x10ac0eb59 WebCore::FrameView::updateWidgets() 23 0x10ac08f4d WebCore::FrameView::performPostLayoutTasks() 24 0x10ac086f9 WebCore::FrameView::layout(bool) 25 0x10a89b2a2 WebCore::Document::implicitClose() 26 0x10abdcdab WebCore::FrameLoader::checkCallImplicitClose() 27 0x10abdca56 WebCore::FrameLoader::checkCompleted() 28 0x10abdb6b3 WebCore::FrameLoader::finishedParsing() 29 0x10a8a5f79 WebCore::Document::finishedParsing() 30 0x10ad47dec WebCore::HTMLConstructionSite::finishedParsing() 31 0x10ae3bceb WebCore::HTMLTreeBuilder::finished()
Simon Fraser (smfr)
Comment 8 2013-03-20 22:03:23 PDT
Easy to hit loading techradar.com/us
Nate Chapin
Comment 9 2013-03-21 08:36:42 PDT
(In reply to comment #4) > What make this difficult for me is that I don't understand all use cases for deferring. A safe browsing check doesn't seem to require deferring an ongoing load - it can happen either from willSendRequest, or before transitioning to committed state. > > Do we ever actually need to defer an ongoing load? Does any platform's ResourceHandle support deferring a load already in progress? I'm pretty confident that WebCore/loader assumes that deferral can't happen after the request is sent.
Alexey Proskuryakov
Comment 10 2013-03-21 08:55:28 PDT
ResourceHandleMac supports that. I don't know what we actually use, but generally speaking, the rest of WebCore shouldn't care - deferring callbacks is indistinguishable from just having a slow connection.
Nate Chapin
Comment 11 2013-03-21 09:03:55 PDT
(In reply to comment #10) > ResourceHandleMac supports that. I don't know what we actually use, but generally speaking, the rest of WebCore shouldn't care - deferring callbacks is indistinguishable from just having a slow connection. Fair point. I'm happy to work on this, I'm just not clear what the preferred solution is. Should we put in a stopgap to prevent re-deferring?
jochen
Comment 12 2013-03-21 09:10:54 PDT
(In reply to comment #11) > (In reply to comment #10) > > ResourceHandleMac supports that. I don't know what we actually use, but generally speaking, the rest of WebCore shouldn't care - deferring callbacks is indistinguishable from just having a slow connection. > > Fair point. > > I'm happy to work on this, I'm just not clear what the preferred solution is. Should we put in a stopgap to prevent re-deferring? Nate, that's the assert I was talking to you about on Monday (which I hit a lot when running layout tests in content shell)
Nate Chapin
Comment 13 2013-03-26 14:30:52 PDT
Created attachment 195164 [details] patch
Alexey Proskuryakov
Comment 14 2013-03-29 10:52:36 PDT
Comment on attachment 195164 [details] patch I guess we can try solving it this way. Unsure if anyone understands what this "deferring" is all about, but if they do, they are keeping the knowledge to themselves.
Nate Chapin
Comment 15 2013-03-29 10:59:01 PDT
Comment on attachment 195164 [details] patch Yeah... :/
WebKit Review Bot
Comment 16 2013-03-29 11:09:00 PDT
Comment on attachment 195164 [details] patch Clearing flags on attachment: 195164 Committed r147228: <http://trac.webkit.org/changeset/147228>
WebKit Review Bot
Comment 17 2013-03-29 11:09:05 PDT
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 18 2013-10-23 15:57:14 PDT
Updated TestExpectations in r157896.
Note You need to log in before you can comment on or make changes to this bug.