Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a cached style cascade bug that only manifested in sequential mode #12839

Merged
merged 3 commits into from Aug 14, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions components/constellation/pipeline.rs
Expand Up @@ -162,7 +162,7 @@ impl Pipeline {
pipeline_port: pipeline_port,
layout_to_constellation_chan: state.layout_to_constellation_chan.clone(),
content_process_shutdown_chan: layout_content_process_shutdown_chan.clone(),
layout_threads: opts::get().layout_threads,
layout_threads: PREFS.get("layout.threads").as_u64().expect("count") as usize,
};

if let Err(e) = script_chan.send(ConstellationControlMsg::AttachLayout(new_layout_info)) {
Expand Down Expand Up @@ -474,7 +474,8 @@ impl UnprivilegedPipelineContent {
self.mem_profiler_chan,
self.layout_content_process_shutdown_chan,
self.webrender_api_sender,
opts::get().layout_threads);
self.prefs.get("layout.threads").expect("exists").value()
.as_u64().expect("count") as usize);

if wait_for_completion {
let _ = self.script_content_process_shutdown_port.recv();
Expand Down
3 changes: 3 additions & 0 deletions components/style/properties/properties.mako.rs
Expand Up @@ -1716,6 +1716,9 @@ fn cascade_with_cached_declarations(
context.mutate_style().mutate_font().compute_font_hash();
}

let mode = get_writing_mode(context.style.get_inheritedbox());
context.style.set_writing_mode(mode);

context.style
}

Expand Down
29 changes: 16 additions & 13 deletions components/util/opts.rs
Expand Up @@ -55,10 +55,6 @@ pub struct Opts {
/// and cause it to produce output on that interval (`-m`).
pub mem_profiler_period: Option<f64>,

/// The number of threads to use for layout (`-y`). Defaults to 1, which results in a recursive
/// sequential algorithm.
pub layout_threads: usize,

pub nonincremental_layout: bool,

/// Where to load userscripts from, if any. An empty string will load from
Expand Down Expand Up @@ -473,7 +469,6 @@ pub fn default_opts() -> Opts {
time_profiling: None,
time_profiler_trace_path: None,
mem_profiler_period: None,
layout_threads: 1,
nonincremental_layout: false,
userscripts: None,
user_stylesheets: Vec::new(),
Expand Down Expand Up @@ -662,7 +657,7 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
Ok(interval) => Some(OutputOptions::Stdout(interval)) ,
Err(_) => Some(OutputOptions::FileName(argument)),
},
None => Some(OutputOptions::Stdout(5 as f64)),
None => Some(OutputOptions::Stdout(5.0 as f64)),
}
} else {
// if the p option doesn't exist:
Expand All @@ -682,11 +677,11 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
period.parse().unwrap_or_else(|err| args_fail(&format!("Error parsing option: -m ({})", err)))
});

let mut layout_threads: usize = match opt_match.opt_str("y") {
Some(layout_threads_str) => layout_threads_str.parse()
.unwrap_or_else(|err| args_fail(&format!("Error parsing option: -y ({})", err))),
None => cmp::max(num_cpus::get() * 3 / 4, 1),
};
let mut layout_threads: Option<usize> = opt_match.opt_str("y")
.map(|layout_threads_str| {
layout_threads_str.parse()
.unwrap_or_else(|err| args_fail(&format!("Error parsing option: -y ({})", err)))
});

let nonincremental_layout = opt_match.opt_present("i");

Expand All @@ -705,7 +700,7 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
let mut bubble_inline_sizes_separately = debug_options.bubble_widths;
if debug_options.trace_layout {
paint_threads = 1;
layout_threads = 1;
layout_threads = Some(1);
bubble_inline_sizes_separately = true;
}

Expand Down Expand Up @@ -777,7 +772,6 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
time_profiling: time_profiling,
time_profiler_trace_path: opt_match.opt_str("profiler-trace-path"),
mem_profiler_period: mem_profiler_period,
layout_threads: layout_threads,
nonincremental_layout: nonincremental_layout,
userscripts: opt_match.opt_default("userscripts", ""),
user_stylesheets: user_stylesheets,
Expand Down Expand Up @@ -845,6 +839,15 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
};
}

