Changeset 214388 in webkit


Ignore:
Timestamp:
Mar 24, 2017 5:10:06 PM (7 years ago)
Author:
Ryan Haddad
Message:

Unreviewed, rolling out r214361.

This change caused flakiness in http/tests/preload tests.

Reverted changeset:

"Add a warning for unused link preloads."
​https://bugs.webkit.org/show_bug.cgi?id=165670
​http://trac.webkit.org/changeset/214361

Location:
trunk
Files:
2 deleted
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r214386 r214388  
     12017-03-24  Ryan Haddad  <ryanhaddad@apple.com>
     2
     3        Unreviewed, rolling out r214361.
     4
     5        This change caused flakiness in http/tests/preload tests.
     6
     7        Reverted changeset:
     8
     9        "Add a warning for unused link preloads."
     10        https://bugs.webkit.org/show_bug.cgi?id=165670
     11        http://trac.webkit.org/changeset/214361
     12
    1132017-03-24  Antoine Quint  <graouts@webkit.org>
    214

  • trunk/LayoutTests/http/tests/preload/download_resources-expected.txt

    r214361 r214388  
    1 CONSOLE MESSAGE: line 13: <link rel=preload> must have a valid `as` value
    2 This test makes sure that link preload preloads resources
     1CONSOLE MESSAGE: line 11: <link rel=preload> must have a valid `as` value
     2PASS internals.isPreloaded('../resources/dummy.js'); is true
     3PASS internals.isPreloaded('../resources/dummy.css'); is true
     4PASS internals.isPreloaded('../resources/square.png'); is true
     5PASS internals.isPreloaded('../resources/Ahem.ttf'); is true
     6PASS internals.isPreloaded('../resources/test.mp4'); is true
     7PASS internals.isPreloaded('../security/resources/captions.vtt'); is true
     8PASS internals.isPreloaded('../resources/dummy.xml?badvalue'); is false
     9PASS internals.isPreloaded('../resources/dummy.xml'); is true
     10PASS successfullyParsed is true
    311
    4 PASS Makes sure that preloaded resources are downloaded
     12TEST COMPLETE
    513

  • trunk/LayoutTests/http/tests/preload/download_resources.html

    r214361 r214388  
    11<!DOCTYPE html>
    2 <script src="/js-test-resources/testharness.js"></script>
    3 <script src="/js-test-resources/testharnessreport.js"></script>
    4 <script>
    5     var t = async_test('Makes sure that preloaded resources are downloaded');
    6 </script>
     2<html>
     3<head>
     4<script src="/js-test-resources/js-test.js"></script>
    75<link rel=preload href="../resources/dummy.js" as=script>
    86<link rel=preload href="../resources/dummy.css" as=style>
    9 <link rel=preload href="../resources/square100.png" as=image>
    10 <link rel=preload href="../resources/Ahem.woff" as=font crossorigin>
     7<link rel=preload href="../resources/square.png" as=image>
     8<link rel=preload href="../resources/Ahem.ttf" as=font crossorigin>
    119<link rel=preload href="../resources/test.mp4" as=media>
    1210<link rel=preload href="../security/resources/captions.vtt" as=track>
    1311<link rel=preload href="../resources/dummy.xml?badvalue" as=foobarxmlthing>
    1412<link rel=preload href="../resources/dummy.xml">
    15 <script src="http://127.0.0.1:8000/resources/slow-script.pl?delay=400"></script>
     13<script src="http://127.0.0.1:8000/resources/slow-script.pl?delay=100"></script>
     14</head>
     15<body>
    1616<script>
    17     setTimeout(t.step_func(function() {
    18         assert_true(internals.isPreloaded("../resources/dummy.js"));
    19         assert_true(internals.isPreloaded("../resources/dummy.css"));
    20         assert_true(internals.isPreloaded("../resources/square100.png"));
    21         // FIXME: RT doesn't show downloads for the resources below. Need to investigate why.
    22         assert_true(internals.isPreloaded("../resources/Ahem.woff"));
    23         assert_true(internals.isPreloaded("../resources/test.mp4"));
    24         assert_true(internals.isPreloaded("../security/resources/captions.vtt"));
    25         assert_false(internals.isPreloaded("../resources/dummy.xml?badvalue"));
    26         assert_true(internals.isPreloaded("../resources/dummy.xml"));
    27         document.write('<script src="../resources/dummy.js"></scr' + 'ipt>' +
    28                        '<link rel=stylesheet href="../resources/dummy.css">' +
    29                        '<img src="../resources/square100.png">' +
    30                        '<video><source src="../resources/test.mp4">' +
    31                        '<track kind=subtitles src="../security/resources/captions.vtt" srclang=en>' +
    32                        '</video>' +
    33                        '<style>' +
    34                        '    @font-face { font-family:ahem; src: url(../resources/Ahem.woff); }' +
    35                        '    span { font-family: ahem, Arial; }' +
    36                        '</style>' +
    37                        '<span>This test makes sure that link preload preloads resources</span>');
    38         var xhr = new XMLHttpRequest();
    39         xhr.open("GET", "../resources/dummy.xml");
    40         xhr.send();
    41         t.done();
    42     }));
     17    if (window.testRunner)
     18        testRunner.dumpAsText();
     19    shouldBeTrue("internals.isPreloaded('../resources/dummy.js');");
     20    shouldBeTrue("internals.isPreloaded('../resources/dummy.css');");
     21    shouldBeTrue("internals.isPreloaded('../resources/square.png');");
     22    shouldBeTrue("internals.isPreloaded('../resources/Ahem.ttf');");
     23    shouldBeTrue("internals.isPreloaded('../resources/test.mp4');");
     24    shouldBeTrue("internals.isPreloaded('../security/resources/captions.vtt');");
     25    shouldBeFalse("internals.isPreloaded('../resources/dummy.xml?badvalue');");
     26    shouldBeTrue("internals.isPreloaded('../resources/dummy.xml');");
    4327</script>

  • trunk/LayoutTests/http/tests/preload/onerror_event-expected.txt

    r214361 r214388  
    1 CONSOLE MESSAGE: line 25: <link rel=preload> must have a valid `as` value
    2 This test makes sure that link preload triggers error events for non-existing resources.
     1CONSOLE MESSAGE: line 27: <link rel=preload> must have a valid `as` value
     2PASS successfullyParsed is true
    33
    4 PASS Makes sure that preloaded resources trigger onerror
     4TEST COMPLETE
     5PASS styleFailed is true
     6PASS scriptFailed is true
     7PASS imageFailed is true
     8PASS fontFailed is true
     9PASS trackFailed is true
     10PASS gibrishFailed is true
     11PASS noTypeFailed is true
    512

  • trunk/LayoutTests/http/tests/preload/onerror_event.html

    r214361 r214388  
    11<!DOCTYPE html>
    2 <script src="/js-test-resources/testharness.js"></script>
    3 <script src="/js-test-resources/testharnessreport.js"></script>
    42<script>
    5     var t = async_test('Makes sure that preloaded resources trigger onerror');
     3    if (window.testRunner) {
     4        testRunner.dumpAsText()
     5        testRunner.waitUntilDone();
     6    }
    67</script>
     8<script src="/js-test-resources/js-test.js"></script>
    79<script>
    810    var scriptFailed = false;
    … …  
    2628<link rel=preload href="http://127.0.0.1:9999/non-existent/dummy.xml" onerror="count();noTypeFailed = true;" onload="count();">
    2729<script>
    28     document.write('<script src="../non-existent/dummy.js"></scr' + 'ipt>' +
    29                    '<link rel=stylesheet href="../non-existent/dummy.css">' +
    30                    '<img src="../non-existent/square.png">' +
    31                    '<video><source src="test.mp4">' +
    32                    '<track kind=subtitles src="../../non-existent/security/captions.vtt" srclang=en>' +
    33                    '</video>' +
    34                    '<style>' +
    35                    '    @font-face { font-family:ahem; src: url(../../non-existent/Ahem.ttf); }' +
    36                    '    span { font-family: ahem, Arial; }' +
    37                    '</style>' +
    38                    '<span>This test makes sure that link preload triggers error events for non-existing resources.</span>');
    39     var xhr = new XMLHttpRequest();
    40     xhr.open("GET", "../non-existent/dummy.xml");
    41     xhr.send();
    42     var xhr2 = new XMLHttpRequest();
    43     xhr2.open("GET", "http://127.0.0.1:9999/non-existent/dummy.xml");
    44     xhr2.send();
    4530    function test() {
    46         assert_true(styleFailed);
    47         assert_true(scriptFailed);
    48         assert_true(imageFailed);
    49         assert_true(fontFailed);
    50         assert_true(trackFailed);
    51         assert_true(gibrishFailed);
    52         assert_true(noTypeFailed);
    53         t.done();
     31        shouldBeTrue("styleFailed");
     32        shouldBeTrue("scriptFailed");
     33        shouldBeTrue("imageFailed");
     34        shouldBeTrue("fontFailed");
     35        shouldBeTrue("trackFailed");
     36        shouldBeTrue("gibrishFailed");
     37        shouldBeTrue("noTypeFailed");
     38        if (window.testRunner)
     39            testRunner.notifyDone();
    5440    };
    55     setInterval(t.step_func(function() {
     41    setInterval(function() {
    5642        if (window.counter >= 7)
    5743            test();
    58     }, 100));
     44    }, 100);
    5945</script>
    6046

  • trunk/LayoutTests/http/tests/preload/onload_event-expected.txt

    r214361 r214388  
    1616PASS noTypeLoaded is true
    1717PASS emptyTypeLoaded is true
    18 This test makes sure that link preload events are fired
     18

  • trunk/LayoutTests/http/tests/preload/onload_event.html

    r214361 r214388  
    3030<link rel=preload href="../resources/Ahem.woff" as=font crossorigin onload="count(); fontLoaded = true;" onerror="count()">
    3131<link rel=preload href="../resources/test.ogv" as=media onload="count(); mediaLoaded = true;" onerror="count()">
    32 <link rel=preload href="../security/resources/captions.vtt" as=track onload="count(); trackLoaded = true;" onerror="count();">
     32<link rel=preload href="../security/resources/captions.vtt" as=track onload="count(); trackLoaded = true;" onerror="count()">
    3333<link rel=preload href="../resources/dummy.xml" as=foobarxmlthing onload="count(); gibberishLoaded = true;" onerror="count(); gibberishErrored = true;">
    3434<link rel=preload href="../resources/dummy.xslt" as=xslt onload="count(); xsltLoaded = true;" onerror="count(); xsltErrored = true;">
    … …  
    5252            testRunner.notifyDone();
    5353    }
    54     document.write('<script src="../resources/dummy.js"></scr' + 'ipt>' +
    55                    '<link rel=stylesheet href="../resources/dummy.css">' +
    56                    '<img src="../resources/square.png">' +
    57                    '<video><source src="test.mp4">' +
    58                    '<track kind=subtitles src="../../security/resources/captions.vtt" srclang=en>' +
    59                    '</video>' +
    60                    '<style>' +
    61                    '    @font-face { font-family:ahem; src: url(../../w3c/webperf/resources/Ahem.ttf); }' +
    62                    '    span { font-family: ahem, Arial; }' +
    63                    '</style>' +
    64                    '<span>This test makes sure that link preload events are fired</span>');
    65     var xhr = new XMLHttpRequest();
    66     xhr.open("GET", "../resources/dummy.xml");
    67     xhr.send();
    68     var xhr2 = new XMLHttpRequest();
    69     xhr2.open("GET", "../resources/dummy.xml?badvalue");
    70     xhr2.send();
    71     addEventListener("load", function(){
     54     addEventListener("load", function(){
    7255        setInterval(function() {
    7356            if (window.counter >= 10)

  • trunk/LayoutTests/http/tests/preload/single_download_preload.html

    r214361 r214388  
    5656            verifyDownloadNumber("http://127.0.0.1:8000/resources/dummy.xml", 2);
    5757            // FIXME: We should verify for video and audio as well, but they seem to (flakily?) trigger multiple partial requests.
    58             var xhr = new XMLHttpRequest();
    59             xhr.open("GET", "../resources/dummy.xml");
    60             xhr.send();
    61             var xhr2 = new XMLHttpRequest();
    62             xhr2.open("GET", "../resources/dummy.xml?badvalue");
    63             xhr2.send();
    6458            t.done();
    6559            }), 100);

  • trunk/Source/WebCore/ChangeLog

    r214386 r214388  
     12017-03-24  Ryan Haddad  <ryanhaddad@apple.com>
     2
     3        Unreviewed, rolling out r214361.
     4
     5        This change caused flakiness in http/tests/preload tests.
     6
     7        Reverted changeset:
     8
     9        "Add a warning for unused link preloads."
     10        https://bugs.webkit.org/show_bug.cgi?id=165670
     11        http://trac.webkit.org/changeset/214361
     12
    1132017-03-24  Antoine Quint  <graouts@webkit.org>
    214

  • trunk/Source/WebCore/dom/Document.cpp

    r214365 r214388  
    22812281#endif
    22822282
    2283     m_cachedResourceLoader->stopUnusedPreloadsTimer();
    2284 
    22852283    detachFromFrame();
    22862284

  • trunk/Source/WebCore/loader/LinkPreloadResourceClients.h

    r214361 r214388  
    8989
    9090    void clear() override { clearResource(*this); }
    91     bool shouldMarkAsReferenced() const override { return false; }
    9291
    9392private:
    … …  
    116115
    117116    void clear() override { clearResource(*this); }
    118     bool shouldMarkAsReferenced() const override { return false; }
    119117
    120118private:
    … …  
    138136
    139137    void clear() override { clearResource(*this); }
    140     bool shouldMarkAsReferenced() const override { return false; }
    141138
    142139private:
    … …  
    164161
    165162    void clear() override { clearResource(*this); }
    166     bool shouldMarkAsReferenced() const override { return false; }
    167163
    168164private:
    … …  
    186182
    187183    void clear() override { clearResource(*this); }
    188     bool shouldMarkAsReferenced() const override { return false; }
    189184
    190185private:

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

    r214361 r214388  
    466466bool CachedResource::addClientToSet(CachedResourceClient& client)
    467467{
    468     if (m_preloadResult == PreloadNotReferenced && client.shouldMarkAsReferenced()) {
     468    if (m_preloadResult == PreloadNotReferenced) {
    469469        if (isLoaded())
    470470            m_preloadResult = PreloadReferencedWhileComplete;

  • trunk/Source/WebCore/loader/cache/CachedResourceClient.h

    r214361 r214388  
    4646    static CachedResourceClientType expectedType() { return BaseResourceType; }
    4747    virtual CachedResourceClientType resourceClientType() const { return expectedType(); }
    48     virtual bool shouldMarkAsReferenced() const { return true; }
    4948
    5049protected:

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

    r214361 r214388  
    8686namespace WebCore {
    8787
    88 // Timeout for link preloads to be used after window.onload
    89 static const int unusedPreloadTimeoutInSeconds = 3;
    90 
    9188static CachedResource* createResource(CachedResource::Type type, CachedResourceRequest&& request, SessionID sessionID)
    9289{
    … …  
    133130    , m_documentLoader(documentLoader)
    134131    , m_requestCount(0)
    135     , m_unusedPreloadsTimer(*this, &CachedResourceLoader::warnUnusedPreloads)
    136132    , m_garbageCollectDocumentResourcesTimer(*this, &CachedResourceLoader::garbageCollectDocumentResources)
    137133    , m_autoLoadImages(true)
    … …  
    152148    // Make sure no requests still point to this CachedResourceLoader
    153149    ASSERT(m_requestCount == 0);
    154     m_unusedPreloadsTimer.stop();
    155150}
    156151
    … …  
    827822{
    828823    m_validatedURLs.clear();
    829 
    830     // If m_preloads is not empty here, it's full of link preloads,
    831     // as speculative preloads were cleared at DCL.
    832     if (m_preloads && m_preloads->size() && !m_unusedPreloadsTimer.isActive())
    833         m_unusedPreloadsTimer.startOneShot(unusedPreloadTimeoutInSeconds);
    834 }
    835 
    836 void CachedResourceLoader::stopUnusedPreloadsTimer()
    837 {
    838     m_unusedPreloadsTimer.stop();
    839824}
    840825
    … …  
    12421227#endif
    12431228    return resource;
    1244 }
    1245 
    1246 void CachedResourceLoader::warnUnusedPreloads()
    1247 {
    1248     if (!m_preloads)
    1249         return;
    1250     for (const auto& resource : *m_preloads) {
    1251         if (resource && resource->isLinkPreload() && resource->preloadResult() == CachedResource::PreloadNotReferenced && document()) {
    1252             document()->addConsoleMessage(MessageSource::Other, MessageLevel::Warning,
    1253                 "The resource " + resource->url().string() +
    1254                 " was preloaded using link preload but not used within a few seconds from the window's load event. Please make sure it wasn't preloaded for nothing.");
    1255         }
    1256     }
    12571229}
    12581230

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

    r214361 r214388  
    134134    CachedResourceHandle<CachedResource> preload(CachedResource::Type, CachedResourceRequest&&);
    135135    void printPreloadStats();
    136     void warnUnusedPreloads();
    137     void stopUnusedPreloadsTimer();
    138136
    139137    bool updateRequestAfterRedirection(CachedResource::Type, ResourceRequest&, const ResourceLoaderOptions&);
    … …  
    191189   
    192190    std::unique_ptr<ListHashSet<CachedResource*>> m_preloads;
    193     Timer m_unusedPreloadsTimer;
    194191
    195192    Timer m_garbageCollectDocumentResourcesTimer;
Note: See TracChangeset for help on using the changeset viewer.