Commit 239b0eaa authored by Dominik Röttsches's avatar Dominik Röttsches Committed by Commit Bot

Ensure font loading promises are rejected in valid execution context

The promise resolution was already wrapped in calling it inside the
right execution context, but the reject was not. Add the correct
execution context and schedule rejecting the promise if font loading for
a local() font fails when it is triggered by an implicit UA-triggered
load.

Spec reference:
https://drafts.csswg.org/css-font-loading/#font-face-load
in particular the last normative paragraph: "User agents can initiate
font loads on their own, whenever they determine that a given font face
is necessary to render something on the page. When this happens, they
must act as if they had called the corresponding FontFace’s load()
method described here."

Thanks to Roel Nieskens (pixelambacht@) for the helpful bug report and
reproduction test case.

Bug: 996687
Change-Id: Icb5d75675b8a2e0b3f100808645a1e2e550cfcd1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1771607
Commit-Queue: Dominik Röttsches <drott@chromium.org>
Reviewed-by: default avatarKunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691092}
parent d0a6cfe6
...@@ -414,11 +414,6 @@ void FontFace::SetLoadStatus(LoadStatusType status) { ...@@ -414,11 +414,6 @@ void FontFace::SetLoadStatus(LoadStatusType status) {
if (!GetExecutionContext()) if (!GetExecutionContext())
return; return;
// When promises are resolved with 'thenables', instead of the object being
// returned directly, the 'then' method is executed (the resolver tries to
// resolve the thenable). This can lead to synchronous script execution, so we
// post a task. This does not apply to promise rejection (i.e. a thenable
// would be returned as is).
if (status_ == kLoaded || status_ == kError) { if (status_ == kLoaded || status_ == kError) {
if (loaded_property_) { if (loaded_property_) {
if (status_ == kLoaded) { if (status_ == kLoaded) {
...@@ -428,8 +423,14 @@ void FontFace::SetLoadStatus(LoadStatusType status) { ...@@ -428,8 +423,14 @@ void FontFace::SetLoadStatus(LoadStatusType status) {
WTF::Bind(&LoadedProperty::Resolve<FontFace*>, WTF::Bind(&LoadedProperty::Resolve<FontFace*>,
WrapPersistent(loaded_property_.Get()), WrapPersistent(loaded_property_.Get()),
WrapPersistent(this))); WrapPersistent(this)));
} else } else {
loaded_property_->Reject(error_.Get()); GetExecutionContext()
->GetTaskRunner(TaskType::kDOMManipulation)
->PostTask(FROM_HERE,
WTF::Bind(&LoadedProperty::Reject<DOMException*>,
WrapPersistent(loaded_property_.Get()),
WrapPersistent(error_.Get())));
}
} }
GetExecutionContext() GetExecutionContext()
......
<!DOCTYPE html>
<meta charset="utf-8">
<link rel="author" title="Dominik Röttsches" href="drott@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-font-loading/#font-face-load">
<meta name="assert" content="Ensure that a UA triggered font load (through the use in the test div) leads to rejecting
the promise." />
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
promise_test(function(t) {
var testFontFace = new FontFace('TestFontFace', 'local("nonexistentfont-9a1a9f78-c8d4-11e9-af16-448a5b2c326f")');
document.fonts.add(testFontFace);
return promise_rejects(t, 'NetworkError', testFontFace.loaded);
})
</script>
<body>
<div style="font-family: TestFontFace;">a</div>
</html>
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