if let Some(layout_threads) = layout_threads {
PREFS.set("layout.threads", PrefValue::Number(layout_threads as f64));
} else if let Some(layout_threads) = PREFS.get("layout.threads").as_string() {
PREFS.set("layout.threads", PrefValue::Number(layout_threads.parse::<f64>().unwrap()));
} else if *PREFS.get("layout.threads") == PrefValue::Missing {
let layout_threads = cmp::max(num_cpus::get() * 3 / 4, 1);
PREFS.set("layout.threads", PrefValue::Number(layout_threads as f64));
}

ArgumentParsingResult::ChromeProcess
}

Expand Down
1 change: 0 additions & 1 deletion ports/cef/core.rs
Expand Up @@ -69,7 +69,6 @@ pub extern "C" fn cef_initialize(args: *const cef_main_args_t,

let mut temp_opts = opts::default_opts();
temp_opts.paint_threads = rendering_threads;
temp_opts.layout_threads = rendering_threads;
temp_opts.headless = false;
temp_opts.hard_fail = false;
temp_opts.enable_text_antialiasing = true;
Expand Down
4 changes: 2 additions & 2 deletions tests/wpt/harness/wptrunner/executors/executorservo.py
Expand Up @@ -211,8 +211,8 @@ def screenshot(self, test, viewport_size, dpi):
for stylesheet in self.browser.user_stylesheets:
command += ["--user-stylesheet", stylesheet]

for pref in test.environment.get('prefs', {}):
command += ["--pref", pref]
for pref, value in test.environment.get('prefs', {}).iteritems():
command += ["--pref", "%s=%s" % (pref, value)]

if viewport_size:
command += ["--resolution", viewport_size]
Expand Down
24 changes: 24 additions & 0 deletions tests/wpt/mozilla/meta/MANIFEST.json
Expand Up @@ -1344,6 +1344,18 @@
"url": "/_mozilla/css/data_img_a.html"
}
],
"css/direction_style_caching.html": [
{
"path": "css/direction_style_caching.html",
"references": [
[
"/_mozilla/css/direction_style_caching_ref.html",
"=="
]
],
"url": "/_mozilla/css/direction_style_caching.html"
}
],
"css/empty_cells_a.html": [
{
"path": "css/empty_cells_a.html",
Expand Down Expand Up @@ -10552,6 +10564,18 @@
"url": "/_mozilla/css/data_img_a.html"
}
],
"css/direction_style_caching.html": [
{
"path": "css/direction_style_caching.html",
"references": [
[
"/_mozilla/css/direction_style_caching_ref.html",
"=="
]
],
"url": "/_mozilla/css/direction_style_caching.html"
}
],
"css/empty_cells_a.html": [
{
"path": "css/empty_cells_a.html",
Expand Down
3 changes: 3 additions & 0 deletions tests/wpt/mozilla/meta/css/direction_style_caching.html.ini
@@ -0,0 +1,3 @@
prefs: [layout.threads:1]
[direction_style_caching.html]
type: reftest
28 changes: 28 additions & 0 deletions tests/wpt/mozilla/tests/css/direction_style_caching.html
@@ -0,0 +1,28 @@
<!doctype html>
<meta charset="utf-8">
<title>Style caching with directions</title>
<link rel="match" href="direction_style_caching_ref.html">
<style>
.wrapper {
width: 200px;
}
p { margin: 0 }
.a p, .b p {
width: 100px;
height: 1px;
background: red;
}
.b {
direction: rtl;
}
</style>
<div class=wrapper>
<div class=a><p></p></div>
<div class=a><p></p></div>
<div class=a><p></p></div>
<div class=a><p></p></div>
<div class=b><p></p></div>
<div class=b><p></p></div>
<div class=b><p></p></div>
<div class=b><p></p></div>
</div>
19 changes: 19 additions & 0 deletions tests/wpt/mozilla/tests/css/direction_style_caching_ref.html
@@ -0,0 +1,19 @@
<!DOCTYPE html>
<style>
.wrapper {
width: 200px;
}
p { margin: 0 }
.a p, .b p {
width: 100px;
height: 4px;
background: red;
}
.b p {
margin-left: 100px;
}
</style>
<div class=wrapper>
<div class=a><p></p></div>
<div class=b><p></p></div>
</div>