Auto merge of #5857 - Ms2ger:preshints, r=pcwalton

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/5857)
<!-- Reviewable:end -->

components/layout/css/matching.rs

@@ -27,13 +27,14 @@ use std::slice::Iter;
 use std::sync::Arc;
 use std::sync::mpsc::Sender;
 use string_cache::{Atom, Namespace};
-use style::node::{TElement, TNode};
+use style::node::{TElement, TElementAttributes, TNode};
 use style::properties::{ComputedValues, cascade};
 use style::selector_matching::{Stylist, DeclarationBlock};
 use util::arc_ptr_eq;
 use util::cache::{LRUCache, SimpleHashCache};
 use util::opts;
 use util::smallvec::{SmallVec, SmallVec16};
+use util::vec::ForgetfulSink;
 
 pub struct ApplicableDeclarations {
     pub normal: SmallVec16<DeclarationBlock>,
@@ -289,6 +290,12 @@ impl StyleSharingCandidate {
             return false
         }
 
+        let mut matching_rules = ForgetfulSink::new();
+        element.synthesize_presentational_hints_for_legacy_attributes(&mut matching_rules);
+        if !matching_rules.is_empty() {
+            return false;
+        }
+
         // FIXME(pcwalton): It's probably faster to iterate over all the element's attributes and
         // use the {common, rare}-style-affecting-attributes tables as lookup tables.
 

components/layout/wrapper.rs

@@ -39,7 +39,6 @@ use incremental::RestyleDamage;
 use data::{LayoutDataAccess, LayoutDataFlags, LayoutDataWrapper, PrivateLayoutData};
 use opaque_node::OpaqueNodeMethods;
 
-use cssparser::RGBA;
 use gfx::display_list::OpaqueNode;
 use script::dom::bindings::codegen::InheritTypes::{CharacterDataCast, ElementCast};
 use script::dom::bindings::codegen::InheritTypes::{HTMLIFrameElementCast, HTMLCanvasElementCast};
@@ -70,11 +69,13 @@ use std::sync::mpsc::Sender;
 use string_cache::{Atom, Namespace};
 use style::computed_values::content::ContentItem;
 use style::computed_values::{content, display, white_space};
+use selectors::matching::DeclarationBlock;
 use selectors::parser::{NamespaceConstraint, AttrSelector};
-use style::legacy::{IntegerAttribute, LengthAttribute, SimpleColorAttribute};
+use selectors::smallvec::VecLike;
+use style::legacy::{IntegerAttribute, LengthAttribute};
 use style::legacy::{UnsignedIntegerAttribute};
 use style::node::{TElement, TElementAttributes, TNode};
-use style::properties::PropertyDeclarationBlock;
+use style::properties::{PropertyDeclaration, PropertyDeclarationBlock};
 use url::Url;
 
 /// Allows some convenience methods on generic layout nodes.
@@ -659,6 +660,14 @@ impl<'le> TElement<'le> for LayoutElement<'le> {
 }
 
 impl<'le> TElementAttributes for LayoutElement<'le> {
+    fn synthesize_presentational_hints_for_legacy_attributes<V>(self, hints: &mut V)
+        where V: VecLike<DeclarationBlock<Vec<PropertyDeclaration>>>
+    {
+        unsafe {
+            self.element.synthesize_presentational_hints_for_legacy_attributes(hints);
+        }
+    }
+
     fn get_length_attribute(self, length_attribute: LengthAttribute) -> LengthOrPercentageOrAuto {
         unsafe {
             self.element.get_length_attribute_for_layout(length_attribute)
@@ -676,12 +685,6 @@ impl<'le> TElementAttributes for LayoutElement<'le> {
             self.element.get_unsigned_integer_attribute_for_layout(attribute)
         }
     }
-
-    fn get_simple_color_attribute(self, attribute: SimpleColorAttribute) -> Option<RGBA> {
-        unsafe {
-            self.element.get_simple_color_attribute_for_layout(attribute)
-        }
-    }
 }
 
 fn get_content(content_list: &content::T) -> Vec<ContentItem> {

components/script/dom/element.rs

@@ -23,6 +23,7 @@ use dom::bindings::codegen::InheritTypes::{HTMLTableElementDerived, HTMLTableCel
 use dom::bindings::codegen::InheritTypes::{HTMLTableRowElementDerived, HTMLTextAreaElementDerived};
 use dom::bindings::codegen::InheritTypes::{HTMLTableSectionElementDerived, NodeCast};
 use dom::bindings::codegen::InheritTypes::HTMLAnchorElementCast;
+use dom::bindings::codegen::InheritTypes::HTMLTableDataCellElementDerived;
 use dom::bindings::codegen::UnionTypes::NodeOrString;
 use dom::bindings::error::{ErrorResult, Fallible};
 use dom::bindings::error::Error::{InvalidCharacter, Syntax};
@@ -55,22 +56,28 @@ use dom::node::{document_from_node, NodeDamage};
 use dom::node::{window_from_node};
 use dom::nodelist::NodeList;
 use dom::virtualmethods::{VirtualMethods, vtable_for};
+
 use devtools_traits::AttrInfo;
-use style::legacy::{SimpleColorAttribute, UnsignedIntegerAttribute, IntegerAttribute, LengthAttribute};
-use selectors::matching::matches;
-use style::properties::{PropertyDeclarationBlock, PropertyDeclaration, parse_style_attribute};
-use selectors::parser::parse_author_origin_selector_list_from_str;
 use style;
+use style::legacy::{UnsignedIntegerAttribute, IntegerAttribute, LengthAttribute, from_declaration};
+use style::properties::{PropertyDeclarationBlock, PropertyDeclaration, parse_style_attribute};
+use style::properties::DeclaredValue::SpecifiedValue;
+use style::values::specified::CSSColor;
 use util::namespace;
 use util::str::{DOMString, LengthOrPercentageOrAuto};
 
+use cssparser::Color;
 use html5ever::serialize;
 use html5ever::serialize::SerializeOpts;
 use html5ever::serialize::TraversalScope;
 use html5ever::serialize::TraversalScope::{IncludeNode, ChildrenOnly};
 use html5ever::tree_builder::{NoQuirks, LimitedQuirks, Quirks};
+use selectors::matching::{matches, DeclarationBlock};
+use selectors::parser::parse_author_origin_selector_list_from_str;
+use selectors::smallvec::VecLike;
+use string_cache::{Atom, Namespace, QualName};
+use url::UrlParser;
 
-use cssparser::RGBA;
 use std::ascii::AsciiExt;
 use std::borrow::{IntoCow, ToOwned};
 use std::cell::{Ref, RefMut};
@@ -78,8 +85,6 @@ use std::default::Default;
 use std::mem;
 use std::old_io::{MemWriter, Writer};
 use std::sync::Arc;
-use string_cache::{Atom, Namespace, QualName};
-use url::UrlParser;
 
 #[dom_struct]
 pub struct Element {
@@ -155,6 +160,9 @@ pub trait RawLayoutElementHelpers {
     unsafe fn get_attr_atom_for_layout(&self, namespace: &Namespace, name: &Atom) -> Option<Atom>;
     unsafe fn has_class_for_layout(&self, name: &Atom) -> bool;
     unsafe fn get_classes_for_layout(&self) -> Option<&'static [Atom]>;
+
+    unsafe fn synthesize_presentational_hints_for_legacy_attributes<V>(&self, &mut V)
+        where V: VecLike<DeclarationBlock<Vec<PropertyDeclaration>>>;
     unsafe fn get_length_attribute_for_layout(&self, length_attribute: LengthAttribute)
                                               -> LengthOrPercentageOrAuto;
     unsafe fn get_integer_attribute_for_layout(&self, integer_attribute: IntegerAttribute)
@@ -163,8 +171,7 @@ pub trait RawLayoutElementHelpers {
     unsafe fn get_indeterminate_state_for_layout(&self) -> bool;
     unsafe fn get_unsigned_integer_attribute_for_layout(&self, attribute: UnsignedIntegerAttribute)
                                                         -> Option<u32>;
-    unsafe fn get_simple_color_attribute_for_layout(&self, attribute: SimpleColorAttribute)
+
-                                                    -> Option<RGBA>;
     fn local_name<'a>(&'a self) -> &'a Atom;
     fn namespace<'a>(&'a self) -> &'a Namespace;
     fn style_attribute<'a>(&'a self) -> &'a DOMRefCell<Option<PropertyDeclarationBlock>>;
@@ -227,6 +234,35 @@ impl RawLayoutElementHelpers for Element {
         })
     }
 
+    unsafe fn synthesize_presentational_hints_for_legacy_attributes<V>(&self, hints: &mut V)
+        where V: VecLike<DeclarationBlock<Vec<PropertyDeclaration>>>
+    {
+        let bgcolor = if self.is_htmlbodyelement() {
+            let this: &HTMLBodyElement = mem::transmute(self);
+            this.get_background_color()
+        } else if self.is_htmltableelement() {
+            let this: &HTMLTableElement = mem::transmute(self);
+            this.get_background_color()
+        } else if self.is_htmltabledatacellelement() {
+            let this: &HTMLTableCellElement = mem::transmute(self);
+            this.get_background_color()
+        } else if self.is_htmltablerowelement() {
+            let this: &HTMLTableRowElement = mem::transmute(self);
+            this.get_background_color()
+        } else if self.is_htmltablesectionelement() {
+            let this: &HTMLTableSectionElement = mem::transmute(self);
+            this.get_background_color()
+        } else {
+            None
+        };
+
+        if let Some(color) = bgcolor {
+            hints.vec_push(from_declaration(
+                PropertyDeclaration::BackgroundColor(SpecifiedValue(
+                    CSSColor { parsed: Color::RGBA(color), authored: None }))));
+        }
+    }
+
     #[inline]
     unsafe fn get_length_attribute_for_layout(&self, length_attribute: LengthAttribute)
                                               -> LengthOrPercentageOrAuto {
@@ -333,34 +369,6 @@ impl RawLayoutElementHelpers for Element {
         }
     }
 
-    #[inline]
-    #[allow(unrooted_must_root)]
-    unsafe fn get_simple_color_attribute_for_layout(&self, attribute: SimpleColorAttribute)
-                                                    -> Option<RGBA> {
-        match attribute {
-            SimpleColorAttribute::BgColor => {
-                if self.is_htmlbodyelement() {
-                    let this: &HTMLBodyElement = mem::transmute(self);
-                    this.get_background_color()
-                } else if self.is_htmltableelement() {
-                    let this: &HTMLTableElement = mem::transmute(self);
-                    this.get_background_color()
-                } else if self.is_htmltablecellelement() {
-                    let this: &HTMLTableCellElement = mem::transmute(self);
-                    this.get_background_color()
-                } else if self.is_htmltablerowelement() {
-                    let this: &HTMLTableRowElement = mem::transmute(self);
-                    this.get_background_color()
-                } else if self.is_htmltablesectionelement() {
-                    let this: &HTMLTableSectionElement = mem::transmute(self);
-                    this.get_background_color()
-                } else {
-                    None
-                }
-            }
-        }
-    }
-
     // Getters used in components/layout/wrapper.rs
 
     fn local_name<'a>(&'a self) -> &'a Atom {

