servo: Merge #19709 - style: Remove -servo-display-for-hypothetical-box from longhand (from CYBAI:servo-display-out-of-mako); r=emilio
authorCYBAI <cyb.ai.815@gmail.com>
Sun, 14 Jan 2018 08:27:14 -0600
changeset 451036 ae38eda6d6ff8fe07e2846693e5547877bd17d57
parent 451035 e8ae7190dd15d67766b396320de894120faa5734
child 451037 74055b2dab456b891e2d373e9999186639bc1ebc
push id8543
push userryanvm@gmail.com
push dateTue, 16 Jan 2018 14:33:22 +0000
treeherdermozilla-beta@a6525ed16a32 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #19709 - style: Remove -servo-display-for-hypothetical-box from longhand (from CYBAI:servo-display-out-of-mako); r=emilio This is a sub-PR of #19015 r? emilio For the `fn set_original_display` inside `properties.mako.rs`, I removed `is_item_or_root` first to see how the tests result is. If it's needed, I'll add it back. --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #19697 - [x] These changes do not require tests Source-Repo: https://github.com/servo/servo Source-Revision: 1b46e2e7597e90a41c6bfb3bd7008bdd922c93c6
servo/components/layout/construct.rs
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/longhand/box.mako.rs
servo/components/style/properties/properties.mako.rs
servo/components/style/values/specified/box.rs
--- a/servo/components/layout/construct.rs
+++ b/servo/components/layout/construct.rs
@@ -1480,17 +1480,17 @@ impl<'a, ConcreteThreadSafeLayoutNode> P
                     PseudoElementType::DetailsContent(maybe_display) |
                     PseudoElementType::DetailsSummary(maybe_display)
                         => maybe_display.unwrap_or(style.get_box().display),
                 };
                 (display, style.get_box().float, style.get_box().position)
             }
             Some(LayoutNodeType::Element(_)) => {
                 let style = node.style(self.style_context());
-                let original_display = style.get_box()._servo_display_for_hypothetical_box;
+                let original_display = style.get_box().original_display;
                 let munged_display = match original_display {
                     Display::Inline | Display::InlineBlock => original_display,
                     _ => style.get_box().display,
                 };
                 (munged_display, style.get_box().float, style.get_box().position)
             }
             Some(LayoutNodeType::Text) =>
                 (Display::Inline, Float::None, Position::Static),
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -3108,54 +3108,52 @@ fn static_assert() {
                                             "inline-flex grid inline-grid ruby ruby-base ruby-base-container " +
                                             "ruby-text ruby-text-container contents flow-root -webkit-box " +
                                             "-webkit-inline-box -moz-box -moz-inline-box -moz-grid -moz-inline-grid " +
                                             "-moz-grid-group -moz-grid-line -moz-stack -moz-inline-stack -moz-deck " +
                                             "-moz-popup -moz-groupbox",
                                             gecko_enum_prefix="StyleDisplay",
                                             gecko_strip_moz_prefix=False) %>
 
