157563 – Resource load cancelled if detaching the first of multiple iframes all loading from the same URL

RESOLVED DUPLICATE of bug 162094 157563

Resource load cancelled if detaching the first of multiple iframes all loading from the same URL

https://bugs.webkit.org/show_bug.cgi?id=157563

Summary Resource load cancelled if detaching the first of multiple iframes all loadin...

Hyongyoub Kim

Reported 2016-05-11 02:57:44 PDT

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...

Attachments
Test html pages and cgi perl scripts (1.31 KB, application/x-zip-compressed) 2016-05-11 02:57 PDT, Hyongyoub Kim	no flags	Details
Patch (2.28 KB, patch) 2016-05-12 13:33 PDT, Alex Christensen	buildbot: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews106 for mac-yosemite-wk2 (800.86 KB, application/zip) 2016-05-12 14:24 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (644.00 KB, application/zip) 2016-05-12 14:28 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews117 for mac-yosemite (1.41 MB, application/zip) 2016-05-12 14:35 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews101 for mac-yosemite (798.49 KB, application/zip) 2016-05-12 15:38 PDT, Build Bot	no flags	Details
sub.html missing from test.zip (96 bytes, text/html) 2016-05-12 17:08 PDT, Hyongyoub Kim	no flags	Details
View All Add attachment proposed patch, testcase, etc.

Alex Christensen

Comment 1 2016-05-12 13:05:41 PDT

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?

Alex Christensen

Comment 2 2016-05-12 13:33:17 PDT

Created attachment 278756 [details] Patch

Alex Christensen

Comment 3 2016-05-12 13:34:17 PDT

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.

Build Bot

Comment 4 2016-05-12 14:24:05 PDT

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

Build Bot

Comment 5 2016-05-12 14:24:09 PDT

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

Build Bot

Comment 6 2016-05-12 14:28:29 PDT

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

Build Bot

Comment 7 2016-05-12 14:28:33 PDT

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

Build Bot

Comment 8 2016-05-12 14:34:55 PDT

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

Build Bot

Comment 9 2016-05-12 14:35:00 PDT

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

Build Bot

Comment 10 2016-05-12 15:38:06 PDT

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

Build Bot

Comment 11 2016-05-12 15:38:11 PDT

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

Hyongyoub Kim

Comment 12 2016-05-12 17:08:15 PDT

Created attachment 278784 [details] sub.html missing from test.zip Attaching sub.html. Forgot to include this in the zip file.

Antti Koivisto

Comment 13 2016-05-13 07:03:56 PDT

I suppose main resource loader cancels the load without taking into account that the cached resource may have other clients.

Alex Christensen

Comment 14 2016-05-16 15:05:09 PDT

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.

Hyongyoub Kim

Comment 15 2016-05-16 21:04:56 PDT

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); }

Chris Dumez

Comment 16 2016-09-16 16:24:29 PDT

*** This bug has been marked as a duplicate of bug 162094 ***

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution DUPLICATE

of bug 162094

Priority P2

Severity Normal

Classification Unclassified

Version WebKit Local Build

Hardware All

OS All

Product WebKit

Component Frames

Assignee

Alex Christensen

Reported

2016-05-11 02:57 PDT

Modified

2016-09-23 11:10 PDT History

CC List

8 users Show

URL

Keywords

Depends on

Blocks