Changeset 206062 in webkit


Ignore:
Timestamp:
Sep 16, 2016 8:18:45 PM (8 years ago)
Author:
Chris Dumez
Message:

Cancelling one frame's load cancels load in other frames that have the same URL as well
​https://bugs.webkit.org/show_bug.cgi?id=162094

Reviewed by Antti Koivisto.

Source/WebCore:

Cancelling one frame's load cancels load in other frames that have the same URL as well.

So if you have several frames that are loading URL X and you navigate one of the frames
to Y, then the load of X will be cancelled and this frame will navigate to Y. All other
frames will not load URL X even though they should.

The issue is that all the DocumentLoaders share the same CachedResource because of the
memoryCache. When we call DocumentLoader::stopLoading(), it will cancel the
CachedResource's load even though there are several clients for this CachedResource
and other clients still want the load.

The approach chosen in this patch is to not reuse CachedResources that are still
loading when trying to load a main resource. This is not the most efficient approach.
I still chose this approach because:

  • It is very unlikely to introduce new bugs.
  • The change is very simple.
  • This is a corner case (several iframes having the same URL and cancelling the load in one of them).

Test: http/tests/navigation/frames-same-url-cancel-load.html

  • loader/cache/CachedResourceLoader.cpp:

(WebCore::CachedResourceLoader::determineRevalidationPolicy):

LayoutTests:

Add layout test coverage.

  • http/tests/cache/iframe-detach-expected.txt: Added.
  • http/tests/cache/iframe-detach.html: Added.
  • http/tests/cache/resources/slow-iframe.php: Added.

Import Alex Christensen's test from Bug 157563.

  • http/tests/navigation/frames-same-url-cancel-load-expected.txt: Added.
  • http/tests/navigation/frames-same-url-cancel-load.html: Added.
  • http/tests/navigation/resources/success.html: Added.
  • http/tests/security/XFrameOptions/x-frame-options-deny-multiple-clients-expected.txt:
Location:
trunk
Files:
6 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r206059 r206062  
     12016-09-16  Chris Dumez  <cdumez@apple.com>
     2
     3        Cancelling one frame's load cancels load in other frames that have the same URL as well
     4        https://bugs.webkit.org/show_bug.cgi?id=162094
     5
     6        Reviewed by Antti Koivisto.
     7
     8        Add layout test coverage.
     9
     10        * http/tests/cache/iframe-detach-expected.txt: Added.
     11        * http/tests/cache/iframe-detach.html: Added.
     12        * http/tests/cache/resources/slow-iframe.php: Added.
     13        Import Alex Christensen's test from Bug 157563.
     14
     15        * http/tests/navigation/frames-same-url-cancel-load-expected.txt: Added.
     16        * http/tests/navigation/frames-same-url-cancel-load.html: Added.
     17        * http/tests/navigation/resources/success.html: Added.
     18        * http/tests/security/XFrameOptions/x-frame-options-deny-multiple-clients-expected.txt:
     19
    1202016-09-16  Joseph Pecoraro  <pecoraro@apple.com>
    221

  • trunk/LayoutTests/http/tests/security/XFrameOptions/x-frame-options-deny-multiple-clients-expected.txt

    r180202 r206062  
     1CONSOLE MESSAGE: Refused to display 'http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-deny.cgi' in a frame because it set 'X-Frame-Options' to 'deny'.
     2ALERT: PASS: onload fired.
    13CONSOLE MESSAGE: Refused to display 'http://127.0.0.1:8000/security/XFrameOptions/resources/x-frame-options-deny.cgi' in a frame because it set 'X-Frame-Options' to 'deny'.
    24ALERT: PASS: onload fired.

  • trunk/Source/WebCore/ChangeLog

    r206061 r206062  
     12016-09-16  Chris Dumez  <cdumez@apple.com>
     2
     3        Cancelling one frame's load cancels load in other frames that have the same URL as well
     4        https://bugs.webkit.org/show_bug.cgi?id=162094
     5
     6        Reviewed by Antti Koivisto.
     7
     8        Cancelling one frame's load cancels load in other frames that have the same URL as well.
     9
     10        So if you have several frames that are loading URL X and you navigate one of the frames
     11        to Y, then the load of X will be cancelled and this frame will navigate to Y. All other
     12        frames will not load URL X even though they should.
     13
     14        The issue is that all the DocumentLoaders share the same CachedResource because of the
     15        memoryCache. When we call DocumentLoader::stopLoading(), it will cancel the
     16        CachedResource's load even though there are several clients for this CachedResource
     17        and other clients still want the load.
     18
     19        The approach chosen in this patch is to not reuse CachedResources that are still
     20        loading when trying to load a main resource. This is not the most efficient approach.
     21        I still chose this approach because:
     22        - It is very unlikely to introduce new bugs.
     23        - The change is very simple.
     24        - This is a corner case (several iframes having the same URL and cancelling the load in
     25          one of them).
     26
     27        Test: http/tests/navigation/frames-same-url-cancel-load.html
     28
     29        * loader/cache/CachedResourceLoader.cpp:
     30        (WebCore::CachedResourceLoader::determineRevalidationPolicy):
     31
    1322016-09-16  Michael Catanzaro  <mcatanzaro@igalia.com>
    233

  • trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp

    r206017 r206062  
    929929        return Reload;
    930930    }
    931    
    932     // For resources that are not yet loaded we ignore the cache policy.
    933     if (existingResource->isLoading())
     931
     932    if (existingResource->isLoading()) {
     933        // Do not use cached main resources that are still loading because sharing
     934        // loading CachedResources in this case causes issues with regards to cancellation.
     935        // If one of the DocumentLoader clients decides to cancel the load, then the load
     936        // would be cancelled for all other DocumentLoaders as well.
     937        if (type == CachedResource::Type::MainResource)
     938            return Reload;
     939        // For cached subresources that are still loading we ignore the cache policy.
    934940        return Use;
     941    }
    935942
    936943    auto revalidationDecision = existingResource->makeRevalidationDecision(cachePolicy(type));
Note: See TracChangeset for help on using the changeset viewer.