servo: Merge #12839 - Fix a cached style cascade bug that only manifested in sequential mode (from notriddle:11818_sequential_layout_bug); r=emilio
authorMichael Howell <michael@notriddle.com>
Sun, 14 Aug 2016 02:27:19 -0500
changeset 339490 fa511d8bdf9530252c8ecbd3a6b1c48b3b91846d
parent 339489 c1b9509be7b04a0267952263d77cfc4c52337df0
child 339491 a9b2c57d1f147f5317e8bfc4c5ab97551eb0f54e
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
servo: Merge #12839 - Fix a cached style cascade bug that only manifested in sequential mode (from notriddle:11818_sequential_layout_bug); r=emilio When copying cached styles, keep the `writing_mode` up to date. --- - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #11818 (github issue number if applicable). - [X] There are tests for these changes EDIT: The test is now working. I ran it with the first commit (the actual fix) reverted and it failed. Source-Repo: https://github.com/servo/servo Source-Revision: 700bb911fcd8b34dcc5fbce22adcd8117cbf60d2
servo/components/constellation/pipeline.rs
servo/components/style/properties/properties.mako.rs
servo/components/util/opts.rs
servo/ports/cef/core.rs
--- a/servo/components/constellation/pipeline.rs
+++ b/servo/components/constellation/pipeline.rs
@@ -157,17 +157,17 @@ impl Pipeline {
                     new_pipeline_id: state.id,
                     subpage_id: subpage_id,
                     frame_type: frame_type,
                     load_data: state.load_data.clone(),
                     paint_chan: layout_to_paint_chan.clone().to_opaque(),
                     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)) {
                     warn!("Sending to script during pipeline creation failed ({})", e);
                 }
                 (script_chan, None)
             }
             None => {
@@ -469,17 +469,18 @@ impl UnprivilegedPipelineContent {
                     self.script_chan,
                     self.layout_to_paint_chan,
                     self.image_cache_thread,
                     self.font_cache_thread,
                     self.time_profiler_chan,
                     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();
             let _ = self.layout_content_process_shutdown_port.recv();
         }
     }
 
     #[cfg(not(target_os = "windows"))]
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -1711,16 +1711,19 @@ fn cascade_with_cached_declarations(
         }
     }
 
     if seen.get_font_style() || seen.get_font_weight() || seen.get_font_stretch() ||
             seen.get_font_family() {
         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
 }
 
 pub type CascadePropertyFn =
     extern "Rust" fn(declaration: &PropertyDeclaration,
                      inherited_style: &ComputedValues,
                      context: &mut computed::Context,
                      seen: &mut PropertyBitField,
--- a/servo/components/util/opts.rs
+++ b/servo/components/util/opts.rs
@@ -50,20 +50,16 @@ pub struct Opts {
     /// When the profiler is enabled, this is an optional path to dump a self-contained HTML file
     /// visualizing the traces as a timeline.
     pub time_profiler_trace_path: Option<String>,
 
     /// `None` to disable the memory profiler or `Some` with an interval in seconds to enable it
     /// 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
     /// the resources/user-agent-js directory, and if the option isn't passed userscripts
     /// won't be loaded
     pub userscripts: Option<String>,
 
     pub user_stylesheets: Vec<(Vec<u8>, Url)>,
@@ -476,17 +472,16 @@ pub fn default_opts() -> Opts {
         is_running_problem_test: false,
         url: Some(Url::parse("about:blank").unwrap()),
         paint_threads: 1,
         tile_size: 512,
         device_pixels_per_px: None,
         time_profiling: None,
         time_profiler_trace_path: None,
         mem_profiler_period: None,
-        layout_threads: 1,
         nonincremental_layout: false,
         userscripts: None,
         user_stylesheets: Vec::new(),
         output_file: None,
         replace_surrogates: false,
         gc_profile: false,
         load_webfonts_synchronously: false,
         headless: true,
@@ -666,17 +661,17 @@ pub fn from_cmdline_args(args: &[String]
 
     // If only the flag is present, default to a 5 second period for both profilers
     let time_profiling = if opt_match.opt_present("p") {
         match opt_match.opt_str("p") {
             Some(argument) => match argument.parse::<f64>() {
                 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:
         None
     };
 
     if let Some(ref time_profiler_trace_path) = opt_match.opt_str("profiler-trace-path") {
         let mut path = PathBuf::from(time_profiler_trace_path);
@@ -686,21 +681,21 @@ pub fn from_cmdline_args(args: &[String]
                 Path::new(time_profiler_trace_path).to_string_lossy(), why);
         }
     }
 
     let mem_profiler_period = opt_match.opt_default("m", "5").map(|period| {
         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");
 
     let random_pipeline_closure_probability = opt_match.opt_str("random-pipeline-closure-probability").map(|prob|
         prob.parse().unwrap_or_else(|err| {
             args_fail(&format!("Error parsing option: --random-pipeline-closure-probability ({})", err))
         })
     );
@@ -709,17 +704,17 @@ pub fn from_cmdline_args(args: &[String]
         seed.parse().unwrap_or_else(|err| {
             args_fail(&format!("Error parsing option: --random-pipeline-closure-seed ({})", err))
         })
     );
 
     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;
     }
 
     let devtools_port = opt_match.opt_default("devtools", "6000").map(|port| {
         port.parse().unwrap_or_else(|err| args_fail(&format!("Error parsing option: --devtools ({})", err)))
     });
 
     let webdriver_port = opt_match.opt_default("webdriver", "7000").map(|port| {
@@ -781,17 +776,16 @@ pub fn from_cmdline_args(args: &[String]
         is_running_problem_test: is_running_problem_test,
         url: Some(url),
         paint_threads: paint_threads,
         tile_size: tile_size,
         device_pixels_per_px: device_pixels_per_px,
         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,
         output_file: opt_match.opt_str("o"),
         replace_surrogates: debug_options.replace_surrogates,
         gc_profile: debug_options.gc_profile,
         load_webfonts_synchronously: debug_options.load_webfonts_synchronously,
         headless: opt_match.opt_present("z"),
@@ -850,16 +844,25 @@ pub fn from_cmdline_args(args: &[String]
         let value = split.get(1);
         match value {
             Some(&"false") => PREFS.set(pref_name, PrefValue::Boolean(false)),
             Some(&"true") | None => PREFS.set(pref_name, PrefValue::Boolean(true)),
             _ => PREFS.set(pref_name, PrefValue::String(value.unwrap().to_string()))
         };
     }
 
+    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
 }
 
 pub enum ArgumentParsingResult {
     ChromeProcess,
     ContentProcess(String),
 }
 
--- a/servo/ports/cef/core.rs
+++ b/servo/ports/cef/core.rs
@@ -64,17 +64,16 @@ pub extern "C" fn cef_initialize(args: *
             MAX_RENDERING_THREADS
         } else {
             (*settings).rendering_threads as usize
         }
     };
 
     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;
     temp_opts.enable_canvas_antialiasing = true;
     temp_opts.url = None;
     opts::set_defaults(temp_opts);
 
     if unsafe { (*settings).windowless_rendering_enabled != 0 } {