| Summary: | Resource load cancelled if detaching the first of multiple iframes all loading from the same URL | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Hyongyoub Kim <hyongyoub.kim> |
| Component: | Frames | Assignee: | Alex Christensen <achristensen> |
| Status: | RESOLVED DUPLICATE | ||
| Severity: | Normal | CC: | achristensen, ap, beidson, buildbot, cdumez, dbates, koivisto, rniwa |
| Priority: | P2 | ||
| Version: | WebKit Local Build | ||
| Hardware: | All | ||
| OS: | All | ||
| Attachments: | |||
This can easily be made into a loading test that shows something that loads in other browsers but not in WebKit. We should probably fix this. Could you please upload sub.html? Created attachment 278756 [details]
Patch
I uploaded a patch with just a test. It passes in Chrome, Firefox, and WebKit with the memory cache disabled, but not in WebKit with the memory cache. Comment on attachment 278756 [details] Patch Attachment 278756 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1311180 New failing tests: http/tests/cache/iframe-detach.html Created attachment 278761 [details]
Archive of layout-test-results from ews106 for mac-yosemite-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Comment on attachment 278756 [details] Patch Attachment 278756 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/1311176 New failing tests: http/tests/cache/iframe-detach.html Created attachment 278762 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.4
Comment on attachment 278756 [details] Patch Attachment 278756 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1311182 New failing tests: http/tests/cache/iframe-detach.html Created attachment 278765 [details]
Archive of layout-test-results from ews117 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 278756 [details] Patch Attachment 278756 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1311508 New failing tests: http/tests/cache/iframe-detach.html Created attachment 278771 [details]
Archive of layout-test-results from ews101 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Created attachment 278784 [details]
sub.html missing from test.zip
Attaching sub.html. Forgot to include this in the zip file.
I suppose main resource loader cancels the load without taking into account that the cached resource may have other clients. Yep, here's the call stack: 1 0x1134d3033 WebCore::MemoryCache::remove(WebCore::CachedResource&) 2 0x113de5999 WebCore::SubresourceLoader::willCancel(WebCore::ResourceError const&) 3 0x111cd5294 WebCore::ResourceLoader::cancel(WebCore::ResourceError const&, WebCore::CachedResourceClient*) 4 0x1122e6cd4 WebCore::DocumentLoader::cancelMainResourceLoad(WebCore::ResourceError const&) 5 0x1122e67f9 WebCore::DocumentLoader::stopLoading() 6 0x11264108a WebCore::FrameLoader::stopAllLoaders(WebCore::ClearProvisionalItemPolicy) 7 0x1126455db WebCore::FrameLoader::frameDetached() The SubresourceLoader's CachedResourceLoader has multiple clients. We need to add a check to say "If there are multiple clients, just disconnect this one instead of cancelling", but I'm not sure where. I've tried this simple hack in the past.
--- a/Source/WebCore/loader/DocumentLoader.cpp
+++ b/Source/WebCore/loader/DocumentLoader.cpp
@@ -1562,7 +1562,12 @@ void DocumentLoader::cancelMainResourceLoad(const ResourceError& resourceError)
cancelPolicyCheckIfNeeded();
- if (mainResourceLoader())
+ // Remove this loader from the client list first. And cancel loading only
+ // if there are no other clients. Another iframe may be loading from the
+ // same CachedResource and we should not disturb that.
+ if (m_mainResource && m_mainResource->hasClient(this))
+ m_mainResource->removeClient(this);
+ if (mainResourceLoader() && !m_mainResource->hasClients())
mainResourceLoader()->cancel(error);
clearMainResource();
One problem is that the frame loader is detached from the page but loading is still happening through it. So if you change slow-iframe to return 302 redirect after 1 second delay, instead of an html page, the redirect response ends up crashing the engine.
WTF.dll!WTFCrash
WebCore::FrameLoader::applyUserAgent
WebCore::ResourceLoadNotifier::willSendRequest
WebCore::ResourceLoader::willSendRequestInternal
WebCore::SubresourceLoader::willSendRequestInternal
ResourceLoader::willSendRequest
WebKit::WebResourceLoader::willSendRequest
[...]
void FrameLoader::applyUserAgent(ResourceRequest& request)
{
String userAgent = this->userAgent(request.url());
ASSERT(!userAgent.isNull()); // This assertion trips
request.setHTTPUserAgent(userAgent);
}
*** This bug has been marked as a duplicate of bug 162094 *** |
Created attachment 278609 [details] Test html pages and cgi perl scripts Multiple iframes are loading from the same URL. Before the main resource load completes, the first iframe that started the load gets detached from the page via javascript. The other iframes all see cancelled main resource. test.zip contains two simple test cases. Need cgi/perl enabled webserver. Case 1: html/main.html Case 2: html/main2.html Here is case 1 so you do not have to unzip the file. <html> <body> <div id="shadow"> <iframe src="/cgi-bin/slow-iframe"></iframe> </div> <div id="empty"></div> </body> <script> function set2() { document.getElementById("shadow").innerHTML = ""; } function set1() { document.getElementById("empty").innerHTML = document.getElementById("shadow").innerHTML; setTimeout(set2, 1000); } setTimeout(set1, 1000); </script> </html> The first iframe ('shadow') starts loading /cgi-bin/slow-iframe. 1 second later: set1 adds the second iframe, which also loads from /cgi-bin/slow-iframe. 2 seconds later: set2 detaches the first iframe from the page. 3 seconds later: slow-iframe finally returns the response. IE/Firefox/Chrome all show slow-iframe's response in the second iframe. But safari and other WebKit ports show a blank because the load gets cancelled when we detach the first iframe from the page. When detached from the page, FrameLoader calls stopAllLoaders, which ends up cancelling the main resource load. The second iframe waiting for the load to complete sees a cancelled load. Case 2 is similar but shows the problem with subresource not the main resource. In this case, the main resouce loads quickly so both iframes see the completed resource. But its subresource loads slowly. When the first iframe gets detached, the subresource load gets cancelled. For case 2, IE/Firefox show the subresource in the second iframe. Safari/Chrome/WebKit ports show blank. Chrome is okay for case 1 because it is not caching main resources in MemoryCache... This behavior feels like a bug because, without MemoryCache, we would not see cancelled loads. In case people feel like this sort of iframe usage is a far fetched scenario, many sites in China actually do employ some versions of it...