servo: Merge #17581 - Fix incremental layout bugs for absolute and cleared elements (from mbrubeck:incremental); r=pcwalton
authorMatt Brubeck <mbrubeck@limpet.net>
Fri, 30 Jun 2017 22:15:26 -0700
changeset 1174639 7e3face23fc335ca7f1491db9936515ef2ff15c2
parent 1174638 b26128086f5b022f580ec31906eff1a50e5d838c
child 1174640 a65fd17cd3db42ec1aa85bf1ce025efa48e3ee94
push id202202
push useraramine@mozilla.com
push dateSat, 01 Jul 2017 08:36:05 +0000
treeherdertry@f178ec68f9d1 [default view] [failures only]
reviewerspcwalton
milestone56.0a1
servo: Merge #17581 - Fix incremental layout bugs for absolute and cleared elements (from mbrubeck:incremental); r=pcwalton Fixes #17307. <del>There were two underlying bugs:</del> * <del>Nodes were not marked dirty when their style attribute changed.</del> * BaseFlow flags set during flow construction could get out of sync with the element's style if repair_style was called. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #17307 - [x] There are tests for these changes Source-Repo: https://github.com/servo/servo Source-Revision: ece35f9caa7c6882adf2927ca7d7ce2bc7d8cef8
servo/Cargo.lock
servo/components/layout/Cargo.toml
servo/components/layout/flow.rs
servo/components/style/servo/restyle_damage.rs
--- a/servo/Cargo.lock
+++ b/servo/Cargo.lock
@@ -1410,17 +1410,17 @@ version = "0.2.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
 name = "layout"
 version = "0.0.1"
 dependencies = [
  "app_units 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "bitflags 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "bitflags 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "canvas_traits 0.0.1",
  "euclid 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "fnv 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "gfx 0.0.1",
  "gfx_traits 0.0.1",
  "heapsize 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "html5ever 0.18.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "ipc-channel 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
--- a/servo/components/layout/Cargo.toml
+++ b/servo/components/layout/Cargo.toml
@@ -7,17 +7,17 @@ publish = false
 
 [lib]
 name = "layout"
 path = "lib.rs"
 
 [dependencies]
 app_units = "0.5"
 atomic_refcell = "0.1"
-bitflags = "0.7"
+bitflags = "0.8"
 canvas_traits = {path = "../canvas_traits"}
 euclid = "0.15"
 fnv = "1.0"
 gfx = {path = "../gfx"}
 gfx_traits = {path = "../gfx_traits"}
 heapsize = "0.4"
 html5ever = "0.18"
 ipc-channel = "0.8"
--- a/servo/components/layout/flow.rs
+++ b/servo/components/layout/flow.rs
@@ -1110,16 +1110,38 @@ impl BaseFlow {
             flags: flags,
             writing_mode: writing_mode,
             thread_id: 0,
             stacking_context_id: StackingContextId::root(),
             scroll_root_id: None,
         }
     }
 
+    /// Update the 'flags' field when computed styles have changed.
+    ///
+    /// These flags are initially set during flow construction.  They only need to be updated here
+    /// if they are based on properties that can change without triggering `RECONSTRUCT_FLOW`.
+    pub fn update_flags_if_needed(&mut self, style: &ServoComputedValues) {
+        // For absolutely-positioned flows, changes to top/bottom/left/right can cause these flags
+        // to get out of date:
+        if self.restyle_damage.contains(REFLOW_OUT_OF_FLOW) {
+            // Note: We don't need to check whether IS_ABSOLUTELY_POSITIONED has changed, because
+            // changes to the 'position' property trigger flow reconstruction.
+            if self.flags.contains(IS_ABSOLUTELY_POSITIONED) {
+                let logical_position = style.logical_position();
+                self.flags.set(INLINE_POSITION_IS_STATIC,
+                    logical_position.inline_start == LengthOrPercentageOrAuto::Auto &&
+                    logical_position.inline_end == LengthOrPercentageOrAuto::Auto);
+                self.flags.set(BLOCK_POSITION_IS_STATIC,
+                    logical_position.block_start == LengthOrPercentageOrAuto::Auto &&
+                    logical_position.block_end == LengthOrPercentageOrAuto::Auto);
+            }
+        }
+    }
+
     /// Return a new BaseFlow like this one but with the given children list
     pub fn clone_with_children(&self, children: FlowList) -> BaseFlow {
         BaseFlow {
             children: children,
             restyle_damage: self.restyle_damage | REPAINT | REFLOW_OUT_OF_FLOW | REFLOW,
             parallel: FlowParallelInfo::new(),
             floats: self.floats.clone(),
             abs_descendants: self.abs_descendants.clone(),
@@ -1356,16 +1378,17 @@ impl<'a> MutableFlowUtils for &'a mut Fl
         }
     }
 
 
     /// Calls `repair_style` and `bubble_inline_sizes`. You should use this method instead of
     /// calling them individually, since there is no reason not to perform both operations.
     fn repair_style_and_bubble_inline_sizes(self, style: &::StyleArc<ServoComputedValues>) {
         self.repair_style(style);
+        mut_base(self).update_flags_if_needed(style);
         self.bubble_inline_sizes();
     }
 
     /// Traverse the Absolute flow tree in preorder.
     ///
     /// Traverse all your direct absolute descendants, who will then traverse
     /// their direct absolute descendants.
     ///
--- a/servo/components/style/servo/restyle_damage.rs
+++ b/servo/components/style/servo/restyle_damage.rs
@@ -197,17 +197,17 @@ fn compute_damage(old: &ServoComputedVal
     // This should check every CSS property, as enumerated in the fields of
     // http://doc.servo.org/style/properties/struct.ServoComputedValues.html
 
     // FIXME: Test somehow that every property is included.
 
     add_if_not_equal!(old, new, damage,
                       [REPAINT, REPOSITION, STORE_OVERFLOW, BUBBLE_ISIZES, REFLOW_OUT_OF_FLOW,
                        REFLOW, RECONSTRUCT_FLOW], [
-        get_box.float, get_box.display, get_box.position, get_counters.content,
+        get_box.clear, get_box.float, get_box.display, get_box.position, get_counters.content,
         get_counters.counter_reset, get_counters.counter_increment,
         get_inheritedbox._servo_under_display_none,
         get_list.quotes, get_list.list_style_type,
 
         // If these text or font properties change, we need to reconstruct the flow so that
         // text shaping is re-run.
         get_inheritedtext.letter_spacing, get_inheritedtext.text_rendering,
         get_inheritedtext.text_transform, get_inheritedtext.word_spacing,