Remove remaining bits of dynamic <link> rel='icon' loading
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Apr 2016 01:33:34 +0000 (01:33 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Apr 2016 01:33:34 +0000 (01:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=156727

Reviewed by Darin Adler.

Source/WebCore:

Don't call 'shouldLoadLink' for 'icon' link types. It performs no
useful checks for 'icon' types, and emits the non-standard
'onbeforeload' event.

This work finishes up https://webkit.org/b/153151, where we removed
FrameLoaderClient::dispatchDidChangeIcons() and related code.

Test: webarchive/test-link-rel-subresource-beforeload.html

* loader/LinkLoader.cpp:
(WebCore::LinkLoader::loadLink): Remove unneeded call to 'shouldLoadLink'.

Tools:

Remove unused test infrastructure. This code never existed in WebKit2
testing since was only used on Windows.

* DumpRenderTree/TestRunner.cpp:
(dumpIconChangesCallback): Deleted.
* DumpRenderTree/TestRunner.h:
(TestRunner::dumpIconChanges): Deleted.
(TestRunner::setDumpIconChanges): Deleted.

LayoutTests:

Rename 'test-link-rel-icon-beforeload.html' to 'test-link-rel-subresource-beforeload.html'.

* fast/dom/icon-url-property-expected.txt: Removed.
* fast/dom/icon-url-property.html: Removed.
* platform/gtk/TestExpectations: Remove references to deleted tests.
* platform/ios-simulator/TestExpectations: Ditto.
* platform/wk2/TestExpectations: Ditto.
* webarchive/test-link-rel-icon-beforeload-expected.webarchive: Removed.
* webarchive/test-link-rel-icon-beforeload.html: Removed.
* webarchive/test-link-rel-subresource-beforeload-expected.webarchive: Copied from LayoutTests/webarchive/test-link-rel-icon-beforeload-expected.webarchive.
* webarchive/test-link-rel-subresource-beforeload.html: Copied from LayoutTests/webarchive/test-link-rel-icon-beforeload.html.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@199752 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/dom/icon-url-property-expected.txt [deleted file]
LayoutTests/fast/dom/icon-url-property.html [deleted file]
LayoutTests/platform/gtk/TestExpectations
LayoutTests/platform/ios-simulator/TestExpectations
LayoutTests/platform/wk2/TestExpectations
LayoutTests/webarchive/test-link-rel-subresource-beforeload-expected.webarchive [moved from LayoutTests/webarchive/test-link-rel-icon-beforeload-expected.webarchive with 83% similarity]
LayoutTests/webarchive/test-link-rel-subresource-beforeload.html [moved from LayoutTests/webarchive/test-link-rel-icon-beforeload.html with 79% similarity]
Source/WebCore/ChangeLog
Source/WebCore/loader/LinkLoader.cpp
Tools/ChangeLog
Tools/DumpRenderTree/TestRunner.cpp
Tools/DumpRenderTree/TestRunner.h

index 197812c..5d07313 100644 (file)
@@ -1,3 +1,22 @@
+2016-04-19  Brent Fulgham  <bfulgham@apple.com>
+
+        Remove remaining bits of dynamic <link> rel='icon' loading
+        https://bugs.webkit.org/show_bug.cgi?id=156727
+
+        Reviewed by Darin Adler.
+
+        Rename 'test-link-rel-icon-beforeload.html' to 'test-link-rel-subresource-beforeload.html'.
+
+        * fast/dom/icon-url-property-expected.txt: Removed.
+        * fast/dom/icon-url-property.html: Removed.
+        * platform/gtk/TestExpectations: Remove references to deleted tests.
+        * platform/ios-simulator/TestExpectations: Ditto. 
+        * platform/wk2/TestExpectations: Ditto.
+        * webarchive/test-link-rel-icon-beforeload-expected.webarchive: Removed.
+        * webarchive/test-link-rel-icon-beforeload.html: Removed.
+        * webarchive/test-link-rel-subresource-beforeload-expected.webarchive: Copied from LayoutTests/webarchive/test-link-rel-icon-beforeload-expected.webarchive.
+        * webarchive/test-link-rel-subresource-beforeload.html: Copied from LayoutTests/webarchive/test-link-rel-icon-beforeload.html.
+
 2016-04-19  Chris Dumez  <cdumez@apple.com>
 
         AudioBufferSourceNode.buffer should be nullable
diff --git a/LayoutTests/fast/dom/icon-url-property-expected.txt b/LayoutTests/fast/dom/icon-url-property-expected.txt
deleted file mode 100644 (file)
index 90e864c..0000000
+++ /dev/null
@@ -1,7 +0,0 @@
-main frame - didChangeIcons
-main frame - didChangeIcons
-main frame - didChangeIcons
-Original iconURL is: http://test.com/oldfavicon.ico
-Setting new icon URL to: http://test.com/newfavicon.ico
-New iconURL is: http://test.com/newfavicon.ico
-PASS - URL list matches expected
diff --git a/LayoutTests/fast/dom/icon-url-property.html b/LayoutTests/fast/dom/icon-url-property.html
deleted file mode 100644 (file)
index 035921d..0000000
+++ /dev/null
@@ -1,75 +0,0 @@
-<html>
-<head>
-<title>Original Title</title>
-<link rel="shortcut icon" type="image/x-icon" href="http://test.com/oldfavicon.ico"/>
-<script>
-function debugOutput(str) {
-    text = document.createTextNode(str);
-    debugDiv = document.getElementById('debugDiv');
-    div = document.createElement ('div');
-    div.appendChild(text);
-    debugDiv.appendChild(div);
-}
-
-function createFavIconElement(iconURL) {
-    var link = document.createElement("link");
-    link.type = "image/x-icon";
-    link.rel = "shortcut icon";
-    link.href = iconURL;
-    return link;
-}
-
-function getHeadElement() {
-    return document.getElementsByTagName("head")[0];
-}
-
-function setFavIcon(iconURL) {
-    var docHead = getHeadElement();
-    var links = docHead.getElementsByTagName("link");
-    for (var i = 0; i < links.length; ++i) {
-      var link = links[i];
-      if (link.type=="image/x-icon" && link.rel=="shortcut icon") {
-        docHead.removeChild(link);
-        break; // Assuming only one match at most.
-      }
-    }
-    docHead.appendChild(createFavIconElement(iconURL));
-}
-
-function runTests() {
-    if (window.testRunner) {
-        testRunner.dumpAsText();
-        if (testRunner.dumpIconChanges)
-            testRunner.dumpIconChanges();
-    }
-
-    iconURL = document.getElementsByTagName("head")[0].getElementsByTagName("link")[0].href;
-    debugOutput ('Original iconURL is: ' + iconURL);
-    newURL = 'http://test.com/newfavicon.ico';
-    debugOutput ('Setting new icon URL to: ' + newURL);
-    setFavIcon(newURL);
-    iconURL = document.getElementsByTagName("head")[0].getElementsByTagName("link")[0].href;
-
-    debugOutput ('New iconURL is: ' + iconURL);
-
-    // check that the URL list in the document is as we expect
-    var expectedURLs = "http://test.com/newfavicon.ico";
-    var iconURLs = window.internals.shortcutIconURLs();
-    if (expectedURLs == iconURLs[0])
-        debugOutput('PASS - URL list matches expected');
-    else
-        debugOutput('FAIL - URL list does not match expected');
-
-    // Add some more fav icons to verify that didChangeIcons gets called.
-    var docHead = getHeadElement();
-    docHead.insertBefore(createFavIconElement("http://example.org/icon1.ico"), docHead.firstChild);
-    docHead.appendChild(createFavIconElement("http://example.org/icon2.ico"));
-}
-
-</script>
-</head>
-<body onload='runTests();'>
-<div id='debugDiv'>
-</div>
-</body>
-</html>
index 70f408d..fd28f1a 100644 (file)
@@ -2076,8 +2076,6 @@ webkit.org/b/112228 svg/custom/resources-css-scaled.html [ ImageOnlyFailure ]
 
 webkit.org/b/99893 svg/animations/mozilla/animateMotion-mpath-targetChange-1.svg [ ImageOnlyFailure Pass ]
 
-webkit.org/b/112705 fast/dom/icon-url-property.html [ Failure ]
-
 webkit.org/b/113772 http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_iframe_self_navigation.html [ Failure ]
 webkit.org/b/113772 http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_cached.html [ Failure ]
 
index d772b9a..cbc85cb 100644 (file)
@@ -1564,7 +1564,6 @@ fast/dom/custom/lifecycle-ready-parser-only.html [ ImageOnlyFailure ]
 fast/dom/custom/lifecycle-ready-parser-script.html [ Failure ]
 fast/dom/horizontal-scrollbar-in-rtl.html [ Failure ]
 fast/dom/horizontal-scrollbar-when-dir-change.html [ Failure ]
-fast/dom/icon-url-property.html [ Failure ]
 fast/dom/jsDevicePixelRatio.html [ Failure ]
 fast/dom/navigator-detached-no-crash.html [ Failure ]
 fast/dom/scroll-reveal-left-overflow.html [ Failure ]
index 16340ea..1620b3f 100644 (file)
 webkit.org/b/156612 http/tests/security/contentSecurityPolicy/embed-redirect-blocked3.html [ Failure ]
 webkit.org/b/156612 http/tests/security/contentSecurityPolicy/object-redirect-blocked3.html [ Failure ]
 
-# WebKitTestRunner needs to implement testRunner.dumpIconChanges().
-webkit.org/b/44046 http/tests/security/contentSecurityPolicy/icon-allowed.html
-webkit.org/b/44046 http/tests/security/contentSecurityPolicy/icon-blocked.html
-
 # AX tests that require hit testing do not work in WK2.
 webkit.org/b/71298 accessibility/loading-iframe-updates-axtree.html
 webkit.org/b/71298 accessibility/mac/html-slider-indicator.html
@@ -72,9 +68,6 @@ webkit.org/b/60648 fast/frames/flattening/frameset-flattening-subframesets.html
 webkit.org/b/42197 fast/harness/override-preferences-2.html [ Failure ]
 webkit.org/b/42197 fast/text/zero-font-size.html [ Pass ]
 
-# setIconDatabaseEnabled not implemented in WebKitTestRunner.
-webkit.org/b/115809 webarchive/test-link-rel-icon-beforeload.html [ Skip ]
-
 webkit.org/b/82665 http/tests/webarchive/test-css-url-encoding.html [ Skip ]
 webkit.org/b/82665 http/tests/webarchive/test-css-url-encoding-shift-jis.html [ Skip ]
 webkit.org/b/82665 http/tests/webarchive/test-css-url-encoding-utf-8.html [ Skip ]
@@ -10,7 +10,6 @@
 function runTest()
 {
     if (window.testRunner) {
-        testRunner.setIconDatabaseEnabled(true);
         testRunner.dumpDOMAsWebArchive();
         testRunner.queueReload();
     }
@@ -31,7 +30,7 @@ function print(message, color)
 }
 &lt;/script&gt;
 &lt;p&gt;This test should have a green 'PASS' just above this line.  If it doesn't, you failed.
-&lt;link rel="icon" onbeforeload="print('PASS','green');return false" href="resources/favicon.ico" type="image/x-icon"&gt;
+&lt;link rel="subresource" onbeforeload="print('PASS','green');return false" href="resources/favicon.ico" type="image/x-icon"&gt;
 
 
 &lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</string>
@@ -42,7 +41,7 @@ function print(message, color)
                <key>WebResourceTextEncodingName</key>
                <string>UTF-8</string>
                <key>WebResourceURL</key>
-               <string>file:///LayoutTests/webarchive/test-link-rel-icon-beforeload.html</string>
+               <string>file:///LayoutTests/webarchive/test-link-rel-subresource-beforeload.html</string>
        </dict>
 </dict>
 </plist>
@@ -4,7 +4,6 @@
 function runTest()
 {
     if (window.testRunner) {
-        testRunner.setIconDatabaseEnabled(true);
         testRunner.dumpDOMAsWebArchive();
         testRunner.queueReload();
     }
@@ -25,6 +24,6 @@ function print(message, color)
 }
 </script>
 <p>This test should have a green 'PASS' just above this line.  If it doesn't, you failed.
-<link rel="icon" onbeforeload="print('PASS','green');return false" href="resources/favicon.ico" type="image/x-icon">
+<link rel="subresource" onbeforeload="print('PASS','green');return false" href="resources/favicon.ico" type="image/x-icon">
 </body>
 </html>
index 1ca3b9f..ba5ef19 100644 (file)
@@ -1,3 +1,22 @@
+2016-04-19  Brent Fulgham  <bfulgham@apple.com>
+
+        Remove remaining bits of dynamic <link> rel='icon' loading
+        https://bugs.webkit.org/show_bug.cgi?id=156727
+
+        Reviewed by Darin Adler.
+
+        Don't call 'shouldLoadLink' for 'icon' link types. It performs no
+        useful checks for 'icon' types, and emits the non-standard
+        'onbeforeload' event.
+
+        This work finishes up https://webkit.org/b/153151, where we removed
+        FrameLoaderClient::dispatchDidChangeIcons() and related code.
+
+        Test: webarchive/test-link-rel-subresource-beforeload.html
+
+        * loader/LinkLoader.cpp:
+        (WebCore::LinkLoader::loadLink): Remove unneeded call to 'shouldLoadLink'.
+
 2016-04-19  Chris Dumez  <cdumez@apple.com>
 
         AudioBufferSourceNode.buffer should be nullable
index 2d8e305..be4031e 100644 (file)
@@ -137,12 +137,6 @@ static void preloadIfNeeded(const LinkRelAttribute& relAttribute, const URL& hre
 
 bool LinkLoader::loadLink(const LinkRelAttribute& relAttribute, const URL& href, const String& as, const String& crossOrigin, Document& document)
 {
-    // We'll record this URL per document, even if we later only use it in top level frames
-    if (relAttribute.iconType != InvalidIcon && href.isValid() && !href.isEmpty()) {
-        if (!m_client.shouldLoadLink())
-            return false;
-    }
-
     if (relAttribute.isDNSPrefetch) {
         Settings* settings = document.settings();
         // FIXME: The href attribute of the link element can be in "//hostname" form, and we shouldn't attempt
index 429cef1..dcce3d0 100644 (file)
@@ -1,3 +1,19 @@
+2016-04-19  Brent Fulgham  <bfulgham@apple.com>
+
+        Remove remaining bits of dynamic <link> rel='icon' loading
+        https://bugs.webkit.org/show_bug.cgi?id=156727
+
+        Reviewed by Darin Adler.
+
+        Remove unused test infrastructure. This code never existed in WebKit2
+        testing since was only used on Windows.
+
+        * DumpRenderTree/TestRunner.cpp:
+        (dumpIconChangesCallback): Deleted.
+        * DumpRenderTree/TestRunner.h:
+        (TestRunner::dumpIconChanges): Deleted.
+        (TestRunner::setDumpIconChanges): Deleted.
+
 2016-04-19  Keith Miller  <keith_miller@apple.com>
 
         ObjcInstance::stringValue should not call NSObject's description method
index 280f73a..a69c0f1 100644 (file)
@@ -81,7 +81,6 @@ TestRunner::TestRunner(const std::string& testURL, const std::string& expectedPi
     , m_dumpSourceAsWebArchive(false)
     , m_dumpStatusCallbacks(false)
     , m_dumpTitleChanges(false)
-    , m_dumpIconChanges(false)
     , m_dumpVisitedLinksCallback(false)
     , m_dumpWillCacheResponse(false)
     , m_generatePixelResults(true)
@@ -263,13 +262,6 @@ static JSValueRef dumpTitleChangesCallback(JSContextRef context, JSObjectRef fun
     return JSValueMakeUndefined(context);
 }
 
-static JSValueRef dumpIconChangesCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
-{
-    TestRunner* controller = static_cast<TestRunner*>(JSObjectGetPrivate(thisObject));
-    controller->setDumpIconChanges(true);
-    return JSValueMakeUndefined(context);
-}
-
 static JSValueRef dumpWillCacheResponseCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
 {
     TestRunner* controller = static_cast<TestRunner*>(JSObjectGetPrivate(thisObject));
@@ -2064,7 +2056,6 @@ JSStaticFunction* TestRunner::staticFunctions()
         { "dumpSourceAsWebArchive", dumpSourceAsWebArchiveCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "dumpStatusCallbacks", dumpStatusCallbacksCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "dumpTitleChanges", dumpTitleChangesCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
-        { "dumpIconChanges", dumpIconChangesCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "dumpWillCacheResponse", dumpWillCacheResponseCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "encodeHostName", encodeHostNameCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "evaluateInWebInspector", evaluateInWebInspectorCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
index e1c000d..731c8d1 100644 (file)
@@ -210,9 +210,6 @@ public:
     bool dumpTitleChanges() const { return m_dumpTitleChanges; }
     void setDumpTitleChanges(bool dumpTitleChanges) { m_dumpTitleChanges = dumpTitleChanges; }
 
-    bool dumpIconChanges() const { return m_dumpIconChanges; }
-    void setDumpIconChanges(bool dumpIconChanges) { m_dumpIconChanges = dumpIconChanges; }
-
     bool dumpVisitedLinksCallback() const { return m_dumpVisitedLinksCallback; }
     void setDumpVisitedLinksCallback(bool dumpVisitedLinksCallback) { m_dumpVisitedLinksCallback = dumpVisitedLinksCallback; }
     
@@ -386,7 +383,6 @@ private:
     bool m_dumpSourceAsWebArchive;
     bool m_dumpStatusCallbacks;
     bool m_dumpTitleChanges;
-    bool m_dumpIconChanges;
     bool m_dumpVisitedLinksCallback;
     bool m_dumpWillCacheResponse;
     bool m_generatePixelResults;