-    pub fn set_display(&mut self, v: longhands::display::computed_value::T) {
+    fn match_display_keyword(
+        v: longhands::display::computed_value::T
+    ) -> structs::root::mozilla::StyleDisplay {
         use properties::longhands::display::computed_value::T as Keyword;
         // FIXME(bholley): Align binary representations and ditch |match| for cast + static_asserts
-        let result = match v {
+        match v {
             % for value in display_keyword.values_for('gecko'):
                 Keyword::${to_camel_case(value)} =>
                     structs::${display_keyword.gecko_constant(value)},
             % endfor
-        };
+        }
+    }
+
+    pub fn set_display(&mut self, v: longhands::display::computed_value::T) {
+        let result = Self::match_display_keyword(v);
         self.gecko.mDisplay = result;
         self.gecko.mOriginalDisplay = result;
     }
 
-    /// Set the display value from the style adjustment code. This is pretty
-    /// much like set_display, but without touching the mOriginalDisplay field,
-    /// which we want to keep.
-    pub fn set_adjusted_display(&mut self,
-                                v: longhands::display::computed_value::T,
-                                _is_item_or_root: bool) {
-        use properties::longhands::display::computed_value::T as Keyword;
-        let result = match v {
-            % for value in display_keyword.values_for('gecko'):
-                Keyword::${to_camel_case(value)} =>
-                    structs::${display_keyword.gecko_constant(value)},
-            % endfor
-        };
-        self.gecko.mDisplay = result;
-    }
-
     pub fn copy_display_from(&mut self, other: &Self) {
         self.gecko.mDisplay = other.gecko.mDisplay;
         self.gecko.mOriginalDisplay = other.gecko.mDisplay;
     }
 
     pub fn reset_display(&mut self, other: &Self) {
         self.copy_display_from(other)
     }
 
+    pub fn set_adjusted_display(
+        &mut self,
+        v: longhands::display::computed_value::T,
+        _is_item_or_root: bool
+    ) {
+        self.gecko.mDisplay = Self::match_display_keyword(v);
+    }
+
     <%call expr="impl_keyword_clone('display', 'mDisplay', display_keyword)"></%call>
 
     <% overflow_x = data.longhands_by_name["overflow-x"] %>
     pub fn set_overflow_y(&mut self, v: longhands::overflow_y::computed_value::T) {
         use properties::longhands::overflow_x::computed_value::T as BaseType;
         // FIXME(bholley): Align binary representations and ditch |match| for cast + static_asserts
         self.gecko.mOverflowY = match v {
             % for value in overflow_x.keyword.values_for('gecko'):
--- a/servo/components/style/properties/longhand/box.mako.rs
+++ b/servo/components/style/properties/longhand/box.mako.rs
@@ -156,37 +156,16 @@
                 % for value in "None Left Right Both".split():
                     computed_value::T::${value} => SpecifiedValue::${value},
                 % endfor
             }
         }
     }
 </%helpers:single_keyword_computed>
 
