ASSERT d->m_defersLoading != defers on detik.com and drive.google.com

• Component: Page Loading
• Status: RESOLVED FIXED

[Alexey Proskuryakov]

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.

[Alexey Proskuryakov]

<rdar://problem/13267540>

[Alexey Proskuryakov]

I'm also constantly hitting this on drive.google.com.

Nate, this sounds like a regression from caching main resources.

[Nate Chapin]

(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]

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]

Or perhaps we should just count deferrals. I don't know why they aren't counted.

[Simon Fraser (smfr)]

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)]

Also fast/frames/sandboxed-iframe-plugins.html
http://build.webkit.org/results/Apple%20Lion%20Debug%20WK1%20(Tests)/r146095%20(7528)/results.html
http://build.webkit.org/results/Apple%20Lion%20Debug%20WK1%20(Tests)/r146095%20(7528)/fast/frames/sandboxed-iframe-plugins-crash-log.txt

[Simon Fraser (smfr)]

Easy to hit loading techradar.com/us

[Nate Chapin]

(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]

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]

(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?

(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]

Created attachment 195164 [details]
patch

[Alexey Proskuryakov]

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 on attachment 195164 [details]
patch

Yeah... :/

[WebKit Review Bot]

Comment on attachment 195164 [details]
patch

Clearing flags on attachment: 195164

Committed r147228: <http://trac.webkit.org/changeset/147228>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

Updated TestExpectations in r157896.