Skip to content

GitLab

T
tangled
Commit 30ecdaf6 authored by Yoav Weiss's avatar Yoav Weiss Committed by Commit Bot
Browse files
Options

Fix CSSPreloadScanner to avoid missing rules (reland) 

This fixes a couple of bugs with the CSSPReloadScanner that made it miss
rules in which the URL wasn't quoted or the rule didn't end with a
semicolon. It also adds tentative WPT tests for that functionality, as
the tests are also relevant for WebKit.

This is a reland of
https://chromium-review.googlesource.com/c/chromium/src/+/1331042

Bug: 903785
Change-Id: I401c252a42fbb96dee9c7942e0a4f8d5b6850244
TBR: kouhei
Reviewed-on: https://chromium-review.googlesource.com/c/1349980Reviewed-by: default avatarYoav Weiss <yoav@yoav.ws>
Commit-Queue: Yoav Weiss <yoav@yoav.ws>
Cr-Commit-Position: refs/heads/master@{#610796}
parent 58d1b65e
Show whitespace changes
Inline Side-by-side
Showing with 174 additions and 18 deletions
third_party/blink/renderer/core/html/parser/css_preload_scanner.cc
View file @ 30ecdaf6
......@@ -156,12 +156,18 @@ inline void CSSPreloadScanner::Tokenize(UChar c,
}
break;
case kRuleValue:
if (IsHTMLSpace<UChar>(c))
if (IsHTMLSpace<UChar>(c)) {
state_ = kAfterRuleValue;
else if (c == ';')
} else if (c == ';') {
EmitRule(source);
else
} else {
rule_value_.Append(c);
// When reading the rule and hitting ')', which signifies the URL end,
// emit the rule.
if (c == ')') {
EmitRule(source);
}
}
break;
case kAfterRuleValue:
if (IsHTMLSpace<UChar>(c))
......@@ -185,14 +191,20 @@ static String ParseCSSStringOrURL(const String& string) {
wtf_size_t offset = 0;
wtf_size_t reduced_length = string.length();
// Remove whitespace from the rule start
while (reduced_length && IsHTMLSpace<UChar>(string[offset])) {
++offset;
--reduced_length;
}
// Remove whitespace from the rule end
// TODO(yoav): Evaluate performance benefits of using raw string operations.
// TODO(yoav): Look into moving parsing to use better parsing primitives.
while (reduced_length &&
IsHTMLSpace<UChar>(string[offset + reduced_length - 1]))
IsHTMLSpace<UChar>(string[offset + reduced_length - 1])) {
--reduced_length;
}
// Skip the "url(" prefix and the ")" suffix
if (reduced_length >= 5 && (string[offset] == 'u' || string[offset] == 'U') &&
(string[offset + 1] == 'r' || string[offset + 1] == 'R') &&
(string[offset + 2] == 'l' || string[offset + 2] == 'L') &&
......@@ -201,28 +213,23 @@ static String ParseCSSStringOrURL(const String& string) {
reduced_length -= 5;
}
// Skip whitespace before and after the URL inside the "url()" parenthesis.
while (reduced_length && IsHTMLSpace<UChar>(string[offset])) {
++offset;
--reduced_length;
}
while (reduced_length &&
IsHTMLSpace<UChar>(string[offset + reduced_length - 1]))
IsHTMLSpace<UChar>(string[offset + reduced_length - 1])) {
--reduced_length;
}
if (reduced_length < 2 ||
string[offset] != string[offset + reduced_length - 1] ||
!(string[offset] == '\'' || string[offset] == '"'))
return String();
// Remove single-quotes or double-quotes from the URL
if ((reduced_length >= 2) &&
(string[offset] == string[offset + reduced_length - 1]) &&
(string[offset] == '\'' || string[offset] == '"')) {
offset++;
reduced_length -= 2;
while (reduced_length && IsHTMLSpace<UChar>(string[offset])) {
++offset;
--reduced_length;
}
while (reduced_length &&
IsHTMLSpace<UChar>(string[offset + reduced_length - 1]))
--reduced_length;
return string.Substring(offset, reduced_length);
}
......
third_party/blink/web_tests/external/wpt/loading/preloader-css-import-no-quote.tentative.html 0 → 100644
View file @ 30ecdaf6
<!DOCTYPE html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
var t = async_test('Imported inline CSS with no quote is not blocked on pending CSS');
</script>
<link rel=stylesheet href="resources/dummy.css?first&pipe=trickle(d1)">
<script>
var this_script_is_neccessary_to_block_the_inline_style_processing = true;
</script>
<style>
@import url(resources/dummy.css?second);
</style>
<script>
window.addEventListener("load", t.step_func_done(() => {
let entries = performance.getEntriesByType('resource');
let first;
let second;
for (entry of entries) {
if (entry.name.includes("first")) {
first = entry;
}
if (entry.name.includes("second")) {
second = entry;
}
}
assert_true(first.responseEnd > second.startTime, "The second resource start time should not be blocked on the first resource response");
}));
</script>
third_party/blink/web_tests/external/wpt/loading/preloader-css-import-no-semicolon.tentative.html 0 → 100644
View file @ 30ecdaf6
<!DOCTYPE html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
var t = async_test('Imported inline CSS with no semicolon is not blocked on pending CSS');
</script>
<link rel=stylesheet href="resources/dummy.css?first&pipe=trickle(d1)">
<script>
var this_script_is_neccessary_to_block_the_inline_style_processing = true;
</script>
<style>
@import url("resources/dummy.css?second")
</style>
<script>
window.addEventListener("load", t.step_func_done(() => {
let entries = performance.getEntriesByType('resource');
let first;
let second;
for (entry of entries) {
if (entry.name.includes("first")) {
first = entry;
}
if (entry.name.includes("second")) {
second = entry;
}
}
assert_true(first.responseEnd > second.startTime, "The second resource start time should not be blocked on the first resource response");
}));
</script>
third_party/blink/web_tests/external/wpt/loading/preloader-css-import-no-space.tentative.html 0 → 100644
View file @ 30ecdaf6
<!DOCTYPE html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
var t = async_test('Imported inline CSS with no quote is not blocked on pending CSS');
</script>
<link rel=stylesheet href="resources/dummy.css?first&pipe=trickle(d1)">
<script>
var this_script_is_neccessary_to_block_the_inline_style_processing = true;
</script>
<style>@import url("resources/dummy.css?second")</style>
<script>
window.addEventListener("load", t.step_func_done(() => {
let entries = performance.getEntriesByType('resource');
let first;
let second;
for (entry of entries) {
if (entry.name.includes("first")) {
first = entry;
}
if (entry.name.includes("second")) {
second = entry;
}
}
assert_true(first.responseEnd > second.startTime, "The second resource start time should not be blocked on the first resource response");
}));
</script>
third_party/blink/web_tests/external/wpt/loading/preloader-css-import-single-quote.tentative.html 0 → 100644
View file @ 30ecdaf6
<!DOCTYPE html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
var t = async_test('Imported inline CSS is not blocked on pending CSS');
</script>
<link rel=stylesheet href="resources/dummy.css?first&pipe=trickle(d1)">
<script>
var this_script_is_neccessary_to_block_the_inline_style_processing = true;
</script>
<style>
@import url('resources/dummy.css?second');
</style>
<script>
window.addEventListener("load", t.step_func_done(() => {
let entries = performance.getEntriesByType('resource');
let first;
let second;
for (entry of entries) {
if (entry.name.includes("first")) {
first = entry;
}
if (entry.name.includes("second")) {
second = entry;
}
}
assert_true(first.responseEnd > second.startTime, "The second resource start time should not be blocked on the first resource response");
}));
</script>
third_party/blink/web_tests/external/wpt/loading/preloader-css-import.tentative.html 0 → 100644
View file @ 30ecdaf6
<!DOCTYPE html>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
var t = async_test('Imported inline CSS is not blocked on pending CSS');
</script>
<link rel=stylesheet href="resources/dummy.css?first&pipe=trickle(d1)">
<script>
var this_script_is_neccessary_to_block_the_inline_style_processing = true;
</script>
<style>
@import url("resources/dummy.css?second");
</style>
<script>
window.addEventListener("load", t.step_func_done(() => {
let entries = performance.getEntriesByType('resource');
let first;
let second;
for (entry of entries) {
if (entry.name.includes("first")) {
first = entry;
}
if (entry.name.includes("second")) {
second = entry;
}
}
assert_true(first.responseEnd > second.startTime, "The second resource start time should not be blocked on the first resource response");
}));
</script>
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment