Source/WebCore:
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 Nov 2017 17:56:35 +0000 (17:56 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 6 Nov 2017 17:56:35 +0000 (17:56 +0000)
Added mime type check to the picture source preloader to avoid downloading incompatible resources.
https://bugs.webkit.org/show_bug.cgi?id=179231

Patch by Colin Bendell <colin@bendell.ca> on 2017-11-06
Reviewed by Alex Christensen.

Test: http/tests/loading/preload-picture-type.html

* html/parser/HTMLPreloadScanner.cpp:
(WebCore::TokenPreloadScanner::StartTagScanner::processAttributes): include type match state when selecting candidate imgs
(WebCore::TokenPreloadScanner::StartTagScanner::processAttribute): added type attribute evaluation and matching

LayoutTests:
Add tests to ensure that <source> tags are only preloaded when the `type`
attribute matches.
https://bugs.webkit.org/show_bug.cgi?id=179231

Patch by Colin Bendell <colin@bendell.ca> on 2017-11-06
Reviewed by Alex Christensen.

* http/tests/loading/preload-picture-type-expected.txt: Added.
* http/tests/loading/preload-picture-type.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/http/tests/loading/preload-picture-type-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/loading/preload-picture-type.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/parser/HTMLPreloadScanner.cpp

index ef870a5..9fc69fc 100644 (file)
@@ -1,3 +1,14 @@
+2017-11-06  Colin Bendell  <colin@bendell.ca>
+
+        Add tests to ensure that <source> tags are only preloaded when the `type`
+        attribute matches.
+        https://bugs.webkit.org/show_bug.cgi?id=179231
+
+        Reviewed by Alex Christensen.
+
+        * http/tests/loading/preload-picture-type-expected.txt: Added.
+        * http/tests/loading/preload-picture-type.html: Added.
+
 2017-11-05  Antoine Quint  <graouts@apple.com>
 
         [Web Animations] Schedule animations registered on the document timeline
diff --git a/LayoutTests/http/tests/loading/preload-picture-type-expected.txt b/LayoutTests/http/tests/loading/preload-picture-type-expected.txt
new file mode 100644 (file)
index 0000000..ea3c7e7
--- /dev/null
@@ -0,0 +1,52 @@
+main frame - didStartProvisionalLoadForFrame
+main frame - didCommitLoadForFrame
+main frame - willPerformClientRedirectToURL: http://127.0.0.1:8000/loading/preload-picture-type.html 
+main frame - didFinishDocumentLoadForFrame
+main frame - didFinishLoadForFrame
+main frame - didStartProvisionalLoadForFrame
+main frame - didCancelClientRedirectForFrame
+main frame - didCommitLoadForFrame
+main frame - didFinishDocumentLoadForFrame
+main frame - didHandleOnloadEventsForFrame
+main frame - didFinishLoadForFrame
+PASS window.devicePixelRatio is 1
+PASS internals.isPreloaded('resources/base-image1.png?0'); is true
+PASS internals.isPreloaded('resources/preload-test.jpg?0'); is false
+PASS internals.isPreloaded('resources/base-image1.png?1'); is true
+PASS internals.isPreloaded('resources/preload-test.jpg?1'); is false
+PASS internals.isPreloaded('resources/base-image1.png?2'); is true
+PASS internals.isPreloaded('resources/preload-test.jpg?2'); is false
+PASS internals.isPreloaded('resources/base-image1.png?3'); is false
+PASS internals.isPreloaded('resources/preload-test.jpg?3'); is true
+PASS internals.isPreloaded('resources/base-image1.png?4'); is false
+PASS internals.isPreloaded('resources/base-image2.png?4'); is false
+PASS internals.isPreloaded('resources/base-image3.png?4'); is true
+PASS internals.isPreloaded('resources/preload-test.jpg?4'); is false
+PASS internals.isPreloaded('resources/base-image1.png?5'); is false
+PASS internals.isPreloaded('resources/base-image2.png?5'); is false
+PASS internals.isPreloaded('resources/base-image3.png?5'); is false
+PASS internals.isPreloaded('resources/preload-test.jpg?5'); is true
+PASS internals.isPreloaded('resources/base-image1.png?6'); is true
+PASS internals.isPreloaded('resources/preload-test.jpg?6'); is false
+PASS internals.isPreloaded('resources/base-image1.png?7'); is false
+PASS internals.isPreloaded('resources/preload-test.jpg?7'); is true
+PASS internals.isPreloaded('resources/base-image1.png?8'); is true
+PASS internals.isPreloaded('resources/preload-test.jpg?8'); is false
+PASS internals.isPreloaded('resources/base-image1.png?9'); is false
+PASS internals.isPreloaded('resources/preload-test.jpg?9'); is true
+PASS internals.isPreloaded('resources/base-image1.png?10'); is true
+PASS internals.isPreloaded('resources/base-image2.png?10'); is false
+PASS internals.isPreloaded('resources/preload-test.jpg?10'); is false
+PASS internals.isPreloaded('resources/base-image1.png?11'); is false
+PASS internals.isPreloaded('resources/base-image2.png?11'); is true
+PASS internals.isPreloaded('resources/preload-test.jpg?11'); is false
+PASS internals.isPreloaded('resources/base-image1.png?12'); is true
+PASS internals.isPreloaded('resources/base-image2.png?12'); is false
+PASS internals.isPreloaded('resources/preload-test.jpg?12'); is false
+PASS internals.isPreloaded('resources/base-image1.png?13'); is false
+PASS internals.isPreloaded('resources/base-image2.png?13'); is false
+PASS internals.isPreloaded('resources/preload-test.jpg?13'); is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+                   
diff --git a/LayoutTests/http/tests/loading/preload-picture-type.html b/LayoutTests/http/tests/loading/preload-picture-type.html
new file mode 100644 (file)
index 0000000..3bdf195
--- /dev/null
@@ -0,0 +1,137 @@
+<html>
+<head>
+    <script>
+      testRunner.waitUntilDone();
+      testRunner.setBackingScaleFactor(1);
+      if (!sessionStorage.pageReloaded) {
+          sessionStorage.pageReloaded = true
+          // have to force a restart because of a bug in dynamic changes with srcset
+          // we have to force the restart even if DPR=1 for consistency
+          document.location.reload(true);
+      }
+    </script>
+    <script src="/js-test-resources/js-test.js"></script>
+    <script src="http://127.0.0.1:8000/resources/slow-script.pl?delay=100"></script>
+</head>
+<body>
+<script>
+    if (window.testRunner)
+        testRunner.dumpAsText();
+
+    shouldBe("window.devicePixelRatio", "1");
+    shouldBeTrue("internals.isPreloaded('resources/base-image1.png?0');");
+    shouldBeFalse("internals.isPreloaded('resources/preload-test.jpg?0');");
+    shouldBeTrue("internals.isPreloaded('resources/base-image1.png?1');");
+    shouldBeFalse("internals.isPreloaded('resources/preload-test.jpg?1');");
+    shouldBeTrue("internals.isPreloaded('resources/base-image1.png?2');");
+    shouldBeFalse("internals.isPreloaded('resources/preload-test.jpg?2');");
+    shouldBeFalse("internals.isPreloaded('resources/base-image1.png?3');");
+    shouldBeTrue("internals.isPreloaded('resources/preload-test.jpg?3');");
+
+    shouldBeFalse("internals.isPreloaded('resources/base-image1.png?4');");
+    shouldBeFalse("internals.isPreloaded('resources/base-image2.png?4');");
+    shouldBeTrue("internals.isPreloaded('resources/base-image3.png?4');");
+    shouldBeFalse("internals.isPreloaded('resources/preload-test.jpg?4');");
+
+    shouldBeFalse("internals.isPreloaded('resources/base-image1.png?5');");
+    shouldBeFalse("internals.isPreloaded('resources/base-image2.png?5');");
+    shouldBeFalse("internals.isPreloaded('resources/base-image3.png?5');");
+    shouldBeTrue("internals.isPreloaded('resources/preload-test.jpg?5');");
+
+    shouldBeTrue("internals.isPreloaded('resources/base-image1.png?6');");
+    shouldBeFalse("internals.isPreloaded('resources/preload-test.jpg?6');");
+    shouldBeFalse("internals.isPreloaded('resources/base-image1.png?7');");
+    shouldBeTrue("internals.isPreloaded('resources/preload-test.jpg?7');");
+
+    shouldBeTrue("internals.isPreloaded('resources/base-image1.png?8');");
+    shouldBeFalse("internals.isPreloaded('resources/preload-test.jpg?8');");
+
+    shouldBeFalse("internals.isPreloaded('resources/base-image1.png?9');");
+    shouldBeTrue("internals.isPreloaded('resources/preload-test.jpg?9');");
+
+    shouldBeTrue("internals.isPreloaded('resources/base-image1.png?10');");
+    shouldBeFalse("internals.isPreloaded('resources/base-image2.png?10');");
+    shouldBeFalse("internals.isPreloaded('resources/preload-test.jpg?10');");
+
+    shouldBeFalse("internals.isPreloaded('resources/base-image1.png?11');");
+    shouldBeTrue("internals.isPreloaded('resources/base-image2.png?11');");
+    shouldBeFalse("internals.isPreloaded('resources/preload-test.jpg?11');");
+
+    shouldBeTrue("internals.isPreloaded('resources/base-image1.png?12');");
+    shouldBeFalse("internals.isPreloaded('resources/base-image2.png?12');");
+    shouldBeFalse("internals.isPreloaded('resources/preload-test.jpg?12');");
+
+    shouldBeFalse("internals.isPreloaded('resources/base-image1.png?13');");
+    shouldBeFalse("internals.isPreloaded('resources/base-image2.png?13');");
+    shouldBeTrue("internals.isPreloaded('resources/preload-test.jpg?13');");
+    testRunner.notifyDone();
+</script>
+<!-- Control group -->
+<picture>
+    <source srcset="resources/base-image1.png?0">
+    <img src="resources/preload-test.jpg?0">
+</picture>
+<!-- All permutations of type, media, srcset and sizes -->
+
+<picture>
+    <source type="image/png" srcset="resources/base-image1.png?1">
+    <img src="resources/preload-test.jpg?1">
+</picture>
+<picture>
+    <source srcset="resources/base-image1.png?2" type="image/png">
+    <img src="resources/preload-test.jpg?2">
+</picture>
+<picture>
+    <source type="image/bad" srcset="resources/base-image1.png?3">
+    <img src="resources/preload-test.jpg?3">
+</picture>
+<picture>
+    <source type="image/png" sizes="400px" srcset="resources/base-image1.png?4 200w, resources/base-image3.png?4 400w, resources/base-image2.png?4 800w">
+    <img src="resources/preload-test.jpg?4">
+</picture>
+<picture>
+    <source type="image/bad" sizes="400px" srcset="resources/base-image1.png?5 200w, resources/base-image3.png?5 400w, resources/base-image2.png?5 800w" >
+    <img src="resources/preload-test.jpg?5">
+</picture>
+<picture>
+    <source type="image/png" media="(min-width: 1px)" srcset="resources/base-image1.png?6">
+    <img src="resources/preload-test.jpg?6">
+</picture>
+<picture>
+    <source type="image/bad" media="(min-width: 1px)" srcset="resources/base-image1.png?7">
+    <img src="resources/preload-test.jpg?7">
+</picture>
+
+<!-- Duplicate attributes -->
+<picture>
+    <source type="image/png" type="image/bad" srcset="resources/base-image1.png?8">
+    <img src="resources/preload-test.jpg?8">
+</picture>
+<picture>
+    <source type="image/bad" type="image/png" srcset="resources/base-image1.png?9">
+    <img src="resources/preload-test.jpg?9">
+</picture>
+<!-- Multiple sources -->
+<picture>
+    <source type="image/png" media="(min-width: 1px)" srcset="resources/base-image1.png?10">
+    <source type="image/png" media="(min-width: 1px)" srcset="resources/base-image2.png?10">
+    <img src="resources/preload-test.jpg?10">
+</picture>
+<picture>
+    <source type="image/bad" media="(min-width: 1px)" srcset="resources/base-image1.png?11">
+    <source type="image/png" media="(min-width: 1px)" srcset="resources/base-image2.png?11">
+    <img src="resources/preload-test.jpg?11">
+</picture>
+<picture>
+    <source type="image/png" media="(min-width: 1px)" srcset="resources/base-image1.png?12">
+    <source type="image/bad" media="(min-width: 1px)" srcset="resources/base-image2.png?12">
+    <img src="resources/preload-test.jpg?12">
+</picture>
+<picture>
+    <source type="image/bad" media="(min-width: 1px)" srcset="resources/base-image1.png?13">
+    <source type="image/invalid" media="(min-width: 1px)" srcset="resources/base-image2.png?13">
+    <img src="resources/preload-test.jpg?13">
+</picture>
+
+</body>
+</html>
index 0e67b3a..4f149c1 100644 (file)
@@ -1,3 +1,16 @@
+2017-11-06  Colin Bendell  <colin@bendell.ca>
+
+        Added mime type check to the picture source preloader to avoid downloading incompatible resources.
+        https://bugs.webkit.org/show_bug.cgi?id=179231
+
+        Reviewed by Alex Christensen.
+
+        Test: http/tests/loading/preload-picture-type.html
+
+        * html/parser/HTMLPreloadScanner.cpp:
+        (WebCore::TokenPreloadScanner::StartTagScanner::processAttributes): include type match state when selecting candidate imgs
+        (WebCore::TokenPreloadScanner::StartTagScanner::processAttribute): added type attribute evaluation and matching
+
 2017-11-06  Youenn Fablet  <youenn@apple.com>
 
         Use VCP H264 encoder for platforms supporting it
index cf38a49..5ab457b 100644 (file)
@@ -36,6 +36,7 @@
 #include "LinkLoader.h"
 #include "LinkRelAttribute.h"
 #include "Logging.h"
+#include "MIMETypeRegistry.h"
 #include "MediaList.h"
 #include "MediaQueryEvaluator.h"
 #include "RenderView.h"
@@ -121,7 +122,7 @@ public:
             processAttribute(attributeName, attributeValue, document, pictureState);
         }
         
