From 8dbf9b1da98478905169f0af887b6aa6b705a5ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 13 Mar 2019 19:55:54 +0000 Subject: [PATCH 01/29] Bug 1535084 - Cleanup contain property. r=dholbert Now that cbindgen supports bitflags, this is trivial. Differential Revision: https://phabricator.services.mozilla.com/D23371 --- components/style/cbindgen.toml | 1 + components/style/properties/gecko.mako.rs | 87 +---------------------- components/style/values/specified/box.rs | 13 ++-- 3 files changed, 10 insertions(+), 91 deletions(-) diff --git a/components/style/cbindgen.toml b/components/style/cbindgen.toml index 5de390cc8a6..0a0db7d1615 100644 --- a/components/style/cbindgen.toml +++ b/components/style/cbindgen.toml @@ -103,6 +103,7 @@ include = [ "ZIndex", "TransformOrigin", "WordBreak", + "Contain", ] item_types = ["enums", "structs", "typedefs"] diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 6a4d4d87a99..258c23a7ebf 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -2751,7 +2751,7 @@ fn static_assert() { transform-style rotate scroll-snap-points-x scroll-snap-points-y scroll-snap-coordinate -moz-binding will-change - offset-path shape-outside contain touch-action + offset-path shape-outside touch-action translate scale""" %> <%self:impl_trait style_struct_name="Box" skip_longhands="${skip_box_longhands}"> #[inline] @@ -3171,91 +3171,6 @@ fn static_assert() { <% impl_shape_source("shape_outside", "mShapeOutside") %> - pub fn set_contain(&mut self, v: longhands::contain::computed_value::T) { - use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_NONE; - use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_STRICT; - use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_CONTENT; - use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_SIZE; - use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_LAYOUT; - use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_PAINT; - use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_ALL_BITS; - use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_CONTENT_BITS; - use crate::properties::longhands::contain::SpecifiedValue; - - if v.is_empty() { - self.gecko.mContain = NS_STYLE_CONTAIN_NONE as u8; - return; - } - - if v.contains(SpecifiedValue::STRICT) { - self.gecko.mContain = (NS_STYLE_CONTAIN_STRICT | NS_STYLE_CONTAIN_ALL_BITS) as u8; - return; - } - if v.contains(SpecifiedValue::CONTENT) { - self.gecko.mContain = (NS_STYLE_CONTAIN_CONTENT | NS_STYLE_CONTAIN_CONTENT_BITS) as u8; - return; - } - - let mut bitfield = 0; - if v.contains(SpecifiedValue::LAYOUT) { - bitfield |= NS_STYLE_CONTAIN_LAYOUT; - } - if v.contains(SpecifiedValue::PAINT) { - bitfield |= NS_STYLE_CONTAIN_PAINT; - } - if v.contains(SpecifiedValue::SIZE) { - bitfield |= NS_STYLE_CONTAIN_SIZE; - } - - self.gecko.mContain = bitfield as u8; - } - - pub fn clone_contain(&self) -> longhands::contain::computed_value::T { - use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_STRICT; - use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_CONTENT; - use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_SIZE; - use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_LAYOUT; - use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_PAINT; - use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_ALL_BITS; - use crate::gecko_bindings::structs::NS_STYLE_CONTAIN_CONTENT_BITS; - use crate::properties::longhands::contain::{self, SpecifiedValue}; - - let mut servo_flags = contain::computed_value::T::empty(); - let gecko_flags = self.gecko.mContain; - - if gecko_flags & (NS_STYLE_CONTAIN_STRICT as u8) != 0 { - debug_assert_eq!( - gecko_flags & (NS_STYLE_CONTAIN_ALL_BITS as u8), - NS_STYLE_CONTAIN_ALL_BITS as u8, - "When strict is specified, ALL_BITS should be specified as well" - ); - servo_flags.insert(SpecifiedValue::STRICT | SpecifiedValue::STRICT_BITS); - return servo_flags; - } - if gecko_flags & (NS_STYLE_CONTAIN_CONTENT as u8) != 0 { - debug_assert_eq!( - gecko_flags & (NS_STYLE_CONTAIN_CONTENT_BITS as u8), - NS_STYLE_CONTAIN_CONTENT_BITS as u8, - "When content is specified, CONTENT_BITS should be specified as well" - ); - servo_flags.insert(SpecifiedValue::CONTENT | SpecifiedValue::CONTENT_BITS); - return servo_flags; - } - if gecko_flags & (NS_STYLE_CONTAIN_LAYOUT as u8) != 0 { - servo_flags.insert(SpecifiedValue::LAYOUT); - } - if gecko_flags & (NS_STYLE_CONTAIN_PAINT as u8) != 0 { - servo_flags.insert(SpecifiedValue::PAINT); - } - if gecko_flags & (NS_STYLE_CONTAIN_SIZE as u8) != 0 { - servo_flags.insert(SpecifiedValue::SIZE); - } - - return servo_flags; - } - - ${impl_simple_copy("contain", "mContain")} - ${impl_simple_type_with_conversion("touch_action")} pub fn set_offset_path(&mut self, v: longhands::offset_path::computed_value::T) { diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index d4b66e42ab9..39917659cc9 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -753,18 +753,21 @@ pub fn assert_touch_action_matches() { bitflags! { #[derive(MallocSizeOf, SpecifiedValueInfo, ToComputedValue)] #[value_info(other_values = "none,strict,content,size,layout,paint")] + #[repr(C)] /// Constants for contain: https://drafts.csswg.org/css-contain/#contain-property pub struct Contain: u8 { + /// `none` variant, just for convenience. + const NONE = 0; /// 'size' variant, turns on size containment - const SIZE = 0x01; + const SIZE = 1 << 0; /// `layout` variant, turns on layout containment - const LAYOUT = 0x02; + const LAYOUT = 1 << 1; /// `paint` variant, turns on paint containment - const PAINT = 0x04; + const PAINT = 1 << 2; /// `strict` variant, turns on all types of containment - const STRICT = 0x08; + const STRICT = 1 << 3; /// 'content' variant, turns on layout and paint containment - const CONTENT = 0x10; + const CONTENT = 1 << 4; /// variant with all the bits that contain: strict turns on const STRICT_BITS = Contain::LAYOUT.bits | Contain::PAINT.bits | Contain::SIZE.bits; /// variant with all the bits that contain: content turns on From 95ee1a5adf64d9c87a7e03f2fb7fdd8017ea2f85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 14 Mar 2019 11:47:50 +0000 Subject: [PATCH 02/29] style: Use a single RestyleHint representation. Differential Revision: https://phabricator.services.mozilla.com/D22828 --- components/style/cbindgen.toml | 7 ++ components/style/gecko/wrapper.rs | 8 +- .../invalidation/element/restyle_hints.rs | 73 +------------------ 3 files changed, 11 insertions(+), 77 deletions(-) diff --git a/components/style/cbindgen.toml b/components/style/cbindgen.toml index 0a0db7d1615..f36fd09a243 100644 --- a/components/style/cbindgen.toml +++ b/components/style/cbindgen.toml @@ -104,6 +104,7 @@ include = [ "TransformOrigin", "WordBreak", "Contain", + "RestyleHint", ] item_types = ["enums", "structs", "typedefs"] @@ -197,3 +198,9 @@ item_types = ["enums", "structs", "typedefs"] "GenericBorderRadius" = """ inline const StyleLengthPercentage& Get(mozilla::HalfCorner) const; """ + +"RestyleHint" = """ + static inline StyleRestyleHint RestyleSubtree(); + static inline StyleRestyleHint RecascadeSubtree(); + static inline StyleRestyleHint ForAnimations(); +""" diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index f5058fc63c4..558ee60f4e2 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -44,7 +44,6 @@ use crate::gecko_bindings::bindings::{Gecko_ElementState, Gecko_GetDocumentLWThe use crate::gecko_bindings::bindings::{Gecko_SetNodeFlags, Gecko_UnsetNodeFlags}; use crate::gecko_bindings::structs; use crate::gecko_bindings::structs::nsChangeHint; -use crate::gecko_bindings::structs::nsRestyleHint; use crate::gecko_bindings::structs::Document_DocumentTheme as DocumentTheme; use crate::gecko_bindings::structs::EffectCompositor_CascadeLevel as CascadeLevel; use crate::gecko_bindings::structs::ELEMENT_HANDLED_SNAPSHOT; @@ -58,6 +57,7 @@ use crate::gecko_bindings::structs::{RawGeckoElement, RawGeckoNode, RawGeckoXBLB use crate::gecko_bindings::sugar::ownership::{HasArcFFI, HasSimpleFFI}; use crate::global_style_data::GLOBAL_STYLE_DATA; use crate::hash::FxHashMap; +use crate::invalidation::element::restyle_hints::RestyleHint; use crate::logical_geometry::WritingMode; use crate::media_queries::Device; use crate::properties::animated_properties::{AnimationValue, AnimationValueMap}; @@ -802,11 +802,10 @@ impl<'le> GeckoElement<'le> { /// Also this function schedules style flush. pub unsafe fn note_explicit_hints( &self, - restyle_hint: nsRestyleHint, + restyle_hint: RestyleHint, change_hint: nsChangeHint, ) { use crate::gecko::restyle_damage::GeckoRestyleDamage; - use crate::invalidation::element::restyle_hints::RestyleHint; let damage = GeckoRestyleDamage::new(change_hint); debug!( @@ -814,7 +813,6 @@ impl<'le> GeckoElement<'le> { self, restyle_hint, change_hint ); - let restyle_hint: RestyleHint = restyle_hint.into(); debug_assert!( !(restyle_hint.has_animation_hint() && restyle_hint.has_non_animation_hint()), "Animation restyle hints should not appear with non-animation restyle hints" @@ -1516,7 +1514,7 @@ impl<'le> TElement for GeckoElement<'le> { ); unsafe { self.note_explicit_hints( - nsRestyleHint::eRestyle_Subtree, + RestyleHint::restyle_subtree(), nsChangeHint::nsChangeHint_Empty, ); } diff --git a/components/style/invalidation/element/restyle_hints.rs b/components/style/invalidation/element/restyle_hints.rs index de84fe56c85..9dbd12bab59 100644 --- a/components/style/invalidation/element/restyle_hints.rs +++ b/components/style/invalidation/element/restyle_hints.rs @@ -4,12 +4,11 @@ //! Restyle hints: an optimization to avoid unnecessarily matching selectors. -#[cfg(feature = "gecko")] -use crate::gecko_bindings::structs::nsRestyleHint; use crate::traversal_flags::TraversalFlags; bitflags! { /// The kind of restyle we need to do for a given element. + #[repr(C)] pub struct RestyleHint: u8 { /// Do a selector match of the element. const RESTYLE_SELF = 1 << 0; @@ -190,75 +189,5 @@ impl Default for RestyleHint { } } -#[cfg(feature = "gecko")] -impl From for RestyleHint { - fn from(mut raw: nsRestyleHint) -> Self { - let mut hint = RestyleHint::empty(); - - debug_assert!( - raw.0 & nsRestyleHint::eRestyle_LaterSiblings.0 == 0, - "Handle later siblings manually if necessary plz." - ); - - if (raw.0 & (nsRestyleHint::eRestyle_Self.0 | nsRestyleHint::eRestyle_Subtree.0)) != 0 { - raw.0 &= !nsRestyleHint::eRestyle_Self.0; - hint.insert(RestyleHint::RESTYLE_SELF); - } - - if (raw.0 & (nsRestyleHint::eRestyle_Subtree.0 | nsRestyleHint::eRestyle_SomeDescendants.0)) != - 0 - { - raw.0 &= !nsRestyleHint::eRestyle_Subtree.0; - raw.0 &= !nsRestyleHint::eRestyle_SomeDescendants.0; - hint.insert(RestyleHint::RESTYLE_DESCENDANTS); - } - - if (raw.0 & (nsRestyleHint::eRestyle_ForceDescendants.0 | nsRestyleHint::eRestyle_Force.0)) != - 0 - { - raw.0 &= !nsRestyleHint::eRestyle_Force.0; - hint.insert(RestyleHint::RECASCADE_SELF); - } - - if (raw.0 & nsRestyleHint::eRestyle_ForceDescendants.0) != 0 { - raw.0 &= !nsRestyleHint::eRestyle_ForceDescendants.0; - hint.insert(RestyleHint::RECASCADE_DESCENDANTS); - } - - hint.insert(RestyleHint::from_bits_truncate(raw.0 as u8)); - - hint - } -} - #[cfg(feature = "servo")] malloc_size_of_is_0!(RestyleHint); - -/// Asserts that all replacement hints have a matching nsRestyleHint value. -#[cfg(feature = "gecko")] -#[inline] -pub fn assert_restyle_hints_match() { - use crate::gecko_bindings::structs; - - macro_rules! check_restyle_hints { - ( $( $a:ident => $b:path),*, ) => { - if cfg!(debug_assertions) { - let mut replacements = RestyleHint::replacements(); - $( - assert_eq!(structs::nsRestyleHint::$a.0 as usize, $b.bits() as usize, stringify!($b)); - replacements.remove($b); - )* - assert_eq!(replacements, RestyleHint::empty(), - "all RestyleHint replacement bits should have an \ - assertion"); - } - } - } - - check_restyle_hints! { - eRestyle_CSSTransitions => RestyleHint::RESTYLE_CSS_TRANSITIONS, - eRestyle_CSSAnimations => RestyleHint::RESTYLE_CSS_ANIMATIONS, - eRestyle_StyleAttribute => RestyleHint::RESTYLE_STYLE_ATTRIBUTE, - eRestyle_StyleAttribute_Animations => RestyleHint::RESTYLE_SMIL, - } -} From 9f4643ac24ec6910115830faf3f74ee564fc7e85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 14 Mar 2019 22:30:37 +0000 Subject: [PATCH 03/29] style: Avoid crashing when calling insertRule("@import ...") on a detached sheet. This should unblock the fuzzers for now, though it's not the ideal solution. It's the only reasonably easy solution to unblock them though, I think. We should probably always keep track of the document a stylesheet was associated with. We'll need that for constructible stylesheets anyway. That requires some though on how to get the cycle-collection and such right, though, and I wouldn't be able to write or land that ASAP. Differential Revision: https://phabricator.services.mozilla.com/D23584 --- components/style/stylesheets/rule_parser.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/components/style/stylesheets/rule_parser.rs b/components/style/stylesheets/rule_parser.rs index 44943259b38..a780ebce250 100644 --- a/components/style/stylesheets/rule_parser.rs +++ b/components/style/stylesheets/rule_parser.rs @@ -189,6 +189,13 @@ impl<'a, 'i> AtRuleParser<'i> for TopLevelRuleParser<'a> { return Err(input.new_custom_error(StyleParseErrorKind::UnexpectedImportRule)) } + // FIXME(emilio): We should always be able to have a loader + // around! See bug 1533783. + if self.loader.is_none() { + error!("Saw @import rule, but no way to trigger the load"); + return Err(input.new_custom_error(StyleParseErrorKind::UnexpectedImportRule)) + } + let url_string = input.expect_url_or_string()?.as_ref().to_owned(); let url = CssUrl::parse_from_string(url_string, &self.context); From 77a75596bbe69e6c5df8706835550f4d3d7e8002 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 12 Mar 2019 00:48:27 +0000 Subject: [PATCH 04/29] style: Don't call PrefillDefaultForGeneric when inheriting font-family. This is the low-risk fix for this issue, that we should get into 67. What's going on here is that font-family is tracked via the font list, from the POV of the style system the font list is generally just the RefPtr, but in Gecko there's also mDefaultGenericId. The way we end up with the right mDefaultGenericId is fishy at best, bogus at worst. I left various fixmes over time related to a bunch of this code. After my patch, we end up with a mDefaultGenericId of serif, rather than the right one (none). The parent font always has none because nsLayoutUtils::ComputeSystemFont always sets it to none if the font is known. Before my patch, PrefillDefaultForGeneric with aGenericId of none (from the parent), which makes it the default generic id for the current language, serif in this case. Before my optimization, apply_declaration_ignoring_phase called copy_font_family_from, which resets both the font list _and_ the default generic. This patch achieves the same effect by not having the first mutation in the first place. This code is still terribly fishy in any case, all the _skip_font_family stuff is just ridiculous. I'll try to clean up a bit after this, but for 68. Differential Revision: https://phabricator.services.mozilla.com/D23026 --- components/style/properties/cascade.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/properties/cascade.rs b/components/style/properties/cascade.rs index 23970d64888..60aa315b62c 100644 --- a/components/style/properties/cascade.rs +++ b/components/style/properties/cascade.rs @@ -744,7 +744,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { // separately, so this boils down to just copying over the // font-family lists (no other aspect of the default font can be // configured). - if self.seen.contains(LonghandId::XLang) || self.seen.contains(LonghandId::FontFamily) { + if self.seen.contains(LonghandId::XLang) || font_family.is_some() { // If just the language changed, the inherited generic is all we // need. let mut generic = self.context.builder.get_parent_font().gecko().mGenericID; From aa5ea337da9a25dd6caf62c787d96f15fc28e908 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 18 Mar 2019 15:37:03 +0000 Subject: [PATCH 05/29] style: Try to bring some more sanity into our font code. It's not very easy to understand on its current state, and it causes subtle bugs like bug 1533654. It could be simpler if we centralized where the interactions between properties are handled. This patch does this. This patch also changes how MathML script sizes are tracked when scriptlevel changes and they have relative fonts in between. With this patch, any explicitly specified font-size is treated the same (being a scriptlevel boundary), regardless of whether it's either an absolute size, a relative size, or a wide keyword. Relative lengths always resolve relative to the constrained size, which allows us to avoid the double font-size computation, and not give up on sanity with keyword font-sizes. I think given no other browser supports scriptlevel it seems like the right trade-off. Differential Revision: https://phabricator.services.mozilla.com/D23070 --- components/style/properties/cascade.rs | 379 ++++++++++-------- components/style/properties/gecko.mako.rs | 263 +++--------- components/style/properties/helpers.mako.rs | 14 +- .../style/properties/longhands/font.mako.rs | 13 +- .../style/properties/properties.mako.rs | 2 +- components/style/values/computed/font.rs | 63 +-- components/style/values/specified/font.rs | 49 +-- components/style/values/specified/length.rs | 5 - 8 files changed, 290 insertions(+), 498 deletions(-) diff --git a/components/style/properties/cascade.rs b/components/style/properties/cascade.rs index 60aa315b62c..638e36e24d4 100644 --- a/components/style/properties/cascade.rs +++ b/components/style/properties/cascade.rs @@ -384,8 +384,6 @@ struct Cascade<'a, 'b: 'a> { cascade_mode: CascadeMode<'a>, seen: LonghandIdSet, reverted: PerOrigin, - saved_font_size: Option, - saved_font_family: Option, } impl<'a, 'b: 'a> Cascade<'a, 'b> { @@ -395,8 +393,6 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { cascade_mode, seen: LonghandIdSet::default(), reverted: Default::default(), - saved_font_size: None, - saved_font_family: None, } } @@ -424,33 +420,8 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { )) } - fn apply_declaration( - &mut self, - longhand_id: LonghandId, - declaration: &PropertyDeclaration, - ) { - // FIXME(emilio): Find a cleaner abstraction for this. - // - // font-size and font-family are special because in Gecko they're - // they're dependent on other early props, like lang and - // -moz-min-font-size-ratio. This sucks a bit, we should ideally - // move the font-size computation code somewhere else... - if Phase::is_early() { - if longhand_id == LonghandId::FontSize { - self.saved_font_size = Some(declaration.clone()); - return; - } - if longhand_id == LonghandId::FontFamily { - self.saved_font_family = Some(declaration.clone()); - return; - } - } - - self.apply_declaration_ignoring_phase(longhand_id, declaration); - } - #[inline(always)] - fn apply_declaration_ignoring_phase( + fn apply_declaration( &mut self, longhand_id: LonghandId, declaration: &PropertyDeclaration, @@ -577,7 +548,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { } if Phase::is_early() { - self.fixup_font_and_apply_saved_font_properties(); + self.fixup_font_stuff(); self.compute_writing_mode(); } else { self.finished_applying_properties(); @@ -702,142 +673,222 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { true } - // FIXME(emilio): It'd be really nice to simplify all this, somehow. This is - // very annoying code in lots of ways, and there are various bits about it - // which I think are broken or could be improved, see the various FIXMEs - // below. - fn fixup_font_and_apply_saved_font_properties(&mut self) { - let font_family = self.saved_font_family.take(); - let font_size = self.saved_font_size.take(); - let mut _skip_font_family = false; + // The default font type (which is stored in FontFamilyList's + // `mDefaultFontType`) depends on the current lang group, so + // we may need to recompute it if it changed. + #[inline] + #[cfg(feature = "gecko")] + fn recompute_default_font_family_type_if_needed(&mut self) { + use crate::gecko_bindings::{bindings, structs}; + if !self.seen.contains(LonghandId::XLang) && + !self.seen.contains(LonghandId::FontFamily) { + return; + } + + let builder = &mut self.context.builder; + let default_font_type = { + let font = builder.get_font().gecko(); + let default_font_type = if font.mFont.systemFont { + structs::FontFamilyType::eFamily_none + } else { + unsafe { + bindings::Gecko_nsStyleFont_ComputeDefaultFontType( + builder.device.document(), + font.mGenericID, + font.mLanguage.mRawPtr, + ) + } + }; + + if font.mFont.fontlist.mDefaultFontType == default_font_type { + return; + } + + default_font_type + }; + + builder.mutate_font().gecko_mut().mFont.fontlist.mDefaultFontType = + default_font_type; + } + + /// Some keyword sizes depend on the font family and language. + #[cfg(feature = "gecko")] + fn recompute_keyword_font_size_if_needed(&mut self) { + use crate::values::computed::ToComputedValue; + use crate::values::specified; + + if !self.seen.contains(LonghandId::XLang) && + !self.seen.contains(LonghandId::FontFamily) { + return; + } + + let new_size = { + let font = self.context.builder.get_font(); + let new_size = match font.clone_font_size().keyword_info { + Some(info) => { + self.context.for_non_inherited_property = None; + specified::FontSize::Keyword(info).to_computed_value(self.context) + } + None => return, + }; + + if font.gecko().mScriptUnconstrainedSize == new_size.size().0 { + return; + } + + new_size + }; + + self.context.builder.mutate_font().set_font_size(new_size); + } + + /// Some properties, plus setting font-size itself, may make us go out of + /// our minimum font-size range. + #[cfg(feature = "gecko")] + fn constrain_font_size_if_needed(&mut self) { + use crate::gecko_bindings::bindings; + + if !self.seen.contains(LonghandId::XLang) && + !self.seen.contains(LonghandId::FontFamily) && + !self.seen.contains(LonghandId::MozMinFontSizeRatio) && + !self.seen.contains(LonghandId::FontSize) { + return; + } + + let builder = &mut self.context.builder; + let min_font_size = { + let font = builder.get_font().gecko(); + let min_font_size = unsafe { + bindings::Gecko_nsStyleFont_ComputeMinSize( + font, + builder.device.document(), + ) + }; + + if font.mFont.size >= min_font_size { + return; + } + + min_font_size + }; + + builder.mutate_font().gecko_mut().mFont.size = min_font_size; + } + + /// is not affected by text zoom, and it uses a preshint + /// to disable it. We fix up the struct when this happens by + /// unzooming its contained font values, which will have been zoomed + /// in the parent. + /// + /// FIXME(emilio): Also, why doing this _before_ handling font-size? That + /// sounds wrong. + #[cfg(feature = "gecko")] + fn unzoom_fonts_if_needed(&mut self) { + if !self.seen.contains(LonghandId::XTextZoom) { + return; + } + + let builder = &mut self.context.builder; + + let parent_zoom = builder.get_parent_font().gecko().mAllowZoom; + let zoom = builder.get_font().gecko().mAllowZoom; + if zoom == parent_zoom { + return; + } + debug_assert!( + !zoom, + "We only ever disable text zoom (in svg:text), never enable it" + ); + let device = builder.device; + builder.mutate_font().unzoom_fonts(device); + } + + /// MathML script* attributes do some very weird shit with font-size. + /// + /// Handle them specially here, separate from other font-size stuff. + /// + /// How this should interact with lang="" and font-family-dependent sizes is + /// not clear to me. For now just pretend those don't exist here. + #[cfg(feature = "gecko")] + fn handle_mathml_scriptlevel_if_needed(&mut self) { + use app_units::Au; + use std::cmp; + + if !self.seen.contains(LonghandId::MozScriptLevel) && + !self.seen.contains(LonghandId::MozScriptMinSize) && + !self.seen.contains(LonghandId::MozScriptSizeMultiplier) { + return; + } + + // If the user specifies a font-size, just let it be. + if self.seen.contains(LonghandId::FontSize) { + return; + } + + let builder = &mut self.context.builder; + let (new_size, new_unconstrained_size) = { + let font = builder.get_font().gecko(); + let parent_font = builder.get_parent_font().gecko(); + + let delta = + font.mScriptLevel.saturating_sub(parent_font.mScriptLevel); + + if delta == 0 { + return; + } + + let mut min = Au(parent_font.mScriptMinSize); + if font.mAllowZoom { + min = builder.device.zoom_text(min); + } + + let scale = (parent_font.mScriptSizeMultiplier as f32).powi(delta as i32); + let parent_size = Au(parent_font.mSize); + let parent_unconstrained_size = Au(parent_font.mScriptUnconstrainedSize); + let new_size = parent_size.scale_by(scale); + let new_unconstrained_size = parent_unconstrained_size.scale_by(scale); + + if scale <= 1. { + // The parent size can be smaller than scriptminsize, e.g. if it + // was specified explicitly. Don't scale in this case, but we + // don't want to set it to scriptminsize either since that will + // make it larger. + if parent_size <= min { + (parent_size, new_unconstrained_size) + } else { + (cmp::max(min, new_size), new_unconstrained_size) + } + } else { + // If the new unconstrained size is larger than the min size, + // this means we have escaped the grasp of scriptminsize and can + // revert to using the unconstrained size. + // However, if the new size is even larger (perhaps due to usage + // of em units), use that instead. + ( + cmp::min(new_size, cmp::max(new_unconstrained_size, min)), + new_unconstrained_size + ) + } + }; + let font = builder.mutate_font().gecko_mut(); + font.mFont.size = new_size.0; + font.mSize = new_size.0; + font.mScriptUnconstrainedSize = new_unconstrained_size.0; + } + + /// Various properties affect how font-size and font-family are computed. + /// + /// These need to be handled here, since relative lengths and ex / ch units + /// for late properties depend on these. + fn fixup_font_stuff(&mut self) { #[cfg(feature = "gecko")] { - // is not affected by text zoom, and it uses a preshint - // to disable it. We fix up the struct when this happens by - // unzooming its contained font values, which will have been zoomed - // in the parent. - // - // FIXME(emilio): Could be cleaner if we just removed this property - // and made a style adjustment o something like that. - if self.seen.contains(LonghandId::XTextZoom) { - let builder = &mut self.context.builder; - - let parent_zoom = builder.get_parent_font().gecko().mAllowZoom; - let zoom = builder.get_font().gecko().mAllowZoom; - if zoom != parent_zoom { - debug_assert!( - !zoom, - "We only ever disable text zoom (in svg:text), never enable it" - ); - let device = builder.device; - builder.mutate_font().unzoom_fonts(device); - } - } - - // Whenever a single generic value is specified, Gecko used to do a - // bunch of recalculation walking up the rule tree, including - // handling the font-size stuff. - // - // It basically repopulated the font struct with the default font - // for a given generic and language. We handle the font-size stuff - // separately, so this boils down to just copying over the - // font-family lists (no other aspect of the default font can be - // configured). - if self.seen.contains(LonghandId::XLang) || font_family.is_some() { - // If just the language changed, the inherited generic is all we - // need. - let mut generic = self.context.builder.get_parent_font().gecko().mGenericID; - - // FIXME(emilio): Isn't this bogus for CSS wide keywords like - // reset or such? - if let Some(ref declaration) = font_family { - if let PropertyDeclaration::FontFamily(ref fam) = *declaration { - if let Some(id) = fam.single_generic() { - generic = id; - // In case of a specified font family with a single - // generic, we will end up setting font family - // below, but its value would get overwritten later - // in the pipeline when cascading. - // - // We instead skip cascading font-family in that - // case. - // - // In case of the language changing, we wish for a - // specified font-family to override this, so we do - // not skip cascading then. - _skip_font_family = true; - } - } - } - - // FIXME(emilio): Why both setting the generic and passing it - // down? - let doc = self.context.builder.device.document(); - let gecko_font = self.context.builder.mutate_font().gecko_mut(); - gecko_font.mGenericID = generic; - unsafe { - crate::gecko_bindings::bindings::Gecko_nsStyleFont_PrefillDefaultForGeneric( - gecko_font, - doc, - generic, - ); - } - } - } - - // It is important that font-size is computed before the late - // properties (for em units), but after font-family (for the - // base-font-size dependence for default and keyword font-sizes). - // - // It's important that font-family comes after the other font properties - // to support system fonts. - // - // NOTE(emilio): I haven't verified that comment, but it was there. - // Verify, and if it's false make font-size the only weird property? - if !_skip_font_family { - if let Some(ref declaration) = font_family { - self.apply_declaration_ignoring_phase(LonghandId::FontFamily, declaration); - #[cfg(feature = "gecko")] - { - let context = &mut self.context; - let device = context.builder.device; - if let PropertyDeclaration::FontFamily(ref val) = *declaration { - if val.get_system().is_some() { - let default = context - .cached_system_font - .as_ref() - .unwrap() - .default_font_type; - context.builder.mutate_font().fixup_system(default); - } else { - context.builder.mutate_font().fixup_none_generic(device); - } - } - } - } - } - - if let Some(declaration) = font_size { - self.apply_declaration_ignoring_phase(LonghandId::FontSize, &declaration); - } else { - #[cfg(feature = "gecko")] - { - if self.seen.contains(LonghandId::XLang) || - self.seen.contains(LonghandId::MozScriptLevel) || - self.seen.contains(LonghandId::MozMinFontSizeRatio) || - self.seen.contains(LonghandId::FontFamily) - { - use crate::values::computed::FontSize; - - // font-size must be explicitly inherited to handle lang - // changes and scriptlevel changes. - // - // FIXME(emilio): That looks a bit bogus... - self.context.for_non_inherited_property = None; - FontSize::cascade_inherit_font_size(&mut self.context); - } - } + self.unzoom_fonts_if_needed(); + self.recompute_default_font_family_type_if_needed(); + self.recompute_keyword_font_size_if_needed(); + self.handle_mathml_scriptlevel_if_needed(); + self.constrain_font_size_if_needed() } } } diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 258c23a7ebf..30369dff62d 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1971,25 +1971,19 @@ fn static_assert() { <% impl_font_settings("font_feature_settings", "gfxFontFeature", "FeatureTagValue", "i32", "u32") %> <% impl_font_settings("font_variation_settings", "gfxFontVariation", "VariationValue", "f32", "f32") %> - pub fn fixup_none_generic(&mut self, device: &Device) { - self.gecko.mFont.systemFont = false; - unsafe { - bindings::Gecko_nsStyleFont_FixupNoneGeneric(&mut self.gecko, device.document()) - } - } - - pub fn fixup_system(&mut self, default_font_type: structs::FontFamilyType) { - self.gecko.mFont.systemFont = true; - self.gecko.mGenericID = structs::kGenericFont_NONE; - self.gecko.mFont.fontlist.mDefaultFontType = default_font_type; - } - pub fn set_font_family(&mut self, v: longhands::font_family::computed_value::T) { - self.gecko.mGenericID = structs::kGenericFont_NONE; - if let Some(generic) = v.0.single_generic() { - self.gecko.mGenericID = generic; - } - self.gecko.mFont.fontlist.mFontlist.mBasePtr.set_move((v.0).0.clone()); + use crate::gecko_bindings::structs::FontFamilyType; + + let is_system_font = v.is_system_font; + self.gecko.mFont.systemFont = is_system_font; + self.gecko.mGenericID = if is_system_font { + structs::kGenericFont_NONE + } else { + v.families.single_generic().unwrap_or(structs::kGenericFont_NONE) + }; + self.gecko.mFont.fontlist.mFontlist.mBasePtr.set_move(v.families.0.clone()); + // Fixed-up if needed in Cascade::fixup_font_stuff. + self.gecko.mFont.fontlist.mDefaultFontType = FontFamilyType::eFamily_none; } pub fn copy_font_family_from(&mut self, other: &Self) { @@ -2009,7 +2003,7 @@ fn static_assert() { let fontlist = &self.gecko.mFont.fontlist; let shared_fontlist = unsafe { fontlist.mFontlist.mBasePtr.to_safe() }; - if shared_fontlist.mNames.is_empty() { + let families = if shared_fontlist.mNames.is_empty() { let default = fontlist.mDefaultFontType; let default = match default { FontFamilyType::eFamily_serif => { @@ -2030,9 +2024,14 @@ fn static_assert() { SingleFontFamily::Generic(atom!("sans-serif")) } }; - FontFamily(FontFamilyList::new(Box::new([default]))) + FontFamilyList::new(Box::new([default])) } else { - FontFamily(FontFamilyList(shared_fontlist)) + FontFamilyList(shared_fontlist) + }; + + FontFamily { + families, + is_system_font: self.gecko.mFont.systemFont, } } @@ -2042,10 +2041,32 @@ fn static_assert() { self.gecko.mFont.size = device.unzoom_text(Au(self.gecko.mFont.size)).0; } + pub fn copy_font_size_from(&mut self, other: &Self) { + self.gecko.mScriptUnconstrainedSize = other.gecko.mScriptUnconstrainedSize; + + self.gecko.mSize = other.gecko.mScriptUnconstrainedSize; + self.gecko.mFont.size = other.gecko.mSize; + self.gecko.mFontSizeKeyword = other.gecko.mFontSizeKeyword; + + // TODO(emilio): Should we really copy over these two? + self.gecko.mFontSizeFactor = other.gecko.mFontSizeFactor; + self.gecko.mFontSizeOffset = other.gecko.mFontSizeOffset; + } + + pub fn reset_font_size(&mut self, other: &Self) { + self.copy_font_size_from(other) + } + pub fn set_font_size(&mut self, v: FontSize) { use crate::values::generics::font::KeywordSize; - self.gecko.mSize = v.size().0; - self.gecko.mScriptUnconstrainedSize = v.size().0; + + let size = v.size(); + self.gecko.mScriptUnconstrainedSize = size.0; + + // These two may be changed from Cascade::fixup_font_stuff. + self.gecko.mSize = size.0; + self.gecko.mFont.size = size.0; + if let Some(info) = v.keyword_info { self.gecko.mFontSizeKeyword = match info.kw { KeywordSize::XXSmall => structs::NS_STYLE_FONT_SIZE_XXSMALL, @@ -2066,196 +2087,6 @@ fn static_assert() { } } - /// Set font size, taking into account scriptminsize and scriptlevel - /// Returns Some(size) if we have to recompute the script unconstrained size - pub fn apply_font_size( - &mut self, - v: FontSize, - parent: &Self, - device: &Device, - ) -> Option { - let (adjusted_size, adjusted_unconstrained_size) = - self.calculate_script_level_size(parent, device); - // In this case, we have been unaffected by scriptminsize, ignore it - if parent.gecko.mSize == parent.gecko.mScriptUnconstrainedSize && - adjusted_size == adjusted_unconstrained_size { - self.set_font_size(v); - self.fixup_font_min_size(device); - None - } else { - self.gecko.mSize = v.size().0; - self.fixup_font_min_size(device); - Some(Au(parent.gecko.mScriptUnconstrainedSize).into()) - } - } - - pub fn fixup_font_min_size(&mut self, device: &Device) { - unsafe { bindings::Gecko_nsStyleFont_FixupMinFontSize(&mut self.gecko, device.document()) } - } - - pub fn apply_unconstrained_font_size(&mut self, v: NonNegativeLength) { - self.gecko.mScriptUnconstrainedSize = v.0.to_i32_au(); - } - - /// Calculates the constrained and unconstrained font sizes to be inherited - /// from the parent. - /// - /// This is a port of Gecko's old ComputeScriptLevelSize function: - /// https://searchfox.org/mozilla-central/rev/c05d9d61188d32b8/layout/style/nsRuleNode.cpp#3103 - /// - /// scriptlevel is a property that affects how font-size is inherited. If scriptlevel is - /// +1, for example, it will inherit as the script size multiplier times - /// the parent font. This does not affect cases where the font-size is - /// explicitly set. - /// - /// However, this transformation is not allowed to reduce the size below - /// scriptminsize. If this inheritance will reduce it to below - /// scriptminsize, it will be set to scriptminsize or the parent size, - /// whichever is smaller (the parent size could be smaller than the min size - /// because it was explicitly specified). - /// - /// Now, within a node that has inherited a font-size which was - /// crossing scriptminsize once the scriptlevel was applied, a negative - /// scriptlevel may be used to increase the size again. - /// - /// This should work, however if we have already been capped by the - /// scriptminsize multiple times, this can lead to a jump in the size. - /// - /// For example, if we have text of the form: - /// - /// huge large medium small tiny reallytiny tiny small medium huge - /// - /// which is represented by progressive nesting and scriptlevel values of - /// +1 till the center after which the scriptlevel is -1, the "tiny"s should - /// be the same size, as should be the "small"s and "medium"s, etc. - /// - /// However, if scriptminsize kicked it at around "medium", then - /// medium/tiny/reallytiny will all be the same size (the min size). - /// A -1 scriptlevel change after this will increase the min size by the - /// multiplier, making the second tiny larger than medium. - /// - /// Instead, we wish for the second "tiny" to still be capped by the script - /// level, and when we reach the second "large", it should be the same size - /// as the original one. - /// - /// We do this by cascading two separate font sizes. The font size (mSize) - /// is the actual displayed font size. The unconstrained font size - /// (mScriptUnconstrainedSize) is the font size in the situation where - /// scriptminsize never applied. - /// - /// We calculate the proposed inherited font size based on scriptlevel and - /// the parent unconstrained size, instead of using the parent font size. - /// This is stored in the node's unconstrained size and will also be stored - /// in the font size provided that it is above the min size. - /// - /// All of this only applies when inheriting. When the font size is - /// manually set, scriptminsize does not apply, and both the real and - /// unconstrained size are set to the explicit value. However, if the font - /// size is manually set to an em or percent unit, the unconstrained size - /// will be set to the value of that unit computed against the parent - /// unconstrained size, whereas the font size will be set computing against - /// the parent font size. - pub fn calculate_script_level_size(&self, parent: &Self, device: &Device) -> (Au, Au) { - use std::cmp; - - let delta = self.gecko.mScriptLevel.saturating_sub(parent.gecko.mScriptLevel); - - let parent_size = Au(parent.gecko.mSize); - let parent_unconstrained_size = Au(parent.gecko.mScriptUnconstrainedSize); - - if delta == 0 { - return (parent_size, parent_unconstrained_size) - } - - - let mut min = Au(parent.gecko.mScriptMinSize); - if self.gecko.mAllowZoom { - min = device.zoom_text(min); - } - - let scale = (parent.gecko.mScriptSizeMultiplier as f32).powi(delta as i32); - - let new_size = parent_size.scale_by(scale); - let new_unconstrained_size = parent_unconstrained_size.scale_by(scale); - - if scale < 1. { - // The parent size can be smaller than scriptminsize, - // e.g. if it was specified explicitly. Don't scale - // in this case, but we don't want to set it to scriptminsize - // either since that will make it larger. - if parent_size < min { - (parent_size, new_unconstrained_size) - } else { - (cmp::max(min, new_size), new_unconstrained_size) - } - } else { - // If the new unconstrained size is larger than the min size, - // this means we have escaped the grasp of scriptminsize - // and can revert to using the unconstrained size. - // However, if the new size is even larger (perhaps due to usage - // of em units), use that instead. - (cmp::min(new_size, cmp::max(new_unconstrained_size, min)), - new_unconstrained_size) - } - } - - /// This function will also handle scriptminsize and scriptlevel - /// so should not be called when you just want the font sizes to be copied. - /// Hence the different name. - pub fn inherit_font_size_from(&mut self, parent: &Self, - kw_inherited_size: Option, - device: &Device) { - let (adjusted_size, adjusted_unconstrained_size) - = self.calculate_script_level_size(parent, device); - if adjusted_size.0 != parent.gecko.mSize || - adjusted_unconstrained_size.0 != parent.gecko.mScriptUnconstrainedSize { - // FIXME(Manishearth): This is incorrect. When there is both a - // keyword size being inherited and a scriptlevel change, we must - // handle the keyword size the same way we handle em units. This - // complicates things because we now have to keep track of the - // adjusted and unadjusted ratios in the kw font size. This only - // affects the use case of a generic font being used in MathML. - // - // If we were to fix this I would prefer doing it not doing - // something like the ruletree walk that Gecko used to do in - // nsRuleNode::SetGenericFont and instead using extra bookkeeping in - // the mSize and mScriptUnconstrainedSize values, and reusing those - // instead of font_size_keyword. - - // In the case that MathML has given us an adjusted size, apply it. - // Keep track of the unconstrained adjusted size. - self.gecko.mSize = adjusted_size.0; - - // Technically the MathML constrained size may also be keyword-derived - // but we ignore this since it would be too complicated - // to correctly track and it's mostly unnecessary. - self.gecko.mFontSizeKeyword = structs::NS_STYLE_FONT_SIZE_NO_KEYWORD as u8; - self.gecko.mFontSizeFactor = 1.; - self.gecko.mFontSizeOffset = 0; - - self.gecko.mScriptUnconstrainedSize = adjusted_unconstrained_size.0; - } else if let Some(size) = kw_inherited_size { - // Parent element was a keyword-derived size. - self.gecko.mSize = size.0.to_i32_au(); - // Copy keyword info over. - self.gecko.mFontSizeFactor = parent.gecko.mFontSizeFactor; - self.gecko.mFontSizeOffset = parent.gecko.mFontSizeOffset; - self.gecko.mFontSizeKeyword = parent.gecko.mFontSizeKeyword; - // MathML constraints didn't apply here, so we can ignore this. - self.gecko.mScriptUnconstrainedSize = size.0.to_i32_au(); - } else { - // MathML isn't affecting us, and our parent element does not - // have a keyword-derived size. Set things normally. - self.gecko.mSize = parent.gecko.mSize; - // copy keyword info over - self.gecko.mFontSizeKeyword = structs::NS_STYLE_FONT_SIZE_NO_KEYWORD as u8; - self.gecko.mFontSizeFactor = 1.; - self.gecko.mFontSizeOffset = 0; - self.gecko.mScriptUnconstrainedSize = parent.gecko.mScriptUnconstrainedSize; - } - self.fixup_font_min_size(device); - } - pub fn clone_font_size(&self) -> FontSize { use crate::values::generics::font::{KeywordInfo, KeywordSize}; let size = Au(self.gecko.mSize).into(); @@ -2270,16 +2101,16 @@ fn static_assert() { structs::NS_STYLE_FONT_SIZE_XXXLARGE => KeywordSize::XXXLarge, structs::NS_STYLE_FONT_SIZE_NO_KEYWORD => { return FontSize { - size: size, + size, keyword_info: None, } } _ => unreachable!("mFontSizeKeyword should be an absolute keyword or NO_KEYWORD") }; FontSize { - size: size, + size, keyword_info: Some(KeywordInfo { - kw: kw, + kw, factor: self.gecko.mFontSizeFactor, offset: Au(self.gecko.mFontSizeOffset).into() }) diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 81931bc713b..b48f0afbf00 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -333,11 +333,7 @@ % if not property.style_struct.inherited: debug_assert!(false, "Should be handled in apply_properties"); % else: - % if property.name == "font-size": - computed::FontSize::cascade_initial_font_size(context); - % else: context.builder.reset_${property.ident}(); - % endif % endif }, % if property.style_struct.inherited: @@ -394,15 +390,7 @@ % else: let computed = specified_value.to_computed_value(context); % endif - % if property.ident == "font_size": - specified::FontSize::cascade_specified_font_size( - context, - &specified_value, - computed, - ); - % else: - context.builder.set_${property.ident}(computed) - % endif + context.builder.set_${property.ident}(computed) % endif } diff --git a/components/style/properties/longhands/font.mako.rs b/components/style/properties/longhands/font.mako.rs index 047ec90c480..88369ab9f21 100644 --- a/components/style/properties/longhands/font.mako.rs +++ b/components/style/properties/longhands/font.mako.rs @@ -379,7 +379,7 @@ ${helpers.predefined_type( use crate::gecko_bindings::structs::{LookAndFeel_FontID, nsFont}; use std::mem; use crate::values::computed::Percentage; - use crate::values::computed::font::{FontSize, FontStretch, FontStyle, FontFamilyList}; + use crate::values::computed::font::{FontFamily, FontSize, FontStretch, FontStyle, FontFamilyList}; use crate::values::generics::NonNegative; let id = match *self { @@ -405,11 +405,12 @@ ${helpers.predefined_type( }))); let font_style = FontStyle::from_gecko(system.style); let ret = ComputedSystemFont { - font_family: longhands::font_family::computed_value::T( - FontFamilyList( - unsafe { system.fontlist.mFontlist.mBasePtr.to_safe() } - ) - ), + font_family: FontFamily { + families: FontFamilyList(unsafe { + system.fontlist.mFontlist.mBasePtr.to_safe() + }), + is_system_font: true, + }, font_size: FontSize { size: Au(system.size).into(), keyword_info: None diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 7e538c4a662..4df99668d18 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -3470,7 +3470,7 @@ impl<'a> StyleBuilder<'a> { % endif ); } - % elif property.name != "font-size": + % else: /// Reset `${property.ident}` to the initial value. #[allow(non_snake_case)] pub fn reset_${property.ident}(&mut self) { diff --git a/components/style/values/computed/font.rs b/components/style/values/computed/font.rs index 765efd9a0d8..6bbcf23bc14 100644 --- a/components/style/values/computed/font.rs +++ b/components/style/values/computed/font.rs @@ -152,49 +152,6 @@ impl FontSize { keyword_info: Some(KeywordInfo::medium()), } } - - /// FIXME(emilio): This is very complex. Also, it should move to - /// StyleBuilder. - pub fn cascade_inherit_font_size(context: &mut Context) { - // If inheriting, we must recompute font-size in case of language - // changes using the font_size_keyword. We also need to do this to - // handle mathml scriptlevel changes - let kw_inherited_size = context - .builder - .get_parent_font() - .clone_font_size() - .keyword_info - .map(|info| { - specified::FontSize::Keyword(info) - .to_computed_value(context) - .size - }); - let mut font = context.builder.take_font(); - font.inherit_font_size_from( - context.builder.get_parent_font(), - kw_inherited_size, - context.builder.device, - ); - context.builder.put_font(font); - } - - /// Cascade the initial value for the `font-size` property. - /// - /// FIXME(emilio): This is the only function that is outside of the - /// `StyleBuilder`, and should really move inside! - /// - /// Can we move the font stuff there? - pub fn cascade_initial_font_size(context: &mut Context) { - // font-size's default ("medium") does not always - // compute to the same value and depends on the font - let computed = specified::FontSize::medium().to_computed_value(context); - context.builder.mutate_font().set_font_size(computed); - #[cfg(feature = "gecko")] - { - let device = context.builder.device; - context.builder.mutate_font().fixup_font_min_size(device); - } - } } /// XXXManishearth it might be better to @@ -221,15 +178,23 @@ impl ToAnimatedValue for FontSize { #[derive(Clone, Debug, Eq, Hash, PartialEq)] #[cfg_attr(feature = "servo", derive(MallocSizeOf))] /// Specifies a prioritized list of font family names or generic family names. -pub struct FontFamily(pub FontFamilyList); +pub struct FontFamily { + /// The actual list of family names. + pub families: FontFamilyList, + /// Whether this font-family came from a specified system-font. + pub is_system_font: bool, +} impl FontFamily { #[inline] /// Get default font family as `serif` which is a generic font-family pub fn serif() -> Self { - FontFamily(FontFamilyList::new(Box::new([SingleFontFamily::Generic( - atom!("serif"), - )]))) + FontFamily { + families: FontFamilyList::new(Box::new([SingleFontFamily::Generic( + atom!("serif"), + )])), + is_system_font: false, + } } } @@ -239,7 +204,7 @@ impl MallocSizeOf for FontFamily { // SharedFontList objects are generally shared from the pointer // stored in the specified value. So only count this if the // SharedFontList is unshared. - unsafe { bindings::Gecko_SharedFontList_SizeOfIncludingThisIfUnshared((self.0).0.get()) } + unsafe { bindings::Gecko_SharedFontList_SizeOfIncludingThisIfUnshared(self.families.0.get()) } } } @@ -248,7 +213,7 @@ impl ToCss for FontFamily { where W: fmt::Write, { - let mut iter = self.0.iter(); + let mut iter = self.families.iter(); iter.next().unwrap().to_css(dest)?; for family in iter { dest.write_str(", ")?; diff --git a/components/style/values/specified/font.rs b/components/style/values/specified/font.rs index 0344af275ce..6c92acbf1b9 100644 --- a/components/style/values/specified/font.rs +++ b/components/style/values/specified/font.rs @@ -548,13 +548,16 @@ impl ToComputedValue for FontFamily { fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { match *self { - FontFamily::Values(ref v) => computed::FontFamily(v.clone()), + FontFamily::Values(ref v) => computed::FontFamily { + families: v.clone(), + is_system_font: false, + }, FontFamily::System(_) => self.compute_system(context), } } fn from_computed_value(other: &computed::FontFamily) -> Self { - FontFamily::Values(other.0.clone()) + FontFamily::Values(other.families.clone()) } } @@ -958,48 +961,6 @@ impl FontSize { "larger" => Ok(FontSize::Larger), } } - - #[allow(unused_mut)] - /// Cascade `font-size` with specified value - pub fn cascade_specified_font_size( - context: &mut Context, - specified_value: &FontSize, - mut computed: computed::FontSize, - ) { - // we could use clone_language and clone_font_family() here but that's - // expensive. Do it only in gecko mode for now. - #[cfg(feature = "gecko")] - { - // if the language or generic changed, we need to recalculate - // the font size from the stored font-size origin information. - if context.builder.get_font().gecko().mLanguage.mRawPtr != - context.builder.get_parent_font().gecko().mLanguage.mRawPtr || - context.builder.get_font().gecko().mGenericID != - context.builder.get_parent_font().gecko().mGenericID - { - if let Some(info) = computed.keyword_info { - computed.size = info.to_computed_value(context); - } - } - } - - let device = context.builder.device; - let mut font = context.builder.take_font(); - let parent_unconstrained = { - let parent_font = context.builder.get_parent_font(); - font.apply_font_size(computed, parent_font, device) - }; - context.builder.put_font(font); - - if let Some(parent) = parent_unconstrained { - let new_unconstrained = specified_value - .to_computed_value_against(context, FontBaseSize::Custom(Au::from(parent))); - context - .builder - .mutate_font() - .apply_unconstrained_font_size(new_unconstrained.size); - } - } } impl Parse for FontSize { diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index acee43f4b38..8888a32c4a5 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -82,17 +82,12 @@ pub enum FontBaseSize { /// /// FIXME(emilio): This is very complex, and should go away. InheritedStyleButStripEmUnits, - /// Use a custom base size. - /// - /// FIXME(emilio): This is very dubious, and only used for MathML. - Custom(Au), } impl FontBaseSize { /// Calculate the actual size for a given context pub fn resolve(&self, context: &Context) -> Au { match *self { - FontBaseSize::Custom(size) => size, FontBaseSize::CurrentStyle => context.style().get_font().clone_font_size().size(), FontBaseSize::InheritedStyleButStripEmUnits | FontBaseSize::InheritedStyle => { context.style().get_parent_font().clone_font_size().size() From d3f254d2e4fc6e516fe267e040db1ebfeea621b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 18 Mar 2019 16:27:54 +0000 Subject: [PATCH 06/29] style: Make all font-metrics-affecting properties cascade early. And make font-size computation work on the whole font of the parent, not just accounting for the parent's font-size. Differential Revision: https://phabricator.services.mozilla.com/D20656 --- components/style/font_metrics.rs | 16 ++------ components/style/gecko/wrapper.rs | 38 ++++++++++++------- .../style/properties/properties.mako.rs | 6 ++- components/style/values/specified/length.rs | 14 ++----- 4 files changed, 36 insertions(+), 38 deletions(-) diff --git a/components/style/font_metrics.rs b/components/style/font_metrics.rs index 0c37a444658..c8b4a655ad8 100644 --- a/components/style/font_metrics.rs +++ b/components/style/font_metrics.rs @@ -7,9 +7,6 @@ #![deny(missing_docs)] use crate::context::SharedStyleContext; -use crate::logical_geometry::WritingMode; -use crate::media_queries::Device; -use crate::properties::style_structs::Font; use crate::Atom; use app_units::Au; @@ -36,22 +33,15 @@ pub enum FontMetricsQueryResult { /// A trait used to represent something capable of providing us font metrics. pub trait FontMetricsProvider { /// Obtain the metrics for given font family. - /// - /// TODO: We could make this take the full list, I guess, and save a few - /// virtual calls in the case we are repeatedly unable to find font metrics? - /// That is not too common in practice though. fn query( &self, - _font: &Font, - _font_size: Au, - _wm: WritingMode, - _in_media_query: bool, - _device: &Device, + _context: &crate::values::computed::Context, + _base_size: crate::values::specified::length::FontBaseSize, ) -> FontMetricsQueryResult { FontMetricsQueryResult::NotAvailable } - /// Get default size of a given language and generic family + /// Get default size of a given language and generic family. fn get_size(&self, font_name: &Atom, font_family: u8) -> Au; /// Construct from a shared style context diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 558ee60f4e2..65a2565b302 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -58,10 +58,8 @@ use crate::gecko_bindings::sugar::ownership::{HasArcFFI, HasSimpleFFI}; use crate::global_style_data::GLOBAL_STYLE_DATA; use crate::hash::FxHashMap; use crate::invalidation::element::restyle_hints::RestyleHint; -use crate::logical_geometry::WritingMode; use crate::media_queries::Device; use crate::properties::animated_properties::{AnimationValue, AnimationValueMap}; -use crate::properties::style_structs::Font; use crate::properties::{ComputedValues, LonghandId}; use crate::properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock}; use crate::rule_tree::CascadeLevel as ServoCascadeLevel; @@ -69,6 +67,7 @@ use crate::selector_parser::{AttrValue, HorizontalDirection, Lang}; use crate::shared_lock::Locked; use crate::string_cache::{Atom, Namespace, WeakAtom, WeakNamespace}; use crate::stylist::CascadeData; +use crate::values::specified::length::FontBaseSize; use crate::CaseSensitivityExt; use app_units::Au; use atomic_refcell::{AtomicRefCell, AtomicRefMut}; @@ -1023,37 +1022,48 @@ impl FontMetricsProvider for GeckoFontMetricsProvider { } fn get_size(&self, font_name: &Atom, font_family: u8) -> Au { - use crate::gecko_bindings::bindings::Gecko_GetBaseSize; let mut cache = self.font_size_cache.borrow_mut(); if let Some(sizes) = cache.iter().find(|el| el.0 == *font_name) { return sizes.1.size_for_generic(font_family); } - let sizes = unsafe { Gecko_GetBaseSize(font_name.as_ptr()) }; + let sizes = unsafe { bindings::Gecko_GetBaseSize(font_name.as_ptr()) }; cache.push((font_name.clone(), sizes)); sizes.size_for_generic(font_family) } fn query( &self, - font: &Font, - font_size: Au, - wm: WritingMode, - in_media_query: bool, - device: &Device, + context: &crate::values::computed::Context, + base_size: FontBaseSize, ) -> FontMetricsQueryResult { - use crate::gecko_bindings::bindings::Gecko_GetFontMetrics; - let pc = match device.pres_context() { + let pc = match context.device().pres_context() { Some(pc) => pc, None => return FontMetricsQueryResult::NotAvailable, }; + + let size = base_size.resolve(context); + let style = context.style(); + + let (wm, font) = match base_size { + FontBaseSize::CurrentStyle => { + (style.writing_mode, style.get_font()) + }, + // These are only used for font-size computation, and the first is + // really dubious... + FontBaseSize::InheritedStyleButStripEmUnits | + FontBaseSize::InheritedStyle => { + (*style.inherited_writing_mode(), style.get_parent_font()) + }, + }; + let gecko_metrics = unsafe { - Gecko_GetFontMetrics( + bindings::Gecko_GetFontMetrics( pc, wm.is_vertical() && !wm.is_sideways(), font.gecko(), - font_size.0, + size.0, // we don't use the user font set in a media query - !in_media_query, + !context.in_media_query, ) }; let metrics = FontMetrics { diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 4df99668d18..40a5456c5e4 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1257,8 +1257,12 @@ impl LonghandId { LonghandId::MozScriptLevel | % endif - // Needed to compute font-relative lengths correctly. + // Needed to compute the first available font, in order to + // compute font-relative units correctly. LonghandId::FontSize | + LonghandId::FontWeight | + LonghandId::FontStretch | + LonghandId::FontStyle | LonghandId::FontFamily | // Needed to resolve currentcolor at computed value time properly. diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index 8888a32c4a5..9a1d366f7fd 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -131,15 +131,9 @@ impl FontRelativeLength { ) -> (Au, CSSFloat) { fn query_font_metrics( context: &Context, - reference_font_size: Au, + base_size: FontBaseSize, ) -> FontMetricsQueryResult { - context.font_metrics_provider.query( - context.style().get_font(), - reference_font_size, - context.style().writing_mode, - context.in_media_query, - context.device(), - ) + context.font_metrics_provider.query(context, base_size) } let reference_font_size = base_size.resolve(context); @@ -164,7 +158,7 @@ impl FontRelativeLength { if context.for_non_inherited_property.is_some() { context.rule_cache_conditions.borrow_mut().set_uncacheable(); } - let reference_size = match query_font_metrics(context, reference_font_size) { + let reference_size = match query_font_metrics(context, base_size) { FontMetricsQueryResult::Available(metrics) => metrics.x_height, // https://drafts.csswg.org/css-values/#ex // @@ -180,7 +174,7 @@ impl FontRelativeLength { if context.for_non_inherited_property.is_some() { context.rule_cache_conditions.borrow_mut().set_uncacheable(); } - let reference_size = match query_font_metrics(context, reference_font_size) { + let reference_size = match query_font_metrics(context, base_size) { FontMetricsQueryResult::Available(metrics) => metrics.zero_advance_measure, // https://drafts.csswg.org/css-values/#ch // From 4b07af2240764e91ae6fd563940e4e7bb6f1fbe0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 18 Mar 2019 16:38:48 +0000 Subject: [PATCH 07/29] style: Make cssPropertySupportsType take an enum. Differential Revision: https://phabricator.services.mozilla.com/D22427 --- components/style_traits/specified_value_info.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style_traits/specified_value_info.rs b/components/style_traits/specified_value_info.rs index 3cb59e796c4..c978727b47a 100644 --- a/components/style_traits/specified_value_info.rs +++ b/components/style_traits/specified_value_info.rs @@ -10,7 +10,7 @@ use std::sync::Arc as StdArc; /// Type of value that a property supports. This is used by Gecko's /// devtools to make sense about value it parses, and types listed -/// here should match TYPE_* constants in InspectorUtils.webidl. +/// here should match InspectorPropertyType in InspectorUtils.webidl. /// /// XXX This should really be a bitflags rather than a namespace mod, /// but currently we cannot use bitflags in const. From b0af5656565f8490f27ccfadfb5afa14126fd858 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 18 Mar 2019 17:58:16 +0000 Subject: [PATCH 08/29] style: Use cbindgen for text-decoration-line. Differential Revision: https://phabricator.services.mozilla.com/D23412 --- components/style/cbindgen.toml | 1 + components/style/properties/gecko.mako.rs | 19 +- .../style/properties/longhands/text.mako.rs | 11 +- .../style/properties/properties.mako.rs | 18 -- components/style/values/computed/text.rs | 10 +- components/style/values/specified/text.rs | 209 +++++++++--------- 6 files changed, 110 insertions(+), 158 deletions(-) diff --git a/components/style/cbindgen.toml b/components/style/cbindgen.toml index f36fd09a243..ae73adeda8c 100644 --- a/components/style/cbindgen.toml +++ b/components/style/cbindgen.toml @@ -105,6 +105,7 @@ include = [ "WordBreak", "Contain", "RestyleHint", + "TextDecorationLine", ] item_types = ["enums", "structs", "typedefs"] diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 30369dff62d..dbeedb9c7db 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -3996,9 +3996,7 @@ fn static_assert() { <%self:impl_trait style_struct_name="Text" - skip_longhands="text-decoration-line text-overflow initial-letter"> - - ${impl_simple_type_with_conversion("text_decoration_line")} + skip_longhands="text-overflow initial-letter"> fn clear_overflow_sides_if_string(&mut self) { use crate::gecko_bindings::structs::nsStyleTextOverflowSide; @@ -4112,21 +4110,6 @@ fn static_assert() { InitialLetter::Specified(self.gecko.mInitialLetterSize, Some(self.gecko.mInitialLetterSink)) } } - - #[inline] - pub fn has_underline(&self) -> bool { - (self.gecko.mTextDecorationLine & (structs::NS_STYLE_TEXT_DECORATION_LINE_UNDERLINE as u8)) != 0 - } - - #[inline] - pub fn has_overline(&self) -> bool { - (self.gecko.mTextDecorationLine & (structs::NS_STYLE_TEXT_DECORATION_LINE_OVERLINE as u8)) != 0 - } - - #[inline] - pub fn has_line_through(&self) -> bool { - (self.gecko.mTextDecorationLine & (structs::NS_STYLE_TEXT_DECORATION_LINE_LINE_THROUGH as u8)) != 0 - } // Set SVGPathData to StyleShapeSource. diff --git a/components/style/properties/longhands/text.mako.rs b/components/style/properties/longhands/text.mako.rs index 308de1b7575..723e26ae22b 100644 --- a/components/style/properties/longhands/text.mako.rs +++ b/components/style/properties/longhands/text.mako.rs @@ -5,16 +5,7 @@ <%namespace name="helpers" file="/helpers.mako.rs" /> <% from data import Method %> -<% data.new_style_struct( - "Text", - inherited=False, - gecko_name="TextReset", - additional_methods=[ - Method("has_underline", "bool"), - Method("has_overline", "bool"), - Method("has_line_through", "bool"), - ] -) %> +<% data.new_style_struct("Text", inherited=False, gecko_name="TextReset") %> ${helpers.predefined_type( "text-overflow", diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 40a5456c5e4..a470eb84b08 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -2638,24 +2638,6 @@ pub mod style_structs { use crate::Zero; !self.outline_width.is_zero() } - % elif style_struct.name == "Text": - /// Whether the text decoration has an underline. - #[inline] - pub fn has_underline(&self) -> bool { - self.text_decoration_line.contains(longhands::text_decoration_line::SpecifiedValue::UNDERLINE) - } - - /// Whether the text decoration has an overline. - #[inline] - pub fn has_overline(&self) -> bool { - self.text_decoration_line.contains(longhands::text_decoration_line::SpecifiedValue::OVERLINE) - } - - /// 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 original_display, /// except when the adjustment comes from root or item display fixups. diff --git a/components/style/values/computed/text.rs b/components/style/values/computed/text.rs index 911891bf33b..a1f68bf1a1b 100644 --- a/components/style/values/computed/text.rs +++ b/components/style/values/computed/text.rs @@ -19,7 +19,7 @@ use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; pub use crate::values::specified::TextAlignKeyword as TextAlign; -pub use crate::values::specified::TextEmphasisPosition; +pub use crate::values::specified::{TextEmphasisPosition, TextDecorationLine}; pub use crate::values::specified::{OverflowWrap, WordBreak}; /// A computed value for the `initial-letter` property. @@ -182,11 +182,11 @@ impl TextDecorationsInEffect { .clone(), }; - let text_style = style.get_text(); + let line = style.get_text().clone_text_decoration_line(); - result.underline |= text_style.has_underline(); - result.overline |= text_style.has_overline(); - result.line_through |= text_style.has_line_through(); + result.underline |= line.contains(TextDecorationLine::UNDERLINE); + result.overline |= line.contains(TextDecorationLine::OVERLINE); + result.line_through |= line.contains(TextDecorationLine::LINE_THROUGH); result } diff --git a/components/style/values/specified/text.rs b/components/style/values/specified/text.rs index 883084e649a..ab4e45fd1ee 100644 --- a/components/style/values/specified/text.rs +++ b/components/style/values/specified/text.rs @@ -22,8 +22,7 @@ use cssparser::{Parser, Token}; use selectors::parser::SelectorParseErrorKind; use std::fmt::{self, Write}; use style_traits::values::SequenceWriter; -use style_traits::{CssWriter, KeywordsCollectFn, ParseError}; -use style_traits::{SpecifiedValueInfo, StyleParseErrorKind, ToCss}; +use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; use unicode_segmentation::UnicodeSegmentation; /// A specified type for the `initial-letter` property. @@ -255,119 +254,115 @@ impl ToComputedValue for TextOverflow { } } -macro_rules! impl_text_decoration_line { - { - $( - $(#[$($meta:tt)+])* - $ident:ident / $css:expr => $value:expr, - )+ - } => { - bitflags! { - #[derive(MallocSizeOf, ToComputedValue)] - /// Specified keyword values for the text-decoration-line property. - pub struct TextDecorationLine: u8 { - /// No text decoration line is specified - const NONE = 0; - $( - $(#[$($meta)+])* - const $ident = $value; - )+ - #[cfg(feature = "gecko")] - /// Only set by presentation attributes - /// - /// Setting this will mean that text-decorations use the color - /// specified by `color` in quirks mode. - /// - /// For example, this gives text - /// a red text decoration - const COLOR_OVERRIDE = 0x10; +bitflags! { + #[derive(MallocSizeOf, SpecifiedValueInfo, ToComputedValue)] + #[value_info(other_values = "none,underline,overline,line-through,blink")] + #[repr(C)] + /// Specified keyword values for the text-decoration-line property. + pub struct TextDecorationLine: u8 { + /// No text decoration line is specified. + const NONE = 0; + /// underline + const UNDERLINE = 1 << 0; + /// overline + const OVERLINE = 1 << 1; + /// line-through + const LINE_THROUGH = 1 << 2; + /// blink + const BLINK = 1 << 3; + /// Only set by presentation attributes + /// + /// Setting this will mean that text-decorations use the color + /// specified by `color` in quirks mode. + /// + /// For example, this gives text + /// a red text decoration + #[cfg(feature = "gecko")] + const COLOR_OVERRIDE = 0x10; + } +} + + +impl Parse for TextDecorationLine { + /// none | [ underline || overline || line-through || blink ] + fn parse<'i, 't>( + _context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + let mut result = TextDecorationLine::empty(); + + // NOTE(emilio): this loop has this weird structure because we run this + // code to parse the text-decoration shorthand as well, so we need to + // ensure we don't return an error if we don't consume the whole thing + // because we find an invalid identifier or other kind of token. + loop { + let flag: Result<_, ParseError<'i>> = input.try(|input| { + let flag = try_match_ident_ignore_ascii_case! { input, + "none" if result.is_empty() => TextDecorationLine::NONE, + "underline" => TextDecorationLine::UNDERLINE, + "overline" => TextDecorationLine::OVERLINE, + "line-through" => TextDecorationLine::LINE_THROUGH, + "blink" => TextDecorationLine::BLINK, + }; + + Ok(flag) + }); + + let flag = match flag { + Ok(flag) => flag, + Err(..) => break, + }; + + if flag.is_empty() { + return Ok(TextDecorationLine::NONE); } + + if result.contains(flag) { + return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) + } + + result.insert(flag) } - impl Parse for TextDecorationLine { - /// none | [ underline || overline || line-through || blink ] - fn parse<'i, 't>( - _context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - let mut result = TextDecorationLine::NONE; - if input - .try(|input| input.expect_ident_matching("none")) - .is_ok() - { - return Ok(result); - } - - loop { - let result = input.try(|input| { - let ident = input.expect_ident().map_err(|_| ())?; - match_ignore_ascii_case! { ident, - $( - $css => { - if result.contains(TextDecorationLine::$ident) { - Err(()) - } else { - result.insert(TextDecorationLine::$ident); - Ok(()) - } - } - )+ - _ => Err(()), - } - }); - if result.is_err() { - break; - } - } - - if !result.is_empty() { - Ok(result) - } else { - Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) - } - } - } - - impl ToCss for TextDecorationLine { - fn to_css(&self, dest: &mut CssWriter) -> fmt::Result - where - W: Write, - { - if self.is_empty() { - return dest.write_str("none"); - } - - let mut writer = SequenceWriter::new(dest, " "); - $( - if self.contains(TextDecorationLine::$ident) { - writer.raw_item($css)?; - } - )+ - Ok(()) - } - } - - impl SpecifiedValueInfo for TextDecorationLine { - fn collect_completion_keywords(f: KeywordsCollectFn) { - f(&["none", $($css,)+]); - } + if !result.is_empty() { + Ok(result) + } else { + Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) } } } -impl_text_decoration_line! { - /// Underline - UNDERLINE / "underline" => 1 << 0, - /// Overline - OVERLINE / "overline" => 1 << 1, - /// Line through - LINE_THROUGH / "line-through" => 1 << 2, - /// Blink - BLINK / "blink" => 1 << 3, -} +impl ToCss for TextDecorationLine { + fn to_css(&self, dest: &mut CssWriter) -> fmt::Result + where + W: Write, + { + if self.is_empty() { + return dest.write_str("none"); + } -#[cfg(feature = "gecko")] -impl_bitflags_conversions!(TextDecorationLine); + let mut writer = SequenceWriter::new(dest, " "); + let mut any = false; + + macro_rules! maybe_write { + ($ident:ident => $str:expr) => { + if self.contains(TextDecorationLine::$ident) { + any = true; + writer.raw_item($str)?; + } + }; + } + + maybe_write!(UNDERLINE => "underline"); + maybe_write!(OVERLINE => "overline"); + maybe_write!(LINE_THROUGH => "line-through"); + maybe_write!(BLINK => "blink"); + + debug_assert!(any || *self == TextDecorationLine::COLOR_OVERRIDE); + + Ok(()) + } +} impl TextDecorationLine { #[inline] From 6e2643c636629f9a953ede313ca896eb60009bca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 18 Mar 2019 18:01:55 +0000 Subject: [PATCH 09/29] style: Use cbindgen for touch-action. And rename the constants to not be prefixed by TOUCH_ACTION_, since that's part of the type name anyway. Differential Revision: https://phabricator.services.mozilla.com/D23413 --- components/style/cbindgen.toml | 1 + components/style/properties/gecko.mako.rs | 4 +- components/style/values/specified/box.rs | 69 +++++++---------------- 3 files changed, 23 insertions(+), 51 deletions(-) diff --git a/components/style/cbindgen.toml b/components/style/cbindgen.toml index ae73adeda8c..0d2af9f21e7 100644 --- a/components/style/cbindgen.toml +++ b/components/style/cbindgen.toml @@ -105,6 +105,7 @@ include = [ "WordBreak", "Contain", "RestyleHint", + "TouchAction", "TextDecorationLine", ] item_types = ["enums", "structs", "typedefs"] diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index dbeedb9c7db..82c8e96ec2d 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -2582,7 +2582,7 @@ fn static_assert() { transform-style rotate scroll-snap-points-x scroll-snap-points-y scroll-snap-coordinate -moz-binding will-change - offset-path shape-outside touch-action + offset-path shape-outside translate scale""" %> <%self:impl_trait style_struct_name="Box" skip_longhands="${skip_box_longhands}"> #[inline] @@ -3002,8 +3002,6 @@ fn static_assert() { <% impl_shape_source("shape_outside", "mShapeOutside") %> - ${impl_simple_type_with_conversion("touch_action")} - pub fn set_offset_path(&mut self, v: longhands::offset_path::computed_value::T) { use crate::gecko_bindings::bindings::{Gecko_NewStyleMotion, Gecko_SetStyleMotion}; use crate::gecko_bindings::structs::StyleShapeSourceType; diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index 39917659cc9..dddf3f9b4af 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -650,21 +650,22 @@ impl Parse for WillChange { } bitflags! { + /// Values for the `touch-action` property. #[cfg_attr(feature = "gecko", derive(MallocSizeOf))] #[derive(SpecifiedValueInfo, ToComputedValue)] - /// These constants match Gecko's `NS_STYLE_TOUCH_ACTION_*` constants. #[value_info(other_values = "auto,none,manipulation,pan-x,pan-y")] + #[repr(C)] pub struct TouchAction: u8 { /// `none` variant - const TOUCH_ACTION_NONE = 1 << 0; + const NONE = 1 << 0; /// `auto` variant - const TOUCH_ACTION_AUTO = 1 << 1; + const AUTO = 1 << 1; /// `pan-x` variant - const TOUCH_ACTION_PAN_X = 1 << 2; + const PAN_X = 1 << 2; /// `pan-y` variant - const TOUCH_ACTION_PAN_Y = 1 << 3; + const PAN_Y = 1 << 3; /// `manipulation` variant - const TOUCH_ACTION_MANIPULATION = 1 << 4; + const MANIPULATION = 1 << 4; } } @@ -672,7 +673,7 @@ impl TouchAction { #[inline] /// Get default `touch-action` as `auto` pub fn auto() -> TouchAction { - TouchAction::TOUCH_ACTION_AUTO + TouchAction::AUTO } } @@ -682,16 +683,14 @@ impl ToCss for TouchAction { W: Write, { match *self { - TouchAction::TOUCH_ACTION_NONE => dest.write_str("none"), - TouchAction::TOUCH_ACTION_AUTO => dest.write_str("auto"), - TouchAction::TOUCH_ACTION_MANIPULATION => dest.write_str("manipulation"), - _ if self - .contains(TouchAction::TOUCH_ACTION_PAN_X | TouchAction::TOUCH_ACTION_PAN_Y) => - { + TouchAction::NONE => dest.write_str("none"), + TouchAction::AUTO => dest.write_str("auto"), + TouchAction::MANIPULATION => dest.write_str("manipulation"), + _ if self.contains(TouchAction::PAN_X | TouchAction::PAN_Y) => { dest.write_str("pan-x pan-y") }, - _ if self.contains(TouchAction::TOUCH_ACTION_PAN_X) => dest.write_str("pan-x"), - _ if self.contains(TouchAction::TOUCH_ACTION_PAN_Y) => dest.write_str("pan-y"), + _ if self.contains(TouchAction::PAN_X) => dest.write_str("pan-x"), + _ if self.contains(TouchAction::PAN_Y) => dest.write_str("pan-y"), _ => panic!("invalid touch-action value"), } } @@ -703,53 +702,27 @@ impl Parse for TouchAction { input: &mut Parser<'i, 't>, ) -> Result> { try_match_ident_ignore_ascii_case! { input, - "auto" => Ok(TouchAction::TOUCH_ACTION_AUTO), - "none" => Ok(TouchAction::TOUCH_ACTION_NONE), - "manipulation" => Ok(TouchAction::TOUCH_ACTION_MANIPULATION), + "auto" => Ok(TouchAction::AUTO), + "none" => Ok(TouchAction::NONE), + "manipulation" => Ok(TouchAction::MANIPULATION), "pan-x" => { if input.try(|i| i.expect_ident_matching("pan-y")).is_ok() { - Ok(TouchAction::TOUCH_ACTION_PAN_X | TouchAction::TOUCH_ACTION_PAN_Y) + Ok(TouchAction::PAN_X | TouchAction::PAN_Y) } else { - Ok(TouchAction::TOUCH_ACTION_PAN_X) + Ok(TouchAction::PAN_X) } }, "pan-y" => { if input.try(|i| i.expect_ident_matching("pan-x")).is_ok() { - Ok(TouchAction::TOUCH_ACTION_PAN_X | TouchAction::TOUCH_ACTION_PAN_Y) + Ok(TouchAction::PAN_X | TouchAction::PAN_Y) } else { - Ok(TouchAction::TOUCH_ACTION_PAN_Y) + Ok(TouchAction::PAN_Y) } }, } } } -#[cfg(feature = "gecko")] -impl_bitflags_conversions!(TouchAction); - -/// Asserts that all touch-action matches its NS_STYLE_TOUCH_ACTION_* value. -#[cfg(feature = "gecko")] -#[inline] -pub fn assert_touch_action_matches() { - use crate::gecko_bindings::structs; - - macro_rules! check_touch_action { - ( $( $a:ident => $b:path),*, ) => { - $( - debug_assert_eq!(structs::$a as u8, $b.bits()); - )* - } - } - - check_touch_action! { - NS_STYLE_TOUCH_ACTION_NONE => TouchAction::TOUCH_ACTION_NONE, - NS_STYLE_TOUCH_ACTION_AUTO => TouchAction::TOUCH_ACTION_AUTO, - NS_STYLE_TOUCH_ACTION_PAN_X => TouchAction::TOUCH_ACTION_PAN_X, - NS_STYLE_TOUCH_ACTION_PAN_Y => TouchAction::TOUCH_ACTION_PAN_Y, - NS_STYLE_TOUCH_ACTION_MANIPULATION => TouchAction::TOUCH_ACTION_MANIPULATION, - } -} - bitflags! { #[derive(MallocSizeOf, SpecifiedValueInfo, ToComputedValue)] #[value_info(other_values = "none,strict,content,size,layout,paint")] From 01e0f37861efc97ee04c900e7c60769d47a3c5ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 18 Mar 2019 18:08:08 +0000 Subject: [PATCH 10/29] style: Make the will-change bitfield use cbindgen. Differential Revision: https://phabricator.services.mozilla.com/D23414 --- components/style/cbindgen.toml | 1 + components/style/macros.rs | 17 ---------------- components/style/properties/gecko.mako.rs | 24 +++++++++++------------ components/style/values/specified/box.rs | 5 ++--- 4 files changed, 14 insertions(+), 33 deletions(-) diff --git a/components/style/cbindgen.toml b/components/style/cbindgen.toml index 0d2af9f21e7..b5d374d77bf 100644 --- a/components/style/cbindgen.toml +++ b/components/style/cbindgen.toml @@ -106,6 +106,7 @@ include = [ "Contain", "RestyleHint", "TouchAction", + "WillChangeBits", "TextDecorationLine", ] item_types = ["enums", "structs", "typedefs"] diff --git a/components/style/macros.rs b/components/style/macros.rs index 6347e5fa4a0..fb189837181 100644 --- a/components/style/macros.rs +++ b/components/style/macros.rs @@ -102,20 +102,3 @@ macro_rules! define_keyword_type { } }; } - -#[cfg(feature = "gecko")] -macro_rules! impl_bitflags_conversions { - ($name:ident) => { - impl From for $name { - fn from(bits: u8) -> $name { - $name::from_bits(bits).expect("bits contain valid flag") - } - } - - impl From<$name> for u8 { - fn from(v: $name) -> u8 { - v.bits() - } - } - }; -} diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 82c8e96ec2d..331ffb5c34f 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -2940,10 +2940,10 @@ fn static_assert() { pub fn set_will_change(&mut self, v: longhands::will_change::computed_value::T) { use crate::gecko_bindings::bindings::{Gecko_AppendWillChange, Gecko_ClearWillChange}; - use crate::properties::longhands::will_change::computed_value::T; + use crate::values::specified::box_::{WillChangeBits, WillChange}; match v { - T::AnimateableFeatures { features, bits } => { + WillChange::AnimateableFeatures { features, bits } => { unsafe { Gecko_ClearWillChange(&mut self.gecko, features.len()); } @@ -2954,13 +2954,13 @@ fn static_assert() { } } - self.gecko.mWillChangeBitField = bits.bits(); + self.gecko.mWillChangeBitField = bits; }, - T::Auto => { + WillChange::Auto => { unsafe { Gecko_ClearWillChange(&mut self.gecko, 0); } - self.gecko.mWillChangeBitField = 0; + self.gecko.mWillChangeBitField = WillChangeBits::empty(); }, }; } @@ -2970,7 +2970,7 @@ fn static_assert() { self.gecko.mWillChangeBitField = other.gecko.mWillChangeBitField; unsafe { - Gecko_CopyWillChangeFrom(&mut self.gecko, &other.gecko as *const _ as *mut _); + Gecko_CopyWillChangeFrom(&mut self.gecko, &other.gecko); } } @@ -2979,24 +2979,22 @@ fn static_assert() { } pub fn clone_will_change(&self) -> longhands::will_change::computed_value::T { - use crate::properties::longhands::will_change::computed_value::T; - use crate::gecko_bindings::structs::nsAtom; use crate::values::CustomIdent; - use crate::values::specified::box_::WillChangeBits; + use crate::values::specified::box_::WillChange; if self.gecko.mWillChange.len() == 0 { - return T::Auto + return WillChange::Auto } let custom_idents: Vec = self.gecko.mWillChange.iter().map(|gecko_atom| { unsafe { - CustomIdent(Atom::from_raw(gecko_atom.mRawPtr as *mut nsAtom)) + CustomIdent(Atom::from_raw(gecko_atom.mRawPtr)) } }).collect(); - T::AnimateableFeatures { + WillChange::AnimateableFeatures { features: custom_idents.into_boxed_slice(), - bits: WillChangeBits::from_bits_truncate(self.gecko.mWillChangeBitField), + bits: self.gecko.mWillChangeBitField, } } diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index dddf3f9b4af..ab81d4c3450 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -555,9 +555,8 @@ impl WillChange { bitflags! { /// The change bits that we care about. - /// - /// These need to be in sync with NS_STYLE_WILL_CHANGE_*. #[derive(MallocSizeOf, SpecifiedValueInfo, ToComputedValue)] + #[repr(C)] pub struct WillChangeBits: u8 { /// Whether the stacking context will change. const STACKING_CONTEXT = 1 << 0; @@ -616,7 +615,7 @@ impl Parse for WillChange { fn parse<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, - ) -> Result> { + ) -> Result> { if input .try(|input| input.expect_ident_matching("auto")) .is_ok() From fe7b3a6b116ec982aed0f478111b120b0f25284d Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Mon, 18 Mar 2019 18:05:00 +0000 Subject: [PATCH 11/29] style: Part 6: Add individual transform properties into compositor animation list. Now, its time to let individual transform run on the compositor thread. Differential Revision: https://phabricator.services.mozilla.com/D22566 --- components/style/properties/longhands/box.mako.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/style/properties/longhands/box.mako.rs b/components/style/properties/longhands/box.mako.rs index 1a8abfb3611..dcfcb343040 100644 --- a/components/style/properties/longhands/box.mako.rs +++ b/components/style/properties/longhands/box.mako.rs @@ -365,7 +365,7 @@ ${helpers.predefined_type( "generics::transform::Rotate::None", animation_value_type="ComputedValue", boxed=True, - flags="CREATES_STACKING_CONTEXT FIXPOS_CB", + flags="CREATES_STACKING_CONTEXT FIXPOS_CB CAN_ANIMATE_ON_COMPOSITOR", gecko_pref="layout.css.individual-transform.enabled", spec="https://drafts.csswg.org/css-transforms-2/#individual-transforms", servo_restyle_damage = "reflow_out_of_flow", @@ -377,7 +377,7 @@ ${helpers.predefined_type( "generics::transform::Scale::None", animation_value_type="ComputedValue", boxed=True, - flags="CREATES_STACKING_CONTEXT FIXPOS_CB", + flags="CREATES_STACKING_CONTEXT FIXPOS_CB CAN_ANIMATE_ON_COMPOSITOR", gecko_pref="layout.css.individual-transform.enabled", spec="https://drafts.csswg.org/css-transforms-2/#individual-transforms", servo_restyle_damage = "reflow_out_of_flow", @@ -389,7 +389,7 @@ ${helpers.predefined_type( "generics::transform::Translate::None", animation_value_type="ComputedValue", boxed=True, - flags="CREATES_STACKING_CONTEXT FIXPOS_CB", + flags="CREATES_STACKING_CONTEXT FIXPOS_CB CAN_ANIMATE_ON_COMPOSITOR", gecko_pref="layout.css.individual-transform.enabled", spec="https://drafts.csswg.org/css-transforms-2/#individual-transforms", servo_restyle_damage="reflow_out_of_flow", From ed4a23eccfd47359df8edd6d0580e2c1859e3ab8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 19 Mar 2019 21:10:13 +0000 Subject: [PATCH 12/29] style: Keep track of whether a style is affected by font metrics. Differential Revision: https://phabricator.services.mozilla.com/D20728 --- .../style/properties/computed_value_flags.rs | 5 ++- .../style/properties/properties.mako.rs | 41 +++++++++++++++---- components/style/style_adjuster.rs | 32 ++++----------- components/style/values/specified/length.rs | 3 ++ 4 files changed, 47 insertions(+), 34 deletions(-) diff --git a/components/style/properties/computed_value_flags.rs b/components/style/properties/computed_value_flags.rs index 18fc886c7ae..2f6851d3f43 100644 --- a/components/style/properties/computed_value_flags.rs +++ b/components/style/properties/computed_value_flags.rs @@ -56,10 +56,13 @@ bitflags! { /// Whether the child explicitly inherits any reset property. const INHERITS_RESET_STYLE = 1 << 8; + /// Whether any value on our style is font-metric-dependent. + const DEPENDS_ON_FONT_METRICS = 1 << 9; + /// Whether the style or any of the ancestors has a multicol style. /// /// Only used in Servo. - const CAN_BE_FRAGMENTED = 1 << 9; + const CAN_BE_FRAGMENTED = 1 << 10; } } diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index a470eb84b08..f57e27eff1e 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -45,6 +45,7 @@ use crate::rule_tree::StrongRuleNode; use crate::Zero; use self::computed_value_flags::*; use crate::str::{CssString, CssStringBorrow, CssStringWriter}; +use std::cell::Cell; pub use self::declaration_block::*; pub use self::cascade::*; @@ -2758,7 +2759,7 @@ pub struct ComputedValuesInner { pub writing_mode: WritingMode, /// A set of flags we use to store misc information regarding this style. - pub flags: ComputedValueFlags, + flags: ComputedValueFlags, /// The rule node representing the ordered list of rules matched for this /// node. Can be None for default values and text nodes. This is @@ -3322,8 +3323,10 @@ pub struct StyleBuilder<'a> { /// /// TODO(emilio): Make private. pub writing_mode: WritingMode, + /// Flags for the computed value. - pub flags: ComputedValueFlags, + pub flags: Cell, + /// The element's style if visited, only computed if there's a relevant link /// for this element. A element's "relevant link" is the element being /// matched if it is a link or the nearest ancestor link. @@ -3365,7 +3368,7 @@ impl<'a> StyleBuilder<'a> { modified_reset: false, custom_properties, writing_mode: inherited_style.writing_mode, - flags, + flags: Cell::new(flags), visited_style: None, % for style_struct in data.active_style_structs(): % if style_struct.inherited: @@ -3404,7 +3407,7 @@ impl<'a> StyleBuilder<'a> { rules: None, custom_properties: style_to_derive_from.custom_properties().cloned(), writing_mode: style_to_derive_from.writing_mode, - flags: style_to_derive_from.flags, + flags: Cell::new(style_to_derive_from.flags), visited_style: None, % for style_struct in data.active_style_structs(): ${style_struct.ident}: StyleStructRef::Borrowed( @@ -3434,14 +3437,14 @@ impl<'a> StyleBuilder<'a> { .get_${property.style_struct.name_lower}(); self.modified_reset = true; - self.flags.insert(ComputedValueFlags::INHERITS_RESET_STYLE); + self.add_flags(ComputedValueFlags::INHERITS_RESET_STYLE); % if property.ident == "content": - self.flags.insert(ComputedValueFlags::INHERITS_CONTENT); + self.add_flags(ComputedValueFlags::INHERITS_CONTENT); % endif % if property.ident == "display": - self.flags.insert(ComputedValueFlags::INHERITS_DISPLAY); + self.add_flags(ComputedValueFlags::INHERITS_DISPLAY); % endif if self.${property.style_struct.ident}.ptr_eq(inherited_struct) { @@ -3628,13 +3631,33 @@ impl<'a> StyleBuilder<'a> { self.modified_reset } + /// Return the current flags. + #[inline] + pub fn flags(&self) -> ComputedValueFlags { + self.flags.get() + } + + /// Add a flag to the current builder. + #[inline] + pub fn add_flags(&self, flag: ComputedValueFlags) { + let flags = self.flags() | flag; + self.flags.set(flags); + } + + /// Removes a flag to the current builder. + #[inline] + pub fn remove_flags(&self, flag: ComputedValueFlags) { + let flags = self.flags() & !flag; + self.flags.set(flags); + } + /// Turns this `StyleBuilder` into a proper `ComputedValues` instance. pub fn build(self) -> Arc { ComputedValues::new( self.pseudo, self.custom_properties, self.writing_mode, - self.flags, + self.flags.get(), self.rules, self.visited_style, % for style_struct in data.active_style_structs(): @@ -3721,7 +3744,7 @@ mod lazy_static_module { writing_mode: WritingMode::empty(), rules: None, visited_style: None, - flags: ComputedValueFlags::empty(), + flags: Cell::new(ComputedValueFlags::empty()), } }; } diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs index 248d0ad0661..8218e2706a3 100644 --- a/components/style/style_adjuster.rs +++ b/components/style/style_adjuster.rs @@ -225,23 +225,17 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { .clone_text_decoration_line() .is_empty() { - self.style - .flags - .insert(ComputedValueFlags::HAS_TEXT_DECORATION_LINES); + self.style.add_flags(ComputedValueFlags::HAS_TEXT_DECORATION_LINES); } if self.style.is_pseudo_element() { - self.style - .flags - .insert(ComputedValueFlags::IS_IN_PSEUDO_ELEMENT_SUBTREE); + self.style.add_flags(ComputedValueFlags::IS_IN_PSEUDO_ELEMENT_SUBTREE); } #[cfg(feature = "servo")] { if self.style.get_parent_column().is_multicol() { - self.style - .flags - .insert(ComputedValueFlags::CAN_BE_FRAGMENTED); + self.style.add_flags(ComputedValueFlags::CAN_BE_FRAGMENTED); } } } @@ -280,9 +274,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { if writing_mode != WritingMode::HorizontalTb && text_combine_upright == TextCombineUpright::All { - self.style - .flags - .insert(ComputedValueFlags::IS_TEXT_COMBINED); + self.style.add_flags(ComputedValueFlags::IS_TEXT_COMBINED); self.style .mutate_inherited_box() .set_writing_mode(WritingMode::HorizontalTb); @@ -303,9 +295,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { .get_parent_flags() .contains(ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK) { - self.style - .flags - .insert(ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK); + self.style.add_flags(ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK); } } @@ -591,9 +581,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { let self_display = self.style.get_box().clone_display(); // Check whether line break should be suppressed for this element. if self.should_suppress_linebreak(layout_parent_style) { - self.style - .flags - .insert(ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK); + self.style.add_flags(ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK); // Inlinify the display type if allowed. if !self.skip_item_display_fixup(element) { let inline_display = self_display.inlinify(); @@ -651,14 +639,10 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { } if element.unwrap().is_visited_link() { - self.style - .flags - .insert(ComputedValueFlags::IS_RELEVANT_LINK_VISITED); + self.style.add_flags(ComputedValueFlags::IS_RELEVANT_LINK_VISITED); } else { // Need to remove to handle unvisited link inside visited. - self.style - .flags - .remove(ComputedValueFlags::IS_RELEVANT_LINK_VISITED); + self.style.remove_flags(ComputedValueFlags::IS_RELEVANT_LINK_VISITED); } } diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index 9a1d366f7fd..b3a7932dd93 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -9,6 +9,7 @@ use super::{AllowQuirks, Number, Percentage, ToComputedValue}; use crate::font_metrics::FontMetricsQueryResult; use crate::parser::{Parse, ParserContext}; +use crate::properties::computed_value_flags::ComputedValueFlags; use crate::values::computed::{self, CSSPixelLength, Context}; use crate::values::generics::length as generics; use crate::values::generics::length::{ @@ -158,6 +159,7 @@ impl FontRelativeLength { if context.for_non_inherited_property.is_some() { context.rule_cache_conditions.borrow_mut().set_uncacheable(); } + context.builder.add_flags(ComputedValueFlags::DEPENDS_ON_FONT_METRICS); let reference_size = match query_font_metrics(context, base_size) { FontMetricsQueryResult::Available(metrics) => metrics.x_height, // https://drafts.csswg.org/css-values/#ex @@ -174,6 +176,7 @@ impl FontRelativeLength { if context.for_non_inherited_property.is_some() { context.rule_cache_conditions.borrow_mut().set_uncacheable(); } + context.builder.add_flags(ComputedValueFlags::DEPENDS_ON_FONT_METRICS); let reference_size = match query_font_metrics(context, base_size) { FontMetricsQueryResult::Available(metrics) => metrics.zero_advance_measure, // https://drafts.csswg.org/css-values/#ch From cf5a2acdf17097d67defe3d989aaa3306516bc05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 19 Mar 2019 21:10:30 +0000 Subject: [PATCH 13/29] style: Remove ComputedStyle::mBits. Use the Servo flags instead. Differential Revision: https://phabricator.services.mozilla.com/D20729 --- .../style/properties/computed_value_flags.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/components/style/properties/computed_value_flags.rs b/components/style/properties/computed_value_flags.rs index 2f6851d3f43..a86f8a4da3a 100644 --- a/components/style/properties/computed_value_flags.rs +++ b/components/style/properties/computed_value_flags.rs @@ -97,3 +97,22 @@ impl ComputedValueFlags { self & Self::maybe_inherited_flags() } } + +/// Asserts that the relevant servo and Gecko representations match. +#[cfg(feature = "gecko")] +#[inline] +pub fn assert_match() { + use crate::gecko_bindings::structs; + macro_rules! assert_bit { + ($rust:ident, $cpp:ident) => { + debug_assert_eq!(ComputedValueFlags::$rust.bits, structs::$cpp); + } + } + + assert_bit!(HAS_TEXT_DECORATION_LINES, ComputedStyleBit_HasTextDecorationLines); + assert_bit!(IS_IN_PSEUDO_ELEMENT_SUBTREE, ComputedStyleBit_HasPseudoElementData); + assert_bit!(SHOULD_SUPPRESS_LINEBREAK, ComputedStyleBit_SuppressLineBreak); + assert_bit!(IS_TEXT_COMBINED, ComputedStyleBit_IsTextCombined); + assert_bit!(IS_RELEVANT_LINK_VISITED, ComputedStyleBit_RelevantLinkVisited); + assert_bit!(DEPENDS_ON_FONT_METRICS, ComputedStyleBit_DependsOnFontMetrics); +} From 39df092bcebe93ac20d724d85640180c02dbdc82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 20 Mar 2019 22:47:30 +0000 Subject: [PATCH 14/29] style: fix some rust-nightly warnings about unused doc comments. Differential Revision: https://phabricator.services.mozilla.com/D24287 --- components/style/bloom.rs | 2 +- components/style/font_face.rs | 1 - components/style/gecko/media_features.rs | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/components/style/bloom.rs b/components/style/bloom.rs index 4e6ae471e50..d75d548ff71 100644 --- a/components/style/bloom.rs +++ b/components/style/bloom.rs @@ -19,7 +19,7 @@ thread_local! { /// such that they can be reused across style traversals. StyleBloom is responsible /// for ensuring that the bloom filter is zeroed when it is dropped. static BLOOM_KEY: Arc> = - Arc::new(AtomicRefCell::new(BloomFilter::new())); + Arc::new(AtomicRefCell::new(BloomFilter::new())); } /// A struct that allows us to fast-reject deep descendant selectors avoiding diff --git a/components/style/font_face.rs b/components/style/font_face.rs index 85dea932173..12ebe3b9864 100644 --- a/components/style/font_face.rs +++ b/components/style/font_face.rs @@ -545,7 +545,6 @@ macro_rules! font_face_descriptors { } } -// css-name rust_identifier: Type, #[cfg(feature = "gecko")] font_face_descriptors! { mandatory descriptors = [ diff --git a/components/style/gecko/media_features.rs b/components/style/gecko/media_features.rs index 9c48836b4d9..12417d38ef0 100644 --- a/components/style/gecko/media_features.rs +++ b/components/style/gecko/media_features.rs @@ -369,8 +369,8 @@ fn eval_prefers_color_scheme(device: &Device, query_value: Option Date: Thu, 21 Mar 2019 04:50:43 +0000 Subject: [PATCH 15/29] style: Allow FontMetricsProvider to produce ex height and zero width independently. We are always able to produce an x height, but depending on whether the glyph exists, we sometimes can't produce a zero glyph width. Differential Revision: https://phabricator.services.mozilla.com/D23424 --- components/style/font_metrics.rs | 18 +++---------- components/style/gecko/wrapper.rs | 19 +++++++------ components/style/values/specified/length.rs | 30 ++++++++++----------- 3 files changed, 29 insertions(+), 38 deletions(-) diff --git a/components/style/font_metrics.rs b/components/style/font_metrics.rs index c8b4a655ad8..c1707d1fbc1 100644 --- a/components/style/font_metrics.rs +++ b/components/style/font_metrics.rs @@ -12,22 +12,12 @@ use app_units::Au; /// Represents the font metrics that style needs from a font to compute the /// value of certain CSS units like `ex`. -#[derive(Clone, Debug, PartialEq)] +#[derive(Clone, Debug, Default, PartialEq)] pub struct FontMetrics { /// The x-height of the font. - pub x_height: Au, + pub x_height: Option, /// The zero advance. This is usually writing mode dependent - pub zero_advance_measure: Au, -} - -/// The result for querying font metrics for a given font family. -#[derive(Clone, Debug, PartialEq)] -pub enum FontMetricsQueryResult { - /// The font is available, but we may or may not have found any font metrics - /// for it. - Available(FontMetrics), - /// The font is not available. - NotAvailable, + pub zero_advance_measure: Option, } /// A trait used to represent something capable of providing us font metrics. @@ -38,7 +28,7 @@ pub trait FontMetricsProvider { _context: &crate::values::computed::Context, _base_size: crate::values::specified::length::FontBaseSize, ) -> FontMetricsQueryResult { - FontMetricsQueryResult::NotAvailable + Default::default() } /// Get default size of a given language and generic family. diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 65a2565b302..8055cd316ea 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -20,7 +20,7 @@ use crate::context::{PostAnimationTasks, QuirksMode, SharedStyleContext, UpdateA use crate::data::ElementData; use crate::dom::{LayoutIterator, NodeInfo, OpaqueNode, TDocument, TElement, TNode, TShadowRoot}; use crate::element_state::{DocumentState, ElementState}; -use crate::font_metrics::{FontMetrics, FontMetricsProvider, FontMetricsQueryResult}; +use crate::font_metrics::{FontMetrics, FontMetricsProvider}; use crate::gecko::data::GeckoStyleSheet; use crate::gecko::selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl}; use crate::gecko::snapshot_helpers; @@ -1035,10 +1035,10 @@ impl FontMetricsProvider for GeckoFontMetricsProvider { &self, context: &crate::values::computed::Context, base_size: FontBaseSize, - ) -> FontMetricsQueryResult { + ) -> FontMetrics { let pc = match context.device().pres_context() { Some(pc) => pc, - None => return FontMetricsQueryResult::NotAvailable, + None => return Default::default(), }; let size = base_size.resolve(context); @@ -1066,11 +1066,14 @@ impl FontMetricsProvider for GeckoFontMetricsProvider { !context.in_media_query, ) }; - let metrics = FontMetrics { - x_height: Au(gecko_metrics.mXSize), - zero_advance_measure: Au(gecko_metrics.mChSize), - }; - FontMetricsQueryResult::Available(metrics) + FontMetrics { + x_height: Some(Au(gecko_metrics.mXSize)), + zero_advance_measure: if gecko_metrics.mChSize >= 0 { + Some(Au(gecko_metrics.mChSize)) + } else { + None + }, + } } } diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index b3a7932dd93..38cf272c795 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -7,7 +7,7 @@ //! [length]: https://drafts.csswg.org/css-values/#lengths use super::{AllowQuirks, Number, Percentage, ToComputedValue}; -use crate::font_metrics::FontMetricsQueryResult; +use crate::font_metrics::FontMetrics; use crate::parser::{Parse, ParserContext}; use crate::properties::computed_value_flags::ComputedValueFlags; use crate::values::computed::{self, CSSPixelLength, Context}; @@ -133,7 +133,7 @@ impl FontRelativeLength { fn query_font_metrics( context: &Context, base_size: FontBaseSize, - ) -> FontMetricsQueryResult { + ) -> FontMetrics { context.font_metrics_provider.query(context, base_size) } @@ -160,16 +160,16 @@ impl FontRelativeLength { context.rule_cache_conditions.borrow_mut().set_uncacheable(); } context.builder.add_flags(ComputedValueFlags::DEPENDS_ON_FONT_METRICS); - let reference_size = match query_font_metrics(context, base_size) { - FontMetricsQueryResult::Available(metrics) => metrics.x_height, + let metrics = query_font_metrics(context, base_size); + let reference_size = metrics.x_height.unwrap_or_else(|| { // https://drafts.csswg.org/css-values/#ex // // In the cases where it is impossible or impractical to // determine the x-height, a value of 0.5em must be // assumed. // - FontMetricsQueryResult::NotAvailable => reference_font_size.scale_by(0.5), - }; + reference_font_size.scale_by(0.5) + }); (reference_size, length) }, FontRelativeLength::Ch(length) => { @@ -177,8 +177,8 @@ impl FontRelativeLength { context.rule_cache_conditions.borrow_mut().set_uncacheable(); } context.builder.add_flags(ComputedValueFlags::DEPENDS_ON_FONT_METRICS); - let reference_size = match query_font_metrics(context, base_size) { - FontMetricsQueryResult::Available(metrics) => metrics.zero_advance_measure, + let metrics = query_font_metrics(context, base_size); + let reference_size = metrics.zero_advance_measure.unwrap_or_else(|| { // https://drafts.csswg.org/css-values/#ch // // In the cases where it is impossible or impractical to @@ -189,14 +189,12 @@ impl FontRelativeLength { // writing-mode is vertical-rl or vertical-lr and // text-orientation is upright). // - FontMetricsQueryResult::NotAvailable => { - if context.style().writing_mode.is_vertical() { - reference_font_size - } else { - reference_font_size.scale_by(0.5) - } - }, - }; + if context.style().writing_mode.is_vertical() { + reference_font_size + } else { + reference_font_size.scale_by(0.5) + } + }); (reference_size, length) }, FontRelativeLength::Rem(length) => { From d4635f1d126ea90d09d00051063fdbfee0abea79 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Thu, 21 Mar 2019 04:50:47 +0000 Subject: [PATCH 16/29] style: Use horizontal metrics for ch in vertical mixed/sideways writing modes and for ex always. Differential Revision: https://phabricator.services.mozilla.com/D23426 --- components/style/font_metrics.rs | 13 +++++++++- components/style/gecko/wrapper.rs | 9 +++++-- components/style/values/specified/length.rs | 28 +++++++++++++++++---- 3 files changed, 42 insertions(+), 8 deletions(-) diff --git a/components/style/font_metrics.rs b/components/style/font_metrics.rs index c1707d1fbc1..15b5fd453e8 100644 --- a/components/style/font_metrics.rs +++ b/components/style/font_metrics.rs @@ -20,6 +20,16 @@ pub struct FontMetrics { pub zero_advance_measure: Option, } +/// Type of font metrics to retrieve. +#[derive(Clone, Debug, PartialEq)] +pub enum FontMetricsOrientation { + /// Get metrics for horizontal or vertical according to the Context's + /// writing mode. + MatchContext, + /// Force getting horizontal metrics. + Horizontal, +} + /// A trait used to represent something capable of providing us font metrics. pub trait FontMetricsProvider { /// Obtain the metrics for given font family. @@ -27,7 +37,8 @@ pub trait FontMetricsProvider { &self, _context: &crate::values::computed::Context, _base_size: crate::values::specified::length::FontBaseSize, - ) -> FontMetricsQueryResult { + _orientation: FontMetricsOrientation, + ) -> FontMetrics { Default::default() } diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 8055cd316ea..ee3dcced3e4 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -20,7 +20,7 @@ use crate::context::{PostAnimationTasks, QuirksMode, SharedStyleContext, UpdateA use crate::data::ElementData; use crate::dom::{LayoutIterator, NodeInfo, OpaqueNode, TDocument, TElement, TNode, TShadowRoot}; use crate::element_state::{DocumentState, ElementState}; -use crate::font_metrics::{FontMetrics, FontMetricsProvider}; +use crate::font_metrics::{FontMetrics, FontMetricsOrientation, FontMetricsProvider}; use crate::gecko::data::GeckoStyleSheet; use crate::gecko::selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl}; use crate::gecko::snapshot_helpers; @@ -1035,6 +1035,7 @@ impl FontMetricsProvider for GeckoFontMetricsProvider { &self, context: &crate::values::computed::Context, base_size: FontBaseSize, + orientation: FontMetricsOrientation, ) -> FontMetrics { let pc = match context.device().pres_context() { Some(pc) => pc, @@ -1056,10 +1057,14 @@ impl FontMetricsProvider for GeckoFontMetricsProvider { }, }; + let vertical_metrics = match orientation { + FontMetricsOrientation::MatchContext => wm.is_vertical() && wm.is_upright(), + FontMetricsOrientation::Horizontal => false, + }; let gecko_metrics = unsafe { bindings::Gecko_GetFontMetrics( pc, - wm.is_vertical() && !wm.is_sideways(), + vertical_metrics, font.gecko(), size.0, // we don't use the user font set in a media query diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index 38cf272c795..750378682bb 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -7,7 +7,7 @@ //! [length]: https://drafts.csswg.org/css-values/#lengths use super::{AllowQuirks, Number, Percentage, ToComputedValue}; -use crate::font_metrics::FontMetrics; +use crate::font_metrics::{FontMetrics, FontMetricsOrientation}; use crate::parser::{Parse, ParserContext}; use crate::properties::computed_value_flags::ComputedValueFlags; use crate::values::computed::{self, CSSPixelLength, Context}; @@ -133,8 +133,9 @@ impl FontRelativeLength { fn query_font_metrics( context: &Context, base_size: FontBaseSize, + orientation: FontMetricsOrientation, ) -> FontMetrics { - context.font_metrics_provider.query(context, base_size) + context.font_metrics_provider.query(context, base_size, orientation) } let reference_font_size = base_size.resolve(context); @@ -160,7 +161,12 @@ impl FontRelativeLength { context.rule_cache_conditions.borrow_mut().set_uncacheable(); } context.builder.add_flags(ComputedValueFlags::DEPENDS_ON_FONT_METRICS); - let metrics = query_font_metrics(context, base_size); + // The x-height is an intrinsically horizontal metric. + let metrics = query_font_metrics( + context, + base_size, + FontMetricsOrientation::Horizontal + ); let reference_size = metrics.x_height.unwrap_or_else(|| { // https://drafts.csswg.org/css-values/#ex // @@ -177,7 +183,18 @@ impl FontRelativeLength { context.rule_cache_conditions.borrow_mut().set_uncacheable(); } context.builder.add_flags(ComputedValueFlags::DEPENDS_ON_FONT_METRICS); - let metrics = query_font_metrics(context, base_size); + // https://drafts.csswg.org/css-values/#ch: + // + // Equal to the used advance measure of the “0” (ZERO, + // U+0030) glyph in the font used to render it. (The advance + // measure of a glyph is its advance width or height, + // whichever is in the inline axis of the element.) + // + let metrics = query_font_metrics( + context, + base_size, + FontMetricsOrientation::MatchContext, + ); let reference_size = metrics.zero_advance_measure.unwrap_or_else(|| { // https://drafts.csswg.org/css-values/#ch // @@ -189,7 +206,8 @@ impl FontRelativeLength { // writing-mode is vertical-rl or vertical-lr and // text-orientation is upright). // - if context.style().writing_mode.is_vertical() { + let wm = context.style().writing_mode; + if wm.is_vertical() && wm.is_upright() { reference_font_size } else { reference_font_size.scale_by(0.5) From 14b176019cb59a7cfaca8095fd0f59082dfb3229 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 21 Mar 2019 17:00:27 +0000 Subject: [PATCH 17/29] style: Simplify a bit our generated bindings by getting rid of FooBorrowed and FooBorrowedMut. This reduces a lot the boilerplate that's needed in order to add simple binding functions. This starts using &Foo and Option<&Foo> instead, and as a result we need to remove the servo_function_signatures test, which is a bit unfortunate. I think it's worth though, this causes problems on some platforms (see bug 1534844), and messing up the functions signature is not something that I've ever seen (other than bug 1308234, which already had all the FooBorrowed mess which I'm removing). Also, cbindgen understands references and Option<&Foo>, so it will be the way to go in the future. After this patch we can also remove HasSimpleFFI, but I've kept it for now since I still use it in a few places, and this patch is quite big on its own. Differential Revision: https://phabricator.services.mozilla.com/D24092 --- components/style/author_styles.rs | 2 +- components/style/build_gecko.rs | 97 +------------------ components/style/gecko/arc_types.rs | 26 ++--- components/style/gecko/conversions.rs | 18 ++-- components/style/gecko/data.rs | 4 +- components/style/gecko/restyle_damage.rs | 4 +- components/style/gecko/wrapper.rs | 46 +++++---- .../style/gecko_bindings/sugar/refptr.rs | 2 +- components/style/properties/gecko.mako.rs | 19 +++- 9 files changed, 68 insertions(+), 150 deletions(-) diff --git a/components/style/author_styles.rs b/components/style/author_styles.rs index 101c2e17349..37eb68ff4f7 100644 --- a/components/style/author_styles.rs +++ b/components/style/author_styles.rs @@ -77,7 +77,7 @@ where #[cfg(feature = "gecko")] unsafe impl HasFFI for AuthorStyles { - type FFIType = crate::gecko_bindings::bindings::RawServoAuthorStyles; + type FFIType = crate::gecko_bindings::structs::RawServoAuthorStyles; } #[cfg(feature = "gecko")] unsafe impl HasSimpleFFI for AuthorStyles {} diff --git a/components/style/build_gecko.rs b/components/style/build_gecko.rs index 734d09569f5..52dc5f51e24 100644 --- a/components/style/build_gecko.rs +++ b/components/style/build_gecko.rs @@ -128,9 +128,6 @@ mod bindings { trait BuilderExt { fn get_initial_builder() -> Builder; fn include>(self, file: T) -> Builder; - fn zero_size_type(self, ty: &str, structs_list: &HashSet<&str>) -> Builder; - fn borrowed_type(self, ty: &str) -> Builder; - fn mutable_borrowed_type(self, ty: &str) -> Builder; } impl BuilderExt for Builder { @@ -181,42 +178,6 @@ mod bindings { fn include>(self, file: T) -> Builder { self.clang_arg("-include").clang_arg(file) } - // This makes an FFI-safe void type that can't be matched on - // &VoidType is UB to have, because you can match on it - // to produce a reachable unreachable. If it's wrapped in - // a struct as a private field it becomes okay again - // - // Not 100% sure of how safe this is, but it's what we're using - // in the XPCOM ffi too - // https://github.com/nikomatsakis/rust-memory-model/issues/2 - fn zero_size_type(self, ty: &str, structs_list: &HashSet<&str>) -> Builder { - if !structs_list.contains(ty) { - self.blacklist_type(ty) - .raw_line(format!("enum {}Void {{ }}", ty)) - .raw_line(format!("pub struct {0}({0}Void);", ty)) - } else { - self - } - } - fn borrowed_type(self, ty: &str) -> Builder { - self.blacklist_type(format!("{}Borrowed", ty)) - .raw_line(format!("pub type {0}Borrowed<'a> = &'a {0};", ty)) - .blacklist_type(format!("{}BorrowedOrNull", ty)) - .raw_line(format!( - "pub type {0}BorrowedOrNull<'a> = Option<&'a {0}>;", - ty - )) - } - fn mutable_borrowed_type(self, ty: &str) -> Builder { - self.borrowed_type(ty) - .blacklist_type(format!("{}BorrowedMut", ty)) - .raw_line(format!("pub type {0}BorrowedMut<'a> = &'a mut {0};", ty)) - .blacklist_type(format!("{}BorrowedMutOrNull", ty)) - .raw_line(format!( - "pub type {0}BorrowedMutOrNull<'a> = Option<&'a mut {0}>;", - ty - )) - } } struct Fixup { @@ -290,13 +251,6 @@ mod bindings { .collect() } - fn get_borrowed_types() -> Vec<(bool, String)> { - get_types("BorrowedTypeList.h", "GECKO_BORROWED_TYPE(?:_MUT)?") - .into_iter() - .map(|(macro_name, type_name)| (macro_name.ends_with("MUT"), type_name)) - .collect() - } - fn get_arc_types() -> Vec { get_types("ServoArcTypeList.h", "SERVO_ARC_TYPE") .into_iter() @@ -480,67 +434,24 @@ mod bindings { } } + // FIXME(emilio): Avoid this altogether. fn generate_bindings() { let builder = Builder::get_initial_builder() .disable_name_namespacing() .with_codegen_config(CodegenConfig::FUNCTIONS); let config = CONFIG["bindings"].as_table().unwrap(); - let mut structs_types = HashSet::new(); let mut fixups = vec![]; let mut builder = BuilderWithConfig::new(builder, config) .handle_common(&mut fixups) .handle_str_items("whitelist-functions", |b, item| b.whitelist_function(item)) - .handle_str_items("structs-types", |mut builder, ty| { - builder = builder - .blacklist_type(ty) - .raw_line(format!("use gecko_bindings::structs::{};", ty)); - structs_types.insert(ty); - // TODO this is hacky, figure out a better way to do it without - // hardcoding everything... - if ty.starts_with("nsStyle") { - builder = builder - .raw_line(format!("unsafe impl Send for {} {{}}", ty)) - .raw_line(format!("unsafe impl Sync for {} {{}}", ty)); - } - builder - }) - // TODO This was added due to servo/rust-bindgen#75, but - // that has been fixed in clang 4.0+. When we switch people - // to libclang 4.0, we can remove this. - .handle_table_items("array-types", |builder, item| { - let cpp_type = item["cpp-type"].as_str().unwrap(); - let rust_type = item["rust-type"].as_str().unwrap(); - builder.raw_line(format!( - concat!( - "pub type nsTArrayBorrowed_{}<'a> = ", - "&'a mut ::gecko_bindings::structs::nsTArray<{}>;" - ), - cpp_type, rust_type - )) - }) - .handle_str_items("servo-immutable-borrow-types", |b, ty| b.borrowed_type(ty)) - // Right now the only immutable borrow types are ones which we import - // from the |structs| module. As such, we don't need to create an opaque - // type with zero_size_type. If we ever introduce immutable borrow types - // which _do_ need to be opaque, we'll need a separate mode. - .handle_str_items("servo-borrow-types", |b, ty| b.mutable_borrowed_type(ty)) .get_builder(); - for (is_mut, ty) in get_borrowed_types().iter() { - if *is_mut { - builder = builder.mutable_borrowed_type(ty); - } else { - builder = builder.borrowed_type(ty); - } - } for ty in get_arc_types().iter() { builder = builder .blacklist_type(format!("{}Strong", ty)) .raw_line(format!( "pub type {0}Strong = ::gecko_bindings::sugar::ownership::Strong<{0}>;", ty - )) - .borrowed_type(ty) - .zero_size_type(ty, &structs_types); + )); } for ty in get_boxed_types().iter() { builder = builder @@ -556,9 +467,7 @@ mod bindings { "::gecko_bindings::sugar::ownership::OwnedOrNull<{0}>;" ), ty - )) - .mutable_borrowed_type(ty) - .zero_size_type(ty, &structs_types); + )); } write_binding_file(builder, BINDINGS_FILE, &fixups); } diff --git a/components/style/gecko/arc_types.rs b/components/style/gecko/arc_types.rs index 95a37f0d905..145670c776f 100644 --- a/components/style/gecko/arc_types.rs +++ b/components/style/gecko/arc_types.rs @@ -9,19 +9,19 @@ #![allow(non_snake_case, missing_docs)] use crate::gecko::url::CssUrlData; -use crate::gecko_bindings::bindings::RawServoCounterStyleRule; -use crate::gecko_bindings::bindings::RawServoFontFeatureValuesRule; -use crate::gecko_bindings::bindings::RawServoImportRule; -use crate::gecko_bindings::bindings::RawServoKeyframe; -use crate::gecko_bindings::bindings::RawServoKeyframesRule; -use crate::gecko_bindings::bindings::RawServoMediaRule; -use crate::gecko_bindings::bindings::RawServoMozDocumentRule; -use crate::gecko_bindings::bindings::RawServoNamespaceRule; -use crate::gecko_bindings::bindings::RawServoPageRule; -use crate::gecko_bindings::bindings::RawServoRuleNode; -use crate::gecko_bindings::bindings::RawServoRuleNodeStrong; -use crate::gecko_bindings::bindings::RawServoSupportsRule; -use crate::gecko_bindings::bindings::ServoCssRules; +use crate::gecko_bindings::structs::RawServoCounterStyleRule; +use crate::gecko_bindings::structs::RawServoFontFeatureValuesRule; +use crate::gecko_bindings::structs::RawServoImportRule; +use crate::gecko_bindings::structs::RawServoKeyframe; +use crate::gecko_bindings::structs::RawServoKeyframesRule; +use crate::gecko_bindings::structs::RawServoMediaRule; +use crate::gecko_bindings::structs::RawServoMozDocumentRule; +use crate::gecko_bindings::structs::RawServoNamespaceRule; +use crate::gecko_bindings::structs::RawServoPageRule; +use crate::gecko_bindings::structs::RawServoRuleNode; +use crate::gecko_bindings::structs::RawServoRuleNodeStrong; +use crate::gecko_bindings::structs::RawServoSupportsRule; +use crate::gecko_bindings::structs::ServoCssRules; use crate::gecko_bindings::structs::RawServoAnimationValue; use crate::gecko_bindings::structs::RawServoCssUrlData; use crate::gecko_bindings::structs::RawServoDeclarationBlock; diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 64f0726b21e..4d667681938 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -12,8 +12,7 @@ use crate::gecko::values::GeckoStyleCoordConvertible; use crate::gecko_bindings::bindings; -use crate::gecko_bindings::structs::RawGeckoGfxMatrix4x4; -use crate::gecko_bindings::structs::{self, nsStyleCoord_CalcValue}; +use crate::gecko_bindings::structs::{self, nsStyleCoord_CalcValue, Matrix4x4Components}; use crate::gecko_bindings::structs::{nsStyleImage, nsresult, SheetType}; use crate::gecko_bindings::sugar::ns_style_coord::{CoordData, CoordDataMut, CoordDataValue}; use crate::stylesheets::{Origin, RulesMutateError}; @@ -986,8 +985,8 @@ pub unsafe fn string_from_chars_pointer(p: *const u16) -> String { String::from_utf16_lossy(char_vec) } -impl<'a> From<&'a RawGeckoGfxMatrix4x4> for Matrix3D { - fn from(m: &'a RawGeckoGfxMatrix4x4) -> Matrix3D { +impl<'a> From<&'a Matrix4x4Components> for Matrix3D { + fn from(m: &'a Matrix4x4Components) -> Matrix3D { Matrix3D { m11: m[0], m12: m[1], @@ -1009,12 +1008,13 @@ impl<'a> From<&'a RawGeckoGfxMatrix4x4> for Matrix3D { } } -impl From for RawGeckoGfxMatrix4x4 { - fn from(matrix: Matrix3D) -> RawGeckoGfxMatrix4x4 { +impl From for Matrix4x4Components { + fn from(matrix: Matrix3D) -> Self { [ - matrix.m11, matrix.m12, matrix.m13, matrix.m14, matrix.m21, matrix.m22, matrix.m23, - matrix.m24, matrix.m31, matrix.m32, matrix.m33, matrix.m34, matrix.m41, matrix.m42, - matrix.m43, matrix.m44, + matrix.m11, matrix.m12, matrix.m13, matrix.m14, matrix.m21, + matrix.m22, matrix.m23, matrix.m24, matrix.m31, matrix.m32, + matrix.m33, matrix.m34, matrix.m41, matrix.m42, matrix.m43, + matrix.m44, ] } } diff --git a/components/style/gecko/data.rs b/components/style/gecko/data.rs index b70f376d7e6..ead7d88502b 100644 --- a/components/style/gecko/data.rs +++ b/components/style/gecko/data.rs @@ -6,8 +6,8 @@ use crate::context::QuirksMode; use crate::dom::TElement; -use crate::gecko_bindings::bindings::{self, RawServoStyleSet}; -use crate::gecko_bindings::structs::{self, ServoStyleSetSizes}; +use crate::gecko_bindings::bindings; +use crate::gecko_bindings::structs::{self, RawServoStyleSet, ServoStyleSetSizes}; use crate::gecko_bindings::structs::{StyleSheet as DomStyleSheet, StyleSheetInfo}; use crate::gecko_bindings::sugar::ownership::{HasArcFFI, HasBoxFFI, HasFFI, HasSimpleFFI}; use crate::invalidation::media_queries::{MediaListKey, ToMediaListKey}; diff --git a/components/style/gecko/restyle_damage.rs b/components/style/gecko/restyle_damage.rs index 9c9d07b7112..a97098363cb 100644 --- a/components/style/gecko/restyle_damage.rs +++ b/components/style/gecko/restyle_damage.rs @@ -51,8 +51,8 @@ impl GeckoRestyleDamage { let mut reset_only = false; let hint = unsafe { bindings::Gecko_CalcStyleDifference( - old_style, - new_style, + old_style.as_gecko_computed_style(), + new_style.as_gecko_computed_style(), &mut any_style_changed, &mut reset_only, ) diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index ee3dcced3e4..93d3290c191 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -53,7 +53,7 @@ use crate::gecko_bindings::structs::ELEMENT_HAS_SNAPSHOT; use crate::gecko_bindings::structs::NODE_DESCENDANTS_NEED_FRAMES; use crate::gecko_bindings::structs::NODE_NEEDS_FRAME; use crate::gecko_bindings::structs::{nsAtom, nsIContent, nsINode_BooleanFlag}; -use crate::gecko_bindings::structs::{RawGeckoElement, RawGeckoNode, RawGeckoXBLBinding}; +use crate::gecko_bindings::structs::{Element as RawGeckoElement, nsINode as RawGeckoNode, nsXBLBinding as RawGeckoXBLBinding}; use crate::gecko_bindings::sugar::ownership::{HasArcFFI, HasSimpleFFI}; use crate::global_style_data::GLOBAL_STYLE_DATA; use crate::hash::FxHashMap; @@ -168,11 +168,7 @@ impl<'lr> TShadowRoot for GeckoShadowRoot<'lr> { where Self: 'a, { - let author_styles = unsafe { - (self.0.mServoStyles.mPtr as *const structs::RawServoAuthorStyles - as *const bindings::RawServoAuthorStyles) - .as_ref()? - }; + let author_styles = unsafe { self.0.mServoStyles.mPtr.as_ref()? }; let author_styles = AuthorStyles::::from_ffi(author_styles); @@ -325,7 +321,7 @@ impl<'ln> GeckoNode<'ln> { // `flattened_tree_parent`. if self.flattened_tree_parent_is_parent() { debug_assert_eq!( - unsafe { bindings::Gecko_GetFlattenedTreeParentNode(self.0).map(GeckoNode) }, + unsafe { bindings::Gecko_GetFlattenedTreeParentNode(self.0).as_ref().map(GeckoNode) }, self.parent_node(), "Fast path stopped holding!" ); @@ -335,7 +331,7 @@ impl<'ln> GeckoNode<'ln> { // NOTE(emilio): If this call is too expensive, we could manually // inline more aggressively. - unsafe { bindings::Gecko_GetFlattenedTreeParentNode(self.0).map(GeckoNode) } + unsafe { bindings::Gecko_GetFlattenedTreeParentNode(self.0).as_ref().map(GeckoNode) } } #[inline] @@ -380,12 +376,12 @@ impl<'ln> TNode for GeckoNode<'ln> { #[inline] fn last_child(&self) -> Option { - unsafe { bindings::Gecko_GetLastChild(self.0).map(GeckoNode) } + unsafe { bindings::Gecko_GetLastChild(self.0).as_ref().map(GeckoNode) } } #[inline] fn prev_sibling(&self) -> Option { - unsafe { bindings::Gecko_GetPreviousSibling(self.0).map(GeckoNode) } + unsafe { bindings::Gecko_GetPreviousSibling(self.0).as_ref().map(GeckoNode) } } #[inline] @@ -502,7 +498,7 @@ impl<'a> Iterator for GeckoChildrenIterator<'a> { // however we can't express this easily with bindgen, and it would // introduce functions with two input lifetimes into bindgen, // which would be out of scope for elision. - bindings::Gecko_GetNextStyleChild(&mut *(it as *mut _)).map(GeckoNode) + bindings::Gecko_GetNextStyleChild(&mut *(it as *mut _)).as_ref().map(GeckoNode) }, } } @@ -548,7 +544,7 @@ impl<'lb> GeckoXBLBinding<'lb> { base.each_xbl_cascade_data(f); } - let data = unsafe { bindings::Gecko_XBLBinding_GetRawServoStyles(self.0) }; + let data = unsafe { bindings::Gecko_XBLBinding_GetRawServoStyles(self.0).as_ref() }; if let Some(data) = data { let data: &'lb _ = AuthorStyles::::from_ffi(data); @@ -709,7 +705,7 @@ impl<'le> GeckoElement<'le> { // FIXME(heycam): Having trouble with bindgen on nsXULElement, // where the binding parent is stored in a member variable // rather than in slots. So just get it through FFI for now. - unsafe { bindings::Gecko_GetBindingParent(self.0).map(GeckoElement) } + unsafe { bindings::Gecko_GetBindingParent(self.0).as_ref().map(GeckoElement) } } else { let binding_parent = unsafe { self.non_xul_xbl_binding_parent_raw_content().as_ref() } .map(GeckoNode::from_content) @@ -717,7 +713,7 @@ impl<'le> GeckoElement<'le> { debug_assert!( binding_parent == - unsafe { bindings::Gecko_GetBindingParent(self.0).map(GeckoElement) } + unsafe { bindings::Gecko_GetBindingParent(self.0).as_ref().map(GeckoElement) } ); binding_parent } @@ -777,7 +773,7 @@ impl<'le> GeckoElement<'le> { return None; } - unsafe { bindings::Gecko_GetBeforeOrAfterPseudo(self.0, is_before).map(GeckoElement) } + unsafe { bindings::Gecko_GetBeforeOrAfterPseudo(self.0, is_before).as_ref().map(GeckoElement) } } #[inline] @@ -887,7 +883,7 @@ impl<'le> GeckoElement<'le> { let mut map = FxHashMap::with_capacity_and_hasher(collection_length, Default::default()); for i in 0..collection_length { - let raw_end_value = unsafe { Gecko_ElementTransitions_EndValueAt(self.0, i) }; + let raw_end_value = unsafe { Gecko_ElementTransitions_EndValueAt(self.0, i).as_ref() }; let end_value = AnimationValue::arc_from_borrowed(&raw_end_value) .expect("AnimationValue not found in ElementTransitions"); @@ -1270,7 +1266,7 @@ impl<'le> TElement for GeckoElement<'le> { return None; } - let declarations = unsafe { Gecko_GetStyleAttrDeclarationBlock(self.0) }; + let declarations = unsafe { Gecko_GetStyleAttrDeclarationBlock(self.0).as_ref() }; let declarations: Option<&RawOffsetArc>> = declarations.and_then(|s| s.as_arc_opt()); declarations.map(|s| s.borrow_arc()) @@ -1551,8 +1547,10 @@ impl<'le> TElement for GeckoElement<'le> { // should destroy all CSS animations in display:none subtree. let computed_data = self.borrow_data(); let computed_values = computed_data.as_ref().map(|d| d.styles.primary()); - let before_change_values = before_change_style.as_ref().map(|x| &**x); - let computed_values_opt = computed_values.as_ref().map(|x| &***x); + let before_change_values = + before_change_style.as_ref().map_or(ptr::null(), |x| x.as_gecko_computed_style()); + let computed_values_opt = + computed_values.as_ref().map_or(ptr::null(), |x| x.as_gecko_computed_style()); unsafe { Gecko_UpdateAnimations( self.0, @@ -1828,7 +1826,7 @@ impl<'le> TElement for GeckoElement<'le> { hints.push(SVG_TEXT_DISABLE_ZOOM_RULE.clone()); } } - let declarations = unsafe { Gecko_GetHTMLPresentationAttrDeclarationBlock(self.0) }; + let declarations = unsafe { Gecko_GetHTMLPresentationAttrDeclarationBlock(self.0).as_ref() }; let declarations: Option<&RawOffsetArc>> = declarations.and_then(|s| s.as_arc_opt()); if let Some(decl) = declarations { @@ -1837,7 +1835,7 @@ impl<'le> TElement for GeckoElement<'le> { ServoCascadeLevel::PresHints, )); } - let declarations = unsafe { Gecko_GetExtraContentStyleDeclarations(self.0) }; + let declarations = unsafe { Gecko_GetExtraContentStyleDeclarations(self.0).as_ref() }; let declarations: Option<&RawOffsetArc>> = declarations.and_then(|s| s.as_arc_opt()); if let Some(decl) = declarations { @@ -1859,10 +1857,10 @@ impl<'le> TElement for GeckoElement<'le> { ); }, VisitedHandlingMode::AllLinksUnvisited => unsafe { - Gecko_GetUnvisitedLinkAttrDeclarationBlock(self.0) + Gecko_GetUnvisitedLinkAttrDeclarationBlock(self.0).as_ref() }, VisitedHandlingMode::RelevantLinkVisited => unsafe { - Gecko_GetVisitedLinkAttrDeclarationBlock(self.0) + Gecko_GetVisitedLinkAttrDeclarationBlock(self.0).as_ref() }, }; let declarations: Option<&RawOffsetArc>> = @@ -1878,7 +1876,7 @@ impl<'le> TElement for GeckoElement<'le> { .state() .intersects(NonTSPseudoClass::Active.state_flag()); if active { - let declarations = unsafe { Gecko_GetActiveLinkAttrDeclarationBlock(self.0) }; + let declarations = unsafe { Gecko_GetActiveLinkAttrDeclarationBlock(self.0).as_ref() }; let declarations: Option<&RawOffsetArc>> = declarations.and_then(|s| s.as_arc_opt()); if let Some(decl) = declarations { diff --git a/components/style/gecko_bindings/sugar/refptr.rs b/components/style/gecko_bindings/sugar/refptr.rs index b289e2e21ec..1ad71afa6bc 100644 --- a/components/style/gecko_bindings/sugar/refptr.rs +++ b/components/style/gecko_bindings/sugar/refptr.rs @@ -285,7 +285,7 @@ macro_rules! impl_threadsafe_refcount { } impl_threadsafe_refcount!( - structs::RawGeckoURLExtraData, + structs::mozilla::URLExtraData, bindings::Gecko_AddRefURLExtraDataArbitraryThread, bindings::Gecko_ReleaseURLExtraDataArbitraryThread ); diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 331ffb5c34f..0879e5846c3 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -66,16 +66,25 @@ use crate::values::generics::url::UrlOrNone; pub mod style_structs { % for style_struct in data.style_structs: pub use super::${style_struct.gecko_struct_name} as ${style_struct.name}; + + unsafe impl Send for ${style_struct.name} {} + unsafe impl Sync for ${style_struct.name} {} % endfor + } /// FIXME(emilio): This is completely duplicated with the other properties code. -pub type ComputedValuesInner = crate::gecko_bindings::structs::ServoComputedData; +pub type ComputedValuesInner = structs::ServoComputedData; #[repr(C)] -pub struct ComputedValues(crate::gecko_bindings::structs::mozilla::ComputedStyle); +pub struct ComputedValues(structs::mozilla::ComputedStyle); impl ComputedValues { + #[inline] + pub (crate) fn as_gecko_computed_style(&self) -> &structs::ComputedStyle { + &self.0 + } + pub fn new( pseudo: Option<<&PseudoElement>, custom_properties: Option>, @@ -929,7 +938,7 @@ transform_functions = [ debug_assert!(!${item}${index + 1}.0.is_empty()); % endif ${css_value_setters[item] % ( - "bindings::Gecko_CSSValue_GetArrayItem(gecko_value, %d)" % (index + 1), + "(&mut *bindings::Gecko_CSSValue_GetArrayItem(gecko_value, %d))" % (index + 1), item + str(index + 1) )}; % endfor @@ -980,7 +989,7 @@ transform_functions = [ % endif <% getter = css_value_getters[item] % ( - "bindings::Gecko_CSSValue_GetArrayItemConst(gecko_value, %d)" % (index + 1) + "(&*bindings::Gecko_CSSValue_GetArrayItemConst(gecko_value, %d))" % (index + 1) ) %> ${getter}, @@ -989,6 +998,7 @@ transform_functions = [ }, +#[allow(unused_parens)] fn set_single_transform_function( servo_value: &values::computed::TransformOperation, gecko_value: &mut structs::nsCSSValue /* output */ @@ -1031,6 +1041,7 @@ pub fn convert_transform( output.set_move(list); } +#[allow(unused_parens)] fn clone_single_transform_function( gecko_value: &structs::nsCSSValue ) -> values::computed::TransformOperation { From 4b4b5b6a1dc7df63d085d2ea6fe09201a841728f Mon Sep 17 00:00:00 2001 From: Mats Palmgren Date: Sun, 24 Mar 2019 23:13:52 +0100 Subject: [PATCH 18/29] style: Implement the counter-set property. Bug: 1518201 Reviewed-by: emilio --- components/style/properties/gecko.mako.rs | 4 ++-- .../style/properties/longhands/counters.mako.rs | 11 ++++++++++- components/style/values/computed/counters.rs | 6 +++--- components/style/values/computed/mod.rs | 2 +- components/style/values/generics/counters.rs | 12 ++++++------ components/style/values/specified/counters.rs | 8 ++++---- components/style/values/specified/mod.rs | 2 +- 7 files changed, 27 insertions(+), 18 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 0879e5846c3..e3bd2ea559f 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -4559,7 +4559,7 @@ clip-path <%self:impl_trait style_struct_name="Counters" - skip_longhands="content counter-increment counter-reset"> + skip_longhands="content counter-increment counter-reset counter-set"> pub fn ineffective_content_property(&self) -> bool { self.gecko.mContents.is_empty() } @@ -4788,7 +4788,7 @@ clip-path ) } - % for counter_property in ["Increment", "Reset"]: + % for counter_property in ["Increment", "Reset", "Set"]: pub fn set_counter_${counter_property.lower()}( &mut self, v: longhands::counter_${counter_property.lower()}::computed_value::T diff --git a/components/style/properties/longhands/counters.mako.rs b/components/style/properties/longhands/counters.mako.rs index b690eeb5c45..715aba1066e 100644 --- a/components/style/properties/longhands/counters.mako.rs +++ b/components/style/properties/longhands/counters.mako.rs @@ -27,9 +27,18 @@ ${helpers.predefined_type( ${helpers.predefined_type( "counter-reset", - "CounterReset", + "CounterSetOrReset", initial_value="Default::default()", animation_value_type="discrete", spec="https://drafts.csswg.org/css-lists-3/#propdef-counter-reset", servo_restyle_damage="rebuild_and_reflow", )} + +${helpers.predefined_type( + "counter-set", + "CounterSetOrReset", + initial_value="Default::default()", + animation_value_type="discrete", + spec="https://drafts.csswg.org/css-lists-3/#propdef-counter-set", + servo_restyle_damage="rebuild_and_reflow", +)} diff --git a/components/style/values/computed/counters.rs b/components/style/values/computed/counters.rs index aaf83afe485..3a083632eb9 100644 --- a/components/style/values/computed/counters.rs +++ b/components/style/values/computed/counters.rs @@ -7,13 +7,13 @@ use crate::values::computed::url::ComputedImageUrl; use crate::values::generics::counters as generics; use crate::values::generics::counters::CounterIncrement as GenericCounterIncrement; -use crate::values::generics::counters::CounterReset as GenericCounterReset; +use crate::values::generics::counters::CounterSetOrReset as GenericCounterSetOrReset; /// A computed value for the `counter-increment` property. pub type CounterIncrement = GenericCounterIncrement; -/// A computed value for the `counter-increment` property. -pub type CounterReset = GenericCounterReset; +/// A computed value for the `counter-set` and `counter-reset` properties. +pub type CounterSetOrReset = GenericCounterSetOrReset; /// A computed value for the `content` property. pub type Content = generics::Content; diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 1f7d1e3eddc..184bac3b12a 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -45,7 +45,7 @@ pub use self::box_::{OverflowClipBox, OverscrollBehavior, Perspective, Resize}; pub use self::box_::{ScrollSnapAlign, ScrollSnapType, TouchAction, VerticalAlign, WillChange}; pub use self::color::{Color, ColorOrAuto, ColorPropertyValue, RGBAColor}; pub use self::column::ColumnCount; -pub use self::counters::{Content, ContentItem, CounterIncrement, CounterReset}; +pub use self::counters::{Content, ContentItem, CounterIncrement, CounterSetOrReset}; pub use self::easing::TimingFunction; pub use self::effects::{BoxShadow, Filter, SimpleShadow}; pub use self::flex::FlexBasis; diff --git a/components/style/values/generics/counters.rs b/components/style/values/generics/counters.rs index 0f5d6bbb939..7bab3c4bbc5 100644 --- a/components/style/values/generics/counters.rs +++ b/components/style/values/generics/counters.rs @@ -45,21 +45,21 @@ impl Deref for CounterIncrement { } } -/// A generic value for the `counter-reset` property. +/// A generic value for the `counter-set` and `counter-reset` properties. #[derive( Clone, Debug, Default, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss, )] -pub struct CounterReset(Counters); +pub struct CounterSetOrReset(Counters); -impl CounterReset { - /// Returns a new value for `counter-reset`. +impl CounterSetOrReset { + /// Returns a new value for `counter-set` / `counter-reset`. #[inline] pub fn new(counters: Vec>) -> Self { - CounterReset(Counters(counters.into_boxed_slice())) + CounterSetOrReset(Counters(counters.into_boxed_slice())) } } -impl Deref for CounterReset { +impl Deref for CounterSetOrReset { type Target = [CounterPair]; #[inline] diff --git a/components/style/values/specified/counters.rs b/components/style/values/specified/counters.rs index 6ee6575fc94..262e7765a4a 100644 --- a/components/style/values/specified/counters.rs +++ b/components/style/values/specified/counters.rs @@ -10,7 +10,7 @@ use crate::parser::{Parse, ParserContext}; use crate::values::generics::counters as generics; use crate::values::generics::counters::CounterIncrement as GenericCounterIncrement; use crate::values::generics::counters::CounterPair; -use crate::values::generics::counters::CounterReset as GenericCounterReset; +use crate::values::generics::counters::CounterSetOrReset as GenericCounterSetOrReset; #[cfg(feature = "gecko")] use crate::values::generics::CounterStyleOrNone; use crate::values::specified::url::SpecifiedImageUrl; @@ -34,10 +34,10 @@ impl Parse for CounterIncrement { } } -/// A specified value for the `counter-increment` property. -pub type CounterReset = GenericCounterReset; +/// A specified value for the `counter-set` and `counter-reset` properties. +pub type CounterSetOrReset = GenericCounterSetOrReset; -impl Parse for CounterReset { +impl Parse for CounterSetOrReset { fn parse<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 7295f060697..e673005f3e6 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -44,7 +44,7 @@ pub use self::box_::{ScrollSnapAlign, ScrollSnapType}; pub use self::box_::{TouchAction, TransitionProperty, VerticalAlign, WillChange}; pub use self::color::{Color, ColorOrAuto, ColorPropertyValue, RGBAColor}; pub use self::column::ColumnCount; -pub use self::counters::{Content, ContentItem, CounterIncrement, CounterReset}; +pub use self::counters::{Content, ContentItem, CounterIncrement, CounterSetOrReset}; pub use self::easing::TimingFunction; pub use self::effects::{BoxShadow, Filter, SimpleShadow}; pub use self::flex::FlexBasis; From 4f44b1c6b1bec49580b65abce1b755a3de7d247b Mon Sep 17 00:00:00 2001 From: Mats Palmgren Date: Sun, 24 Mar 2019 23:13:52 +0100 Subject: [PATCH 19/29] style: Add an inherited internal UA sheet property (-moz-list-reversed:true|false) to propagate
    to its relevant descendants. Bug: 288704 Reviewed-by: emilio --- components/style/cbindgen.toml | 1 + components/style/properties/data.py | 1 + components/style/properties/gecko.mako.rs | 1 + components/style/properties/longhands/list.mako.rs | 12 ++++++++++++ components/style/values/computed/list.rs | 1 + components/style/values/computed/mod.rs | 1 + components/style/values/specified/list.rs | 10 ++++++++++ components/style/values/specified/mod.rs | 1 + 8 files changed, 28 insertions(+) diff --git a/components/style/cbindgen.toml b/components/style/cbindgen.toml index b5d374d77bf..54288dff7ad 100644 --- a/components/style/cbindgen.toml +++ b/components/style/cbindgen.toml @@ -108,6 +108,7 @@ include = [ "TouchAction", "WillChangeBits", "TextDecorationLine", + "MozListReversed", ] item_types = ["enums", "structs", "typedefs"] diff --git a/components/style/properties/data.py b/components/style/properties/data.py index 9347d9b9428..52cdcf15d22 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -324,6 +324,7 @@ class Longhand(object): "JustifyItems", "JustifySelf", "MozForceBrokenImageIcon", + "MozListReversed", "MozScriptLevel", "MozScriptMinSize", "MozScriptSizeMultiplier", diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index e3bd2ea559f..7c5f4a94c01 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1247,6 +1247,7 @@ impl Clone for ${style_struct.gecko_struct_name} { "Length": impl_absolute_length, "LengthOrNormal": impl_style_coord, "LengthPercentageOrAuto": impl_style_coord, + "MozListReversed": impl_simple, "MozScriptMinSize": impl_absolute_length, "RGBAColor": impl_rgba_color, "SVGLength": impl_svg_length, diff --git a/components/style/properties/longhands/list.mako.rs b/components/style/properties/longhands/list.mako.rs index 4b368ea2ba5..6d79d3ad9ee 100644 --- a/components/style/properties/longhands/list.mako.rs +++ b/components/style/properties/longhands/list.mako.rs @@ -68,3 +68,15 @@ ${helpers.predefined_type( boxed=True, spec="Nonstandard (https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-image-region)", )} + +${helpers.predefined_type( + "-moz-list-reversed", + "MozListReversed", + "computed::MozListReversed::False", + animation_value_type="discrete", + products="gecko", + enabled_in="ua", + needs_context=False, + spec="Internal implementation detail for
      ", + servo_restyle_damage="rebuild_and_reflow", +)} diff --git a/components/style/values/computed/list.rs b/components/style/values/computed/list.rs index 622c021554f..2bde35e3b6b 100644 --- a/components/style/values/computed/list.rs +++ b/components/style/values/computed/list.rs @@ -6,6 +6,7 @@ #[cfg(feature = "gecko")] pub use crate::values::specified::list::ListStyleType; +pub use crate::values::specified::list::MozListReversed; pub use crate::values::specified::list::{QuotePair, Quotes}; use servo_arc::Arc; diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 184bac3b12a..4c92218e69b 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -64,6 +64,7 @@ pub use self::length::{LengthOrAuto, LengthPercentageOrAuto, MaxSize, Size}; pub use self::length::{NonNegativeLengthPercentage, NonNegativeLengthPercentageOrAuto}; #[cfg(feature = "gecko")] pub use self::list::ListStyleType; +pub use self::list::MozListReversed; pub use self::list::{QuotePair, Quotes}; pub use self::motion::OffsetPath; pub use self::outline::OutlineStyle; diff --git a/components/style/values/specified/list.rs b/components/style/values/specified/list.rs index b37ade47081..d75455a1c71 100644 --- a/components/style/values/specified/list.rs +++ b/components/style/values/specified/list.rs @@ -123,3 +123,13 @@ impl Parse for Quotes { } } } + +/// Specified and computed `-moz-list-reversed` property (for UA sheets only). +#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss)] +#[repr(u8)] +pub enum MozListReversed { + /// the initial value + False, + /// exclusively used for
        in our html.css UA sheet + True, +} diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index e673005f3e6..919cb0cb341 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -66,6 +66,7 @@ pub use self::length::{NoCalcLength, ViewportPercentageLength}; pub use self::length::{NonNegativeLengthPercentage, NonNegativeLengthPercentageOrAuto}; #[cfg(feature = "gecko")] pub use self::list::ListStyleType; +pub use self::list::MozListReversed; pub use self::list::{QuotePair, Quotes}; pub use self::motion::OffsetPath; pub use self::outline::OutlineStyle; From ed74e8acbb9095dc903d6037a8a652e497da6a21 Mon Sep 17 00:00:00 2001 From: Mats Palmgren Date: Sun, 24 Mar 2019 23:13:52 +0100 Subject: [PATCH 20/29] style: Implement display:list-item counters using a built-in 'list-item' CSS counter. Bug: 288704 Reviewed-by: emilio --- components/style/style_adjuster.rs | 42 ++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs index 8218e2706a3..8df5bc222bf 100644 --- a/components/style/style_adjuster.rs +++ b/components/style/style_adjuster.rs @@ -708,6 +708,47 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { } } + /// For HTML elements with 'display:list-item' we add a default 'counter-increment:list-item' + /// unless 'counter-increment' already has a value for 'list-item'. + /// + /// https://drafts.csswg.org/css-lists-3/#declaring-a-list-item + #[cfg(feature = "gecko")] + fn adjust_for_list_item(&mut self, element: Option) + where + E: TElement, + { + use crate::properties::longhands::counter_increment::computed_value::T as ComputedIncrement; + use crate::values::CustomIdent; + use crate::values::generics::counters::{CounterPair}; + use crate::values::specified::list::MozListReversed; + + if self.style.get_box().clone_display() != Display::ListItem { + return; + } + if self.style.pseudo.is_some() { + return; + } + if !element.map_or(false, |e| e.is_html_element()) { + return; + } + // Note that we map
      1. to 'counter-set: list-item INTEGER; + // counter-increment: list-item 0;' so we'll return here unless the author + // explicitly specified something else. + let increments = self.style.get_counters().clone_counter_increment(); + if increments.iter().any(|i| i.name.0 == atom!("list-item")) { + return; + } + + let reversed = self.style.get_list().clone__moz_list_reversed() == MozListReversed::True; + let increment = if reversed { -1 } else { 1 }; + let list_increment = CounterPair { + name: CustomIdent(atom!("list-item")), + value: increment, + }; + let increments = increments.iter().cloned().chain(std::iter::once(list_increment)); + self.style.mutate_counters().set_counter_increment(ComputedIncrement::new(increments.collect())); + } + /// Adjusts the style to account for various fixups that don't fit naturally /// into the cascade. /// @@ -772,6 +813,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { #[cfg(feature = "gecko")] { self.adjust_for_appearance(element); + self.adjust_for_list_item(element); } self.set_bits(); } From ab8c00e41a99cbfb72e232879c6100c96c18a40d Mon Sep 17 00:00:00 2001 From: Mats Palmgren Date: Sun, 24 Mar 2019 23:13:53 +0100 Subject: [PATCH 21/29] style: Add support for the ::marker pseudo element on list items. Alias :-moz-list-bullet/number to that in the parser. Bug: 205202 Reviewed-by: emilio --- components/style/dom.rs | 5 +++++ components/style/gecko/pseudo_element.rs | 4 +++- components/style/gecko/pseudo_element_definition.mako.rs | 5 ++++- components/style/gecko/wrapper.rs | 8 ++++++++ components/style/invalidation/element/invalidator.rs | 4 ++++ 5 files changed, 24 insertions(+), 2 deletions(-) diff --git a/components/style/dom.rs b/components/style/dom.rs index 1c64543310c..28798e280b6 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -441,6 +441,11 @@ pub trait TElement: None } + /// The ::marker pseudo-element of this element, if it exists. + fn marker_pseudo_element(&self) -> Option { + None + } + /// Execute `f` for each anonymous content child (apart from ::before and /// ::after) whose originating element is `self`. fn each_anonymous_content_child(&self, _f: F) diff --git a/components/style/gecko/pseudo_element.rs b/components/style/gecko/pseudo_element.rs index 8160ea2c9b8..c49277409e0 100644 --- a/components/style/gecko/pseudo_element.rs +++ b/components/style/gecko/pseudo_element.rs @@ -33,7 +33,7 @@ impl ::selectors::parser::PseudoElement for PseudoElement { fn valid_after_slotted(&self) -> bool { matches!( *self, - PseudoElement::Before | PseudoElement::After | PseudoElement::Placeholder + PseudoElement::Before | PseudoElement::After | PseudoElement::Marker | PseudoElement::Placeholder ) } @@ -180,6 +180,8 @@ impl PseudoElement { /// Whether this pseudo-element should actually exist if it has /// the given styles. pub fn should_exist(&self, style: &ComputedValues) -> bool { + debug_assert!(self.is_eager()); + if style.get_box().clone_display() == Display::None { return false; } diff --git a/components/style/gecko/pseudo_element_definition.mako.rs b/components/style/gecko/pseudo_element_definition.mako.rs index be156b54a14..e2dffcbdd74 100644 --- a/components/style/gecko/pseudo_element_definition.mako.rs +++ b/components/style/gecko/pseudo_element_definition.mako.rs @@ -195,13 +195,16 @@ impl PseudoElement { return Some(${pseudo_element_variant(pseudo)}) } % endfor - // Alias "-moz-selection" to "selection" at parse time. + // Alias some legacy prefixed pseudos to their standardized name at parse time: "-moz-selection" => { return Some(PseudoElement::Selection); } "-moz-placeholder" => { return Some(PseudoElement::Placeholder); } + "-moz-list-bullet" | "-moz-list-number" => { + return Some(PseudoElement::Marker); + } _ => { if starts_with_ignore_ascii_case(name, "-moz-tree-") { return PseudoElement::tree_pseudo_element(name, Box::new([])) diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index 93d3290c191..dedfe7715e8 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1137,6 +1137,14 @@ impl<'le> TElement for GeckoElement<'le> { self.before_or_after_pseudo(/* is_before = */ false) } + fn marker_pseudo_element(&self) -> Option { + if !self.has_properties() { + return None; + } + + unsafe { bindings::Gecko_GetMarkerPseudo(self.0).as_ref().map(GeckoElement) } + } + #[inline] fn is_html_element(&self) -> bool { self.namespace_id() == structs::kNameSpaceID_XHTML as i32 diff --git a/components/style/invalidation/element/invalidator.rs b/components/style/invalidation/element/invalidator.rs index 4640c7c0442..7ed97c0b197 100644 --- a/components/style/invalidation/element/invalidator.rs +++ b/components/style/invalidation/element/invalidator.rs @@ -542,6 +542,10 @@ where any_descendant |= self.invalidate_dom_descendants_of(anon_content, invalidations); } + if let Some(marker) = self.element.marker_pseudo_element() { + any_descendant |= self.invalidate_pseudo_element_or_nac(marker, invalidations); + } + if let Some(before) = self.element.before_pseudo_element() { any_descendant |= self.invalidate_pseudo_element_or_nac(before, invalidations); } From 64f19ae34db3e754dbc62288e01598700315d978 Mon Sep 17 00:00:00 2001 From: Mats Palmgren Date: Sun, 24 Mar 2019 23:13:53 +0100 Subject: [PATCH 22/29] style: Add support for the 'content' CSS property on ::marker pseudo elements. Bug: 205202 Reviewed-by: emilio --- components/style/style_adjuster.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs index 8df5bc222bf..54b7dfc8a5b 100644 --- a/components/style/style_adjuster.rs +++ b/components/style/style_adjuster.rs @@ -174,10 +174,15 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { /// Apply the blockification rules based on the table in CSS 2.2 section 9.7. /// + /// A ::marker pseudo-element with 'list-style-position:outside' needs to + /// have its 'display' blockified. fn blockify_if_necessary(&mut self, layout_parent_style: &ComputedValues, element: Option) where E: TElement, { + use crate::selector_parser::PseudoElement; + use crate::computed_values::list_style_position::T as ListStylePosition; + let mut blockify = false; macro_rules! blockify_if { ($if_what:expr) => { @@ -200,6 +205,8 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { blockify_if!(self.style.floated()); blockify_if!(self.style.out_of_flow_positioned()); + blockify_if!(self.style.pseudo == Some(&PseudoElement::Marker) && + self.style.get_parent_list().clone_list_style_position() == ListStylePosition::Outside); if !blockify { return; From 02bc29a11b06de7e6de290d5c2f31fda69c8e9b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 26 Mar 2019 13:25:42 +0000 Subject: [PATCH 23/29] style: Simplify Owned FFI types. And make them actually sound. We're defining functions on Rust-land that get structs as arguments, but declaring them in C++ as getting pointers. This is another step in order to be able to autogenerate ServoBindings.h and remove bindings.rs altogether. We remove FooOwned in favor of Owned, which is generated via cbindgen. It'd be good to actually mark Owned and such as MOZ_MUST_USE_TYPE, so I sent https://github.com/eqrion/cbindgen/pull/307 for that. Differential Revision: https://phabricator.services.mozilla.com/D24681 --- components/style/build_gecko.rs | 23 ------------------- components/style/cbindgen.toml | 20 ++++++++++++++++ .../style/gecko_bindings/sugar/ownership.rs | 21 ----------------- 3 files changed, 20 insertions(+), 44 deletions(-) diff --git a/components/style/build_gecko.rs b/components/style/build_gecko.rs index 52dc5f51e24..6b536c4122b 100644 --- a/components/style/build_gecko.rs +++ b/components/style/build_gecko.rs @@ -258,13 +258,6 @@ mod bindings { .collect() } - fn get_boxed_types() -> Vec { - get_types("ServoBoxedTypeList.h", "SERVO_BOXED_TYPE") - .into_iter() - .map(|(_, type_name)| type_name) - .collect() - } - struct BuilderWithConfig<'a> { builder: Builder, config: &'a Table, @@ -453,22 +446,6 @@ mod bindings { ty )); } - for ty in get_boxed_types().iter() { - builder = builder - .blacklist_type(format!("{}Owned", ty)) - .raw_line(format!( - "pub type {0}Owned = ::gecko_bindings::sugar::ownership::Owned<{0}>;", - ty - )) - .blacklist_type(format!("{}OwnedOrNull", ty)) - .raw_line(format!( - concat!( - "pub type {0}OwnedOrNull = ", - "::gecko_bindings::sugar::ownership::OwnedOrNull<{0}>;" - ), - ty - )); - } write_binding_file(builder, BINDINGS_FILE, &fixups); } diff --git a/components/style/cbindgen.toml b/components/style/cbindgen.toml index 54288dff7ad..41da88bbb35 100644 --- a/components/style/cbindgen.toml +++ b/components/style/cbindgen.toml @@ -109,6 +109,8 @@ include = [ "WillChangeBits", "TextDecorationLine", "MozListReversed", + "Owned", + "OwnedOrNull", ] item_types = ["enums", "structs", "typedefs"] @@ -208,3 +210,21 @@ item_types = ["enums", "structs", "typedefs"] static inline StyleRestyleHint RecascadeSubtree(); static inline StyleRestyleHint ForAnimations(); """ + +# TODO(emilio): Add hooks to cbindgen to be able to generate MOZ_MUST_USE_TYPE +# or MOZ_MUST_USE on the functions. +"Owned" = """ + UniquePtr Consume() { + UniquePtr ret(ptr); + ptr = nullptr; + return ret; + } +""" + +"OwnedOrNull" = """ + UniquePtr Consume() { + UniquePtr ret(ptr); + ptr = nullptr; + return ret; + } +""" diff --git a/components/style/gecko_bindings/sugar/ownership.rs b/components/style/gecko_bindings/sugar/ownership.rs index 3f1f1315eda..33f5b39553b 100644 --- a/components/style/gecko_bindings/sugar/ownership.rs +++ b/components/style/gecko_bindings/sugar/ownership.rs @@ -320,27 +320,6 @@ impl OwnedOrNull { self.ptr.is_null() } - /// Returns an owned pointer if this is non-null, and `None` otherwise. - pub fn into_box_opt(self) -> Option> - where - ServoType: HasBoxFFI, - { - if self.is_null() { - None - } else { - Some(unsafe { transmute(self) }) - } - } - - /// Returns an `Owned` if non-null, `None` otherwise. - pub fn into_owned_opt(self) -> Option> { - if self.is_null() { - None - } else { - Some(unsafe { transmute(self) }) - } - } - /// Gets a immutable reference to the underlying Gecko type, or `None` if /// null. pub fn borrow(&self) -> Option<&GeckoType> { From ecda72a5fd2ca41b1150efc2d80d89eb20f0d388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 26 Mar 2019 13:26:33 +0000 Subject: [PATCH 24/29] style: Cleanup refcounted types. And make the handling of ComputedStyle more similar to these. Differential Revision: https://phabricator.services.mozilla.com/D24703 --- components/style/build_gecko.rs | 48 +------------------ components/style/cbindgen.toml | 9 ++++ components/style/gecko/arc_types.rs | 28 ----------- .../style/gecko_bindings/sugar/ownership.rs | 2 +- 4 files changed, 11 insertions(+), 76 deletions(-) diff --git a/components/style/build_gecko.rs b/components/style/build_gecko.rs index 6b536c4122b..ba53068be82 100644 --- a/components/style/build_gecko.rs +++ b/components/style/build_gecko.rs @@ -220,44 +220,6 @@ mod bindings { .expect("Unable to write output"); } - fn get_types(filename: &str, macro_pat: &str) -> Vec<(String, String)> { - // Read the file - let path = DISTDIR_PATH.join("include/mozilla/").join(filename); - let mut list_file = File::open(path).expect(&format!("Unable to open {}", filename)); - let mut content = String::new(); - list_file - .read_to_string(&mut content) - .expect(&format!("Failed to read {}", filename)); - // Remove comments - let block_comment_re = Regex::new(r#"(?s)/\*.*?\*/"#).unwrap(); - let line_comment_re = Regex::new(r#"//.*"#).unwrap(); - let content = block_comment_re.replace_all(&content, ""); - let content = line_comment_re.replace_all(&content, ""); - // Extract the list - let re_string = format!(r#"^({})\(.+,\s*(\w+)\)$"#, macro_pat); - let re = Regex::new(&re_string).unwrap(); - content - .lines() - .map(|line| line.trim()) - .filter(|line| !line.is_empty()) - .map(|line| { - let captures = re - .captures(&line) - .expect(&format!("Unrecognized line in {}: '{}'", filename, line)); - let macro_name = captures.get(1).unwrap().as_str().to_string(); - let type_name = captures.get(2).unwrap().as_str().to_string(); - (macro_name, type_name) - }) - .collect() - } - - fn get_arc_types() -> Vec { - get_types("ServoArcTypeList.h", "SERVO_ARC_TYPE") - .into_iter() - .map(|(_, type_name)| type_name) - .collect() - } - struct BuilderWithConfig<'a> { builder: Builder, config: &'a Table, @@ -434,18 +396,10 @@ mod bindings { .with_codegen_config(CodegenConfig::FUNCTIONS); let config = CONFIG["bindings"].as_table().unwrap(); let mut fixups = vec![]; - let mut builder = BuilderWithConfig::new(builder, config) + let builder = BuilderWithConfig::new(builder, config) .handle_common(&mut fixups) .handle_str_items("whitelist-functions", |b, item| b.whitelist_function(item)) .get_builder(); - for ty in get_arc_types().iter() { - builder = builder - .blacklist_type(format!("{}Strong", ty)) - .raw_line(format!( - "pub type {0}Strong = ::gecko_bindings::sugar::ownership::Strong<{0}>;", - ty - )); - } write_binding_file(builder, BINDINGS_FILE, &fixups); } diff --git a/components/style/cbindgen.toml b/components/style/cbindgen.toml index 41da88bbb35..cd910f0be9b 100644 --- a/components/style/cbindgen.toml +++ b/components/style/cbindgen.toml @@ -111,6 +111,7 @@ include = [ "MozListReversed", "Owned", "OwnedOrNull", + "Strong", ] item_types = ["enums", "structs", "typedefs"] @@ -228,3 +229,11 @@ item_types = ["enums", "structs", "typedefs"] return ret; } """ + +"Strong" = """ + already_AddRefed Consume() { + already_AddRefed ret(const_cast(ptr)); + ptr = nullptr; + return ret; + } +""" diff --git a/components/style/gecko/arc_types.rs b/components/style/gecko/arc_types.rs index 145670c776f..58d2fe5af9d 100644 --- a/components/style/gecko/arc_types.rs +++ b/components/style/gecko/arc_types.rs @@ -18,8 +18,6 @@ use crate::gecko_bindings::structs::RawServoMediaRule; use crate::gecko_bindings::structs::RawServoMozDocumentRule; use crate::gecko_bindings::structs::RawServoNamespaceRule; use crate::gecko_bindings::structs::RawServoPageRule; -use crate::gecko_bindings::structs::RawServoRuleNode; -use crate::gecko_bindings::structs::RawServoRuleNodeStrong; use crate::gecko_bindings::structs::RawServoSupportsRule; use crate::gecko_bindings::structs::ServoCssRules; use crate::gecko_bindings::structs::RawServoAnimationValue; @@ -34,7 +32,6 @@ use crate::gecko_bindings::sugar::ownership::{HasArcFFI, HasFFI, Strong}; use crate::media_queries::MediaList; use crate::properties::animated_properties::AnimationValue; use crate::properties::{ComputedValues, PropertyDeclarationBlock}; -use crate::rule_tree::StrongRuleNode; use crate::shared_lock::Locked; use crate::stylesheets::keyframes_rule::Keyframe; use crate::stylesheets::{CounterStyleRule, CssRules, FontFaceRule, FontFeatureValuesRule}; @@ -121,31 +118,6 @@ impl_arc_ffi!(CssUrlData => RawServoCssUrlData impl_arc_ffi!(Box<[QuotePair]> => RawServoQuotes [Servo_Quotes_AddRef, Servo_Quotes_Release]); -// RuleNode is a Arc-like type but it does not use Arc. - -impl StrongRuleNode { - pub fn into_strong(self) -> RawServoRuleNodeStrong { - let ptr = self.ptr(); - mem::forget(self); - unsafe { mem::transmute(ptr) } - } - - pub fn from_ffi<'a>(ffi: &'a &RawServoRuleNode) -> &'a Self { - unsafe { &*(ffi as *const &RawServoRuleNode as *const StrongRuleNode) } - } -} - -#[no_mangle] -pub unsafe extern "C" fn Servo_RuleNode_AddRef(obj: &RawServoRuleNode) { - mem::forget(StrongRuleNode::from_ffi(&obj).clone()); -} - -#[no_mangle] -pub unsafe extern "C" fn Servo_RuleNode_Release(obj: &RawServoRuleNode) { - let ptr = StrongRuleNode::from_ffi(&obj); - ptr::read(ptr as *const StrongRuleNode); -} - // ComputedStyle is not an opaque type on any side of FFI. // This means that doing the HasArcFFI type trick is actually unsound, // since it gives us a way to construct an Arc from diff --git a/components/style/gecko_bindings/sugar/ownership.rs b/components/style/gecko_bindings/sugar/ownership.rs index 33f5b39553b..04fbf2e55c8 100644 --- a/components/style/gecko_bindings/sugar/ownership.rs +++ b/components/style/gecko_bindings/sugar/ownership.rs @@ -135,12 +135,12 @@ pub unsafe trait HasArcFFI: HasFFI { } } -#[repr(C)] /// Gecko-FFI-safe Arc (T is an ArcInner). /// /// This can be null. /// /// Leaks on drop. Please don't drop this. +#[repr(C)] pub struct Strong { ptr: *const GeckoType, _marker: PhantomData, From f637d93fc18a4de8bce732fb2da246d9e203de3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 26 Mar 2019 13:38:14 +0000 Subject: [PATCH 25/29] style: Remove bindings.rs. I kept it building the most straight-forward way possible (pub use) because it seems to me that bindings is not a bad name, and we should probably move structs.rs to be bindings.rs rather than the other way around. But that's a different bug in any case, need to think more about it. Differential Revision: https://phabricator.services.mozilla.com/D24713 --- components/style/build_gecko.rs | 39 ++++---------------------- components/style/gecko_bindings/mod.rs | 7 ++--- 2 files changed, 7 insertions(+), 39 deletions(-) diff --git a/components/style/build_gecko.rs b/components/style/build_gecko.rs index ba53068be82..f68ac639697 100644 --- a/components/style/build_gecko.rs +++ b/components/style/build_gecko.rs @@ -32,7 +32,6 @@ mod bindings { use toml::value::Table; const STRUCTS_FILE: &'static str = "structs.rs"; - const BINDINGS_FILE: &'static str = "bindings.rs"; fn read_config(path: &PathBuf) -> Table { println!("cargo:rerun-if-changed={}", path.to_str().unwrap()); @@ -295,10 +294,11 @@ mod bindings { fn generate_structs() { let builder = Builder::get_initial_builder() .enable_cxx_namespaces() - .with_codegen_config(CodegenConfig::TYPES | CodegenConfig::VARS); + .with_codegen_config(CodegenConfig::TYPES | CodegenConfig::VARS | CodegenConfig::FUNCTIONS); let mut fixups = vec![]; let builder = BuilderWithConfig::new(builder, CONFIG["structs"].as_table().unwrap()) .handle_common(&mut fixups) + .handle_str_items("whitelist-functions", |b, item| b.whitelist_function(item)) .handle_str_items("bitfield-enums", |b, item| b.bitfield_enum(item)) .handle_str_items("rusty-enums", |b, item| b.rustified_enum(item)) .handle_str_items("whitelist-vars", |b, item| b.whitelist_var(item)) @@ -389,20 +389,6 @@ mod bindings { } } - // FIXME(emilio): Avoid this altogether. - fn generate_bindings() { - let builder = Builder::get_initial_builder() - .disable_name_namespacing() - .with_codegen_config(CodegenConfig::FUNCTIONS); - let config = CONFIG["bindings"].as_table().unwrap(); - let mut fixups = vec![]; - let builder = BuilderWithConfig::new(builder, config) - .handle_common(&mut fixups) - .handle_str_items("whitelist-functions", |b, item| b.whitelist_function(item)) - .get_builder(); - write_binding_file(builder, BINDINGS_FILE, &fixups); - } - fn generate_atoms() { let script = PathBuf::from(env::var_os("CARGO_MANIFEST_DIR").unwrap()) .join("gecko") @@ -420,24 +406,9 @@ mod bindings { } pub fn generate() { - use std::thread; - macro_rules! run_tasks { - ($($task:expr,)+) => { - if setup_logging() { - $($task;)+ - } else { - let threads = vec![$( thread::spawn(|| $task) ),+]; - for thread in threads.into_iter() { - thread.join().unwrap(); - } - } - } - } - run_tasks! { - generate_structs(), - generate_bindings(), - generate_atoms(), - } + setup_logging(); + generate_structs(); + generate_atoms(); for path in ADDED_PATHS.lock().unwrap().iter() { println!("cargo:rerun-if-changed={}", path.to_str().unwrap()); diff --git a/components/style/gecko_bindings/mod.rs b/components/style/gecko_bindings/mod.rs index 78fc8e3ae76..87f444f5a91 100644 --- a/components/style/gecko_bindings/mod.rs +++ b/components/style/gecko_bindings/mod.rs @@ -4,11 +4,6 @@ //! Gecko's C++ bindings, along with some rust helpers to ease its use. -#[allow(dead_code, improper_ctypes, non_camel_case_types, missing_docs)] -pub mod bindings { - include!(concat!(env!("OUT_DIR"), "/gecko/bindings.rs")); -} - // FIXME: We allow `improper_ctypes` (for now), because the lint doesn't allow // foreign structs to have `PhantomData`. We should remove this once the lint // ignores this case. @@ -25,4 +20,6 @@ pub mod structs { include!(concat!(env!("OUT_DIR"), "/gecko/structs.rs")); } +pub use self::structs as bindings; + pub mod sugar; From f9ce3a9da284ad3ff9d87c105abbcc1a7e4a08be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 26 Mar 2019 18:42:27 +0000 Subject: [PATCH 26/29] style: Honor browser.display.use_document_fonts again. Differential Revision: https://phabricator.services.mozilla.com/D24218 --- components/style/properties/cascade.rs | 65 ++++++++++++++++++-------- 1 file changed, 46 insertions(+), 19 deletions(-) diff --git a/components/style/properties/cascade.rs b/components/style/properties/cascade.rs index 638e36e24d4..7d4c8c6275c 100644 --- a/components/style/properties/cascade.rs +++ b/components/style/properties/cascade.rs @@ -673,9 +673,12 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { true } - // The default font type (which is stored in FontFamilyList's - // `mDefaultFontType`) depends on the current lang group, so - // we may need to recompute it if it changed. + /// The default font type (which is stored in FontFamilyList's + /// `mDefaultFontType`) depends on the current lang group and generic font + /// family, so we may need to recompute it if or the family changed. + /// + /// Also, we prioritize non-document fonts here if we need to (see the pref + /// `browser.display.use_document_fonts`). #[inline] #[cfg(feature = "gecko")] fn recompute_default_font_family_type_if_needed(&mut self) { @@ -686,30 +689,54 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { return; } + let use_document_fonts = unsafe { structs::StaticPrefs_sVarCache_browser_display_use_document_fonts != 0 }; let builder = &mut self.context.builder; - let default_font_type = { + let (default_font_type, prioritize_user_fonts) = { let font = builder.get_font().gecko(); - let default_font_type = if font.mFont.systemFont { - structs::FontFamilyType::eFamily_none - } else { - unsafe { - bindings::Gecko_nsStyleFont_ComputeDefaultFontType( - builder.device.document(), - font.mGenericID, - font.mLanguage.mRawPtr, - ) - } - }; - if font.mFont.fontlist.mDefaultFontType == default_font_type { + // System fonts are all right, and should have the default font type + // set to none already, so bail out early. + if font.mFont.systemFont { + debug_assert_eq!(font.mFont.fontlist.mDefaultFontType, structs::FontFamilyType::eFamily_none); return; } - default_font_type + let default_font_type = unsafe { + bindings::Gecko_nsStyleFont_ComputeDefaultFontType( + builder.device.document(), + font.mGenericID, + font.mLanguage.mRawPtr, + ) + }; + + // We prioritize user fonts over document fonts if the pref is set, + // and we don't have a generic family already (or we're using + // cursive or fantasy, since they're ignored, see bug 789788), and + // we have a generic family to actually replace it with. + let prioritize_user_fonts = + !use_document_fonts && + matches!( + font.mGenericID, + structs::kGenericFont_NONE | + structs::kGenericFont_fantasy | + structs::kGenericFont_cursive + ) && + default_font_type != structs::FontFamilyType::eFamily_none; + + if !prioritize_user_fonts && default_font_type == font.mFont.fontlist.mDefaultFontType { + // Nothing to do. + return; + } + (default_font_type, prioritize_user_fonts) }; - builder.mutate_font().gecko_mut().mFont.fontlist.mDefaultFontType = - default_font_type; + let font = builder.mutate_font().gecko_mut(); + font.mFont.fontlist.mDefaultFontType = default_font_type; + if prioritize_user_fonts { + unsafe { + bindings::Gecko_nsStyleFont_PrioritizeUserFonts(font, default_font_type) + } + } } /// Some keyword sizes depend on the font family and language. From ff41f827208ade9a49ae23dc799ab8f13f1c226d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 26 Mar 2019 16:23:12 +0100 Subject: [PATCH 27/29] style: Fix Gecko and Servo builds, and appease tidy. --- components/style/build_gecko.rs | 4 +- components/style/gecko/arc_types.rs | 14 ++-- components/style/gecko/conversions.rs | 7 +- components/style/gecko/pseudo_element.rs | 11 ++- components/style/gecko/wrapper.rs | 79 +++++++++++++------ components/style/global_style_data.rs | 2 +- .../properties/longhands/counters.mako.rs | 1 + .../style/properties/properties.mako.rs | 6 +- components/style/servo/selector_parser.rs | 6 ++ components/style/style_adjuster.rs | 39 ++++++--- components/style/stylesheets/viewport_rule.rs | 2 +- components/style/values/computed/font.rs | 8 +- components/style/values/computed/text.rs | 2 +- components/style/values/specified/length.rs | 26 +++--- components/style/values/specified/list.rs | 14 +++- components/style/values/specified/text.rs | 12 ++- 16 files changed, 155 insertions(+), 78 deletions(-) diff --git a/components/style/build_gecko.rs b/components/style/build_gecko.rs index f68ac639697..4866909899c 100644 --- a/components/style/build_gecko.rs +++ b/components/style/build_gecko.rs @@ -294,7 +294,9 @@ mod bindings { fn generate_structs() { let builder = Builder::get_initial_builder() .enable_cxx_namespaces() - .with_codegen_config(CodegenConfig::TYPES | CodegenConfig::VARS | CodegenConfig::FUNCTIONS); + .with_codegen_config( + CodegenConfig::TYPES | CodegenConfig::VARS | CodegenConfig::FUNCTIONS, + ); let mut fixups = vec![]; let builder = BuilderWithConfig::new(builder, CONFIG["structs"].as_table().unwrap()) .handle_common(&mut fixups) diff --git a/components/style/gecko/arc_types.rs b/components/style/gecko/arc_types.rs index 58d2fe5af9d..309f2395085 100644 --- a/components/style/gecko/arc_types.rs +++ b/components/style/gecko/arc_types.rs @@ -9,25 +9,25 @@ #![allow(non_snake_case, missing_docs)] use crate::gecko::url::CssUrlData; +use crate::gecko_bindings::structs::RawServoAnimationValue; use crate::gecko_bindings::structs::RawServoCounterStyleRule; +use crate::gecko_bindings::structs::RawServoCssUrlData; +use crate::gecko_bindings::structs::RawServoDeclarationBlock; +use crate::gecko_bindings::structs::RawServoFontFaceRule; use crate::gecko_bindings::structs::RawServoFontFeatureValuesRule; use crate::gecko_bindings::structs::RawServoImportRule; use crate::gecko_bindings::structs::RawServoKeyframe; use crate::gecko_bindings::structs::RawServoKeyframesRule; +use crate::gecko_bindings::structs::RawServoMediaList; use crate::gecko_bindings::structs::RawServoMediaRule; use crate::gecko_bindings::structs::RawServoMozDocumentRule; use crate::gecko_bindings::structs::RawServoNamespaceRule; use crate::gecko_bindings::structs::RawServoPageRule; -use crate::gecko_bindings::structs::RawServoSupportsRule; -use crate::gecko_bindings::structs::ServoCssRules; -use crate::gecko_bindings::structs::RawServoAnimationValue; -use crate::gecko_bindings::structs::RawServoCssUrlData; -use crate::gecko_bindings::structs::RawServoDeclarationBlock; -use crate::gecko_bindings::structs::RawServoFontFaceRule; -use crate::gecko_bindings::structs::RawServoMediaList; use crate::gecko_bindings::structs::RawServoQuotes; use crate::gecko_bindings::structs::RawServoStyleRule; use crate::gecko_bindings::structs::RawServoStyleSheetContents; +use crate::gecko_bindings::structs::RawServoSupportsRule; +use crate::gecko_bindings::structs::ServoCssRules; use crate::gecko_bindings::sugar::ownership::{HasArcFFI, HasFFI, Strong}; use crate::media_queries::MediaList; use crate::properties::animated_properties::AnimationValue; diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 4d667681938..356636fc6bb 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -1011,10 +1011,9 @@ impl<'a> From<&'a Matrix4x4Components> for Matrix3D { impl From for Matrix4x4Components { fn from(matrix: Matrix3D) -> Self { [ - matrix.m11, matrix.m12, matrix.m13, matrix.m14, matrix.m21, - matrix.m22, matrix.m23, matrix.m24, matrix.m31, matrix.m32, - matrix.m33, matrix.m34, matrix.m41, matrix.m42, matrix.m43, - matrix.m44, + matrix.m11, matrix.m12, matrix.m13, matrix.m14, matrix.m21, matrix.m22, matrix.m23, + matrix.m24, matrix.m31, matrix.m32, matrix.m33, matrix.m34, matrix.m41, matrix.m42, + matrix.m43, matrix.m44, ] } } diff --git a/components/style/gecko/pseudo_element.rs b/components/style/gecko/pseudo_element.rs index c49277409e0..479c12b9cde 100644 --- a/components/style/gecko/pseudo_element.rs +++ b/components/style/gecko/pseudo_element.rs @@ -33,7 +33,10 @@ impl ::selectors::parser::PseudoElement for PseudoElement { fn valid_after_slotted(&self) -> bool { matches!( *self, - PseudoElement::Before | PseudoElement::After | PseudoElement::Marker | PseudoElement::Placeholder + PseudoElement::Before | + PseudoElement::After | + PseudoElement::Marker | + PseudoElement::Placeholder ) } @@ -109,6 +112,12 @@ impl PseudoElement { *self == PseudoElement::After } + /// Whether this pseudo-element is the ::marker pseudo. + #[inline] + pub fn is_marker(&self) -> bool { + *self == PseudoElement::Marker + } + /// Whether this pseudo-element is ::first-letter. #[inline] pub fn is_first_letter(&self) -> bool { diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index dedfe7715e8..46eaa427b3a 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -53,7 +53,9 @@ use crate::gecko_bindings::structs::ELEMENT_HAS_SNAPSHOT; use crate::gecko_bindings::structs::NODE_DESCENDANTS_NEED_FRAMES; use crate::gecko_bindings::structs::NODE_NEEDS_FRAME; use crate::gecko_bindings::structs::{nsAtom, nsIContent, nsINode_BooleanFlag}; -use crate::gecko_bindings::structs::{Element as RawGeckoElement, nsINode as RawGeckoNode, nsXBLBinding as RawGeckoXBLBinding}; +use crate::gecko_bindings::structs::{ + nsINode as RawGeckoNode, nsXBLBinding as RawGeckoXBLBinding, Element as RawGeckoElement, +}; use crate::gecko_bindings::sugar::ownership::{HasArcFFI, HasSimpleFFI}; use crate::global_style_data::GLOBAL_STYLE_DATA; use crate::hash::FxHashMap; @@ -321,7 +323,11 @@ impl<'ln> GeckoNode<'ln> { // `flattened_tree_parent`. if self.flattened_tree_parent_is_parent() { debug_assert_eq!( - unsafe { bindings::Gecko_GetFlattenedTreeParentNode(self.0).as_ref().map(GeckoNode) }, + unsafe { + bindings::Gecko_GetFlattenedTreeParentNode(self.0) + .as_ref() + .map(GeckoNode) + }, self.parent_node(), "Fast path stopped holding!" ); @@ -331,7 +337,11 @@ impl<'ln> GeckoNode<'ln> { // NOTE(emilio): If this call is too expensive, we could manually // inline more aggressively. - unsafe { bindings::Gecko_GetFlattenedTreeParentNode(self.0).as_ref().map(GeckoNode) } + unsafe { + bindings::Gecko_GetFlattenedTreeParentNode(self.0) + .as_ref() + .map(GeckoNode) + } } #[inline] @@ -381,7 +391,11 @@ impl<'ln> TNode for GeckoNode<'ln> { #[inline] fn prev_sibling(&self) -> Option { - unsafe { bindings::Gecko_GetPreviousSibling(self.0).as_ref().map(GeckoNode) } + unsafe { + bindings::Gecko_GetPreviousSibling(self.0) + .as_ref() + .map(GeckoNode) + } } #[inline] @@ -498,7 +512,9 @@ impl<'a> Iterator for GeckoChildrenIterator<'a> { // however we can't express this easily with bindgen, and it would // introduce functions with two input lifetimes into bindgen, // which would be out of scope for elision. - bindings::Gecko_GetNextStyleChild(&mut *(it as *mut _)).as_ref().map(GeckoNode) + bindings::Gecko_GetNextStyleChild(&mut *(it as *mut _)) + .as_ref() + .map(GeckoNode) }, } } @@ -705,7 +721,11 @@ impl<'le> GeckoElement<'le> { // FIXME(heycam): Having trouble with bindgen on nsXULElement, // where the binding parent is stored in a member variable // rather than in slots. So just get it through FFI for now. - unsafe { bindings::Gecko_GetBindingParent(self.0).as_ref().map(GeckoElement) } + unsafe { + bindings::Gecko_GetBindingParent(self.0) + .as_ref() + .map(GeckoElement) + } } else { let binding_parent = unsafe { self.non_xul_xbl_binding_parent_raw_content().as_ref() } .map(GeckoNode::from_content) @@ -713,7 +733,11 @@ impl<'le> GeckoElement<'le> { debug_assert!( binding_parent == - unsafe { bindings::Gecko_GetBindingParent(self.0).as_ref().map(GeckoElement) } + unsafe { + bindings::Gecko_GetBindingParent(self.0) + .as_ref() + .map(GeckoElement) + } ); binding_parent } @@ -773,7 +797,11 @@ impl<'le> GeckoElement<'le> { return None; } - unsafe { bindings::Gecko_GetBeforeOrAfterPseudo(self.0, is_before).as_ref().map(GeckoElement) } + unsafe { + bindings::Gecko_GetBeforeOrAfterPseudo(self.0, is_before) + .as_ref() + .map(GeckoElement) + } } #[inline] @@ -795,11 +823,7 @@ impl<'le> GeckoElement<'le> { /// animation. /// /// Also this function schedules style flush. - pub unsafe fn note_explicit_hints( - &self, - restyle_hint: RestyleHint, - change_hint: nsChangeHint, - ) { + pub unsafe fn note_explicit_hints(&self, restyle_hint: RestyleHint, change_hint: nsChangeHint) { use crate::gecko::restyle_damage::GeckoRestyleDamage; let damage = GeckoRestyleDamage::new(change_hint); @@ -1042,13 +1066,10 @@ impl FontMetricsProvider for GeckoFontMetricsProvider { let style = context.style(); let (wm, font) = match base_size { - FontBaseSize::CurrentStyle => { - (style.writing_mode, style.get_font()) - }, + FontBaseSize::CurrentStyle => (style.writing_mode, style.get_font()), // These are only used for font-size computation, and the first is // really dubious... - FontBaseSize::InheritedStyleButStripEmUnits | - FontBaseSize::InheritedStyle => { + FontBaseSize::InheritedStyleButStripEmUnits | FontBaseSize::InheritedStyle => { (*style.inherited_writing_mode(), style.get_parent_font()) }, }; @@ -1142,7 +1163,11 @@ impl<'le> TElement for GeckoElement<'le> { return None; } - unsafe { bindings::Gecko_GetMarkerPseudo(self.0).as_ref().map(GeckoElement) } + unsafe { + bindings::Gecko_GetMarkerPseudo(self.0) + .as_ref() + .map(GeckoElement) + } } #[inline] @@ -1555,10 +1580,12 @@ impl<'le> TElement for GeckoElement<'le> { // should destroy all CSS animations in display:none subtree. let computed_data = self.borrow_data(); let computed_values = computed_data.as_ref().map(|d| d.styles.primary()); - let before_change_values = - before_change_style.as_ref().map_or(ptr::null(), |x| x.as_gecko_computed_style()); - let computed_values_opt = - computed_values.as_ref().map_or(ptr::null(), |x| x.as_gecko_computed_style()); + let before_change_values = before_change_style + .as_ref() + .map_or(ptr::null(), |x| x.as_gecko_computed_style()); + let computed_values_opt = computed_values + .as_ref() + .map_or(ptr::null(), |x| x.as_gecko_computed_style()); unsafe { Gecko_UpdateAnimations( self.0, @@ -1834,7 +1861,8 @@ impl<'le> TElement for GeckoElement<'le> { hints.push(SVG_TEXT_DISABLE_ZOOM_RULE.clone()); } } - let declarations = unsafe { Gecko_GetHTMLPresentationAttrDeclarationBlock(self.0).as_ref() }; + let declarations = + unsafe { Gecko_GetHTMLPresentationAttrDeclarationBlock(self.0).as_ref() }; let declarations: Option<&RawOffsetArc>> = declarations.and_then(|s| s.as_arc_opt()); if let Some(decl) = declarations { @@ -1884,7 +1912,8 @@ impl<'le> TElement for GeckoElement<'le> { .state() .intersects(NonTSPseudoClass::Active.state_flag()); if active { - let declarations = unsafe { Gecko_GetActiveLinkAttrDeclarationBlock(self.0).as_ref() }; + let declarations = + unsafe { Gecko_GetActiveLinkAttrDeclarationBlock(self.0).as_ref() }; let declarations: Option<&RawOffsetArc>> = declarations.and_then(|s| s.as_arc_opt()); if let Some(decl) = declarations { diff --git a/components/style/global_style_data.rs b/components/style/global_style_data.rs index 8f852dccb45..90936ff7172 100644 --- a/components/style/global_style_data.rs +++ b/components/style/global_style_data.rs @@ -11,7 +11,6 @@ use crate::parallel::STYLE_THREAD_STACK_SIZE_KB; use crate::shared_lock::SharedRwLock; use crate::thread_state; use rayon; -use servo_config::pref; use std::env; /// Global style data @@ -67,6 +66,7 @@ lazy_static! { Ok(num) => num, #[cfg(feature = "servo")] _ => { + use servo_config::pref; // We always set this pref on startup, before layout or script // have had a chance of accessing (and thus creating) the // thread-pool. diff --git a/components/style/properties/longhands/counters.mako.rs b/components/style/properties/longhands/counters.mako.rs index 715aba1066e..f8971315603 100644 --- a/components/style/properties/longhands/counters.mako.rs +++ b/components/style/properties/longhands/counters.mako.rs @@ -41,4 +41,5 @@ ${helpers.predefined_type( animation_value_type="discrete", spec="https://drafts.csswg.org/css-lists-3/#propdef-counter-set", servo_restyle_damage="rebuild_and_reflow", + products="gecko", )} diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index f57e27eff1e..c9bc2bd8e2f 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -2759,7 +2759,7 @@ pub struct ComputedValuesInner { pub writing_mode: WritingMode, /// A set of flags we use to store misc information regarding this style. - flags: ComputedValueFlags, + pub flags: ComputedValueFlags, /// The rule node representing the ordered list of rules matched for this /// node. Can be None for default values and text nodes. This is @@ -3719,8 +3719,8 @@ mod lazy_static_module { use super::{ComputedValues, ComputedValuesInner, longhands, style_structs}; use super::computed_value_flags::ComputedValueFlags; - /// The initial values for all style structs as defined by the specification. lazy_static! { + /// The initial values for all style structs as defined by the specification. pub static ref INITIAL_SERVO_VALUES: ComputedValues = ComputedValues { inner: ComputedValuesInner { % for style_struct in data.active_style_structs(): @@ -3744,7 +3744,7 @@ mod lazy_static_module { writing_mode: WritingMode::empty(), rules: None, visited_style: None, - flags: Cell::new(ComputedValueFlags::empty()), + flags: ComputedValueFlags::empty(), } }; } diff --git a/components/style/servo/selector_parser.rs b/components/style/servo/selector_parser.rs index 9abfe976919..229f7dccdd2 100644 --- a/components/style/servo/selector_parser.rs +++ b/components/style/servo/selector_parser.rs @@ -142,6 +142,12 @@ impl PseudoElement { false } + /// Whether this pseudo-element is the ::marker pseudo. + #[inline] + pub fn is_marker(&self) -> bool { + false + } + /// Whether this pseudo-element is the ::before pseudo. #[inline] pub fn is_before(&self) -> bool { diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs index 54b7dfc8a5b..070a2299969 100644 --- a/components/style/style_adjuster.rs +++ b/components/style/style_adjuster.rs @@ -180,7 +180,6 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { where E: TElement, { - use crate::selector_parser::PseudoElement; use crate::computed_values::list_style_position::T as ListStylePosition; let mut blockify = false; @@ -205,8 +204,11 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { blockify_if!(self.style.floated()); blockify_if!(self.style.out_of_flow_positioned()); - blockify_if!(self.style.pseudo == Some(&PseudoElement::Marker) && - self.style.get_parent_list().clone_list_style_position() == ListStylePosition::Outside); + blockify_if!( + self.style.pseudo.map_or(false, |p| p.is_marker()) && + self.style.get_parent_list().clone_list_style_position() == + ListStylePosition::Outside + ); if !blockify { return; @@ -232,11 +234,13 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { .clone_text_decoration_line() .is_empty() { - self.style.add_flags(ComputedValueFlags::HAS_TEXT_DECORATION_LINES); + self.style + .add_flags(ComputedValueFlags::HAS_TEXT_DECORATION_LINES); } if self.style.is_pseudo_element() { - self.style.add_flags(ComputedValueFlags::IS_IN_PSEUDO_ELEMENT_SUBTREE); + self.style + .add_flags(ComputedValueFlags::IS_IN_PSEUDO_ELEMENT_SUBTREE); } #[cfg(feature = "servo")] @@ -302,7 +306,8 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { .get_parent_flags() .contains(ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK) { - self.style.add_flags(ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK); + self.style + .add_flags(ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK); } } @@ -588,7 +593,8 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { let self_display = self.style.get_box().clone_display(); // Check whether line break should be suppressed for this element. if self.should_suppress_linebreak(layout_parent_style) { - self.style.add_flags(ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK); + self.style + .add_flags(ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK); // Inlinify the display type if allowed. if !self.skip_item_display_fixup(element) { let inline_display = self_display.inlinify(); @@ -646,10 +652,12 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { } if element.unwrap().is_visited_link() { - self.style.add_flags(ComputedValueFlags::IS_RELEVANT_LINK_VISITED); + self.style + .add_flags(ComputedValueFlags::IS_RELEVANT_LINK_VISITED); } else { // Need to remove to handle unvisited link inside visited. - self.style.remove_flags(ComputedValueFlags::IS_RELEVANT_LINK_VISITED); + self.style + .remove_flags(ComputedValueFlags::IS_RELEVANT_LINK_VISITED); } } @@ -725,9 +733,9 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { E: TElement, { use crate::properties::longhands::counter_increment::computed_value::T as ComputedIncrement; - use crate::values::CustomIdent; - use crate::values::generics::counters::{CounterPair}; + use crate::values::generics::counters::CounterPair; use crate::values::specified::list::MozListReversed; + use crate::values::CustomIdent; if self.style.get_box().clone_display() != Display::ListItem { return; @@ -752,8 +760,13 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { name: CustomIdent(atom!("list-item")), value: increment, }; - let increments = increments.iter().cloned().chain(std::iter::once(list_increment)); - self.style.mutate_counters().set_counter_increment(ComputedIncrement::new(increments.collect())); + let increments = increments + .iter() + .cloned() + .chain(std::iter::once(list_increment)); + self.style + .mutate_counters() + .set_counter_increment(ComputedIncrement::new(increments.collect())); } /// Adjusts the style to account for various fixups that don't fit naturally diff --git a/components/style/stylesheets/viewport_rule.rs b/components/style/stylesheets/viewport_rule.rs index 3d7b29f19bf..02be35261db 100644 --- a/components/style/stylesheets/viewport_rule.rs +++ b/components/style/stylesheets/viewport_rule.rs @@ -27,7 +27,6 @@ use cssparser::CowRcStr; use cssparser::{parse_important, AtRuleParser, DeclarationListParser, DeclarationParser, Parser}; use euclid::TypedSize2D; use selectors::parser::SelectorParseErrorKind; -use servo_config::pref; use std::borrow::Cow; use std::cell::RefCell; use std::fmt::{self, Write}; @@ -39,6 +38,7 @@ use style_traits::{CssWriter, ParseError, PinchZoomFactor, StyleParseErrorKind, /// Whether parsing and processing of `@viewport` rules is enabled. #[cfg(feature = "servo")] pub fn enabled() -> bool { + use servo_config::pref; pref!(layout.viewport.enabled) } diff --git a/components/style/values/computed/font.rs b/components/style/values/computed/font.rs index 6bbcf23bc14..c78eb9df2e3 100644 --- a/components/style/values/computed/font.rs +++ b/components/style/values/computed/font.rs @@ -190,9 +190,7 @@ impl FontFamily { /// Get default font family as `serif` which is a generic font-family pub fn serif() -> Self { FontFamily { - families: FontFamilyList::new(Box::new([SingleFontFamily::Generic( - atom!("serif"), - )])), + families: FontFamilyList::new(Box::new([SingleFontFamily::Generic(atom!("serif"))])), is_system_font: false, } } @@ -204,7 +202,9 @@ impl MallocSizeOf for FontFamily { // SharedFontList objects are generally shared from the pointer // stored in the specified value. So only count this if the // SharedFontList is unshared. - unsafe { bindings::Gecko_SharedFontList_SizeOfIncludingThisIfUnshared(self.families.0.get()) } + unsafe { + bindings::Gecko_SharedFontList_SizeOfIncludingThisIfUnshared(self.families.0.get()) + } } } diff --git a/components/style/values/computed/text.rs b/components/style/values/computed/text.rs index a1f68bf1a1b..db56a16a82d 100644 --- a/components/style/values/computed/text.rs +++ b/components/style/values/computed/text.rs @@ -19,8 +19,8 @@ use std::fmt::{self, Write}; use style_traits::{CssWriter, ToCss}; pub use crate::values::specified::TextAlignKeyword as TextAlign; -pub use crate::values::specified::{TextEmphasisPosition, TextDecorationLine}; pub use crate::values::specified::{OverflowWrap, WordBreak}; +pub use crate::values::specified::{TextDecorationLine, TextEmphasisPosition}; /// A computed value for the `initial-letter` property. pub type InitialLetter = GenericInitialLetter; diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index 750378682bb..631de5c87ec 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -135,7 +135,9 @@ impl FontRelativeLength { base_size: FontBaseSize, orientation: FontMetricsOrientation, ) -> FontMetrics { - context.font_metrics_provider.query(context, base_size, orientation) + context + .font_metrics_provider + .query(context, base_size, orientation) } let reference_font_size = base_size.resolve(context); @@ -160,13 +162,12 @@ impl FontRelativeLength { if context.for_non_inherited_property.is_some() { context.rule_cache_conditions.borrow_mut().set_uncacheable(); } - context.builder.add_flags(ComputedValueFlags::DEPENDS_ON_FONT_METRICS); + context + .builder + .add_flags(ComputedValueFlags::DEPENDS_ON_FONT_METRICS); // The x-height is an intrinsically horizontal metric. - let metrics = query_font_metrics( - context, - base_size, - FontMetricsOrientation::Horizontal - ); + let metrics = + query_font_metrics(context, base_size, FontMetricsOrientation::Horizontal); let reference_size = metrics.x_height.unwrap_or_else(|| { // https://drafts.csswg.org/css-values/#ex // @@ -182,7 +183,9 @@ impl FontRelativeLength { if context.for_non_inherited_property.is_some() { context.rule_cache_conditions.borrow_mut().set_uncacheable(); } - context.builder.add_flags(ComputedValueFlags::DEPENDS_ON_FONT_METRICS); + context + .builder + .add_flags(ComputedValueFlags::DEPENDS_ON_FONT_METRICS); // https://drafts.csswg.org/css-values/#ch: // // Equal to the used advance measure of the “0” (ZERO, @@ -190,11 +193,8 @@ impl FontRelativeLength { // measure of a glyph is its advance width or height, // whichever is in the inline axis of the element.) // - let metrics = query_font_metrics( - context, - base_size, - FontMetricsOrientation::MatchContext, - ); + let metrics = + query_font_metrics(context, base_size, FontMetricsOrientation::MatchContext); let reference_size = metrics.zero_advance_measure.unwrap_or_else(|| { // https://drafts.csswg.org/css-values/#ch // diff --git a/components/style/values/specified/list.rs b/components/style/values/specified/list.rs index d75455a1c71..dffce9a3001 100644 --- a/components/style/values/specified/list.rs +++ b/components/style/values/specified/list.rs @@ -125,7 +125,19 @@ impl Parse for Quotes { } /// Specified and computed `-moz-list-reversed` property (for UA sheets only). -#[derive(Clone, Copy, Debug, Eq, Hash, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss)] +#[derive( + Clone, + Copy, + Debug, + Eq, + Hash, + MallocSizeOf, + Parse, + PartialEq, + SpecifiedValueInfo, + ToComputedValue, + ToCss, +)] #[repr(u8)] pub enum MozListReversed { /// the initial value diff --git a/components/style/values/specified/text.rs b/components/style/values/specified/text.rs index ab4e45fd1ee..33bda52aa9d 100644 --- a/components/style/values/specified/text.rs +++ b/components/style/values/specified/text.rs @@ -282,7 +282,6 @@ bitflags! { } } - impl Parse for TextDecorationLine { /// none | [ underline || overline || line-through || blink ] fn parse<'i, 't>( @@ -318,7 +317,7 @@ impl Parse for TextDecorationLine { } if result.contains(flag) { - return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) + return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); } result.insert(flag) @@ -341,6 +340,13 @@ impl ToCss for TextDecorationLine { return dest.write_str("none"); } + #[cfg(feature = "gecko")] + { + if *self == TextDecorationLine::COLOR_OVERRIDE { + return Ok(()); + } + } + let mut writer = SequenceWriter::new(dest, " "); let mut any = false; @@ -358,7 +364,7 @@ impl ToCss for TextDecorationLine { maybe_write!(LINE_THROUGH => "line-through"); maybe_write!(BLINK => "blink"); - debug_assert!(any || *self == TextDecorationLine::COLOR_OVERRIDE); + debug_assert!(any); Ok(()) } From 6283837613e3b8f9c79092d305d862bcb0ef84e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 27 Mar 2019 02:07:11 +0100 Subject: [PATCH 28/29] layout: Fix Servo layout build. --- components/gfx/font.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/gfx/font.rs b/components/gfx/font.rs index 6a0000494c5..cd9513a3f43 100644 --- a/components/gfx/font.rs +++ b/components/gfx/font.rs @@ -367,7 +367,7 @@ impl FontGroup { let families = style .font_family - .0 + .families .iter() .map(|family| FontGroupFamily::new(descriptor.clone(), &family)) .collect(); From 1f28a486e3c713e61f69d204a74efe3465c7a1e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 27 Mar 2019 14:35:04 +0100 Subject: [PATCH 29/29] gfx: Fix unit tests. --- components/gfx/tests/font_context.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/components/gfx/tests/font_context.rs b/components/gfx/tests/font_context.rs index 2f3f12b97b9..3a3f73671ac 100644 --- a/components/gfx/tests/font_context.rs +++ b/components/gfx/tests/font_context.rs @@ -119,7 +119,10 @@ fn font_family(names: Vec<&str>) -> FontFamily { }) .collect(); - FontFamily(FontFamilyList::new(names.into_boxed_slice())) + FontFamily { + families: FontFamilyList::new(names.into_boxed_slice()), + is_system_font: false, + } } #[test]