Skip to content

Commit

Permalink
Fix crash when HTMLPreloadScanner encounters atypical picture children
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
Yoav Weiss authored and Commit Bot committed May 23, 2019
1 parent 1289dbf commit c1f636e
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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">
Expand Down Expand Up @@ -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">

0 comments on commit c1f636e

Please sign in to comment.