-<%helpers:longhand name="-servo-display-for-hypothetical-box"
-                   animation_value_type="none"
-                   derived_from="display"
-                   products="servo"
-                   spec="Internal (not web-exposed)">
-    pub use super::display::{SpecifiedValue, get_initial_value};
-    pub use super::display::{parse};
-
-    pub mod computed_value {
-        pub type T = super::SpecifiedValue;
-    }
-
-    #[inline]
-    pub fn derive_from_display(context: &mut Context) {
-        let d = context.style().get_box().clone_display();
-        context.builder.set__servo_display_for_hypothetical_box(d);
-    }
-
-</%helpers:longhand>
-
-
 ${helpers.predefined_type(
     "vertical-align",
     "VerticalAlign",
     "computed::VerticalAlign::baseline()",
     animation_value_type="ComputedValue",
     flags="APPLIES_TO_FIRST_LETTER APPLIES_TO_FIRST_LINE APPLIES_TO_PLACEHOLDER",
     spec="https://www.w3.org/TR/CSS2/visudet.html#propdef-vertical-align",
 )}
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -1858,16 +1858,21 @@ pub mod style_structs {
             % for longhand in style_struct.longhands:
                 /// The ${longhand.name} computed value.
                 pub ${longhand.ident}: longhands::${longhand.ident}::computed_value::T,
             % endfor
             % if style_struct.name == "Font":
                 /// The font hash, used for font caching.
                 pub hash: u64,
             % endif
+            % if style_struct.name == "Box":
+                /// The display value specified by the CSS stylesheets (without any style adjustments),
+                /// which is needed for hypothetical layout boxes.
+                pub original_display: longhands::display::computed_value::T,
+            % endif
         }
         % if style_struct.name == "Font":
 
         impl PartialEq for ${style_struct.name} {
             fn eq(&self, other: &${style_struct.name}) -> bool {
                 self.hash == other.hash
                 % for longhand in style_struct.longhands:
                     && self.${longhand.ident} == other.${longhand.ident}
@@ -1888,31 +1893,54 @@ pub mod style_structs {
                         pub fn set_${longhand.ident}<I>(&mut self, v: I)
                             where I: IntoIterator<Item = longhands::${longhand.ident}
                                                                   ::computed_value::single_value::T>,
                                   I::IntoIter: ExactSizeIterator
                         {
                             self.${longhand.ident} = longhands::${longhand.ident}::computed_value
                                                               ::T(v.into_iter().collect());
                         }
+                    % elif longhand.ident == "display":
+                        /// Set `display`.
+                        ///
+                        /// We need to keep track of the original display for hypothetical boxes,
+                        /// so we need to special-case this.
+                        #[allow(non_snake_case)]
+                        #[inline]
+                        pub fn set_display(&mut self, v: longhands::display::computed_value::T) {
+                            self.display = v;
+                            self.original_display = v;
+                        }
                     % else:
                         /// Set ${longhand.name}.
                         #[allow(non_snake_case)]
                         #[inline]
                         pub fn set_${longhand.ident}(&mut self, v: longhands::${longhand.ident}::computed_value::T) {
                             self.${longhand.ident} = v;
                         }
                     % endif
-                    /// Set ${longhand.name} from other struct.
-                    #[allow(non_snake_case)]
-                    #[inline]
-                    pub fn copy_${longhand.ident}_from(&mut self, other: &Self) {
-                        self.${longhand.ident} = other.${longhand.ident}.clone();
-                    }
-
+                    % if longhand.ident == "display":
+                        /// Set `display` from other struct.
+                        ///
+                        /// Same as `set_display` above.
+                        /// Thus, we need to special-case this.
+                        #[allow(non_snake_case)]
+                        #[inline]
+                        pub fn copy_display_from(&mut self, other: &Self) {
+                            self.display = other.display.clone();
+                            self.original_display = other.display.clone();
+                        }
+                    % else:
+                        /// Set ${longhand.name} from other struct.
+                        #[allow(non_snake_case)]
+                        #[inline]
+                        pub fn copy_${longhand.ident}_from(&mut self, other: &Self) {
+                            self.${longhand.ident} = other.${longhand.ident}.clone();
+                        }
+                    % endif
                     /// Reset ${longhand.name} from the initial struct.
                     #[allow(non_snake_case)]
                     #[inline]
                     pub fn reset_${longhand.ident}(&mut self, other: &Self) {
                         self.copy_${longhand.ident}_from(other)
                     }
 
                     /// Get the computed value for ${longhand.name}.
@@ -1997,25 +2025,26 @@ pub mod style_structs {
                 }
 
                 /// Whether the text decoration has a line through.
                 #[inline]
                 pub fn has_line_through(&self) -> bool {
                     self.text_decoration_line.contains(longhands::text_decoration_line::SpecifiedValue::LINE_THROUGH)
                 }
             % elif style_struct.name == "Box":
-                /// Sets the display property, but without touching
-                /// __servo_display_for_hypothetical_box, except when the
-                /// adjustment comes from root or item display fixups.
-                pub fn set_adjusted_display(&mut self,
-                                            dpy: longhands::display::computed_value::T,
-                                            is_item_or_root: bool) {
-                    self.set_display(dpy);
+                /// Sets the display property, but without touching original_display,
+                /// except when the adjustment comes from root or item display fixups.
+                pub fn set_adjusted_display(
+                    &mut self,
+                    dpy: longhands::display::computed_value::T,
+                    is_item_or_root: bool
+                ) {
+                    self.display = dpy;
                     if is_item_or_root {
-                        self.set__servo_display_for_hypothetical_box(dpy);
+                        self.original_display = dpy;
                     }
                 }
             % endif
         }
 
     % endfor
 }
 
@@ -3043,16 +3072,19 @@ mod lazy_static_module {
                 % for style_struct in data.active_style_structs():
                     ${style_struct.ident}: Arc::new(style_structs::${style_struct.name} {
                         % for longhand in style_struct.longhands:
                             ${longhand.ident}: longhands::${longhand.ident}::get_initial_value(),
                         % endfor
                         % if style_struct.name == "Font":
                             hash: 0,
                         % endif
+                        % if style_struct.name == "Box":
+                            original_display: longhands::display::get_initial_value(),
+                        % endif
                     }),
                 % endfor
                 custom_properties: None,
                 writing_mode: WritingMode::empty(),
                 rules: None,
                 visited_style: None,
                 flags: ComputedValueFlags::empty(),
             }
--- a/servo/components/style/values/specified/box.rs
+++ b/servo/components/style/values/specified/box.rs
@@ -223,17 +223,16 @@ impl Display {
 
     #[cfg(feature = "servo")]
     #[inline]
     /// Custom cascade for the `display` property in servo
     pub fn cascade_property_custom(
         _declaration: &PropertyDeclaration,
         context: &mut Context
     ) {
-        longhands::_servo_display_for_hypothetical_box::derive_from_display(context);
         longhands::_servo_text_decorations_in_effect::derive_from_display(context);
     }
 }
 
 /// A specified value for the `vertical-align` property.
 pub type VerticalAlign = GenericVerticalAlign<LengthOrPercentage>;
 
 impl Parse for VerticalAlign {