From a061f2a89f4b733216e9d5b3c0dbc5dfa49fcc26 Mon Sep 17 00:00:00 2001 From: CYBAI Date: Sat, 13 Jan 2018 00:14:53 +0800 Subject: [PATCH 1/3] Use Python's True instead of string for arbitrary Python code in mako --- components/style/properties/longhand/border.mako.rs | 2 +- components/style/properties/longhand/box.mako.rs | 6 +++--- components/style/properties/longhand/effects.mako.rs | 2 +- components/style/properties/longhand/inherited_svg.mako.rs | 6 +++--- .../style/properties/longhand/inherited_table.mako.rs | 2 +- components/style/properties/longhand/list.mako.rs | 4 ++-- components/style/properties/longhand/position.mako.rs | 2 +- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/components/style/properties/longhand/border.mako.rs b/components/style/properties/longhand/border.mako.rs index 505be70d012..11fdf30b983 100644 --- a/components/style/properties/longhand/border.mako.rs +++ b/components/style/properties/longhand/border.mako.rs @@ -210,7 +210,7 @@ ${helpers.predefined_type("border-image-source", "ImageLayer", vector=False, animation_value_type="discrete", flags="APPLIES_TO_FIRST_LETTER", - boxed="True")} + boxed=True)} ${helpers.predefined_type("border-image-outset", "LengthOrNumberRect", parse_method="parse_non_negative", diff --git a/components/style/properties/longhand/box.mako.rs b/components/style/properties/longhand/box.mako.rs index 5a026ae0632..2810f5a513b 100644 --- a/components/style/properties/longhand/box.mako.rs +++ b/components/style/properties/longhand/box.mako.rs @@ -389,7 +389,7 @@ ${helpers.predefined_type("scroll-snap-destination", "computed::Position::zero()", products="gecko", gecko_pref="layout.css.scroll-snap.enabled", - boxed="True", + boxed=True, spec="Nonstandard (https://developer.mozilla.org/en-US/docs/Web/CSS/scroll-snap-destination)", animation_value_type="discrete")} @@ -505,7 +505,7 @@ ${helpers.predefined_type("perspective", ${helpers.predefined_type("perspective-origin", "position::Position", "computed::position::Position::center()", - boxed="True", + boxed=True, extra_prefixes="moz webkit", spec="https://drafts.csswg.org/css-transforms-2/#perspective-origin-property", animation_value_type="ComputedValue")} @@ -589,7 +589,7 @@ ${helpers.single_keyword("-moz-appearance", ${helpers.predefined_type("-moz-binding", "UrlOrNone", "Either::Second(None_)", products="gecko", - boxed="True" if product == "gecko" else "False", + boxed= product == "gecko", animation_value_type="none", gecko_ffi_name="mBinding", spec="Nonstandard (https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-binding)")} diff --git a/components/style/properties/longhand/effects.mako.rs b/components/style/properties/longhand/effects.mako.rs index 1e9cb575f3f..98630f0ef6a 100644 --- a/components/style/properties/longhand/effects.mako.rs +++ b/components/style/properties/longhand/effects.mako.rs @@ -30,7 +30,7 @@ ${helpers.predefined_type("clip", "ClipRectOrAuto", "computed::ClipRectOrAuto::auto()", animation_value_type="ComputedValue", - boxed="True", + boxed=True, allow_quirks=True, spec="https://drafts.fxtf.org/css-masking/#clip-property")} diff --git a/components/style/properties/longhand/inherited_svg.mako.rs b/components/style/properties/longhand/inherited_svg.mako.rs index 11e39bd0917..909cce282fd 100644 --- a/components/style/properties/longhand/inherited_svg.mako.rs +++ b/components/style/properties/longhand/inherited_svg.mako.rs @@ -114,19 +114,19 @@ ${helpers.single_keyword("clip-rule", "nonzero evenodd", ${helpers.predefined_type("marker-start", "UrlOrNone", "Either::Second(None_)", products="gecko", - boxed="True" if product == "gecko" else "False", + boxed= product == "gecko", animation_value_type="discrete", spec="https://www.w3.org/TR/SVG2/painting.html#VertexMarkerProperties")} ${helpers.predefined_type("marker-mid", "UrlOrNone", "Either::Second(None_)", products="gecko", - boxed="True" if product == "gecko" else "False", + boxed= product == "gecko", animation_value_type="discrete", spec="https://www.w3.org/TR/SVG2/painting.html#VertexMarkerProperties")} ${helpers.predefined_type("marker-end", "UrlOrNone", "Either::Second(None_)", products="gecko", - boxed="True" if product == "gecko" else "False", + boxed= product == "gecko", animation_value_type="discrete", spec="https://www.w3.org/TR/SVG2/painting.html#VertexMarkerProperties")} diff --git a/components/style/properties/longhand/inherited_table.mako.rs b/components/style/properties/longhand/inherited_table.mako.rs index 99363d082c2..3a1fe70021d 100644 --- a/components/style/properties/longhand/inherited_table.mako.rs +++ b/components/style/properties/longhand/inherited_table.mako.rs @@ -24,5 +24,5 @@ ${helpers.predefined_type("border-spacing", "BorderSpacing", "computed::BorderSpacing::zero()", animation_value_type="BorderSpacing", - boxed="True", + boxed=True, spec="https://drafts.csswg.org/css-tables/#propdef-border-spacing")} diff --git a/components/style/properties/longhand/list.mako.rs b/components/style/properties/longhand/list.mako.rs index bead5054b95..5819b2b8d02 100644 --- a/components/style/properties/longhand/list.mako.rs +++ b/components/style/properties/longhand/list.mako.rs @@ -35,7 +35,7 @@ ${helpers.single_keyword("list-style-position", "outside inside", animation_valu "computed::ListStyleType::disc()", initial_specified_value="specified::ListStyleType::disc()", animation_value_type="discrete", - boxed="True", + boxed=True, spec="https://drafts.csswg.org/css-lists/#propdef-list-style-type")} % endif @@ -58,5 +58,5 @@ ${helpers.predefined_type("-moz-image-region", "computed::ClipRectOrAuto::auto()", animation_value_type="ComputedValue", products="gecko", - boxed="True", + boxed=True, spec="Nonstandard (https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-image-region)")} diff --git a/components/style/properties/longhand/position.mako.rs b/components/style/properties/longhand/position.mako.rs index 8d04c8eba8b..cb666a27f38 100644 --- a/components/style/properties/longhand/position.mako.rs +++ b/components/style/properties/longhand/position.mako.rs @@ -247,7 +247,7 @@ ${helpers.predefined_type("object-position", "Position", "computed::Position::zero()", products="gecko", - boxed="True", + boxed=True, spec="https://drafts.csswg.org/css-images-3/#the-object-position", animation_value_type="ComputedValue")} From ac74cf7a603621444411b3dd4b37a157df73b5cc Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 11 Jan 2018 13:48:36 -0800 Subject: [PATCH 2/3] Add machinery to assert single-threadedness from geckolib. MozReview-Commit-ID: 9LBNm2h5Ct3 --- components/style/gecko/generated/bindings.rs | 4 ++++ ports/geckolib/glue.rs | 11 +++++++++++ 2 files changed, 15 insertions(+) diff --git a/components/style/gecko/generated/bindings.rs b/components/style/gecko/generated/bindings.rs index 3e8276c8885..eacccb5dafc 100644 --- a/components/style/gecko/generated/bindings.rs +++ b/components/style/gecko/generated/bindings.rs @@ -1597,4 +1597,8 @@ extern "C" { pub fn Gecko_GetElementsWithId ( aDocument : * const nsIDocument , aId : * mut nsAtom , ) -> * const nsTArray < * mut Element > ; } extern "C" { pub fn Gecko_GetBoolPrefValue ( pref_name : * const :: std :: os :: raw :: c_char , ) -> bool ; +} extern "C" { + pub fn Gecko_IsInServoTraversal ( ) -> bool ; +} extern "C" { + pub fn Gecko_IsMainThread ( ) -> bool ; } \ No newline at end of file diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 31ec4bfa6f7..abf966ce937 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -216,6 +216,16 @@ unsafe fn dummy_url_data() -> &'static RefPtr { RefPtr::from_ptr_ref(&DUMMY_URL_DATA) } +#[allow(dead_code)] +fn is_main_thread() -> bool { + unsafe { bindings::Gecko_IsMainThread() } +} + +#[allow(dead_code)] +fn is_in_servo_traversal() -> bool { + unsafe { bindings::Gecko_IsInServoTraversal() } +} + fn create_shared_context<'a>( global_style_data: &GlobalStyleData, guard: &'a SharedRwLockReadGuard, @@ -1335,6 +1345,7 @@ fn read_locked_arc(raw: & as HasFFI>::FFIType, func: F) -> R unsafe fn read_locked_arc_unchecked(raw: & as HasFFI>::FFIType, func: F) -> R where Locked: HasArcFFI, F: FnOnce(&T) -> R { + debug_assert!(is_main_thread() && !is_in_servo_traversal()); read_locked_arc(raw, func) } From 2b1c1d7b6870902f13c1df136471176998731319 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 11 Jan 2018 12:22:07 -0800 Subject: [PATCH 3/3] Avoid atomic overhead in Servo_Element_IsDisplayNone. This reduces time spent in NoteDirtyElement by about 15% in the testcase. Even though the subsequent patch will cause us to call Servo_Element_IsDisplayNone less for this particular testcase, it could still be hot on other testcases, and so it's worth optimizing. MozReview-Commit-ID: 3F3Zfp48dDW --- ports/geckolib/glue.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index abf966ce937..8ccbcc84e71 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -1024,8 +1024,13 @@ pub extern "C" fn Servo_Element_GetPseudoComputedValues(element: RawGeckoElement #[no_mangle] pub extern "C" fn Servo_Element_IsDisplayNone(element: RawGeckoElementBorrowed) -> bool { let element = GeckoElement(element); - let data = element.borrow_data().expect("Invoking Servo_Element_IsDisplayNone on unstyled element"); - data.styles.is_display_none() + let data = element.get_data().expect("Invoking Servo_Element_IsDisplayNone on unstyled element"); + + // This function is hot, so we bypass the AtomicRefCell. It would be nice to also assert that + // we're not in the servo traversal, but this function is called at various intermediate + // checkpoints when managing the traversal on the Gecko side. + debug_assert!(is_main_thread()); + unsafe { &*data.as_ptr() }.styles.is_display_none() } #[no_mangle]