Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-06 14:10:11 +01:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #19256 - yati-sagade:master, r=asajeffrey

Browse source 
Paint worklets: Implement timeout for worklet painter threads

When a paint worklet thread takes too long, we would like to move on,
since we have a ~16ms budget for rendering at 60fps. At the moment, there
is no provision in the paintworklet spec to signal such timeouts to the
developer. ajeffrey opened an [issue][1] for this, but it got punted to
v2 of the spec. Hence we are silently timing out unresponsive paint
scripts.

The timeout value is chosen to be 10ms by default, and can be overridden
by setting the SERVO_PAINT_WORKLET_TIMEOUT_MS environment variable.

In the absence of such a timeout, the reftest in this commit would fail
by timing out the testrunner itself, since the paint script never
returns. From my discussions with ajeffrey, this should do until we spec
out a way to signal timeouts to the script developer.

This fixes #17370.

[1]: https://github.com/w3c/css-houdini-drafts/issues/507

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19256)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-12-22 04:50:16 -06:00 • committed by GitHub
commit 54f8cc37c5
9 changed files with 189 additions and 70 deletions
Download patch file Download diff file Expand all files Collapse all files

27
components/layout/display_list_builder.rs
View file

@ -1382,7 +1382,7 @@ impl FragmentDisplayListBuilding for Fragment {
.map(|argument| argument.to_css_string())
.collect();
let mut draw_result = match state.layout_context.registered_painters.get(&name) {
let draw_result = match state.layout_context.registered_painters.get(&name) {
Some(painter) => {
debug!("Drawing a paint image {}({},{}).", name, size_in_px.width, size_in_px.height);
let properties = painter.properties().iter()
@ -1397,19 +1397,22 @@ impl FragmentDisplayListBuilding for Fragment {
},
};
let webrender_image = WebRenderImageInfo {
width: draw_result.width,
height: draw_result.height,
format: draw_result.format,
key: draw_result.image_key,
};
if let Ok(draw_result) = draw_result {
let webrender_image = WebRenderImageInfo {
width: draw_result.width,
height: draw_result.height,
format: draw_result.format,
key: draw_result.image_key,
};
for url in draw_result.missing_image_urls.drain(..) {
debug!("Requesting missing image URL {}.", url);
state.layout_context.get_webrender_image_for_url(self.node, url, UsePlaceholder::No);
for url in draw_result.missing_image_urls.into_iter() {
debug!("Requesting missing image URL {}.", url);
state.layout_context.get_webrender_image_for_url(self.node, url, UsePlaceholder::No);
}
Some(webrender_image)
} else {
None
}
Some(webrender_image)
}
fn build_display_list_for_background_gradient(&self,

4
components/layout_thread/lib.rs
View file

@ -100,8 +100,8 @@ use script_layout_interface::rpc::{LayoutRPC, MarginStyleResponse, NodeOverflowR
use script_layout_interface::rpc::TextIndexResponse;
use script_layout_interface::wrapper_traits::LayoutNode;
use script_traits::{ConstellationControlMsg, LayoutControlMsg, LayoutMsg as ConstellationMsg};
use script_traits::{DrawAPaintImageResult, PaintWorkletError};
use script_traits::{ScrollState, UntrustedNodeAddress};
use script_traits::DrawAPaintImageResult;
use script_traits::Painter;
use selectors::Element;
use servo_arc::Arc as ServoArc;
@ -1787,7 +1787,7 @@ impl Painter for RegisteredPainterImpl {
device_pixel_ratio: TypedScale<f32, CSSPixel, DevicePixel>,
properties: Vec<(Atom, String)>,
arguments: Vec<String>)
-> DrawAPaintImageResult
-> Result<DrawAPaintImageResult, PaintWorkletError>
{
self.painter.draw_a_paint_image(size, device_pixel_ratio, properties, arguments)
}

130
components/script/dom/paintworkletglobalscope.rs
View file

@ -45,9 +45,10 @@ use js::rust::Runtime;
use msg::constellation_msg::PipelineId;
use net_traits::image::base::PixelFormat;
use net_traits::image_cache::ImageCache;
use script_traits::DrawAPaintImageResult;
use script_traits::{DrawAPaintImageResult, PaintWorkletError};
use script_traits::Painter;
use servo_atoms::Atom;
use servo_config::prefs::PREFS;
use servo_url::ServoUrl;
use std::cell::Cell;
use std::collections::HashMap;
@ -58,6 +59,8 @@ use std::sync::Arc;
use std::sync::Mutex;
use std::sync::mpsc;
use std::sync::mpsc::Sender;
use std::thread;
use std::time::Duration;
use style_traits::CSSPixel;
use style_traits::DevicePixel;
use style_traits::SpeculativePainter;
@ -343,7 +346,7 @@ impl PaintWorkletGlobalScope {
device_pixel_ratio: TypedScale<f32, CSSPixel, DevicePixel>,
properties: Vec<(Atom, String)>,
arguments: Vec<String>)
-> DrawAPaintImageResult {
-> Result<DrawAPaintImageResult, PaintWorkletError> {
let name = self.name.clone();
let (sender, receiver) = mpsc::channel();
let task = PaintWorkletTask::DrawAPaintImage(name,
@ -354,7 +357,14 @@ impl PaintWorkletGlobalScope {
sender);
self.executor.lock().expect("Locking a painter.")
.schedule_a_worklet_task(WorkletTask::Paint(task));
receiver.recv().expect("Worklet thread died?")
let timeout = PREFS.get("dom.worklet.timeout_ms")
.as_u64()
.unwrap_or(10u64);
let timeout_duration = Duration::from_millis(timeout);
receiver.recv_timeout(timeout_duration)
.map_err(|e| PaintWorkletError::from(e))
}
}
Box::new(WorkletPainter {
@ -364,6 +374,60 @@ impl PaintWorkletGlobalScope {
}
}
/// Tasks which can be peformed by a paint worklet
pub enum PaintWorkletTask {
DrawAPaintImage(Atom,
TypedSize2D<f32, CSSPixel>,
TypedScale<f32, CSSPixel, DevicePixel>,
Vec<(Atom, String)>,
Vec<String>,
Sender<DrawAPaintImageResult>),
SpeculativelyDrawAPaintImage(Atom,
Vec<(Atom, String)>,
Vec<String>),
}
/// A paint definition
/// <https://drafts.css-houdini.org/css-paint-api/#paint-definition>
/// This type is dangerous, because it contains uboxed `Heap<JSVal>` values,
/// which can't be moved.
#[derive(JSTraceable, MallocSizeOf)]
#[must_root]
struct PaintDefinition {
class_constructor: Heap<JSVal>,
paint_function: Heap<JSVal>,
constructor_valid_flag: Cell<bool>,
context_alpha_flag: bool,
// TODO: this should be a list of CSS syntaxes.
input_arguments_len: usize,
// TODO: the spec calls for fresh rendering contexts each time a paint image is drawn,
// but to avoid having the primary worklet thread create a new renering context,
// we recycle them.
context: Dom<PaintRenderingContext2D>,
}
impl PaintDefinition {
fn new(class_constructor: HandleValue,
paint_function: HandleValue,
alpha: bool,
input_arguments_len: usize,
context: &PaintRenderingContext2D)
-> Box<PaintDefinition>
{
let result = Box::new(PaintDefinition {
class_constructor: Heap::default(),
paint_function: Heap::default(),
constructor_valid_flag: Cell::new(true),
context_alpha_flag: alpha,
input_arguments_len: input_arguments_len,
context: Dom::from_ref(context),
});
result.class_constructor.set(class_constructor.get());
result.paint_function.set(paint_function.get());
result
}
}
impl PaintWorkletGlobalScopeMethods for PaintWorkletGlobalScope {
#[allow(unsafe_code)]
#[allow(unrooted_must_root)]
@ -445,58 +509,14 @@ impl PaintWorkletGlobalScopeMethods for PaintWorkletGlobalScope {
Ok(())
}
}
/// Tasks which can be peformed by a paint worklet
pub enum PaintWorkletTask {
DrawAPaintImage(Atom,
TypedSize2D<f32, CSSPixel>,
TypedScale<f32, CSSPixel, DevicePixel>,
Vec<(Atom, String)>,
Vec<String>,
Sender<DrawAPaintImageResult>),
SpeculativelyDrawAPaintImage(Atom,
Vec<(Atom, String)>,
Vec<String>),
}
/// A paint definition
/// <https://drafts.css-houdini.org/css-paint-api/#paint-definition>
/// This type is dangerous, because it contains uboxed `Heap<JSVal>` values,
/// which can't be moved.
#[derive(JSTraceable, MallocSizeOf)]
#[must_root]
struct PaintDefinition {
class_constructor: Heap<JSVal>,
paint_function: Heap<JSVal>,
constructor_valid_flag: Cell<bool>,
context_alpha_flag: bool,
// TODO: this should be a list of CSS syntaxes.
input_arguments_len: usize,
// TODO: the spec calls for fresh rendering contexts each time a paint image is drawn,
// but to avoid having the primary worklet thread create a new renering context,
// we recycle them.
context: Dom<PaintRenderingContext2D>,
}
impl PaintDefinition {
fn new(class_constructor: HandleValue,
paint_function: HandleValue,
alpha: bool,
input_arguments_len: usize,
context: &PaintRenderingContext2D)
-> Box<PaintDefinition>
{
let result = Box::new(PaintDefinition {
class_constructor: Heap::default(),
paint_function: Heap::default(),
constructor_valid_flag: Cell::new(true),
context_alpha_flag: alpha,
input_arguments_len: input_arguments_len,
context: Dom::from_ref(context),
});
result.class_constructor.set(class_constructor.get());
result.paint_function.set(paint_function.get());
result
/// This is a blocking sleep function available in the paint worklet
/// global scope behind the dom.worklet.enabled +
/// dom.worklet.blockingsleep.enabled prefs. It is to be used only for
/// testing, e.g., timeouts, where otherwise one would need busy waiting
/// to make sure a certain timeout is triggered.
/// check-tidy: no specs after this line
fn Sleep(&self, ms: u64) {
thread::sleep(Duration::from_millis(ms));
}
}

4
components/script/dom/webidls/PaintWorkletGlobalScope.webidl
View file

@ -6,4 +6,8 @@
[Global=(Worklet,PaintWorklet), Pref="dom.worklet.enabled", Exposed=PaintWorklet]
interface PaintWorkletGlobalScope : WorkletGlobalScope {
[Throws] void registerPaint(DOMString name, VoidFunction paintCtor);
// This function is to be used only for testing, and should not be
// accessible outside of that use.
[Pref="dom.worklet.blockingsleep.enabled"]
void sleep(unsigned long long ms);
};

2
components/script_traits/lib.rs
View file

@ -869,7 +869,7 @@ pub trait Painter: SpeculativePainter {
zoom: TypedScale<f32, CSSPixel, DevicePixel>,
properties: Vec<(Atom, String)>,
arguments: Vec<String>)
-> DrawAPaintImageResult;
-> Result<DrawAPaintImageResult, PaintWorkletError>;
}
impl fmt::Debug for Painter {

34
tests/wpt/mozilla/meta/MANIFEST.json
View file

@ -7804,6 +7804,18 @@
],
{}
]
],
"mozilla/worklets/test_paint_worklet_timeout.html": [
[
"/_mozilla/mozilla/worklets/test_paint_worklet_timeout.html",
[
[
"/_mozilla/mozilla/worklets/test_paint_worklet_timeout_ref.html",
"=="
]
],
{}
]
]
},
"reftest_node": {
@ -12701,6 +12713,16 @@
{}
]
],
"mozilla/worklets/test_paint_worklet_timeout.js": [
[
{}
]
],
"mozilla/worklets/test_paint_worklet_timeout_ref.html": [
[
{}
]
],
"mozilla/worklets/test_worklet.js": [
[
{}
@ -72432,6 +72454,18 @@
"7c92e508e08d7d146354a5be7fa256ccbd465dde",
"support"
],
"mozilla/worklets/test_paint_worklet_timeout.html": [
"dde3d2d6359d39282cf8dfdfabebed735c7815a8",
"reftest"
],
"mozilla/worklets/test_paint_worklet_timeout.js": [
"83cdbd59c905de898b72ba47d3ae10bb95d76301",
"support"
],
"mozilla/worklets/test_paint_worklet_timeout_ref.html": [
"193529394981b09b56404678124f6b08ff230a74",
"support"
],
"mozilla/worklets/test_worklet.html": [
"fe9c93a5307c616f878b6623155e1b04c86dd994",
"testharness"

31
tests/wpt/mozilla/tests/mozilla/worklets/test_paint_worklet_timeout.html Normal file
View file

@ -0,0 +1,31 @@
<!doctype html>
<html class="reftest-wait">
<head>
<meta charset="utf-8">
<title>reftest001</title>
<link rel="match" href="test_paint_worklet_timeout_ref.html">
</head>
<body>
<!--
When the paint worklet script takes too long, we should timeout (before the
reftest itself timing out). The result should be the same as having
a broken image url as the background of the element in question.
-->
<button id="btn">Click me!</button>
<style>
#btn {
background-image: paint(testgreen);
}
</style>
<script>
window.paintWorklet.addModule('test_paint_worklet_timeout.js')
.then(() => document.documentElement.classList.remove('reftest-wait'));
</script>
</body>
</html>

7
tests/wpt/mozilla/tests/mozilla/worklets/test_paint_worklet_timeout.js Normal file
View file

@ -0,0 +1,7 @@
registerPaint("testgreen", class {
paint(ctx, size) {
ctx.fillStyle = 'green';
ctx.fillRect(0, 0, size.width, size.height);
sleep(30); // too long for a paintworklet to init
}
});

20
tests/wpt/mozilla/tests/mozilla/worklets/test_paint_worklet_timeout_ref.html Normal file
View file

@ -0,0 +1,20 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>reftest001</title>
</head>
<body>
<button id="btn">Click me!</button>
<style>
#btn {
background-image: url(broken.png);
}
</style>
</body>
</html>