-        if (m_tagId == TagId::Source && !pictureState.isEmpty() && !pictureState.last() && m_mediaMatched && !m_srcSetAttribute.isEmpty()) {
+        if (m_tagId == TagId::Source && !pictureState.isEmpty() && !pictureState.last() && m_mediaMatched && m_typeMatched && !m_srcSetAttribute.isEmpty()) {
             
             auto sourceSize = SizesAttributeParser(m_sizesAttribute, document).length();
             ImageCandidate imageCandidate = bestFitSourceForImageAttributes(m_deviceScaleFactor, m_urlToLoad, m_srcSetAttribute, sourceSize);
@@ -219,6 +220,11 @@ private:
                 LOG(MediaQueries, "HTMLPreloadScanner %p processAttribute evaluating media queries", this);
                 m_mediaMatched = MediaQueryEvaluator { document.printing() ? "print" : "screen", document, documentElement ? documentElement->computedStyle() : nullptr }.evaluate(mediaSet.get());
             }
+            if (match(attributeName, typeAttr) && m_typeAttribute.isNull()) {
+                // when multiple type attributes present: first value wins, ignore subsequent (to match ImageElement parser and Blink behaviours)
+                m_typeAttribute = attributeValue;
+                m_typeMatched &= MIMETypeRegistry::isSupportedImageOrSVGMIMEType(m_typeAttribute);
+            }
             break;
         case TagId::Script:
             if (match(attributeName, typeAttr)) {
@@ -341,6 +347,7 @@ private:
     String m_srcSetAttribute;
     String m_sizesAttribute;
     bool m_mediaMatched { true };
+    bool m_typeMatched { true };
     String m_charset;
     String m_crossOriginMode;
     bool m_linkIsStyleSheet;