Skip to content

Commit

Permalink
layout: Support more types of selectors for style sharing.
Browse files Browse the repository at this point in the history
This helps avoid problems with style sharing in common cases, often
caused by the user agent stylesheet.
  • Loading branch information
pcwalton committed Oct 28, 2014
1 parent 2d8bd10 commit 4bf0acb
Show file tree
Hide file tree
Showing 3 changed files with 146 additions and 10 deletions.
79 changes: 75 additions & 4 deletions components/layout/css/matching.rs
Expand Up @@ -17,10 +17,10 @@ use servo_util::arc_ptr_eq;
use std::mem;
use std::hash::{Hash, sip};
use std::slice::Items;
use style::{After, Before, ComputedValues, DeclarationBlock, Stylist, TElement, TNode};
use style::cascade;
use string_cache::{Atom, Namespace};
use style::{mod, After, Before, ComputedValues, DeclarationBlock, Stylist, TElement, TNode};
use style::{AttrIsEqualMode, AttrIsPresentMode, CommonStyleAffectingAttributes, cascade};
use sync::Arc;
use string_cache::Atom;

pub struct ApplicableDeclarations {
pub normal: SmallVec16<DeclarationBlock>,
Expand Down Expand Up @@ -148,21 +148,50 @@ pub struct StyleSharingCandidateCache {
cache: LRUCache<StyleSharingCandidate,()>,
}

fn create_common_style_affecting_attributes_from_element(element: &LayoutElement)
-> CommonStyleAffectingAttributes {
let mut flags = CommonStyleAffectingAttributes::empty();
for attribute_info in style::common_style_affecting_attributes().iter() {
match attribute_info.mode {
AttrIsPresentMode(flag) => {
if element.get_attr(&ns!(""), &attribute_info.atom).is_some() {
flags.insert(flag)
}
}
AttrIsEqualMode(target_value, flag) => {
match element.get_attr(&ns!(""), &attribute_info.atom) {
Some(element_value) if element_value == target_value => {
flags.insert(flag)
}
_ => {}
}
}
}
}
flags
}

#[deriving(Clone)]
pub struct StyleSharingCandidate {
pub style: Arc<ComputedValues>,
pub parent_style: Arc<ComputedValues>,
pub local_name: Atom,
// FIXME(pcwalton): Should be a list of atoms instead.
pub class: Option<String>,
pub namespace: Namespace,
pub common_style_affecting_attributes: CommonStyleAffectingAttributes,
pub link: bool,
}

impl PartialEq for StyleSharingCandidate {
fn eq(&self, other: &StyleSharingCandidate) -> bool {
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.class == other.class &&
self.link == other.link &&
self.namespace == other.namespace &&
self.common_style_affecting_attributes == other.common_style_affecting_attributes
}
}

Expand Down Expand Up @@ -213,6 +242,10 @@ impl StyleSharingCandidate {
local_name: element.get_local_name().clone(),
class: element.get_attr(&ns!(""), &atom!("class"))
.map(|string| string.to_string()),
link: element.get_link().is_some(),
namespace: (*element.get_namespace()).clone(),
common_style_affecting_attributes:
create_common_style_affecting_attributes_from_element(&element)
})
}

Expand All @@ -231,6 +264,44 @@ impl StyleSharingCandidate {
(&Some(_), Some(_)) | (&None, None) => {}
}

if *element.get_namespace() != self.namespace {
return false
}

for attribute_info in style::common_style_affecting_attributes().iter() {
match attribute_info.mode {
AttrIsPresentMode(flag) => {
if self.common_style_affecting_attributes.contains(flag) !=
element.get_attr(&ns!(""), &attribute_info.atom).is_some() {
return false
}
}
AttrIsEqualMode(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 => {
return false
}
Some(_) if !self.common_style_affecting_attributes.contains(flag) => {
return false
}
None if self.common_style_affecting_attributes.contains(flag) => {
return false
}
_ => {}
}
}
}
}

if element.get_link().is_some() != self.link {
return false
}

// TODO(pcwalton): We don't support visited links yet, but when we do there will need to
// be some logic here.

true
}
}
Expand Down
5 changes: 4 additions & 1 deletion components/style/lib.rs
Expand Up @@ -38,7 +38,10 @@ extern crate "util" as servo_util;
pub use media_queries::{Device, Screen};
pub use stylesheets::{Stylesheet, iter_font_face_rules};
pub use selector_matching::{Stylist, StylesheetOrigin, UserAgentOrigin, AuthorOrigin, UserOrigin};
pub use selector_matching::{DeclarationBlock, matches,matches_simple_selector};
pub use selector_matching::{DeclarationBlock, CommonStyleAffectingAttributes};
pub use selector_matching::{CommonStyleAffectingAttributeInfo, CommonStyleAffectingAttributeMode};
pub use selector_matching::{AttrIsPresentMode, AttrIsEqualMode};
pub use selector_matching::{matches, matches_simple_selector, common_style_affecting_attributes};
pub use selector_matching::{RECOMMENDED_SELECTOR_BLOOM_FILTER_SIZE,SELECTOR_WHITESPACE};
pub use properties::{cascade, cascade_anonymous};
pub use properties::{PropertyDeclaration, ComputedValues, computed_values, style_structs};
Expand Down
72 changes: 67 additions & 5 deletions components/style/selector_matching.rs
Expand Up @@ -761,6 +761,53 @@ fn matches_compound_selector_internal<'a,E,N>(selector: &CompoundSelector,
}
}

