From 74ffb16a1283c931d6cc51a107160b2343a20683 Mon Sep 17 00:00:00 2001 From: Gurzau Raul Date: Sat, 9 Jun 2018 10:45:08 +0300 Subject: [PATCH] Backed out 4 changesets (bug 1467536) for permafailing on layout/style/test/test_bug418986-2.html. a=backout Backed out changeset 4e1cee0e0a48 (bug 1467536) Backed out changeset 697892c54d63 (bug 1467536) Backed out changeset 13b38c2d3251 (bug 1467536) Backed out changeset 086eaeb69efa (bug 1467536) --- layout/style/CSSPropFlags.h | 3 - layout/style/ServoBindingList.h | 5 +- layout/style/ServoCSSPropList.mako.py | 30 ----- layout/style/nsComputedDOMStyle.cpp | 118 ++++++++++++------ layout/style/nsComputedDOMStyle.h | 7 ++ servo/components/style/properties/data.py | 12 -- .../style/properties/properties.mako.rs | 93 ++++---------- servo/ports/geckolib/glue.rs | 19 --- 8 files changed, 116 insertions(+), 171 deletions(-) diff --git a/layout/style/CSSPropFlags.h b/layout/style/CSSPropFlags.h index e5faad425c40..8f72adebe1b8 100644 --- a/layout/style/CSSPropFlags.h +++ b/layout/style/CSSPropFlags.h @@ -49,9 +49,6 @@ enum class CSSPropFlags : uint8_t // the DOM. Properties with this flag are defined in an #ifndef // CSS_PROP_LIST_EXCLUDE_INTERNAL section. Internal = 1 << 5, - - // Whether this property should be serialized by Servo in getComputedStyle. - SerializedByServo = 1 << 6, }; MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(CSSPropFlags) diff --git a/layout/style/ServoBindingList.h b/layout/style/ServoBindingList.h index a53c6fe8b2e2..33f20c480ec9 100644 --- a/layout/style/ServoBindingList.h +++ b/layout/style/ServoBindingList.h @@ -825,7 +825,7 @@ SERVO_BINDING_FUNC(Servo_SerializeFontValueForCanvas, void, RawServoDeclarationBlockBorrowed declarations, nsAString* buffer) -// GetComputedStyle APIs. +// Get custom property value. SERVO_BINDING_FUNC(Servo_GetCustomPropertyValue, bool, ComputedStyleBorrowed computed_values, const nsAString* name, nsAString* value) @@ -837,9 +837,6 @@ SERVO_BINDING_FUNC(Servo_GetCustomPropertyNameAt, bool, ComputedStyleBorrowed, uint32_t index, nsAString* name) -SERVO_BINDING_FUNC(Servo_GetPropertyValue, void, - ComputedStyleBorrowed computed_values, - nsCSSPropertyID property, nsAString* value) SERVO_BINDING_FUNC(Servo_ProcessInvalidations, void, RawServoStyleSetBorrowed set, diff --git a/layout/style/ServoCSSPropList.mako.py b/layout/style/ServoCSSPropList.mako.py index b1569b1eeab6..659f0ffa01a5 100644 --- a/layout/style/ServoCSSPropList.mako.py +++ b/layout/style/ServoCSSPropList.mako.py @@ -68,34 +68,6 @@ def method(prop): return prop.camel_case[1:] return prop.camel_case -# Colors, integers and lengths are easy as well. -# -# TODO(emilio): This will go away once the rest of the longhands have been -# moved or perhaps using a blacklist for the ones with non-layout-dependence -# but other non-trivial dependence like scrollbar colors. -SERIALIZED_PREDEFINED_TYPES = [ - "Color", - "Integer", - "Length", - "Opacity", -] - -def serialized_by_servo(prop): - # If the property requires layout information, no such luck. - if "GETCS_NEEDS_LAYOUT_FLUSH" in prop.flags: - return False - # No shorthands yet. - if prop.type() == "shorthand": - return False - # Keywords are all fine. - if prop.keyword: - return True - if prop.predefined_type in SERIALIZED_PREDEFINED_TYPES: - return True - # TODO(emilio): Enable the rest of the longhands. - return False - - def flags(prop): result = [] if prop.explicitly_enabled_in_chrome(): @@ -110,8 +82,6 @@ def flags(prop): result.append("GetCSNeedsLayoutFlush") if "CAN_ANIMATE_ON_COMPOSITOR" in prop.flags: result.append("CanAnimateOnCompositor") - if serialized_by_servo(prop): - result.append("SerializedByServo") return ", ".join('"CSSPropFlags::{}"'.format(flag) for flag in result) def pref(prop): diff --git a/layout/style/nsComputedDOMStyle.cpp b/layout/style/nsComputedDOMStyle.cpp index 11c93732b692..c165e90d2755 100644 --- a/layout/style/nsComputedDOMStyle.cpp +++ b/layout/style/nsComputedDOMStyle.cpp @@ -435,47 +435,20 @@ nsComputedDOMStyle::GetPropertyValue(const nsAString& aPropertyName, { aReturn.Truncate(); - nsCSSPropertyID prop = - nsCSSProps::LookupProperty(aPropertyName, CSSEnabledState::eForAllContent); - - const ComputedStyleMap::Entry* entry = nullptr; - if (prop != eCSSPropertyExtra_variable) { - entry = GetComputedStyleMap()->FindEntryForProperty(prop); - if (!entry) { - return NS_OK; - } + ErrorResult error; + RefPtr val = + GetPropertyCSSValueWithoutWarning(aPropertyName, error); + if (error.Failed()) { + return error.StealNSResult(); } - const bool layoutFlushIsNeeded = entry && entry->IsLayoutFlushNeeded(); - UpdateCurrentStyleSources(layoutFlushIsNeeded); - if (!mComputedStyle) { - return NS_ERROR_NOT_AVAILABLE; + if (val) { + nsString text; + val->GetCssText(text, error); + aReturn.Assign(text); + return error.StealNSResult(); } - auto cleanup = mozilla::MakeScopeExit([&] { - ClearCurrentStyleSources(); - }); - - if (!entry) { - MOZ_ASSERT(nsCSSProps::IsCustomPropertyName(aPropertyName)); - const nsAString& name = - Substring(aPropertyName, CSS_CUSTOM_NAME_PREFIX_LENGTH); - Servo_GetCustomPropertyValue(mComputedStyle, &name, &aReturn); - return NS_OK; - } - - if (!nsCSSProps::PropHasFlags(prop, CSSPropFlags::SerializedByServo)) { - if (RefPtr value = (this->*entry->mGetter)()) { - ErrorResult rv; - nsString text; - value->GetCssText(text, rv); - aReturn.Assign(text); - return rv.StealNSResult(); - } - return NS_OK; - } - - Servo_GetPropertyValue(mComputedStyle, prop, &aReturn); return NS_OK; } @@ -1014,6 +987,57 @@ nsComputedDOMStyle::ClearCurrentStyleSources() mPresShell = nullptr; } +already_AddRefed +nsComputedDOMStyle::GetPropertyCSSValueWithoutWarning( + const nsAString& aPropertyName, + ErrorResult& aRv) +{ + nsCSSPropertyID prop = + nsCSSProps::LookupProperty(aPropertyName, CSSEnabledState::eForAllContent); + + bool needsLayoutFlush; + ComputedStyleMap::Entry::ComputeMethod getter; + + if (prop == eCSSPropertyExtra_variable) { + needsLayoutFlush = false; + getter = nullptr; + } else { + const ComputedStyleMap::Entry* propEntry = + GetComputedStyleMap()->FindEntryForProperty(prop); + + if (!propEntry) { +#ifdef DEBUG_ComputedDOMStyle + NS_WARNING(PromiseFlatCString(NS_ConvertUTF16toUTF8(aPropertyName) + + NS_LITERAL_CSTRING(" is not queryable!")).get()); +#endif + + // NOTE: For branches, we should flush here for compatibility! + return nullptr; + } + + needsLayoutFlush = propEntry->IsLayoutFlushNeeded(); + getter = propEntry->mGetter; + } + + UpdateCurrentStyleSources(needsLayoutFlush); + if (!mComputedStyle) { + aRv.Throw(NS_ERROR_NOT_AVAILABLE); + return nullptr; + } + + RefPtr val; + if (prop == eCSSPropertyExtra_variable) { + val = DoGetCustomProperty(aPropertyName); + } else { + // Call our pointer-to-member-function. + val = (this->*getter)(); + } + + ClearCurrentStyleSources(); + + return val.forget(); +} + NS_IMETHODIMP nsComputedDOMStyle::RemoveProperty(const nsAString& aPropertyName, nsAString& aReturn) @@ -7168,6 +7192,26 @@ MarkComputedStyleMapDirty(const char* aPref, void* aData) static_cast(aData)->MarkDirty(); } +already_AddRefed +nsComputedDOMStyle::DoGetCustomProperty(const nsAString& aPropertyName) +{ + MOZ_ASSERT(nsCSSProps::IsCustomPropertyName(aPropertyName)); + + nsString variableValue; + const nsAString& name = Substring(aPropertyName, + CSS_CUSTOM_NAME_PREFIX_LENGTH); + bool present = + Servo_GetCustomPropertyValue(mComputedStyle, &name, &variableValue); + if (!present) { + return nullptr; + } + + RefPtr val = new nsROCSSPrimitiveValue; + val->SetString(variableValue); + + return val.forget(); +} + void nsComputedDOMStyle::ParentChainChanged(nsIContent* aContent) { diff --git a/layout/style/nsComputedDOMStyle.h b/layout/style/nsComputedDOMStyle.h index e4ad2af5593e..ffd7d790be02 100644 --- a/layout/style/nsComputedDOMStyle.h +++ b/layout/style/nsComputedDOMStyle.h @@ -59,6 +59,10 @@ private: typedef mozilla::dom::CSSValue CSSValue; typedef mozilla::StyleGeometryBox StyleGeometryBox; + already_AddRefed + GetPropertyCSSValueWithoutWarning(const nsAString& aProp, + mozilla::ErrorResult& aRv); + public: NS_DECL_CYCLE_COLLECTING_ISUPPORTS NS_DECL_CYCLE_COLLECTION_SKIPPABLE_SCRIPT_HOLDER_CLASS_AMBIGUOUS(nsComputedDOMStyle, @@ -613,6 +617,9 @@ private: already_AddRefed DoGetContextProperties(); + /* Custom properties */ + already_AddRefed DoGetCustomProperty(const nsAString& aPropertyName); + /* Helper functions */ void SetToRGBAColor(nsROCSSPrimitiveValue* aValue, nscolor aColor); void SetValueFromComplexColor(nsROCSSPrimitiveValue* aValue, diff --git a/servo/components/style/properties/data.py b/servo/components/style/properties/data.py index fca4b536e013..0ac42b365eea 100644 --- a/servo/components/style/properties/data.py +++ b/servo/components/style/properties/data.py @@ -226,10 +226,6 @@ class Longhand(object): # See compute_damage for the various values this can take self.servo_restyle_damage = servo_restyle_damage - @staticmethod - def type(): - return "longhand" - def experimental(self, product): if product == "gecko": return bool(self.gecko_pref) @@ -365,10 +361,6 @@ class Shorthand(object): animatable = property(get_animatable) transitionable = property(get_transitionable) - @staticmethod - def type(): - return "shorthand" - def experimental(self, product): if product == "gecko": return bool(self.gecko_pref) @@ -400,10 +392,6 @@ class Alias(object): self.allowed_in_page_rule = original.allowed_in_page_rule self.allowed_in_keyframe_block = original.allowed_in_keyframe_block - @staticmethod - def type(): - return "alias" - def experimental(self, product): if product == "gecko": return bool(self.gecko_pref) diff --git a/servo/components/style/properties/properties.mako.rs b/servo/components/style/properties/properties.mako.rs index cdebe662279c..4e52412c18d4 100644 --- a/servo/components/style/properties/properties.mako.rs +++ b/servo/components/style/properties/properties.mako.rs @@ -22,7 +22,8 @@ use std::cell::RefCell; use std::fmt::{self, Write}; use std::mem::{self, ManuallyDrop}; -use cssparser::{Parser, RGBA, TokenSerializationType}; +#[cfg(feature = "servo")] use cssparser::RGBA; +use cssparser::{Parser, TokenSerializationType}; use cssparser::ParserInput; #[cfg(feature = "servo")] use euclid::SideOffsets2D; use context::QuirksMode; @@ -44,6 +45,7 @@ use shared_lock::StylesheetGuards; use style_traits::{CssWriter, KeywordsCollectFn, ParseError, ParsingMode}; use style_traits::{SpecifiedValueInfo, StyleParseErrorKind, ToCss}; use stylesheets::{CssRuleType, Origin, UrlExtraData}; +#[cfg(feature = "servo")] use values::Either; use values::generics::text::LineHeight; use values::computed; use values::computed::NonNegativeLength; @@ -832,13 +834,13 @@ bitflags! { const APPLIES_TO_FIRST_LINE = 1 << 4; /// This longhand property applies to ::placeholder. const APPLIES_TO_PLACEHOLDER = 1 << 5; - /// This property's getComputedStyle implementation requires layout - /// to be flushed. - const GETCS_NEEDS_LAYOUT_FLUSH = 1 << 6; /* The following flags are currently not used in Rust code, they * only need to be listed in corresponding properties so that * they can be checked in the C++ side via ServoCSSPropList.h. */ + /// This property's getComputedStyle implementation requires layout + /// to be flushed. + const GETCS_NEEDS_LAYOUT_FLUSH = 0; /// This property can be animated on the compositor. const CAN_ANIMATE_ON_COMPOSITOR = 0; } @@ -2612,59 +2614,6 @@ impl ComputedValues { pub fn custom_properties(&self) -> Option<<&Arc<::custom_properties::CustomPropertiesMap>> { self.custom_properties.as_ref() } - - /// Writes the value of the given longhand as a string in `dest`. - /// - /// Note that the value will usually be the computed value, except for - /// colors, where it's resolved. - pub fn get_longhand_property_value( - &self, - property_id: LonghandId, - dest: &mut CssWriter - ) -> fmt::Result - where - W: Write, - { - // TODO(emilio): Is it worth to merge branches here just like - // PropertyDeclaration::to_css does? - // - // We'd need to get a concept of ~resolved value, which may not be worth - // it. - match property_id { - % for prop in data.longhands: - LonghandId::${prop.camel_case} => { - let style_struct = - self.get_${prop.style_struct.ident.strip("_")}(); - let value = - style_struct - % if prop.logical: - .clone_${prop.ident}(self.writing_mode); - % else: - .clone_${prop.ident}(); - % endif - - % if prop.predefined_type == "Color": - let value = self.resolve_color(value); - % endif - - value.to_css(dest) - } - % endfor - } - } - - /// Resolves the currentColor keyword. - /// - /// Any color value from computed values (except for the 'color' property - /// itself) should go through this method. - /// - /// Usage example: - /// let top_color = - /// style.resolve_color(style.get_border().clone_border_top_color()); - #[inline] - pub fn resolve_color(&self, color: computed::Color) -> RGBA { - color.to_rgba(self.get_color().clone_color()) - } } #[cfg(feature = "servo")] @@ -2777,6 +2726,18 @@ impl ComputedValuesInner { self.get_column().is_multicol() } + /// Resolves the currentColor keyword. + /// + /// Any color value from computed values (except for the 'color' property + /// itself) should go through this method. + /// + /// Usage example: + /// let top_color = style.resolve_color(style.Border.border_top_color); + #[inline] + pub fn resolve_color(&self, color: computed::Color) -> RGBA { + color.to_rgba(self.get_color().color) + } + /// Get the logical computed inline size. #[inline] pub fn content_inline_size(&self) -> computed::LengthOrPercentageOrAuto { @@ -2941,19 +2902,19 @@ impl ComputedValuesInner { /// Serializes the computed value of this property as a string. pub fn computed_value_to_string(&self, property: PropertyDeclarationId) -> String { match property { - PropertyDeclarationId::Longhand(id) => { - let mut s = String::new(); - self.get_longhand_property_value( - property, - &mut CssWriter::new(&mut s) - ).unwrap(); - s - } + % for style_struct in data.active_style_structs(): + % for longhand in style_struct.longhands: + PropertyDeclarationId::Longhand(LonghandId::${longhand.camel_case}) => { + self.${style_struct.ident}.${longhand.ident}.to_css_string() + } + % endfor + % endfor PropertyDeclarationId::Custom(name) => { self.custom_properties .as_ref() .and_then(|map| map.get(name)) - .map_or(String::new(), |value| value.to_css_string()) + .map(|value| value.to_css_string()) + .unwrap_or(String::new()) } } } diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs index a66866c19c70..948286ebd86c 100644 --- a/servo/ports/geckolib/glue.rs +++ b/servo/ports/geckolib/glue.rs @@ -5132,25 +5132,6 @@ pub extern "C" fn Servo_StyleSet_HasDocumentStateDependency( data.stylist.has_document_state_dependency(state) } -#[no_mangle] -pub unsafe extern "C" fn Servo_GetPropertyValue( - computed_values: ComputedStyleBorrowed, - prop: nsCSSPropertyID, - value: *mut nsAString, -) { - use style::properties::PropertyFlags; - - let longhand = LonghandId::from_nscsspropertyid(prop).expect("Not a longhand?"); - debug_assert!( - !longhand.flags().contains(PropertyFlags::GETCS_NEEDS_LAYOUT_FLUSH), - "We're not supposed to serialize layout-dependent properties" - ); - computed_values.get_longhand_property_value( - longhand, - &mut CssWriter::new(&mut *value), - ).unwrap(); -} - #[no_mangle] pub unsafe extern "C" fn Servo_GetCustomPropertyValue( computed_values: ComputedStyleBorrowed,