Remove parsed attribute 'background' field on HTMLBodyElement

https://github.com/servo/servo/issues/7863

3 files changed, 40 insertions, 18 deletions

diff --git a/components/script/dom/htmlbodyelement.rs b/components/script/dom/htmlbodyelement.rs
index cdc8f4cfac0..f082a4c8ced 100644
--- a/components/script/dom/htmlbodyelement.rs
+++ b/components/script/dom/htmlbodyelement.rs
@@ -4,7 +4,6 @@
 
 use cssparser::RGBA;
 use dom::attr::{Attr, AttrValue};
-use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::EventHandlerBinding::EventHandlerNonNull;
 use dom::bindings::codegen::Bindings::HTMLBodyElementBinding::{self, HTMLBodyElementMethods};
 use dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
@@ -32,7 +31,6 @@ const INITIAL_REFLOW_DELAY: u64 = 200_000_000;
 #[dom_struct]
 pub struct HTMLBodyElement {
     htmlelement: HTMLElement,
-    background: DOMRefCell<Option<Url>>
 }
 
 impl HTMLBodyElement {
@@ -40,7 +38,6 @@ impl HTMLBodyElement {
                      -> HTMLBodyElement {
         HTMLBodyElement {
             htmlelement: HTMLElement::new_inherited(localName, prefix, document),
-            background: DOMRefCell::new(None)
         }
     }
 
@@ -89,12 +86,7 @@ impl HTMLBodyElementMethods for HTMLBodyElement {
     make_getter!(Background, "background");
 
     // https://html.spec.whatwg.org/multipage/#dom-body-background
-    fn SetBackground(&self, value: DOMString) {
-        let document = document_from_node(self);
-        let base = document.url();
-        *self.background.borrow_mut() = base.join(&value).ok();
-        self.upcast::<Element>().set_string_attribute(&atom!("background"), value);
-    }
+    make_url_setter!(SetBackground, "background");
 }
 
 pub trait HTMLBodyElementLayoutHelpers {
@@ -127,7 +119,10 @@ impl HTMLBodyElementLayoutHelpers for LayoutJS<HTMLBodyElement> {
     #[allow(unsafe_code)]
     fn get_background(&self) -> Option<Url> {
         unsafe {
-            (*self.unsafe_get()).background.borrow_for_layout().clone()
+            (*self.upcast::<Element>().unsafe_get())
+                .get_attr_for_layout(&ns!(), &atom!("background"))
+                .and_then(AttrValue::as_url)
+                .cloned()
         }
     }
 }
@@ -158,20 +153,13 @@ impl VirtualMethods for HTMLBodyElement {
         match *name {
             atom!("bgcolor") |
             atom!("text") => AttrValue::from_legacy_color(value),
+            atom!("background") => AttrValue::from_url(document_from_node(self).url(), value),
             _ => self.super_type().unwrap().parse_plain_attribute(name, value),
         }
     }
 
     fn attribute_mutated(&self, attr: &Attr, mutation: AttributeMutation) {
         let do_super_mutate = match (attr.local_name(), mutation) {
-            (&atom!("background"), _) => {
-                *self.background.borrow_mut() = mutation.new_value(attr).and_then(|value| {
-                    let document = document_from_node(self);
-                    let base = document.url();
-                    base.join(&value).ok()
-                });
-                true
-            },
             (name, AttributeMutation::Set(_)) if name.starts_with("on") => {
                 let window = window_from_node(self);
                 let (cx, url, reflector) = (window.get_cx(),
diff --git a/components/script/dom/macros.rs b/components/script/dom/macros.rs
index eb60a2c5798..60ed65860f8 100644
--- a/components/script/dom/macros.rs
+++ b/components/script/dom/macros.rs
@@ -153,6 +153,20 @@ macro_rules! make_bool_setter(
 );
 
 #[macro_export]
+macro_rules! make_url_setter(
+    ( $attr:ident, $htmlname:tt ) => (
+        fn $attr(&self, value: DOMString) {
+            use dom::bindings::inheritance::Castable;
+            use dom::element::Element;
+            use dom::node::document_from_node;
+            let value = AttrValue::from_url(document_from_node(self).url(), value);
+            let element = self.upcast::<Element>();
+            element.set_attribute(&atom!($htmlname), value);
+        }
+    );
+);
+
+#[macro_export]
 macro_rules! make_uint_setter(
     ($attr:ident, $htmlname:tt, $default:expr) => (
         fn $attr(&self, value: u32) {
diff --git a/components/style/attr.rs b/components/style/attr.rs
index aa7eb621839..dadc07eac29 100644
--- a/components/style/attr.rs
+++ b/components/style/attr.rs
@@ -5,6 +5,7 @@
 use cssparser::RGBA;
 use std::ops::Deref;
 use string_cache::{Atom, Namespace};
+use url::Url;
 use util::str::{DOMString, LengthOrPercentageOrAuto, parse_unsigned_integer, parse_legacy_color, parse_length};
 use util::str::{parse_nonzero_length, split_html_space_chars, str_join, parse_integer};
 use values::specified::{Length};
@@ -22,6 +23,7 @@ pub enum AttrValue {
     Length(DOMString, Option<Length>),
     Color(DOMString, Option<RGBA>),
     Dimension(DOMString, LengthOrPercentageOrAuto),
+    Url(DOMString, Option<Url>),
 }
 
 impl AttrValue {
@@ -86,6 +88,11 @@ impl AttrValue {
         AttrValue::Atom(value)
     }
 
+    pub fn from_url(base: &Url, url: DOMString) -> AttrValue {
+        let joined = base.join(&url).ok();
+        AttrValue::Url(url, joined)
+    }
+
     pub fn from_legacy_color(string: DOMString) -> AttrValue {
         let parsed = parse_legacy_color(&string).ok();
         AttrValue::Color(string, parsed)
@@ -161,6 +168,18 @@ impl AttrValue {
         }
     }
 
+    /// Assumes the `AttrValue` is a `Url` and returns its value
+    ///
+    /// ## Panics
+    ///
+    /// Panics if the `AttrValue` is not a `Url`
+    pub fn as_url(&self) -> Option<&Url> {
+        match *self {
+            AttrValue::Url(_, ref url) => url.as_ref(),
+            _ => panic!("Url not found"),
+        }
+    }
+
     /// Return the AttrValue as its integer representation, if any.
     /// This corresponds to attribute values returned as `AttrValue::UInt(_)`
     /// by `VirtualMethods::parse_plain_attribute()`.
@@ -188,6 +207,7 @@ impl Deref for AttrValue {
                 AttrValue::Length(ref value, _) |
                 AttrValue::Color(ref value, _) |
                 AttrValue::Int(ref value, _) |
+                AttrValue::Url(ref value, _) |
                 AttrValue::Dimension(ref value, _) => &value,
             AttrValue::Atom(ref value) => &value,
         }