bitflags! {
flags CommonStyleAffectingAttributes: u8 {
static HiddenAttribute = 0x01,
static NoWrapAttribute = 0x02,
static AlignLeftAttribute = 0x04,
static AlignCenterAttribute = 0x08,
static AlignRightAttribute = 0x10,
}
}

pub struct CommonStyleAffectingAttributeInfo {
pub atom: Atom,
pub mode: CommonStyleAffectingAttributeMode,
}

pub enum CommonStyleAffectingAttributeMode {
AttrIsPresentMode(CommonStyleAffectingAttributes),
AttrIsEqualMode(&'static str, CommonStyleAffectingAttributes),
}

// NB: This must match the order in `layout::css::matching::CommonStyleAffectingAttributes`.
#[inline]
pub fn common_style_affecting_attributes() -> [CommonStyleAffectingAttributeInfo, ..5] {
[
CommonStyleAffectingAttributeInfo {
atom: atom!("hidden"),
mode: AttrIsPresentMode(HiddenAttribute),
},
CommonStyleAffectingAttributeInfo {
atom: atom!("nowrap"),
mode: AttrIsPresentMode(NoWrapAttribute),
},
CommonStyleAffectingAttributeInfo {
atom: atom!("align"),
mode: AttrIsEqualMode("left", AlignLeftAttribute),
},
CommonStyleAffectingAttributeInfo {
atom: atom!("align"),
mode: AttrIsEqualMode("center", AlignCenterAttribute),
},
CommonStyleAffectingAttributeInfo {
atom: atom!("align"),
mode: AttrIsEqualMode("right", AlignRightAttribute),
}
]
}

/// Determines whether the given element matches the given single selector.
///
/// NB: If you add support for any new kinds of selectors to this routine, be sure to set
Expand All @@ -781,7 +828,6 @@ pub fn matches_simple_selector<'a,E,N>(selector: &SimpleSelector,
}

NamespaceSelector(ref namespace) => {
*shareable = false;
let element = element.as_element();
element.get_namespace() == namespace
}
Expand All @@ -799,11 +845,26 @@ pub fn matches_simple_selector<'a,E,N>(selector: &SimpleSelector,
}

AttrExists(ref attr) => {
*shareable = false;
// NB(pcwalton): If you update this, remember to update the corresponding list in
// `can_share_style_with()` as well.
if common_style_affecting_attributes().iter().all(|common_attr_info| {
!(common_attr_info.atom == attr.name && match common_attr_info.mode {
AttrIsPresentMode(_) => true,
AttrIsEqualMode(..) => false,
})
}) {
*shareable = false;
}
element.match_attr(attr, |_| true)
}
AttrEqual(ref attr, ref value, case_sensitivity) => {
if value.as_slice() != "DIR" {
if value.as_slice() != "DIR" &&
common_style_affecting_attributes().iter().all(|common_attr_info| {
!(common_attr_info.atom == attr.name && match common_attr_info.mode {
AttrIsEqualMode(target_value, _) => target_value == value.as_slice(),
AttrIsPresentMode(_) => false,
})
}) {
// FIXME(pcwalton): Remove once we start actually supporting RTL text. This is in
// here because the UA style otherwise disables all style sharing completely.
*shareable = false
Expand Down Expand Up @@ -853,15 +914,15 @@ pub fn matches_simple_selector<'a,E,N>(selector: &SimpleSelector,
element.get_link().is_some()
}
Link => {
*shareable = false;
let elem = element.as_element();
match elem.get_link() {
Some(url) => !url_is_visited(url),
None => false,
}
}
Visited => {
*shareable = false;
// NB(pcwalton): When we actually start supporting visited links, remember to update
// `can_share_style_with`.
let elem = element.as_element();
match elem.get_link() {
Some(url) => url_is_visited(url),
Expand Down Expand Up @@ -947,6 +1008,7 @@ fn url_is_visited(_url: &str) -> bool {
// FIXME: implement this.
// This function will probably need to take a "session"
// or something containing browsing history as an additional parameter.
// NB(pcwalton): When you implement this, remember to update `can_share_style_with`!
false
}

Expand Down

5 comments on commit 4bf0acb

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from SimonSapin
at pcwalton@4bf0acb

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging pcwalton/servo/style-sharing-enhancements = 4bf0acb into auto

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pcwalton/servo/style-sharing-enhancements = 4bf0acb merged ok, testing candidate = ba13e44

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = ba13e44

Please sign in to comment.