Changeset 207080 in webkit


Ignore:
Timestamp:
Oct 11, 2016 3:38:37 AM (8 years ago)
Author:
Carlos Garcia Campos
Message:

Merge r206062 - 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:
releases/WebKitGTK/webkit-2.14
Files:
6 added
4 edited

Legend:

Unmodified
Added
Removed

  • releases/WebKitGTK/webkit-2.14/LayoutTests/ChangeLog

    r207078 r207080  
     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-15  Zalan Bujtas  <zalan@apple.com>
    221

  • releases/WebKitGTK/webkit-2.14/LayoutTests/http/tests/security/XFrameOptions/x-frame-options-deny-multiple-clients-expected.txt

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

  • releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog

    r207079 r207080  
     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

  • releases/WebKitGTK/webkit-2.14/Source/WebCore/loader/cache/CachedResourceLoader.cpp

    r204976 r207080  
    865865        return Reload;
    866866    }
    867    
    868     // For resources that are not yet loaded we ignore the cache policy.
    869     if (existingResource->isLoading())
     867
     868    if (existingResource->isLoading()) {
     869        // Do not use cached main resources that are still loading because sharing
     870        // loading CachedResources in this case causes issues with regards to cancellation.
     871        // If one of the DocumentLoader clients decides to cancel the load, then the load
     872        // would be cancelled for all other DocumentLoaders as well.
     873        if (type == CachedResource::Type::MainResource)
     874            return Reload;
     875        // For cached subresources that are still loading we ignore the cache policy.
    870876        return Use;
     877    }
    871878
    872879    auto revalidationDecision = existingResource->makeRevalidationDecision(cachePolicy(type));
Note: See TracChangeset for help on using the changeset viewer.