From c1f636e0e3e9f51cf90195570b561bb97a478e38 Mon Sep 17 00:00:00 2001
From: Yoav Weiss <yoavweiss@chromium.org>
Date: Thu, 23 May 2019 16:34:58 +0000
Subject: [PATCH] Fix crash when HTMLPreloadScanner encounters atypical picture
 children

When a picture element was opened but not closed, internal tags would
get preloaded as image_set typed resources, which shouldn't happen when
the loaded resources are not images.

This CL fixes that by terminating picture-like processing as soon as an
atypical tag is encountered inside `<picture>`.
That may reduce preloading for invalid `<picture>` tags, but that is
most-probably fine.

Bug: 961151
Change-Id: I392b87e51100175b38461d47f7677c840448c78d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1626590
Commit-Queue: Yoav Weiss <yoavweiss@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662674}
---
 .../core/html/parser/html_preload_scanner.cc        |  8 +++++++-
 .../loading/preload-picture-invalid-expected.txt    | 12 ++++++------
 .../http/tests/loading/preload-picture-invalid.html | 13 +++++++------
 3 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/third_party/blink/renderer/core/html/parser/html_preload_scanner.cc b/third_party/blink/renderer/core/html/parser/html_preload_scanner.cc
index b814db9d7839bd..d23aebb948f9be 100644
--- a/third_party/blink/renderer/core/html/parser/html_preload_scanner.cc
+++ b/third_party/blink/renderer/core/html/parser/html_preload_scanner.cc
@@ -931,6 +931,11 @@ void TokenPreloadScanner::ScanCommon(const Token& token,
         in_picture_ = true;
         picture_data_ = PictureData();
         return;
+      } else if (!Match(tag_impl, kSourceTag) && !Match(tag_impl, kImgTag)) {
+        // If found an "atypical" picture child, don't process it as a picture
+        // child.
+        in_picture_ = false;
+        picture_data_.picked = false;
       }
 
       StartTagScanner scanner(
@@ -945,8 +950,9 @@ void TokenPreloadScanner::ScanCommon(const Token& token,
       std::unique_ptr<PreloadRequest> request = scanner.CreatePreloadRequest(
           predicted_base_element_url_, source, client_hints_preferences_,
           picture_data_, *document_parameters_);
-      if (request)
+      if (request) {
         requests.push_back(std::move(request));
+      }
       return;
     }
     default: { return; }
diff --git a/third_party/blink/web_tests/http/tests/loading/preload-picture-invalid-expected.txt b/third_party/blink/web_tests/http/tests/loading/preload-picture-invalid-expected.txt
index 8e9379779b893e..b3305eae452323 100644
--- a/third_party/blink/web_tests/http/tests/loading/preload-picture-invalid-expected.txt
+++ b/third_party/blink/web_tests/http/tests/loading/preload-picture-invalid-expected.txt
@@ -15,18 +15,18 @@ PASS internals.isPreloaded('resources/preload-test.jpg?2'); is true
 PASS internals.isPreloaded('resources/base-image1.png?2'); is false
 PASS internals.isPreloaded('resources/base-image2.png?2'); is false
 PASS internals.isPreloaded('resources/base-image3.png?2'); is false
-PASS internals.isPreloaded('resources/preload-test.jpg?3'); is false
+PASS internals.isPreloaded('resources/preload-test.jpg?3'); is true
 PASS internals.isPreloaded('resources/base-image1.png?3'); is false
 PASS internals.isPreloaded('resources/base-image2.png?3'); is false
-PASS internals.isPreloaded('resources/base-image3.png?3'); is true
-PASS internals.isPreloaded('resources/preload-test.jpg?4'); is false
+PASS internals.isPreloaded('resources/base-image3.png?3'); is false
+PASS internals.isPreloaded('resources/preload-test.jpg?4'); 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?5'); is false
+PASS internals.isPreloaded('resources/base-image3.png?4'); is false
+PASS internals.isPreloaded('resources/preload-test.jpg?5'); is true
 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 true
+PASS internals.isPreloaded('resources/base-image3.png?5'); is false
 PASS internals.isPreloaded('resources/preload-test.jpg?6'); is true
 PASS internals.isPreloaded('resources/base-image1.png?6'); is false
 PASS internals.isPreloaded('resources/base-image2.png?6'); is false
diff --git a/third_party/blink/web_tests/http/tests/loading/preload-picture-invalid.html b/third_party/blink/web_tests/http/tests/loading/preload-picture-invalid.html
index 96b4f07bcf8f96..ab9a3b6e2bec32 100644
--- a/third_party/blink/web_tests/http/tests/loading/preload-picture-invalid.html
+++ b/third_party/blink/web_tests/http/tests/loading/preload-picture-invalid.html
@@ -12,9 +12,9 @@
     shouldBeTrue("internals.isPreloaded('resources/image2.png');");
     loadFromImg(1);
     loadFromImg(2);
-    loadFromSource(3);
-    loadFromSource(4);
-    loadFromSource(5);
+    loadFromImg(3);
+    loadFromImg(4);
+    loadFromImg(5);
     loadFromImg(6);
     loadFromImg(7);
 </script>
@@ -43,7 +43,7 @@
     <source sizes="400px" srcset="resources/base-image1.png?2 200w, resources/base-image3.png?2 400w, resources/base-image2.png?2 800w">
     <img src="resources/preload-test.jpg?2">
 </picture>
-<!-- Here we preload the correct resource, since document.write is not modifying the DOM -->
+<!-- Here we preload the wrong resource, as we encounter a script element -->
 <picture>
     <script>document.write("bla");</script>
     <source sizes="400px" srcset="resources/base-image1.png?3 200w, resources/base-image3.png?3 400w, resources/base-image2.png?3 800w">
@@ -81,5 +81,6 @@
     <script>document.write('<source sizes="400px" srcset="resources/base-image1.png?7 200w, resources/base-image3.png?7 400w, resources/base-image2.png?7 800w">');</script>
     <img src="resources/preload-test.jpg?7">
 </picture>
-</body>
-</html>
+<!-- This one below makes sure that the preloadScanner doesn't crash -->
+<picture>
+<source media="(max-width: 2000px)" sizes="2000px" srcset="../../hidpi/resources/image-set-1x.png?1 400w"> <script src="../../forms/resources/picker-common.js">