From 99e3e122591b204b33e8a3190438d71d512d611c Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 4 Feb 2016 16:08:14 -0800 Subject: [PATCH 01/11] Add stub for missing je_malloc_usable_size symbol in geckolib. --- ports/geckolib/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ports/geckolib/lib.rs b/ports/geckolib/lib.rs index c3c90da9cb3..357cb9a3d70 100644 --- a/ports/geckolib/lib.rs +++ b/ports/geckolib/lib.rs @@ -39,3 +39,7 @@ mod wrapper; pub mod properties { include!(concat!(env!("OUT_DIR"), "/properties.rs")); } + +// FIXME(bholley): This should probably go away once we harmonize the allocators. +#[no_mangle] +pub extern "C" fn je_malloc_usable_size(_: *const ::libc::c_void) -> ::libc::size_t { 0 } From fd09035c65052b8c6ce1e1becd61701244a10425 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Fri, 3 Jun 2016 16:33:49 +1000 Subject: [PATCH 02/11] Disable align-items in geckolib. --- components/style/properties/longhand/position.mako.rs | 5 ++++- components/style/properties/properties.mako.rs | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/components/style/properties/longhand/position.mako.rs b/components/style/properties/longhand/position.mako.rs index a143dab44c5..3ad07a9c2c3 100644 --- a/components/style/properties/longhand/position.mako.rs +++ b/components/style/properties/longhand/position.mako.rs @@ -77,11 +77,14 @@ ${helpers.single_keyword("justify-content", "flex-start flex-end center space-be products="servo", animatable=False)} +// FIXME(heycam): Disable align-items in geckolib since we don't support the Gecko initial value +// 'normal' yet. ${helpers.single_keyword("align-items", "stretch flex-start flex-end center baseline", experimental=True, need_clone=True, gecko_constant_prefix="NS_STYLE_ALIGN", - animatable=False)} + animatable=False, + products="servo")} ${helpers.single_keyword("align-content", "stretch flex-start flex-end center space-between space-around", experimental=True, diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index d2d07c79a73..c5b08b44aac 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1921,6 +1921,7 @@ pub fn cascade( } } + % if "align-items" in data.longhands_by_name: { use self::style_struct_traits::Position; use computed_values::align_self::T as align_self; @@ -1937,6 +1938,7 @@ pub fn cascade( style.mutate_position().set_align_self(self_align); } } + % endif // The initial value of border-*-width may be changed at computed value time. % for side in ["top", "right", "bottom", "left"]: From 00fe12d3ad257d01dc78360032ca81034a845fa6 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Fri, 27 May 2016 15:32:21 +1000 Subject: [PATCH 03/11] Add STYLO_THREADS environment variable to control number of style worker threads. --- ports/geckolib/data.rs | 16 ++++++++++++++-- ports/geckolib/glue.rs | 8 +++++++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/ports/geckolib/data.rs b/ports/geckolib/data.rs index 1962243a310..02fdece2411 100644 --- a/ports/geckolib/data.rs +++ b/ports/geckolib/data.rs @@ -9,6 +9,7 @@ use num_cpus; use selector_impl::{Animation, SharedStyleContext, Stylist, Stylesheet}; use std::cmp; use std::collections::HashMap; +use std::env; use std::sync::mpsc::{channel, Receiver, Sender}; use std::sync::{Arc, RwLock}; use style::dom::OpaqueNode; @@ -36,6 +37,17 @@ pub struct PerDocumentStyleData { // FIXME(bholley): This shouldn't be per-document. pub work_queue: WorkQueue, + + pub num_threads: usize, +} + +lazy_static! { + pub static ref NUM_THREADS: usize = { + match env::var("STYLO_THREADS").map(|s| s.parse::().expect("invalid STYLO_THREADS")) { + Ok(num) => num, + _ => cmp::max(num_cpus::get() * 3 / 4, 1), + } + }; } impl PerDocumentStyleData { @@ -45,7 +57,6 @@ impl PerDocumentStyleData { let device = Device::new(MediaType::Screen, window_size); let (new_anims_sender, new_anims_receiver) = channel(); - let num_threads = cmp::max(num_cpus::get() * 3 / 4, 1); PerDocumentStyleData { stylist: Arc::new(Stylist::new(device)), @@ -55,7 +66,8 @@ impl PerDocumentStyleData { new_animations_receiver: new_anims_receiver, running_animations: Arc::new(RwLock::new(HashMap::new())), expired_animations: Arc::new(RwLock::new(HashMap::new())), - work_queue: WorkQueue::new("StyleWorker", thread_state::LAYOUT, num_threads), + work_queue: WorkQueue::new("StyleWorker", thread_state::LAYOUT, *NUM_THREADS), + num_threads: *NUM_THREADS, } } diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 8f76407749c..baf3b3b22ac 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -29,6 +29,7 @@ use style::parallel; use style::parser::ParserContextExtraData; use style::properties::{ComputedValues, PropertyDeclarationBlock}; use style::selector_impl::{SelectorImplExt, PseudoElementCascadeType}; +use style::sequential; use style::stylesheets::Origin; use traversal::RecalcStyleOnly; use url::Url; @@ -109,7 +110,12 @@ fn restyle_subtree(node: GeckoNode, raw_data: *mut RawServoStyleSet) { }; if node.is_dirty() || node.has_dirty_descendants() { - parallel::traverse_dom::(node, &shared_style_context, &mut per_doc_data.work_queue); + if per_doc_data.num_threads == 1 { + sequential::traverse_dom::(node, &shared_style_context); + } else { + parallel::traverse_dom::(node, &shared_style_context, + &mut per_doc_data.work_queue); + } } } From 0417022ecb706b5ed000e912e981126c7f5cbdd5 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Tue, 31 May 2016 17:12:57 +1000 Subject: [PATCH 04/11] Add Servo_StyleWorkerThreadCount. --- ports/geckolib/glue.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index baf3b3b22ac..f1650be83f6 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -5,7 +5,7 @@ #![allow(unsafe_code)] use app_units::Au; -use data::PerDocumentStyleData; +use data::{NUM_THREADS, PerDocumentStyleData}; use env_logger; use euclid::Size2D; use gecko_bindings::bindings::{RawGeckoDocument, RawGeckoElement, RawGeckoNode}; @@ -136,6 +136,11 @@ pub extern "C" fn Servo_RestyleDocument(doc: *mut RawGeckoDocument, raw_data: *m restyle_subtree(node, raw_data); } +#[no_mangle] +pub extern "C" fn Servo_StyleWorkerThreadCount() -> u32 { + *NUM_THREADS as u32 +} + #[no_mangle] pub extern "C" fn Servo_DropNodeData(data: *mut ServoNodeData) -> () { unsafe { From 7f2a0a15473d21f11e7dbbabf78f2452d7cfae44 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 21 Jun 2016 18:27:54 -0700 Subject: [PATCH 05/11] Regenerate bindings. --- .../style/properties/longhand/box.mako.rs | 3 +- ports/geckolib/gecko_bindings/bindings.rs | 24 +++++++-- .../geckolib/gecko_bindings/structs_debug.rs | 52 +++++++++++-------- .../gecko_bindings/structs_release.rs | 52 +++++++++++-------- 4 files changed, 79 insertions(+), 52 deletions(-) diff --git a/components/style/properties/longhand/box.mako.rs b/components/style/properties/longhand/box.mako.rs index 580926889be..381ca4df1d9 100644 --- a/components/style/properties/longhand/box.mako.rs +++ b/components/style/properties/longhand/box.mako.rs @@ -94,8 +94,7 @@ ${helpers.single_keyword("position", "static absolute relative fixed", <%helpers:single_keyword_computed name="float" values="none left right" animatable="False" - need_clone="True" - gecko_ffi_name="mFloats"> + need_clone="True"> impl ToComputedValue for SpecifiedValue { type ComputedValue = computed_value::T; diff --git a/ports/geckolib/gecko_bindings/bindings.rs b/ports/geckolib/gecko_bindings/bindings.rs index 6edc03d88ff..5014e701cf4 100644 --- a/ports/geckolib/gecko_bindings/bindings.rs +++ b/ports/geckolib/gecko_bindings/bindings.rs @@ -169,6 +169,25 @@ extern "C" { pub fn Gecko_LocalName(element: *mut RawGeckoElement) -> *mut nsIAtom; pub fn Gecko_Namespace(element: *mut RawGeckoElement) -> *mut nsIAtom; pub fn Gecko_GetElementId(element: *mut RawGeckoElement) -> *mut nsIAtom; + pub fn Gecko_HasAttr(element: *mut RawGeckoElement, ns: *mut nsIAtom, + name: *mut nsIAtom) -> bool; + pub fn Gecko_AttrEquals(element: *mut RawGeckoElement, ns: *mut nsIAtom, + name: *mut nsIAtom, str: *mut nsIAtom, + ignoreCase: bool) -> bool; + pub fn Gecko_AttrDashEquals(element: *mut RawGeckoElement, + ns: *mut nsIAtom, name: *mut nsIAtom, + str: *mut nsIAtom) -> bool; + pub fn Gecko_AttrIncludes(element: *mut RawGeckoElement, ns: *mut nsIAtom, + name: *mut nsIAtom, str: *mut nsIAtom) -> bool; + pub fn Gecko_AttrHasSubstring(element: *mut RawGeckoElement, + ns: *mut nsIAtom, name: *mut nsIAtom, + str: *mut nsIAtom) -> bool; + pub fn Gecko_AttrHasPrefix(element: *mut RawGeckoElement, + ns: *mut nsIAtom, name: *mut nsIAtom, + str: *mut nsIAtom) -> bool; + pub fn Gecko_AttrHasSuffix(element: *mut RawGeckoElement, + ns: *mut nsIAtom, name: *mut nsIAtom, + str: *mut nsIAtom) -> bool; pub fn Gecko_ClassOrClassList(element: *mut RawGeckoElement, class_: *mut *mut nsIAtom, classList: *mut *mut *mut nsIAtom) -> u32; @@ -182,7 +201,6 @@ extern "C" { -> *mut nsIAtom; pub fn Gecko_AddRefAtom(aAtom: *mut nsIAtom); pub fn Gecko_ReleaseAtom(aAtom: *mut nsIAtom); - pub fn Gecko_HashAtom(aAtom: *mut nsIAtom) -> u32; pub fn Gecko_GetAtomAsUTF16(aAtom: *mut nsIAtom, aLength: *mut u32) -> *const u16; pub fn Gecko_AtomEqualsUTF8(aAtom: *mut nsIAtom, @@ -209,9 +227,6 @@ extern "C" { pub fn Gecko_CreateGradient(shape: u8, size: u8, repeating: bool, legacy_syntax: bool, stops: u32) -> *mut nsStyleGradient; - pub fn Gecko_SetGradientStop(gradient: *mut nsStyleGradient, index: u32, - location: *const nsStyleCoord, - color: nscolor, is_interpolation_hint: bool); pub fn Gecko_AddRefPrincipalArbitraryThread(aPtr: *mut ThreadSafePrincipalHolder); pub fn Gecko_ReleasePrincipalArbitraryThread(aPtr: @@ -282,6 +297,7 @@ extern "C" { set: *mut RawServoStyleSet); pub fn Servo_RestyleSubtree(node: *mut RawGeckoNode, set: *mut RawServoStyleSet); + pub fn Servo_StyleWorkerThreadCount() -> u32; pub fn Gecko_GetAttrAsUTF8(element: *mut RawGeckoElement, ns: *mut nsIAtom, name: *mut nsIAtom, length: *mut u32) diff --git a/ports/geckolib/gecko_bindings/structs_debug.rs b/ports/geckolib/gecko_bindings/structs_debug.rs index 4a21baf0a99..b08a42d5084 100644 --- a/ports/geckolib/gecko_bindings/structs_debug.rs +++ b/ports/geckolib/gecko_bindings/structs_debug.rs @@ -188,8 +188,6 @@ pub const NS_ERROR_MODULE_BASE_OFFSET: ::std::os::raw::c_uint = 69; pub const MOZ_STRING_WITH_OBSOLETE_API: ::std::os::raw::c_uint = 1; pub const NSID_LENGTH: ::std::os::raw::c_uint = 39; pub const NS_NUMBER_OF_FLAGS_IN_REFCNT: ::std::os::raw::c_uint = 2; -pub const _STL_PAIR_H: ::std::os::raw::c_uint = 1; -pub const _GLIBCXX_UTILITY: ::std::os::raw::c_uint = 1; pub const TWIPS_PER_POINT_INT: ::std::os::raw::c_uint = 20; pub const POINTS_PER_INCH_INT: ::std::os::raw::c_uint = 72; pub const NS_FONT_VARIANT_NORMAL: ::std::os::raw::c_uint = 0; @@ -2903,6 +2901,16 @@ pub struct pair<_T1, _T2> { pub first: _T1, pub second: _T2, } +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct __make_pair_return_impl<_Tp> { + pub _phantom0: ::std::marker::PhantomData<_Tp>, +} +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct __make_pair_return<_Tp> { + pub _phantom0: ::std::marker::PhantomData<_Tp>, +} pub type Float = f32; #[repr(i8)] #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] @@ -2931,12 +2939,17 @@ pub enum SurfaceFormat { R8G8B8X8 = 3, A8R8G8B8 = 4, X8R8G8B8 = 5, - R5G6B5_UINT16 = 6, - A8 = 7, - YUV = 8, - NV12 = 9, - YUV422 = 10, - UNKNOWN = 11, + R8G8B8 = 6, + B8G8R8 = 7, + R5G6B5_UINT16 = 8, + A8 = 9, + YUV = 10, + NV12 = 11, + YUV422 = 12, + HSV = 13, + Lab = 14, + Depth = 15, + UNKNOWN = 16, } #[repr(i8)] #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] @@ -3072,7 +3085,7 @@ pub enum FillRule { FILL_WINDING = 0, FILL_EVEN_ODD = 1, } pub enum AntialiasMode { NONE = 0, GRAY = 1, SUBPIXEL = 2, DEFAULT = 3, } #[repr(i8)] #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] -pub enum Filter { GOOD = 0, LINEAR = 1, POINT = 2, SENTINEL = 3, } +pub enum SamplingFilter { GOOD = 0, LINEAR = 1, POINT = 2, SENTINEL = 3, } #[repr(i8)] #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] pub enum PatternType { @@ -3159,16 +3172,6 @@ pub enum SideBits { eSideBitsLeftRight = 10, eSideBitsAll = 15, } -#[repr(C)] -#[derive(Debug, Copy, Clone)] -pub struct tuple_size<_Tp> { - pub _phantom0: ::std::marker::PhantomData<_Tp>, -} -#[repr(C)] -#[derive(Debug, Copy, Clone)] -pub struct tuple_element<_Tp> { - pub _phantom0: ::std::marker::PhantomData<_Tp>, -} pub type nscoord = i32; #[repr(C)] pub struct nsIntPoint { @@ -3269,7 +3272,7 @@ pub const eFamily_generic_count: FontFamilyType = * generic (e.g. serif, sans-serif), with the ability to distinguish * between unquoted and quoted names for serializaiton */ -#[repr(u32)] +#[repr(i32)] #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] pub enum FontFamilyType { eFamily_none = 0, @@ -3389,7 +3392,7 @@ fn bindgen_test_layout_nsFont() { } #[repr(i8)] #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] -pub enum StyleBoxSizing { Content = 0, Padding = 1, Border = 2, } +pub enum StyleBoxSizing { Content = 0, Border = 1, } #[repr(i32)] #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] pub enum PlaybackDirection { _BindgenOpaqueEnum = 0, } @@ -5069,6 +5072,9 @@ pub enum nsStyleImageLayers_nsStyleStruct_h_unnamed_19 { maskMode = 10, composite = 11, } +#[repr(i8)] +#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] +pub enum nsStyleImageLayers_LayerType { Background = 0, Mask = 1, } #[repr(C)] #[derive(Debug, Copy)] pub struct nsStyleImageLayers_Position { @@ -5645,8 +5651,8 @@ pub struct nsStyleDisplay { pub mContain: u8, pub mAppearance: u8, pub mPosition: u8, - pub mFloats: u8, - pub mOriginalFloats: u8, + pub mFloat: u8, + pub mOriginalFloat: u8, pub mBreakType: u8, pub mBreakInside: u8, pub mBreakBefore: bool, diff --git a/ports/geckolib/gecko_bindings/structs_release.rs b/ports/geckolib/gecko_bindings/structs_release.rs index 457f8c17295..c7559d71b43 100644 --- a/ports/geckolib/gecko_bindings/structs_release.rs +++ b/ports/geckolib/gecko_bindings/structs_release.rs @@ -188,8 +188,6 @@ pub const NS_ERROR_MODULE_BASE_OFFSET: ::std::os::raw::c_uint = 69; pub const MOZ_STRING_WITH_OBSOLETE_API: ::std::os::raw::c_uint = 1; pub const NSID_LENGTH: ::std::os::raw::c_uint = 39; pub const NS_NUMBER_OF_FLAGS_IN_REFCNT: ::std::os::raw::c_uint = 2; -pub const _STL_PAIR_H: ::std::os::raw::c_uint = 1; -pub const _GLIBCXX_UTILITY: ::std::os::raw::c_uint = 1; pub const TWIPS_PER_POINT_INT: ::std::os::raw::c_uint = 20; pub const POINTS_PER_INCH_INT: ::std::os::raw::c_uint = 72; pub const NS_FONT_VARIANT_NORMAL: ::std::os::raw::c_uint = 0; @@ -2882,6 +2880,16 @@ pub struct pair<_T1, _T2> { pub first: _T1, pub second: _T2, } +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct __make_pair_return_impl<_Tp> { + pub _phantom0: ::std::marker::PhantomData<_Tp>, +} +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct __make_pair_return<_Tp> { + pub _phantom0: ::std::marker::PhantomData<_Tp>, +} pub type Float = f32; #[repr(i8)] #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] @@ -2910,12 +2918,17 @@ pub enum SurfaceFormat { R8G8B8X8 = 3, A8R8G8B8 = 4, X8R8G8B8 = 5, - R5G6B5_UINT16 = 6, - A8 = 7, - YUV = 8, - NV12 = 9, - YUV422 = 10, - UNKNOWN = 11, + R8G8B8 = 6, + B8G8R8 = 7, + R5G6B5_UINT16 = 8, + A8 = 9, + YUV = 10, + NV12 = 11, + YUV422 = 12, + HSV = 13, + Lab = 14, + Depth = 15, + UNKNOWN = 16, } #[repr(i8)] #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] @@ -3051,7 +3064,7 @@ pub enum FillRule { FILL_WINDING = 0, FILL_EVEN_ODD = 1, } pub enum AntialiasMode { NONE = 0, GRAY = 1, SUBPIXEL = 2, DEFAULT = 3, } #[repr(i8)] #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] -pub enum Filter { GOOD = 0, LINEAR = 1, POINT = 2, SENTINEL = 3, } +pub enum SamplingFilter { GOOD = 0, LINEAR = 1, POINT = 2, SENTINEL = 3, } #[repr(i8)] #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] pub enum PatternType { @@ -3138,16 +3151,6 @@ pub enum SideBits { eSideBitsLeftRight = 10, eSideBitsAll = 15, } -#[repr(C)] -#[derive(Debug, Copy, Clone)] -pub struct tuple_size<_Tp> { - pub _phantom0: ::std::marker::PhantomData<_Tp>, -} -#[repr(C)] -#[derive(Debug, Copy, Clone)] -pub struct tuple_element<_Tp> { - pub _phantom0: ::std::marker::PhantomData<_Tp>, -} pub type nscoord = i32; #[repr(C)] pub struct nsIntPoint { @@ -3248,7 +3251,7 @@ pub const eFamily_generic_count: FontFamilyType = * generic (e.g. serif, sans-serif), with the ability to distinguish * between unquoted and quoted names for serializaiton */ -#[repr(u32)] +#[repr(i32)] #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] pub enum FontFamilyType { eFamily_none = 0, @@ -3368,7 +3371,7 @@ fn bindgen_test_layout_nsFont() { } #[repr(i8)] #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] -pub enum StyleBoxSizing { Content = 0, Padding = 1, Border = 2, } +pub enum StyleBoxSizing { Content = 0, Border = 1, } #[repr(i32)] #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] pub enum PlaybackDirection { _BindgenOpaqueEnum = 0, } @@ -5047,6 +5050,9 @@ pub enum nsStyleImageLayers_nsStyleStruct_h_unnamed_19 { maskMode = 10, composite = 11, } +#[repr(i8)] +#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] +pub enum nsStyleImageLayers_LayerType { Background = 0, Mask = 1, } #[repr(C)] #[derive(Debug, Copy)] pub struct nsStyleImageLayers_Position { @@ -5623,8 +5629,8 @@ pub struct nsStyleDisplay { pub mContain: u8, pub mAppearance: u8, pub mPosition: u8, - pub mFloats: u8, - pub mOriginalFloats: u8, + pub mFloat: u8, + pub mOriginalFloat: u8, pub mBreakType: u8, pub mBreakInside: u8, pub mBreakBefore: bool, From a0c425fbef3f1db45a392b27b1cc869e8fca3bc3 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 23 Jun 2016 16:03:43 -0700 Subject: [PATCH 06/11] Use each_class rather than get_attr when comparing class lists during style sharing. --- components/style/matching.rs | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/components/style/matching.rs b/components/style/matching.rs index d3dd3742d3d..289410af3f6 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -189,8 +189,7 @@ pub struct StyleSharingCandidate { pub style: Arc, pub parent_style: Arc, pub local_name: Atom, - // FIXME(pcwalton): Should be a list of atoms instead. - pub class: Option, + pub classes: Vec, pub namespace: Namespace, pub common_style_affecting_attributes: CommonStyleAffectingAttributes, pub link: bool, @@ -201,7 +200,7 @@ impl PartialEq for StyleSharingCandidate { arc_ptr_eq(&self.style, &other.style) && arc_ptr_eq(&self.parent_style, &other.parent_style) && self.local_name == other.local_name && - self.class == other.class && + self.classes == other.classes && self.link == other.link && self.namespace == other.namespace && self.common_style_affecting_attributes == other.common_style_affecting_attributes @@ -246,12 +245,13 @@ impl StyleSharingCandidate { return None } + let mut classes = Vec::new(); + element.each_class(|c| classes.push(c.clone())); Some(StyleSharingCandidate { style: style, parent_style: parent_style, local_name: element.get_local_name().clone(), - class: element.get_attr(&ns!(), &atom!("class")) - .map(|string| string.to_owned()), + classes: classes, link: element.is_link(), namespace: (*element.get_namespace()).clone(), common_style_affecting_attributes: @@ -264,14 +264,19 @@ impl StyleSharingCandidate { return false } - // FIXME(pcwalton): Use `each_class` here instead of slow string comparison. - match (&self.class, element.get_attr(&ns!(), &atom!("class"))) { - (&None, Some(_)) | (&Some(_), None) => return false, - (&Some(ref this_class), Some(element_class)) if - element_class != &**this_class => { - return false + let mut num_classes = 0; + let mut classes_match = true; + element.each_class(|c| { + num_classes += 1; + // Note that we could do this check more cheaply if we decided to + // only consider class lists as equal if the orders match, since + // we could then index by num_classes instead of using .contains(). + if classes_match && !self.classes.contains(c) { + classes_match = false; } - (&Some(_), Some(_)) | (&None, None) => {} + }); + if !classes_match || num_classes != self.classes.len() { + return false; } if *element.get_namespace() != self.namespace { From 7947afc6991f0ba8db73d23014e8264534d801a8 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 23 Jun 2016 14:06:19 -0700 Subject: [PATCH 07/11] Remove get_attrs from TElement. This should just be a helper. --- components/script/layout_wrapper.rs | 12 ++++-------- components/style/dom.rs | 1 - ports/geckolib/wrapper.rs | 9 +++------ 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index 7085fcae002..46f5f1f9e63 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -392,13 +392,6 @@ impl<'le> TElement for ServoLayoutElement<'le> { (*self.element.unsafe_get()).get_attr_val_for_layout(namespace, name) } } - - #[inline] - fn get_attrs(&self, name: &Atom) -> Vec<&str> { - unsafe { - (*self.element.unsafe_get()).get_attr_vals_for_layout(name) - } - } } @@ -561,7 +554,10 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { self.get_attr(ns, name).map_or(false, |attr| test(attr)) }, NamespaceConstraint::Any => { - self.get_attrs(name).iter().any(|attr| test(*attr)) + let attrs = unsafe { + (*self.element.unsafe_get()).get_attr_vals_for_layout(name) + }; + attrs.iter().any(|attr| test(*attr)) } } } diff --git a/components/style/dom.rs b/components/style/dom.rs index 11cc5f93a0e..76445e6f60b 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -208,7 +208,6 @@ pub trait TElement : Sized + Copy + Clone + ElementExt + PresentationalHintsSynt fn get_state(&self) -> ElementState; fn get_attr<'a>(&'a self, namespace: &Namespace, attr: &Atom) -> Option<&'a str>; - fn get_attrs<'a>(&'a self, attr: &Atom) -> Vec<&'a str>; /// Properly marks nodes as dirty in response to restyle hints. fn note_restyle_hint(&self, mut hint: RestyleHint) { diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs index 2c32643c008..8ef5161cf97 100644 --- a/ports/geckolib/wrapper.rs +++ b/ports/geckolib/wrapper.rs @@ -369,11 +369,6 @@ impl<'le> TElement for GeckoElement<'le> { reinterpret_string(ptr, length) } } - - #[inline] - fn get_attrs<'a>(&'a self, _name: &Atom) -> Vec<&'a str> { - unimplemented!() - } } impl<'le> PresentationalHintsSynthetizer for GeckoElement<'le> { @@ -519,7 +514,9 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { self.get_attr(ns, name).map_or(false, |attr| test(attr)) }, NamespaceConstraint::Any => { - self.get_attrs(name).iter().any(|attr| test(*attr)) + // We should probably pass the atom into gecko and let it match + // against attributes across namespaces. + unimplemented!() } } } From 96af00fbb9270f1704b36bebbbc47493d3a8f813 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 23 Jun 2016 14:13:50 -0700 Subject: [PATCH 08/11] Add has_attr method to TElement. If this is all the information the caller needs, we can get it from gecko without worrying about atomization and string conversions. --- components/script/layout_wrapper.rs | 5 +++++ components/style/dom.rs | 2 ++ components/style/matching.rs | 8 ++++---- ports/geckolib/wrapper.rs | 9 +++++++++ 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index 46f5f1f9e63..7c7c9cc3505 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -386,6 +386,11 @@ impl<'le> TElement for ServoLayoutElement<'le> { self.element.get_state_for_layout() } + #[inline] + fn has_attr(&self, namespace: &Namespace, attr: &Atom) -> bool { + self.get_attr(namespace, attr).is_some() + } + #[inline] fn get_attr(&self, namespace: &Namespace, name: &Atom) -> Option<&str> { unsafe { diff --git a/components/style/dom.rs b/components/style/dom.rs index 76445e6f60b..436bf57aebd 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -207,6 +207,8 @@ pub trait TElement : Sized + Copy + Clone + ElementExt + PresentationalHintsSynt fn get_state(&self) -> ElementState; + fn has_attr(&self, namespace: &Namespace, attr: &Atom) -> bool; + fn get_attr<'a>(&'a self, namespace: &Namespace, attr: &Atom) -> Option<&'a str>; /// Properly marks nodes as dirty in response to restyle hints. diff --git a/components/style/matching.rs b/components/style/matching.rs index 289410af3f6..aa7d751267d 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -34,7 +34,7 @@ fn create_common_style_affecting_attributes_from_element(element: & for attribute_info in &common_style_affecting_attributes() { match attribute_info.mode { CommonStyleAffectingAttributeMode::IsPresent(flag) => { - if element.get_attr(&ns!(), &attribute_info.atom).is_some() { + if element.has_attr(&ns!(), &attribute_info.atom) { flags.insert(flag) } } @@ -296,7 +296,7 @@ impl StyleSharingCandidate { match attribute_info.mode { CommonStyleAffectingAttributeMode::IsPresent(flag) => { if self.common_style_affecting_attributes.contains(flag) != - element.get_attr(&ns!(), &attribute_info.atom).is_some() { + element.has_attr(&ns!(), &attribute_info.atom) { return false } } @@ -320,7 +320,7 @@ impl StyleSharingCandidate { } for attribute_name in &rare_style_affecting_attributes() { - if element.get_attr(&ns!(), attribute_name).is_some() { + if element.has_attr(&ns!(), attribute_name) { return false } } @@ -606,7 +606,7 @@ pub trait ElementMatchMethods : TElement if self.style_attribute().is_some() { return StyleSharingResult::CannotShare } - if self.get_attr(&ns!(), &atom!("id")).is_some() { + if self.has_attr(&ns!(), &atom!("id")) { return StyleSharingResult::CannotShare } diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs index 8ef5161cf97..7dc4545be2f 100644 --- a/ports/geckolib/wrapper.rs +++ b/ports/geckolib/wrapper.rs @@ -360,6 +360,15 @@ impl<'le> TElement for GeckoElement<'le> { } } + #[inline] + fn has_attr(&self, namespace: &Namespace, attr: &Atom) -> bool { + unsafe { + bindings::Gecko_HasAttr(self.element, + namespace.0.as_ptr(), + attr.as_ptr()) + } + } + #[inline] fn get_attr<'a>(&'a self, namespace: &Namespace, name: &Atom) -> Option<&'a str> { unsafe { From 364c8e297607f131468c4def668c7ba9933984e5 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 23 Jun 2016 15:36:25 -0700 Subject: [PATCH 09/11] Add attr_equals to TElement. Same reasons as the previous patch. --- components/script/layout_wrapper.rs | 5 +++++ components/style/dom.rs | 1 + components/style/matching.rs | 25 +++++++++---------------- ports/geckolib/wrapper.rs | 11 +++++++++++ 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index 7c7c9cc3505..a916ee73512 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -391,6 +391,11 @@ impl<'le> TElement for ServoLayoutElement<'le> { self.get_attr(namespace, attr).is_some() } + #[inline] + fn attr_equals(&self, namespace: &Namespace, attr: &Atom, val: &Atom) -> bool { + self.get_attr(namespace, attr).map_or(false, |x| x == val) + } + #[inline] fn get_attr(&self, namespace: &Namespace, name: &Atom) -> Option<&str> { unsafe { diff --git a/components/style/dom.rs b/components/style/dom.rs index 436bf57aebd..9cdce7b3852 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -208,6 +208,7 @@ pub trait TElement : Sized + Copy + Clone + ElementExt + PresentationalHintsSynt fn get_state(&self) -> ElementState; fn has_attr(&self, namespace: &Namespace, attr: &Atom) -> bool; + fn attr_equals(&self, namespace: &Namespace, attr: &Atom, value: &Atom) -> bool; fn get_attr<'a>(&'a self, namespace: &Namespace, attr: &Atom) -> Option<&'a str>; diff --git a/components/style/matching.rs b/components/style/matching.rs index aa7d751267d..f97ee2aab2c 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -39,11 +39,9 @@ fn create_common_style_affecting_attributes_from_element(element: & } } CommonStyleAffectingAttributeMode::IsEqual(target_value, flag) => { - match element.get_attr(&ns!(), &attribute_info.atom) { - Some(element_value) if element_value == target_value => { - flags.insert(flag) - } - _ => {} + let atom = Atom::from(target_value); // FIXME(bholley): This goes away in the next patch. + if element.attr_equals(&ns!(), &attribute_info.atom, &atom) { + flags.insert(flag) } } } @@ -301,19 +299,14 @@ impl StyleSharingCandidate { } } CommonStyleAffectingAttributeMode::IsEqual(target_value, flag) => { - match element.get_attr(&ns!(), &attribute_info.atom) { - Some(ref element_value) if self.common_style_affecting_attributes - .contains(flag) && - *element_value != target_value => { + let atom = Atom::from(target_value); // FIXME(bholley): This goes away in the next patch. + let contains = self.common_style_affecting_attributes.contains(flag); + if element.has_attr(&ns!(), &attribute_info.atom) { + if !contains || !element.attr_equals(&ns!(), &attribute_info.atom, &atom) { return false } - Some(_) if !self.common_style_affecting_attributes.contains(flag) => { - return false - } - None if self.common_style_affecting_attributes.contains(flag) => { - return false - } - _ => {} + } else if contains { + return false } } } diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs index 7dc4545be2f..247d3b9216c 100644 --- a/ports/geckolib/wrapper.rs +++ b/ports/geckolib/wrapper.rs @@ -369,6 +369,17 @@ impl<'le> TElement for GeckoElement<'le> { } } + #[inline] + fn attr_equals(&self, namespace: &Namespace, attr: &Atom, val: &Atom) -> bool { + unsafe { + bindings::Gecko_AttrEquals(self.element, + namespace.0.as_ptr(), + attr.as_ptr(), + val.as_ptr(), + /* ignoreCase = */ false) + } + } + #[inline] fn get_attr<'a>(&'a self, namespace: &Namespace, name: &Atom) -> Option<&'a str> { unsafe { From 1d8d1cb9d90e163cfc10328ceed0a2347279eb78 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Fri, 24 Jun 2016 15:01:37 -0700 Subject: [PATCH 10/11] Upgrade rust-selectors. --- components/layout/Cargo.toml | 2 +- components/layout/traversal.rs | 2 +- components/script/Cargo.toml | 2 +- components/script/dom/element.rs | 53 ++++---- components/script/layout_wrapper.rs | 74 +++++------ components/script_layout_interface/Cargo.toml | 2 +- .../script_layout_interface/wrapper_traits.rs | 4 +- components/servo/Cargo.lock | 12 +- components/style/Cargo.toml | 4 +- components/style/matching.rs | 10 +- components/style/restyle_hints.rs | 117 +++++++++++------- components/style/selector_impl.rs | 3 +- components/style/selector_matching.rs | 34 ++--- ports/cef/Cargo.lock | 10 +- ports/geckolib/Cargo.lock | 6 +- ports/geckolib/Cargo.toml | 2 +- ports/geckolib/selector_impl.rs | 2 + ports/geckolib/wrapper.rs | 113 ++++++++++++++--- tests/unit/style/Cargo.toml | 2 +- 19 files changed, 280 insertions(+), 174 deletions(-) diff --git a/components/layout/Cargo.toml b/components/layout/Cargo.toml index 188b39e9207..05f9fcda82c 100644 --- a/components/layout/Cargo.toml +++ b/components/layout/Cargo.toml @@ -31,7 +31,7 @@ range = {path = "../range"} rustc-serialize = "0.3" script_layout_interface = {path = "../script_layout_interface"} script_traits = {path = "../script_traits"} -selectors = {version = "0.6", features = ["heap_size"]} +selectors = {version = "0.7", features = ["heap_size"]} serde_macros = "0.7.11" smallvec = "0.1" string_cache = {version = "0.2.20", features = ["heap_size"]} diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index 5db766f68b5..581fee5e03d 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -28,7 +28,7 @@ pub struct RecalcStyleAndConstructFlows<'lc> { impl<'lc, N> DomTraversalContext for RecalcStyleAndConstructFlows<'lc> where N: LayoutNode + TNode, - N::ConcreteElement: ::selectors::Element + N::ConcreteElement: ::selectors::Element { type SharedContext = SharedLayoutContext; diff --git a/components/script/Cargo.toml b/components/script/Cargo.toml index d9bdeac843b..fa2d75c633d 100644 --- a/components/script/Cargo.toml +++ b/components/script/Cargo.toml @@ -57,7 +57,7 @@ regex = "0.1.43" rustc-serialize = "0.3" script_layout_interface = {path = "../script_layout_interface"} script_traits = {path = "../script_traits"} -selectors = {version = "0.6", features = ["heap_size"]} +selectors = {version = "0.7", features = ["heap_size"]} serde = "0.7.11" smallvec = "0.1" string_cache = {version = "0.2.20", features = ["heap_size", "unstable"]} diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index d9535663d46..e53f7dbb77b 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -2217,6 +2217,34 @@ impl VirtualMethods for Element { } } +impl<'a> ::selectors::MatchAttrGeneric for Root { + fn match_attr(&self, attr: &AttrSelector, test: F) -> bool + where F: Fn(&str) -> bool + { + use ::selectors::Element; + let local_name = { + if self.is_html_element_in_html_document() { + &attr.lower_name + } else { + &attr.name + } + }; + match attr.namespace { + NamespaceConstraint::Specific(ref ns) => { + self.get_attribute(ns, local_name) + .map_or(false, |attr| { + test(&attr.value()) + }) + }, + NamespaceConstraint::Any => { + self.attrs.borrow().iter().any(|attr| { + attr.local_name() == local_name && test(&attr.value()) + }) + } + } + } +} + impl<'a> ::selectors::Element for Root { type Impl = ServoSelectorImpl; @@ -2317,31 +2345,6 @@ impl<'a> ::selectors::Element for Root { } } - fn match_attr(&self, attr: &AttrSelector, test: F) -> bool - where F: Fn(&str) -> bool - { - let local_name = { - if self.is_html_element_in_html_document() { - &attr.lower_name - } else { - &attr.name - } - }; - match attr.namespace { - NamespaceConstraint::Specific(ref ns) => { - self.get_attribute(ns, local_name) - .map_or(false, |attr| { - test(&attr.value()) - }) - }, - NamespaceConstraint::Any => { - self.attrs.borrow().iter().any(|attr| { - attr.local_name() == local_name && test(&attr.value()) - }) - } - } - } - fn is_html_element_in_html_document(&self) -> bool { self.html_element_in_html_document() } diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index a916ee73512..7f3aaf3f242 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -418,6 +418,28 @@ fn as_element<'le>(node: LayoutJS) -> Option> { node.downcast().map(ServoLayoutElement::from_layout_js) } +impl<'le> ::selectors::MatchAttrGeneric for ServoLayoutElement<'le> { + fn match_attr(&self, attr: &AttrSelector, test: F) -> bool where F: Fn(&str) -> bool { + use ::selectors::Element; + let name = if self.is_html_element_in_html_document() { + &attr.lower_name + } else { + &attr.name + }; + match attr.namespace { + NamespaceConstraint::Specific(ref ns) => { + self.get_attr(ns, name).map_or(false, |attr| test(attr)) + }, + NamespaceConstraint::Any => { + let attrs = unsafe { + (*self.element.unsafe_get()).get_attr_vals_for_layout(name) + }; + attrs.iter().any(|attr| test(*attr)) + } + } + } +} + impl<'le> ::selectors::Element for ServoLayoutElement<'le> { type Impl = ServoSelectorImpl; @@ -553,25 +575,6 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> { } } - fn match_attr(&self, attr: &AttrSelector, test: F) -> bool where F: Fn(&str) -> bool { - let name = if self.is_html_element_in_html_document() { - &attr.lower_name - } else { - &attr.name - }; - match attr.namespace { - NamespaceConstraint::Specific(ref ns) => { - self.get_attr(ns, name).map_or(false, |attr| test(attr)) - }, - NamespaceConstraint::Any => { - let attrs = unsafe { - (*self.element.unsafe_get()).get_attr_vals_for_layout(name) - }; - attrs.iter().any(|attr| test(*attr)) - } - } - } - fn is_html_element_in_html_document(&self) -> bool { unsafe { self.element.html_element_in_html_document_for_layout() @@ -906,7 +909,23 @@ impl<'le> ThreadSafeLayoutElement for ServoThreadSafeLayoutElement<'le> { /// /// Note that the element implementation is needed only for selector matching, /// not for inheritance (styles are inherited appropiately). -impl <'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> { +impl<'le> ::selectors::MatchAttrGeneric for ServoThreadSafeLayoutElement<'le> { + fn match_attr(&self, attr: &AttrSelector, test: F) -> bool + where F: Fn(&str) -> bool { + match attr.namespace { + NamespaceConstraint::Specific(ref ns) => { + self.get_attr(ns, &attr.name).map_or(false, |attr| test(attr)) + }, + NamespaceConstraint::Any => { + unsafe { + self.element.get_attr_vals_for_layout(&attr.name).iter() + .any(|attr| test(*attr)) + } + } + } + } +} +impl<'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> { type Impl = ServoSelectorImpl; fn parent_element(&self) -> Option { @@ -968,21 +987,6 @@ impl <'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> { false } - fn match_attr(&self, attr: &AttrSelector, test: F) -> bool - where F: Fn(&str) -> bool { - match attr.namespace { - NamespaceConstraint::Specific(ref ns) => { - self.get_attr(ns, &attr.name).map_or(false, |attr| test(attr)) - }, - NamespaceConstraint::Any => { - unsafe { - self.element.get_attr_vals_for_layout(&attr.name).iter() - .any(|attr| test(*attr)) - } - } - } - } - fn is_empty(&self) -> bool { warn!("ServoThreadSafeLayoutElement::is_empty called"); false diff --git a/components/script_layout_interface/Cargo.toml b/components/script_layout_interface/Cargo.toml index 51b767c753f..41c1f6e2598 100644 --- a/components/script_layout_interface/Cargo.toml +++ b/components/script_layout_interface/Cargo.toml @@ -26,7 +26,7 @@ plugins = {path = "../plugins"} profile_traits = {path = "../profile_traits"} range = {path = "../range"} script_traits = {path = "../script_traits"} -selectors = {version = "0.6", features = ["heap_size"]} +selectors = {version = "0.7", features = ["heap_size"]} string_cache = {version = "0.2.20", features = ["heap_size"]} style = {path = "../style"} url = {version = "1.0.0", features = ["heap_size"]} diff --git a/components/script_layout_interface/wrapper_traits.rs b/components/script_layout_interface/wrapper_traits.rs index a816321ec28..bdbf31c37c8 100644 --- a/components/script_layout_interface/wrapper_traits.rs +++ b/components/script_layout_interface/wrapper_traits.rs @@ -86,7 +86,7 @@ pub trait LayoutNode: TNode { pub trait ThreadSafeLayoutNode: Clone + Copy + Sized + PartialEq { type ConcreteThreadSafeLayoutElement: ThreadSafeLayoutElement - + ::selectors::Element; + + ::selectors::Element; type ChildrenIterator: Iterator + Sized; /// Creates a new `ThreadSafeLayoutNode` for the same `LayoutNode` @@ -351,7 +351,7 @@ pub trait DangerousThreadSafeLayoutNode: ThreadSafeLayoutNode { } pub trait ThreadSafeLayoutElement: Clone + Copy + Sized + - ::selectors::Element + + ::selectors::Element + PresentationalHintsSynthetizer { type ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode; diff --git a/components/servo/Cargo.lock b/components/servo/Cargo.lock index 3424416cead..997fc8ab250 100644 --- a/components/servo/Cargo.lock +++ b/components/servo/Cargo.lock @@ -1159,7 +1159,7 @@ dependencies = [ "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", "script_layout_interface 0.0.1", "script_traits 0.0.1", - "selectors 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", + "selectors 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "serde_macros 0.7.11 (registry+https://github.com/rust-lang/crates.io-index)", "smallvec 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", "string_cache 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1942,7 +1942,7 @@ dependencies = [ "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", "script_layout_interface 0.0.1", "script_traits 0.0.1", - "selectors 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", + "selectors 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "serde 0.7.11 (registry+https://github.com/rust-lang/crates.io-index)", "smallvec 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", "string_cache 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1978,7 +1978,7 @@ dependencies = [ "profile_traits 0.0.1", "range 0.0.1", "script_traits 0.0.1", - "selectors 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", + "selectors 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "string_cache 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)", "style 0.0.1", "url 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2026,7 +2026,7 @@ dependencies = [ [[package]] name = "selectors" -version = "0.6.0" +version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "bitflags 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2243,7 +2243,7 @@ dependencies = [ "num-traits 0.1.32 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", - "selectors 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", + "selectors 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "serde 0.7.11 (registry+https://github.com/rust-lang/crates.io-index)", "serde_macros 0.7.11 (registry+https://github.com/rust-lang/crates.io-index)", "smallvec 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2263,7 +2263,7 @@ dependencies = [ "cssparser 0.5.6 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", - "selectors 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", + "selectors 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "string_cache 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)", "style 0.0.1", "style_traits 0.0.1", diff --git a/components/style/Cargo.toml b/components/style/Cargo.toml index a90673b9e40..049dd560eeb 100644 --- a/components/style/Cargo.toml +++ b/components/style/Cargo.toml @@ -11,7 +11,7 @@ name = "style" path = "lib.rs" [features] -gecko = ["gecko_bindings"] +gecko = ["gecko_bindings", "selectors/gecko"] servo = ["serde", "serde/nightly", "serde_macros", "heapsize", "heapsize_plugin", "style_traits/servo", "app_units/plugins", "euclid/plugins", "cssparser/heap_size", "cssparser/serde-serialization", @@ -36,7 +36,7 @@ matches = "0.1" num-traits = "0.1.32" rand = "0.3" rustc-serialize = "0.3" -selectors = "0.6" +selectors = "0.7" serde = {version = "0.7.11", optional = true} serde_macros = {version = "0.7.11", optional = true} smallvec = "0.1" diff --git a/components/style/matching.rs b/components/style/matching.rs index f97ee2aab2c..5d4fa82b8ce 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -38,9 +38,8 @@ fn create_common_style_affecting_attributes_from_element(element: & flags.insert(flag) } } - CommonStyleAffectingAttributeMode::IsEqual(target_value, flag) => { - let atom = Atom::from(target_value); // FIXME(bholley): This goes away in the next patch. - if element.attr_equals(&ns!(), &attribute_info.atom, &atom) { + CommonStyleAffectingAttributeMode::IsEqual(ref target_value, flag) => { + if element.attr_equals(&ns!(), &attribute_info.atom, target_value) { flags.insert(flag) } } @@ -298,11 +297,10 @@ impl StyleSharingCandidate { return false } } - CommonStyleAffectingAttributeMode::IsEqual(target_value, flag) => { - let atom = Atom::from(target_value); // FIXME(bholley): This goes away in the next patch. + CommonStyleAffectingAttributeMode::IsEqual(ref target_value, flag) => { let contains = self.common_style_affecting_attributes.contains(flag); if element.has_attr(&ns!(), &attribute_info.atom) { - if !contains || !element.attr_equals(&ns!(), &attribute_info.atom, &atom) { + if !contains || !element.attr_equals(&ns!(), &attribute_info.atom, target_value) { return false } } else if contains { diff --git a/components/style/restyle_hints.rs b/components/style/restyle_hints.rs index c517a6f1a7b..cd9957322e4 100644 --- a/components/style/restyle_hints.rs +++ b/components/style/restyle_hints.rs @@ -7,9 +7,9 @@ use attr::{AttrIdentifier, AttrValue}; use element_state::*; use selector_impl::SelectorImplExt; -use selectors::Element; use selectors::matching::matches_compound_selector; use selectors::parser::{AttrSelector, Combinator, CompoundSelector, SelectorImpl, SimpleSelector}; +use selectors::{Element, MatchAttrGeneric}; use std::clone::Clone; use std::sync::Arc; use string_cache::{Atom, BorrowedAtom, BorrowedNamespace, Namespace}; @@ -81,15 +81,21 @@ impl ElementSnapshot { static EMPTY_SNAPSHOT: ElementSnapshot = ElementSnapshot { state: None, attrs: None }; +// FIXME(bholley): This implementation isn't going to work for geckolib, because +// it's fundamentally based on get_attr/match_attr, which we don't want to support +// that configuration due to the overhead of converting between UTF-16 and UTF-8. +// We'll need to figure something out when we start using restyle hints with +// geckolib, but in the mean time we can just use the trait parameters to +// specialize it to the Servo configuration. struct ElementWrapper<'a, E> - where E: Element, + where E: Element, E::Impl: SelectorImplExt { element: E, snapshot: &'a ElementSnapshot, } impl<'a, E> ElementWrapper<'a, E> - where E: Element, + where E: Element, E::Impl: SelectorImplExt { pub fn new(el: E) -> ElementWrapper<'a, E> { ElementWrapper { element: el, snapshot: &EMPTY_SNAPSHOT } @@ -100,8 +106,42 @@ impl<'a, E> ElementWrapper<'a, E> } } +#[cfg(not(feature = "gecko"))] +impl<'a, E> MatchAttrGeneric for ElementWrapper<'a, E> + where E: Element, + E: MatchAttrGeneric, + E::Impl: SelectorImplExt { + fn match_attr(&self, attr: &AttrSelector, test: F) -> bool + where F: Fn(&str) -> bool { + use selectors::parser::NamespaceConstraint; + match self.snapshot.attrs { + Some(_) => { + let html = self.is_html_element_in_html_document(); + let local_name = if html { &attr.lower_name } else { &attr.name }; + match attr.namespace { + NamespaceConstraint::Specific(ref ns) => self.snapshot.get_attr(ns, local_name), + NamespaceConstraint::Any => self.snapshot.get_attr_ignore_ns(local_name), + }.map_or(false, |v| test(v)) + }, + None => self.element.match_attr(attr, test) + } + } +} + +#[cfg(feature = "gecko")] +impl<'a, E> MatchAttrGeneric for ElementWrapper<'a, E> + where E: Element, + E: MatchAttrGeneric, + E::Impl: SelectorImplExt { + fn match_attr(&self, _: &AttrSelector, _: F) -> bool + where F: Fn(&str) -> bool { + panic!("Not implemented for Gecko - this system will need to be redesigned"); + } +} + impl<'a, E> Element for ElementWrapper<'a, E> - where E: Element, + where E: Element, + E: MatchAttrGeneric, E::Impl: SelectorImplExt { type Impl = E::Impl; @@ -155,27 +195,6 @@ impl<'a, E> Element for ElementWrapper<'a, E> None => self.element.has_class(name), } } - #[cfg(feature = "gecko")] - fn match_attr(&self, _: &AttrSelector, _: F) -> bool - where F: Fn(&str) -> bool { - panic!("Gecko can't borrow atoms as UTF-8."); - } - #[cfg(not(feature = "gecko"))] - fn match_attr(&self, attr: &AttrSelector, test: F) -> bool - where F: Fn(&str) -> bool { - use selectors::parser::NamespaceConstraint; - match self.snapshot.attrs { - Some(_) => { - let html = self.is_html_element_in_html_document(); - let local_name = if html { &attr.lower_name } else { &attr.name }; - match attr.namespace { - NamespaceConstraint::Specific(ref ns) => self.snapshot.get_attr(ns, local_name), - NamespaceConstraint::Any => self.snapshot.get_attr_ignore_ns(local_name), - }.map_or(false, |v| test(v)) - }, - None => self.element.match_attr(attr, test) - } - } fn is_empty(&self) -> bool { self.element.is_empty() } @@ -208,7 +227,7 @@ fn is_attr_selector(sel: &SimpleSelector) -> bool { SimpleSelector::AttrExists(_) | SimpleSelector::AttrEqual(_, _, _) | SimpleSelector::AttrIncludes(_, _) | - SimpleSelector::AttrDashMatch(_, _, _) | + SimpleSelector::AttrDashMatch(_, _) | SimpleSelector::AttrPrefixMatch(_, _) | SimpleSelector::AttrSubstringMatch(_, _) | SimpleSelector::AttrSuffixMatch(_, _) => true, @@ -285,28 +304,6 @@ impl DependencySet { DependencySet { deps: Vec::new() } } - pub fn compute_hint(&self, el: &E, snapshot: &ElementSnapshot, current_state: ElementState) - -> RestyleHint - where E: Element + Clone { - let state_changes = snapshot.state.map_or(ElementState::empty(), |old_state| current_state ^ old_state); - let attrs_changed = snapshot.attrs.is_some(); - let mut hint = RestyleHint::empty(); - for dep in &self.deps { - if state_changes.intersects(dep.sensitivities.states) || (attrs_changed && dep.sensitivities.attrs) { - let old_el: ElementWrapper = ElementWrapper::new_with_snapshot(el.clone(), snapshot); - let matched_then = matches_compound_selector(&*dep.selector, &old_el, None, &mut false); - let matches_now = matches_compound_selector(&*dep.selector, el, None, &mut false); - if matched_then != matches_now { - hint.insert(combinator_to_restyle_hint(dep.combinator)); - if hint.is_all() { - break - } - } - } - } - hint - } - pub fn note_selector(&mut self, selector: Arc>) { let mut cur = selector; let mut combinator: Option = None; @@ -340,3 +337,27 @@ impl DependencySet { self.deps.clear(); } } + +impl> DependencySet { + pub fn compute_hint(&self, el: &E, snapshot: &ElementSnapshot, current_state: ElementState) + -> RestyleHint + where E: Element + Clone + MatchAttrGeneric { + let state_changes = snapshot.state.map_or(ElementState::empty(), |old_state| current_state ^ old_state); + let attrs_changed = snapshot.attrs.is_some(); + let mut hint = RestyleHint::empty(); + for dep in &self.deps { + if state_changes.intersects(dep.sensitivities.states) || (attrs_changed && dep.sensitivities.attrs) { + let old_el: ElementWrapper = ElementWrapper::new_with_snapshot(el.clone(), snapshot); + let matched_then = matches_compound_selector(&*dep.selector, &old_el, None, &mut false); + let matches_now = matches_compound_selector(&*dep.selector, el, None, &mut false); + if matched_then != matches_now { + hint.insert(combinator_to_restyle_hint(dep.combinator)); + if hint.is_all() { + break + } + } + } + } + hint + } +} diff --git a/components/style/selector_impl.rs b/components/style/selector_impl.rs index cf95f4a9048..cd47d0dc7f5 100644 --- a/components/style/selector_impl.rs +++ b/components/style/selector_impl.rs @@ -181,6 +181,7 @@ impl NonTSPseudoClass { pub struct ServoSelectorImpl; impl SelectorImpl for ServoSelectorImpl { + type AttrString = String; type PseudoElement = PseudoElement; type NonTSPseudoClass = NonTSPseudoClass; @@ -278,7 +279,7 @@ impl SelectorImplExt for ServoSelectorImpl { } } -impl> ElementExt for E { +impl> ElementExt for E { fn is_link(&self) -> bool { self.match_non_ts_pseudo_class(NonTSPseudoClass::AnyLink) } diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs index 74718d4b69b..b632d783e1e 100644 --- a/components/style/selector_matching.rs +++ b/components/style/selector_matching.rs @@ -13,11 +13,11 @@ use parser::ParserContextExtraData; use properties::{self, PropertyDeclaration, PropertyDeclarationBlock}; use restyle_hints::{ElementSnapshot, RestyleHint, DependencySet}; use selector_impl::{SelectorImplExt, ServoSelectorImpl}; -use selectors::Element; use selectors::bloom::BloomFilter; use selectors::matching::DeclarationBlock as GenericDeclarationBlock; use selectors::matching::{Rule, SelectorMap}; use selectors::parser::SelectorImpl; +use selectors::{Element, MatchAttrGeneric}; use sink::Push; use smallvec::VecLike; use std::collections::HashMap; @@ -311,7 +311,7 @@ impl Stylist { pseudo: &Impl::PseudoElement, parent: &Arc) -> Option> - where E: Element + + where E: Element + PresentationalHintsSynthetizer { debug_assert!(Impl::pseudo_element_cascade_type(pseudo).is_lazy()); if self.pseudos_map.get(pseudo).is_none() { @@ -336,18 +336,6 @@ impl Stylist { Some(Arc::new(computed)) } - pub fn compute_restyle_hint(&self, element: &E, - snapshot: &ElementSnapshot, - // NB: We need to pass current_state as an argument because - // selectors::Element doesn't provide access to ElementState - // directly, and computing it from the ElementState would be - // more expensive than getting it directly from the caller. - current_state: ElementState) - -> RestyleHint - where E: Element + Clone { - self.state_deps.compute_hint(element, snapshot, current_state) - } - pub fn set_device(&mut self, mut device: Device, stylesheets: &[Arc>]) { let cascaded_rule = stylesheets.iter() .flat_map(|s| s.effective_rules(&self.device).viewport()) @@ -389,7 +377,8 @@ impl Stylist { pseudo_element: Option<&Impl::PseudoElement>, applicable_declarations: &mut V) -> bool - where E: Element + PresentationalHintsSynthetizer, + where E: Element + + PresentationalHintsSynthetizer, V: Push + VecLike { assert!(!self.is_device_dirty); assert!(style_attribute.is_none() || pseudo_element.is_none(), @@ -474,6 +463,21 @@ impl Stylist { } } +impl> Stylist { + pub fn compute_restyle_hint(&self, element: &E, + snapshot: &ElementSnapshot, + // NB: We need to pass current_state as an argument because + // selectors::Element doesn't provide access to ElementState + // directly, and computing it from the ElementState would be + // more expensive than getting it directly from the caller. + current_state: ElementState) + -> RestyleHint + where E: Element + Clone + MatchAttrGeneric { + self.state_deps.compute_hint(element, snapshot, current_state) + } +} + + /// Map that contains the CSS rules for a given origin. #[cfg_attr(feature = "servo", derive(HeapSizeOf))] struct PerOriginSelectorMap { diff --git a/ports/cef/Cargo.lock b/ports/cef/Cargo.lock index d2acbc92d13..4907cc20629 100644 --- a/ports/cef/Cargo.lock +++ b/ports/cef/Cargo.lock @@ -1068,7 +1068,7 @@ dependencies = [ "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", "script_layout_interface 0.0.1", "script_traits 0.0.1", - "selectors 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", + "selectors 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "serde_macros 0.7.11 (registry+https://github.com/rust-lang/crates.io-index)", "smallvec 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", "string_cache 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1796,7 +1796,7 @@ dependencies = [ "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", "script_layout_interface 0.0.1", "script_traits 0.0.1", - "selectors 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", + "selectors 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "serde 0.7.11 (registry+https://github.com/rust-lang/crates.io-index)", "smallvec 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", "string_cache 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1832,7 +1832,7 @@ dependencies = [ "profile_traits 0.0.1", "range 0.0.1", "script_traits 0.0.1", - "selectors 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", + "selectors 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "string_cache 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)", "style 0.0.1", "url 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1870,7 +1870,7 @@ dependencies = [ [[package]] name = "selectors" -version = "0.6.0" +version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "bitflags 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2128,7 +2128,7 @@ dependencies = [ "num-traits 0.1.32 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", - "selectors 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", + "selectors 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "serde 0.7.11 (registry+https://github.com/rust-lang/crates.io-index)", "serde_macros 0.7.11 (registry+https://github.com/rust-lang/crates.io-index)", "smallvec 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/ports/geckolib/Cargo.lock b/ports/geckolib/Cargo.lock index 3fe70abd1d9..27b841fb790 100644 --- a/ports/geckolib/Cargo.lock +++ b/ports/geckolib/Cargo.lock @@ -12,7 +12,7 @@ dependencies = [ "libc 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", "num_cpus 0.2.12 (registry+https://github.com/rust-lang/crates.io-index)", - "selectors 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", + "selectors 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "smallvec 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", "string_cache 0.2.20 (registry+https://github.com/rust-lang/crates.io-index)", "style 0.0.1", @@ -379,7 +379,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "selectors" -version = "0.6.0" +version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "bitflags 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -465,7 +465,7 @@ dependencies = [ "num-traits 0.1.32 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", - "selectors 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", + "selectors 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "serde 0.7.11 (registry+https://github.com/rust-lang/crates.io-index)", "serde_macros 0.7.11 (registry+https://github.com/rust-lang/crates.io-index)", "smallvec 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/ports/geckolib/Cargo.toml b/ports/geckolib/Cargo.toml index 6ca0d6d4e2e..8f1112ccddc 100644 --- a/ports/geckolib/Cargo.toml +++ b/ports/geckolib/Cargo.toml @@ -37,7 +37,7 @@ lazy_static = "0.2" libc = "0.2" log = {version = "0.3.5", features = ["release_max_level_info"]} num_cpus = "0.2.2" -selectors = "0.6" +selectors = "0.7" smallvec = "0.1" string_cache = {version = "0.2.20", features = ["unstable"]} style = {path = "../../components/style", features = ["gecko"]} diff --git a/ports/geckolib/selector_impl.rs b/ports/geckolib/selector_impl.rs index c1284015414..a2f0d1eba92 100644 --- a/ports/geckolib/selector_impl.rs +++ b/ports/geckolib/selector_impl.rs @@ -4,6 +4,7 @@ use properties::GeckoComputedValues; use selectors::parser::{ParserContext, SelectorImpl}; +use string_cache::Atom; use style; use style::element_state::ElementState; use style::selector_impl::{PseudoElementCascadeType, SelectorImplExt}; @@ -157,6 +158,7 @@ impl NonTSPseudoClass { } impl SelectorImpl for GeckoSelectorImpl { + type AttrString = Atom; type PseudoElement = PseudoElement; type NonTSPseudoClass = NonTSPseudoClass; fn parse_non_ts_pseudo_class(_context: &ParserContext, diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs index 247d3b9216c..7e06c533ef1 100644 --- a/ports/geckolib/wrapper.rs +++ b/ports/geckolib/wrapper.rs @@ -4,6 +4,7 @@ #![allow(unsafe_code)] +use gecko_bindings::bindings; use gecko_bindings::bindings::Gecko_ChildrenCount; use gecko_bindings::bindings::Gecko_ClassOrClassList; use gecko_bindings::bindings::Gecko_GetElementId; @@ -521,26 +522,6 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { } } - fn match_attr(&self, attr: &AttrSelector, test: F) -> bool where F: Fn(&str) -> bool { - // FIXME(bholley): This is copy-pasted from the servo wrapper's version. - // We should find a way to share it. - let name = if self.is_html_element_in_html_document() { - &attr.lower_name - } else { - &attr.name - }; - match attr.namespace { - NamespaceConstraint::Specific(ref ns) => { - self.get_attr(ns, name).map_or(false, |attr| test(attr)) - }, - NamespaceConstraint::Any => { - // We should probably pass the atom into gecko and let it match - // against attributes across namespaces. - unimplemented!() - } - } - } - fn is_html_element_in_html_document(&self) -> bool { unsafe { Gecko_IsHTMLElementInHTMLDocument(self.element) @@ -548,6 +529,98 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { } } +trait AttrSelectorHelpers { + fn ns_or_null(&self) -> *mut nsIAtom; + fn select_name<'le>(&self, el: &GeckoElement<'le>) -> *mut nsIAtom; +} + +impl AttrSelectorHelpers for AttrSelector { + fn ns_or_null(&self) -> *mut nsIAtom { + match self.namespace { + NamespaceConstraint::Any => ptr::null_mut(), + NamespaceConstraint::Specific(ref ns) => ns.0.as_ptr(), + } + } + + fn select_name<'le>(&self, el: &GeckoElement<'le>) -> *mut nsIAtom { + if el.is_html_element_in_html_document() { + self.lower_name.as_ptr() + } else { + self.name.as_ptr() + } + + } +} + +impl<'le> ::selectors::MatchAttr for GeckoElement<'le> { + type AttrString = Atom; + fn match_attr_has(&self, attr: &AttrSelector) -> bool { + unsafe { + bindings::Gecko_HasAttr(self.element, + attr.ns_or_null(), + attr.select_name(self)) + } + } + fn match_attr_equals(&self, attr: &AttrSelector, value: &Self::AttrString) -> bool { + unsafe { + bindings::Gecko_AttrEquals(self.element, + attr.ns_or_null(), + attr.select_name(self), + value.as_ptr(), + /* ignoreCase = */ false) + } + } + fn match_attr_equals_ignore_ascii_case(&self, attr: &AttrSelector, value: &Self::AttrString) -> bool { + unsafe { + bindings::Gecko_AttrEquals(self.element, + attr.ns_or_null(), + attr.select_name(self), + value.as_ptr(), + /* ignoreCase = */ false) + } + } + fn match_attr_includes(&self, attr: &AttrSelector, value: &Self::AttrString) -> bool { + unsafe { + bindings::Gecko_AttrIncludes(self.element, + attr.ns_or_null(), + attr.select_name(self), + value.as_ptr()) + } + } + fn match_attr_dash(&self, attr: &AttrSelector, value: &Self::AttrString) -> bool { + unsafe { + bindings::Gecko_AttrDashEquals(self.element, + attr.ns_or_null(), + attr.select_name(self), + value.as_ptr()) + } + } + fn match_attr_prefix(&self, attr: &AttrSelector, value: &Self::AttrString) -> bool { + unsafe { + bindings::Gecko_AttrHasPrefix(self.element, + attr.ns_or_null(), + attr.select_name(self), + value.as_ptr()) + } + } + fn match_attr_substring(&self, attr: &AttrSelector, value: &Self::AttrString) -> bool { + unsafe { + bindings::Gecko_AttrHasSubstring(self.element, + attr.ns_or_null(), + attr.select_name(self), + value.as_ptr()) + } + } + fn match_attr_suffix(&self, attr: &AttrSelector, value: &Self::AttrString) -> bool { + unsafe { + bindings::Gecko_AttrHasSuffix(self.element, + attr.ns_or_null(), + attr.select_name(self), + value.as_ptr()) + } + } +} + impl<'le> ElementExt for GeckoElement<'le> { fn is_link(&self) -> bool { self.match_non_ts_pseudo_class(NonTSPseudoClass::AnyLink) diff --git a/tests/unit/style/Cargo.toml b/tests/unit/style/Cargo.toml index 1e8e120651f..8063e1a5b8d 100644 --- a/tests/unit/style/Cargo.toml +++ b/tests/unit/style/Cargo.toml @@ -13,7 +13,7 @@ app_units = "0.2.5" cssparser = {version = "0.5.4", features = ["heap_size"]} euclid = "0.7.1" rustc-serialize = "0.3" -selectors = {version = "0.6", features = ["heap_size"]} +selectors = {version = "0.7", features = ["heap_size"]} string_cache = {version = "0.2", features = ["heap_size"]} style = {path = "../../../components/style"} style_traits = {path = "../../../components/style_traits"} From 187a47d89d2a4187abca85690429f85293e618cd Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 30 Jun 2016 14:15:20 -0700 Subject: [PATCH 11/11] Remove get_attr from TElement. \o/ --- components/script/layout_wrapper.rs | 14 +++++++------- components/style/dom.rs | 2 -- ports/geckolib/wrapper.rs | 10 ---------- 3 files changed, 7 insertions(+), 19 deletions(-) diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index 7f3aaf3f242..ebc2da9c529 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -395,13 +395,6 @@ impl<'le> TElement for ServoLayoutElement<'le> { fn attr_equals(&self, namespace: &Namespace, attr: &Atom, val: &Atom) -> bool { self.get_attr(namespace, attr).map_or(false, |x| x == val) } - - #[inline] - fn get_attr(&self, namespace: &Namespace, name: &Atom) -> Option<&str> { - unsafe { - (*self.element.unsafe_get()).get_attr_val_for_layout(namespace, name) - } - } } @@ -412,6 +405,13 @@ impl<'le> ServoLayoutElement<'le> { chain: PhantomData, } } + + #[inline] + fn get_attr(&self, namespace: &Namespace, name: &Atom) -> Option<&str> { + unsafe { + (*self.element.unsafe_get()).get_attr_val_for_layout(namespace, name) + } + } } fn as_element<'le>(node: LayoutJS) -> Option> { diff --git a/components/style/dom.rs b/components/style/dom.rs index 9cdce7b3852..0e94c2c5c1d 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -210,8 +210,6 @@ pub trait TElement : Sized + Copy + Clone + ElementExt + PresentationalHintsSynt fn has_attr(&self, namespace: &Namespace, attr: &Atom) -> bool; fn attr_equals(&self, namespace: &Namespace, attr: &Atom, value: &Atom) -> bool; - fn get_attr<'a>(&'a self, namespace: &Namespace, attr: &Atom) -> Option<&'a str>; - /// Properly marks nodes as dirty in response to restyle hints. fn note_restyle_hint(&self, mut hint: RestyleHint) { // Bail early if there's no restyling to do. diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs index 7e06c533ef1..29f3ee99900 100644 --- a/ports/geckolib/wrapper.rs +++ b/ports/geckolib/wrapper.rs @@ -380,16 +380,6 @@ impl<'le> TElement for GeckoElement<'le> { /* ignoreCase = */ false) } } - - #[inline] - fn get_attr<'a>(&'a self, namespace: &Namespace, name: &Atom) -> Option<&'a str> { - unsafe { - let mut length: u32 = 0; - let ptr = Gecko_GetAttrAsUTF8(self.element, namespace.0.as_ptr(), name.as_ptr(), - &mut length); - reinterpret_string(ptr, length) - } - } } impl<'le> PresentationalHintsSynthetizer for GeckoElement<'le> {