components/style/legacy.rs

@@ -10,14 +10,12 @@ use std::sync::Arc;
 use selectors::tree::{TElement, TNode};
 use selectors::matching::DeclarationBlock;
 use node::TElementAttributes;
-use values::specified::CSSColor;
 use values::{CSSFloat, specified};
 use properties::DeclaredValue::SpecifiedValue;
 use properties::PropertyDeclaration;
 use properties::longhands::{self, border_spacing};
 use selector_matching::Stylist;
 
-use cssparser::Color;
 use selectors::smallvec::VecLike;
 use util::geometry::Au;
 use util::str::LengthOrPercentageOrAuto;
@@ -49,13 +47,6 @@ pub enum UnsignedIntegerAttribute {
     ColSpan,
 }
 
-/// Legacy presentational attributes that take a simple color as defined in HTML5 § 2.4.6.
-#[derive(Copy, PartialEq, Eq)]
-pub enum SimpleColorAttribute {
-    /// `<body bgcolor>`
-    BgColor,
-}
-
 /// Extension methods for `Stylist` that cause rules to be synthesized for legacy attributes.
 pub trait PresentationalHintSynthesis {
     /// Synthesizes rules from various HTML attributes (mostly legacy junk from HTML4) that confer
@@ -74,18 +65,6 @@ pub trait PresentationalHintSynthesis {
                                                              where N: TNode<'a>,
                                                                    N::Element: TElementAttributes,
                                                                    V: VecLike<DeclarationBlock<Vec<PropertyDeclaration>>>;
-    /// Synthesizes rules for the legacy `bgcolor` attribute.
-    fn synthesize_presentational_hint_for_legacy_background_color_attribute<'a,E,V>(
-                                                                            &self,
-                                                                            element: E,
-                                                                            matching_rules_list:
-                                                                                &mut V,
-                                                                            shareable: &mut bool)
-                                                                            where
-                                                                            E: TElement<'a> +
-                                                                               TElementAttributes,
-                                                                            V: VecLike<
-                                                                                DeclarationBlock<Vec<PropertyDeclaration>>>;
     /// Synthesizes rules for the legacy `border` attribute.
     fn synthesize_presentational_hint_for_legacy_border_attribute<'a,E,V>(
                                                                   &self,
@@ -108,6 +87,14 @@ impl PresentationalHintSynthesis for Stylist {
                                                                    N::Element: TElementAttributes,
                                                                    V: VecLike<DeclarationBlock<Vec<PropertyDeclaration>>> {
         let element = node.as_element();
+
+        let length = matching_rules_list.vec_len();
+        element.synthesize_presentational_hints_for_legacy_attributes(matching_rules_list);
+        if matching_rules_list.vec_len() != length {
+            // Never share style for elements with preshints
+            *shareable = false;
+        }
+
         match element.get_local_name() {
             name if *name == atom!("td") => {
                 match element.get_length_attribute(LengthAttribute::Width) {
@@ -125,20 +112,12 @@ impl PresentationalHintSynthesis for Stylist {
                         *shareable = false
                     }
                 }
-                self.synthesize_presentational_hint_for_legacy_background_color_attribute(
-                    element,
-                    matching_rules_list,
-                    shareable);
                 self.synthesize_presentational_hint_for_legacy_border_attribute(
                     element,
                     matching_rules_list,
                     shareable);
             }
             name if *name == atom!("table") => {
-                self.synthesize_presentational_hint_for_legacy_background_color_attribute(
-                    element,
-                    matching_rules_list,
-                    shareable);
                 self.synthesize_presentational_hint_for_legacy_border_attribute(
                     element,
                     matching_rules_list,
@@ -159,13 +138,6 @@ impl PresentationalHintSynthesis for Stylist {
                     }
                 }
             }
-            name if *name == atom!("body") || *name == atom!("tr") || *name == atom!("thead") ||
-                    *name == atom!("tbody") || *name == atom!("tfoot") => {
-                self.synthesize_presentational_hint_for_legacy_background_color_attribute(
-                    element,
-                    matching_rules_list,
-                    shareable);
-            }
             name if *name == atom!("input") => {
                 // FIXME(pcwalton): More use of atoms, please!
                 match element.get_attr(&ns!(""), &atom!("type")) {
@@ -220,28 +192,6 @@ impl PresentationalHintSynthesis for Stylist {
         }
     }
 
-    fn synthesize_presentational_hint_for_legacy_background_color_attribute<'a,E,V>(
-                                                                            &self,
-                                                                            element: E,
-                                                                            matching_rules_list:
-                                                                                &mut V,
-                                                                            shareable: &mut bool)
-                                                                            where
-                                                                            E: TElement<'a> +
-                                                                               TElementAttributes,
-                                                                            V: VecLike<
-                                                                                DeclarationBlock<Vec<PropertyDeclaration>>> {
-        match element.get_simple_color_attribute(SimpleColorAttribute::BgColor) {
-            None => {}
-            Some(color) => {
-                matching_rules_list.vec_push(from_declaration(
-                        PropertyDeclaration::BackgroundColor(SpecifiedValue(
-                            CSSColor { parsed: Color::RGBA(color), authored: None }))));
-                *shareable = false
-            }
-        }
-    }
-
     fn synthesize_presentational_hint_for_legacy_border_attribute<'a,E,V>(
                                                                   &self,
                                                                   element: E,

components/style/node.rs

@@ -5,15 +5,18 @@
 //! Traits that nodes must implement. Breaks the otherwise-cyclic dependency between layout and
 //! style.
 
-use cssparser::RGBA;
+use legacy::{IntegerAttribute, LengthAttribute, UnsignedIntegerAttribute};
-use legacy::{IntegerAttribute, LengthAttribute, SimpleColorAttribute, UnsignedIntegerAttribute};
+use properties::PropertyDeclaration;
 use util::str::LengthOrPercentageOrAuto;
 
+use selectors::matching::DeclarationBlock;
+use selectors::smallvec::VecLike;
 pub use selectors::tree::{TNode, TElement};
 
 pub trait TElementAttributes : Copy {
+    fn synthesize_presentational_hints_for_legacy_attributes<V>(self, &mut V)
+        where V: VecLike<DeclarationBlock<Vec<PropertyDeclaration>>>;
     fn get_length_attribute(self, attribute: LengthAttribute) -> LengthOrPercentageOrAuto;
     fn get_integer_attribute(self, attribute: IntegerAttribute) -> Option<i32>;
     fn get_unsigned_integer_attribute(self, attribute: UnsignedIntegerAttribute) -> Option<u32>;
-    fn get_simple_color_attribute(self, attribute: SimpleColorAttribute) -> Option<RGBA>;
 }

components/util/vec.rs

@@ -2,6 +2,8 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+use selectors::smallvec::VecLike;
+
 use std::cmp::{PartialOrd, PartialEq, Ordering};
 use std::iter::range_step;
 
@@ -72,3 +74,37 @@ pub fn byte_swap(data: &mut [u8]) {
         data[i + 0] = r;
     }
 }
+
+/// A `VecLike` that only tracks whether or not something was ever pushed to it.
+pub struct ForgetfulSink {
+    empty: bool,
+}
+
+impl ForgetfulSink {
+    pub fn new() -> ForgetfulSink {
+        ForgetfulSink {
+            empty: true,
+        }
+    }
+
+    pub fn is_empty(&self) -> bool {
+        self.empty
+    }
+}
+
+impl<T> VecLike<T> for ForgetfulSink {
+    #[inline]
+    fn vec_len(&self) -> usize {
+        unreachable!()
+    }
+
+    #[inline]
+    fn vec_push(&mut self, _value: T) {
+        self.empty = false;
+    }
+
+    #[inline]
+    fn vec_slice_mut<'a>(&'a mut self, _start: usize, _end: usize) -> &'a mut [T] {
+        unreachable!()
+    }
+}