script: Add presentation attributes as part of a single PropertyDeclarationBlock (#38684)

Instead of making a block for each attribute, use a single block as
described in a `FIXME` comment by @emilio. This also switch to using
`map` and `and_then` in more places to make the code a bit more concise.

Testing: This should not change behavior other than to incraese
efficiency a
bit and is thus covered by existing tests.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>

components/script/dom/element.rs

@@ -51,7 +51,7 @@ use style::selector_parser::{
     NonTSPseudoClass, PseudoElement, RestyleDamage, SelectorImpl, SelectorParser,
     extended_filtering,
 };
-use style::shared_lock::{Locked, SharedRwLock};
+use style::shared_lock::Locked;
 use style::stylesheets::layer_rule::LayerOrder;
 use style::stylesheets::{CssRuleType, Origin as CssOrigin, UrlExtraData};
 use style::values::computed::Overflow;
@@ -1368,32 +1368,19 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> {
     where
         V: Push<ApplicableDeclarationBlock>,
     {
-        // FIXME(emilio): Just a single PDB should be enough.
+        let mut property_declaration_block = None;
-        #[inline]
+        let mut push = |declaration| {
-        fn from_declaration(
+            property_declaration_block
-            shared_lock: &SharedRwLock,
+                .get_or_insert_with(PropertyDeclarationBlock::default)
-            declaration: PropertyDeclaration,
+                .push(declaration, Importance::Normal);
-        ) -> ApplicableDeclarationBlock {
+        };
-            ApplicableDeclarationBlock::from_declarations(
-                Arc::new(shared_lock.wrap(PropertyDeclarationBlock::with_one(
-                    declaration,
-                    Importance::Normal,
-                ))),
-                CascadeLevel::PresHints,
-                LayerOrder::root(),
-            )
-        }
-
-        let document = self.upcast::<Node>().owner_doc_for_layout();
-        let shared_lock = document.style_shared_lock();
 
         // TODO(xiaochengh): This is probably not enough. When the root element doesn't have a `lang`,
         // we should check the browser settings and system locale.
         if let Some(lang) = self.get_lang_attr_val_for_layout() {
-            hints.push(from_declaration(
+            push(PropertyDeclaration::XLang(specified::XLang(Atom::from(
-                shared_lock,
+                lang.to_owned(),
-                PropertyDeclaration::XLang(specified::XLang(Atom::from(lang.to_owned()))),
+            ))));
-            ));
         }
 
         let bgcolor = if let Some(this) = self.downcast::<HTMLBodyElement>() {
@@ -1411,24 +1398,19 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> {
         };
 
         if let Some(color) = bgcolor {
-            hints.push(from_declaration(
+            push(PropertyDeclaration::BackgroundColor(
-                shared_lock,
+                specified::Color::from_absolute_color(color),
-                PropertyDeclaration::BackgroundColor(specified::Color::from_absolute_color(color)),
             ));
         }
 
-        let background = if let Some(this) = self.downcast::<HTMLBodyElement>() {
+        let background = self
-            this.get_background()
+            .downcast::<HTMLBodyElement>()
-        } else {
+            .and_then(HTMLBodyElementLayoutHelpers::get_background);
-            None
-        };
-
         if let Some(url) = background {
-            hints.push(from_declaration(
+            push(PropertyDeclaration::BackgroundImage(
-                shared_lock,
+                background_image::SpecifiedValue(
-                PropertyDeclaration::BackgroundImage(background_image::SpecifiedValue(
                     vec![specified::Image::for_cascade(url.get_arc())].into(),
-                )),
+                ),
             ));
         }
 
@@ -1445,61 +1427,41 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> {
         };
 
         if let Some(color) = color {
-            hints.push(from_declaration(
+            push(PropertyDeclaration::Color(
-                shared_lock,
+                longhands::color::SpecifiedValue(specified::Color::from_absolute_color(color)),
-                PropertyDeclaration::Color(longhands::color::SpecifiedValue(
-                    specified::Color::from_absolute_color(color),
-                )),
             ));
         }
 
-        let font_face = if let Some(this) = self.downcast::<HTMLFontElement>() {
+        let font_face = self
-            this.get_face()
+            .downcast::<HTMLFontElement>()
-        } else {
+            .and_then(HTMLFontElementLayoutHelpers::get_face);
-            None
-        };
-
         if let Some(font_face) = font_face {
-            hints.push(from_declaration(
+            push(PropertyDeclaration::FontFamily(
-                shared_lock,
+                font_family::SpecifiedValue::Values(computed::font::FontFamilyList {
-                PropertyDeclaration::FontFamily(font_family::SpecifiedValue::Values(
-                    computed::font::FontFamilyList {
                     list: ArcSlice::from_iter(
                         HTMLFontElement::parse_face_attribute(font_face).into_iter(),
                     ),
-                    },
+                }),
-                )),
             ));
         }
 
         let font_size = self
             .downcast::<HTMLFontElement>()
-            .and_then(|this| this.get_size());
+            .and_then(HTMLFontElementLayoutHelpers::get_size);
-
         if let Some(font_size) = font_size {
-            hints.push(from_declaration(
+            push(PropertyDeclaration::FontSize(
-                shared_lock,
+                font_size::SpecifiedValue::from_html_size(font_size as u8),
-                PropertyDeclaration::FontSize(font_size::SpecifiedValue::from_html_size(
+            ));
-                    font_size as u8,
-                )),
-            ))
         }
 
-        let cellspacing = if let Some(this) = self.downcast::<HTMLTableElement>() {
+        let cellspacing = self
-            this.get_cellspacing()
+            .downcast::<HTMLTableElement>()
-        } else {
+            .and_then(HTMLTableElementLayoutHelpers::get_cellspacing);
-            None
-        };
-
         if let Some(cellspacing) = cellspacing {
             let width_value = specified::Length::from_px(cellspacing as f32);
-            hints.push(from_declaration(
+            push(PropertyDeclaration::BorderSpacing(Box::new(
-                shared_lock,
+                border_spacing::SpecifiedValue::new(width_value.clone().into(), width_value.into()),
-                PropertyDeclaration::BorderSpacing(Box::new(border_spacing::SpecifiedValue::new(
+            )));
-                    width_value.clone().into(),
-                    width_value.into(),
-                ))),
-            ));
         }
 
         // Textual input, specifically text entry and domain specific input has
@@ -1507,30 +1469,29 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> {
         //
         // <https://html.spec.whatwg.org/multipage/#the-input-element-as-a-text-entry-widget>
         // <https://html.spec.whatwg.org/multipage/#the-input-element-as-domain-specific-widgets>
-        let size = if let Some(this) = self.downcast::<HTMLInputElement>() {
+        let size = self
+            .downcast::<HTMLInputElement>()
+            .and_then(|input_element| {
                 // FIXME(pcwalton): More use of atoms, please!
                 match self.get_attr_val_for_layout(&ns!(), &local_name!("type")) {
                     Some("hidden") | Some("range") | Some("color") | Some("checkbox") |
-                Some("radio") | Some("file") | Some("submit") | Some("image") | Some("reset") |
+                    Some("radio") | Some("file") | Some("submit") | Some("image") |
-                Some("button") => None,
+                    Some("reset") | Some("button") => None,
                     // Others
-                _ => match this.size_for_layout() {
+                    _ => match input_element.size_for_layout() {
                         0 => None,
                         s => Some(s as i32),
                     },
                 }
-        } else {
+            });
-            None
-        };
 
         if let Some(size) = size {
             let value =
                 specified::NoCalcLength::ServoCharacterWidth(specified::CharacterWidth(size));
-            hints.push(from_declaration(
+            push(PropertyDeclaration::Width(
-                shared_lock,
+                specified::Size::LengthPercentage(NonNegative(
-                PropertyDeclaration::Width(specified::Size::LengthPercentage(NonNegative(
                     specified::LengthPercentage::Length(value),
-                ))),
+                )),
             ));
         }
 
@@ -1560,10 +1521,7 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> {
                 let width_value = specified::Size::LengthPercentage(NonNegative(
                     specified::LengthPercentage::Percentage(computed::Percentage(percentage)),
                 ));
-                hints.push(from_declaration(
+                push(PropertyDeclaration::Width(width_value));
-                    shared_lock,
-                    PropertyDeclaration::Width(width_value),
-                ));
             },
             LengthOrPercentageOrAuto::Length(length) => {
                 let width_value = specified::Size::LengthPercentage(NonNegative(
@@ -1571,10 +1529,7 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> {
                         specified::AbsoluteLength::Px(length.to_f32_px()),
                     )),
                 ));
-                hints.push(from_declaration(
+                push(PropertyDeclaration::Width(width_value));
-                    shared_lock,
-                    PropertyDeclaration::Width(width_value),
-                ));
             },
         }
 
@@ -1602,10 +1557,7 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> {
                 let height_value = specified::Size::LengthPercentage(NonNegative(
                     specified::LengthPercentage::Percentage(computed::Percentage(percentage)),
                 ));
-                hints.push(from_declaration(
+                push(PropertyDeclaration::Height(height_value));
-                    shared_lock,
-                    PropertyDeclaration::Height(height_value),
-                ));
             },
             LengthOrPercentageOrAuto::Length(length) => {
                 let height_value = specified::Size::LengthPercentage(NonNegative(
@@ -1613,10 +1565,7 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> {
                         specified::AbsoluteLength::Px(length.to_f32_px()),
                     )),
                 ));
-                hints.push(from_declaration(
+                push(PropertyDeclaration::Height(height_value));
-                    shared_lock,
-                    PropertyDeclaration::Height(height_value),
-                ));
             },
         }
 
@@ -1633,24 +1582,17 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> {
                         auto: true,
                         ratio: PreferredRatio::Ratio(Ratio(width_value, height_value)),
                     };
-                    hints.push(from_declaration(
+                    push(PropertyDeclaration::AspectRatio(aspect_ratio));
-                        shared_lock,
-                        PropertyDeclaration::AspectRatio(aspect_ratio),
-                    ));
                 }
             }
         }
 
-        let cols = if let Some(this) = self.downcast::<HTMLTextAreaElement>() {
+        let cols = self
-            match this.get_cols() {
+            .downcast::<HTMLTextAreaElement>()
-                0 => None,
+            .map(LayoutHTMLTextAreaElementHelpers::get_cols);
-                c => Some(c as i32),
-            }
-        } else {
-            None
-        };
-
         if let Some(cols) = cols {
+            let cols = cols as i32;
+            if cols > 0 {
                 // TODO(mttr) ServoCharacterWidth uses the size math for <input type="text">, but
                 // the math for <textarea> is a little different since we need to take
                 // scrollbar size into consideration (but we don't have a scrollbar yet!)
@@ -1658,62 +1600,43 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> {
                 // https://html.spec.whatwg.org/multipage/#textarea-effective-width
                 let value =
                     specified::NoCalcLength::ServoCharacterWidth(specified::CharacterWidth(cols));
-            hints.push(from_declaration(
+                push(PropertyDeclaration::Width(
-                shared_lock,
+                    specified::Size::LengthPercentage(NonNegative(
-                PropertyDeclaration::Width(specified::Size::LengthPercentage(NonNegative(
                         specified::LengthPercentage::Length(value),
-                ))),
+                    )),
                 ));
             }
-
-        let rows = if let Some(this) = self.downcast::<HTMLTextAreaElement>() {
-            match this.get_rows() {
-                0 => None,
-                r => Some(r as i32),
         }
-        } else {
-            None
-        };
 
+        let rows = self
+            .downcast::<HTMLTextAreaElement>()
+            .map(LayoutHTMLTextAreaElementHelpers::get_rows);
         if let Some(rows) = rows {
+            let rows = rows as i32;
+            if rows > 0 {
                 // TODO(mttr) This should take scrollbar size into consideration.
                 //
                 // https://html.spec.whatwg.org/multipage/#textarea-effective-height
-            let value = specified::NoCalcLength::FontRelative(specified::FontRelativeLength::Em(
+                let value = specified::NoCalcLength::FontRelative(
-                rows as CSSFloat,
+                    specified::FontRelativeLength::Em(rows as CSSFloat),
-            ));
+                );
-            hints.push(from_declaration(
+                push(PropertyDeclaration::Height(
-                shared_lock,
+                    specified::Size::LengthPercentage(NonNegative(
-                PropertyDeclaration::Height(specified::Size::LengthPercentage(NonNegative(
                         specified::LengthPercentage::Length(value),
-                ))),
+                    )),
                 ));
             }
+        }
 
-        let border = if let Some(this) = self.downcast::<HTMLTableElement>() {
+        let border = self
-            this.get_border()
+            .downcast::<HTMLTableElement>()
-        } else {
+            .and_then(|table| table.get_border());
-            None
-        };
-
         if let Some(border) = border {
             let width_value = specified::BorderSideWidth::from_px(border as f32);
-            hints.push(from_declaration(
+            push(PropertyDeclaration::BorderTopWidth(width_value.clone()));
-                shared_lock,
+            push(PropertyDeclaration::BorderLeftWidth(width_value.clone()));
-                PropertyDeclaration::BorderTopWidth(width_value.clone()),
+            push(PropertyDeclaration::BorderBottomWidth(width_value.clone()));
-            ));
+            push(PropertyDeclaration::BorderRightWidth(width_value));
-            hints.push(from_declaration(
-                shared_lock,
-                PropertyDeclaration::BorderLeftWidth(width_value.clone()),
-            ));
-            hints.push(from_declaration(
-                shared_lock,
-                PropertyDeclaration::BorderBottomWidth(width_value.clone()),
-            ));
-            hints.push(from_declaration(
-                shared_lock,
-                PropertyDeclaration::BorderRightWidth(width_value),
-            ));
         }
 
         if let Some(cellpadding) = self
@@ -1724,22 +1647,10 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> {
             let cellpadding = NonNegative(specified::LengthPercentage::Length(
                 specified::NoCalcLength::from_px(cellpadding as f32),
             ));
-            hints.push(from_declaration(
+            push(PropertyDeclaration::PaddingTop(cellpadding.clone()));
-                shared_lock,
+            push(PropertyDeclaration::PaddingLeft(cellpadding.clone()));
-                PropertyDeclaration::PaddingTop(cellpadding.clone()),
+            push(PropertyDeclaration::PaddingBottom(cellpadding.clone()));
-            ));
+            push(PropertyDeclaration::PaddingRight(cellpadding));
-            hints.push(from_declaration(
-                shared_lock,
-                PropertyDeclaration::PaddingLeft(cellpadding.clone()),
-            ));
-            hints.push(from_declaration(
-                shared_lock,
-                PropertyDeclaration::PaddingBottom(cellpadding.clone()),
-            ));
-            hints.push(from_declaration(
-                shared_lock,
-                PropertyDeclaration::PaddingRight(cellpadding),
-            ));
         }
 
         // https://html.spec.whatwg.org/multipage/#the-hr-element-2
@@ -1749,39 +1660,33 @@ impl<'dom> LayoutElementHelpers<'dom> for LayoutDom<'dom, Element> {
         {
             match size_info {
                 SizePresentationalHint::SetHeightTo(height) => {
-                    hints.push(from_declaration(
+                    push(PropertyDeclaration::Height(height));
-                        shared_lock,
-                        PropertyDeclaration::Height(height),
-                    ));
                 },
                 SizePresentationalHint::SetAllBorderWidthValuesTo(border_width) => {
-                    hints.push(from_declaration(
+                    push(PropertyDeclaration::BorderLeftWidth(border_width.clone()));
-                        shared_lock,
+                    push(PropertyDeclaration::BorderRightWidth(border_width.clone()));
-                        PropertyDeclaration::BorderLeftWidth(border_width.clone()),
+                    push(PropertyDeclaration::BorderTopWidth(border_width.clone()));
-                    ));
+                    push(PropertyDeclaration::BorderBottomWidth(border_width));
-                    hints.push(from_declaration(
-                        shared_lock,
-                        PropertyDeclaration::BorderRightWidth(border_width.clone()),
-                    ));
-                    hints.push(from_declaration(
-                        shared_lock,
-                        PropertyDeclaration::BorderTopWidth(border_width.clone()),
-                    ));
-                    hints.push(from_declaration(
-                        shared_lock,
-                        PropertyDeclaration::BorderBottomWidth(border_width),
-                    ));
                 },
                 SizePresentationalHint::SetBottomBorderWidthToZero => {
-                    hints.push(from_declaration(
+                    push(PropertyDeclaration::BorderBottomWidth(
-                        shared_lock,
-                        PropertyDeclaration::BorderBottomWidth(
                         specified::border::BorderSideWidth::from_px(0.),
-                        ),
                     ));
                 },
             }
         }
+
+        let Some(property_declaration_block) = property_declaration_block else {
+            return;
+        };
+
+        let document = self.upcast::<Node>().owner_doc_for_layout();
+        let shared_lock = document.style_shared_lock();
+        hints.push(ApplicableDeclarationBlock::from_declarations(
+            Arc::new(shared_lock.wrap(property_declaration_block)),
+            CascadeLevel::PresHints,
+            LayerOrder::root(),
+        ));
     }
 
     fn get_span(self) -> Option<u32> {