auto merge of #1751 : Ms2ger/servo/image-loading, r=jdm

src/components/script/dom/htmlimageelement.rs

@@ -16,9 +16,7 @@ use extra::url::Url;
 use servo_util::geometry::to_px;
 use layout_interface::{ContentBoxQuery, ContentBoxResponse};
 use servo_net::image_cache_task;
-use servo_net::image_cache_task::ImageCacheTask;
 use servo_util::url::parse_url;
-use servo_util::namespace::Null;
 use servo_util::str::DOMString;
 
 use extra::serialize::{Encoder, Encodable};
@@ -66,11 +64,15 @@ impl HTMLImageElement {
 impl HTMLImageElement {
     /// Makes the local `image` member match the status of the `src` attribute and starts
     /// prefetching the image. This method must be called after `src` is changed.
-    pub fn update_image(&mut self, image_cache: ImageCacheTask, url: Option<Url>) {
+    fn update_image(&mut self, value: Option<DOMString>, url: Option<Url>) {
         let elem = &mut self.htmlelement.element;
-        let src_opt = elem.get_attribute(Null, "src").map(|x| x.get().Value());
-        match src_opt {
-            None => {}
+        let document = elem.node.owner_doc();
+        let window = document.get().window.get();
+        let image_cache = &window.image_cache_task;
+        match value {
+            None => {
+                self.extra.image = None;
+            }
             Some(src) => {
                 let img_url = parse_url(src, url);
                 self.extra.image = Some(img_url.clone());
@@ -85,23 +87,18 @@ impl HTMLImageElement {
         }
     }
 
-    pub fn AfterSetAttr(&mut self, name: DOMString, _value: DOMString) {
+    pub fn AfterSetAttr(&mut self, name: DOMString, value: DOMString) {
         if "src" == name {
             let document = self.htmlelement.element.node.owner_doc();
             let window = document.get().window.get();
             let url = window.page.url.as_ref().map(|&(ref url, _)| url.clone());
-            self.update_image(window.image_cache_task.clone(), url);
+            self.update_image(Some(value), url);
         }
     }
 
     pub fn AfterRemoveAttr(&mut self, name: DOMString) {
-        // FIXME (#1469):
-        // This might not handle remove src attribute actually since
-        // `self.update_image()` will see the missing src attribute and return early.
         if "src" == name {
-            let document = self.htmlelement.element.node.owner_doc();
-            let window = document.get().window.get();
-            self.update_image(window.image_cache_task.clone(), None);
+            self.update_image(None, None);
         }
     }
 

src/components/script/html/hubbub_html_parser.rs

@@ -3,11 +3,11 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use dom::bindings::codegen::InheritTypes::{NodeBase, NodeCast, TextCast, ElementCast};
-use dom::bindings::codegen::InheritTypes::{HTMLIFrameElementCast, HTMLImageElementCast};
+use dom::bindings::codegen::InheritTypes::HTMLIFrameElementCast;
 use dom::bindings::js::JS;
 use dom::bindings::utils::Reflectable;
 use dom::document::Document;
-use dom::element::{HTMLLinkElementTypeId, HTMLIframeElementTypeId, HTMLImageElementTypeId};
+use dom::element::{HTMLLinkElementTypeId, HTMLIframeElementTypeId};
 use dom::htmlelement::HTMLElement;
 use dom::htmlheadingelement::{Heading1, Heading2, Heading3, Heading4, Heading5, Heading6};
 use dom::htmliframeelement::IFrameSize;
@@ -21,7 +21,6 @@ use extra::url::Url;
 use hubbub::hubbub;
 use js::jsapi::JSContext;
 use servo_msg::constellation_msg::SubpageId;
-use servo_net::image_cache_task::ImageCacheTask;
 use servo_net::resource_task::{Load, Payload, Done, ResourceTask, load_whole_resource};
 use servo_util::namespace::Null;
 use servo_util::str::DOMString;
@@ -251,7 +250,6 @@ pub fn parse_html(cx: *JSContext,
                   document: &mut JS<Document>,
                   url: Url,
                   resource_task: ResourceTask,
-                  image_cache_task: ImageCacheTask,
                   next_subpage_id: SubpageId)
                   -> HtmlParserResult {
     debug!("Hubbub: parsing {:?}", url);
@@ -382,13 +380,6 @@ pub fn parse_html(cx: *JSContext,
                                                                sandboxed)));
                     }
                 }
-
-                //FIXME: This should be taken care of by set_attr, but we don't have
-                //       access to a window so HTMLImageElement::AfterSetAttr bails.
-                ElementNodeTypeId(HTMLImageElementTypeId) => {
-                    let mut image_element: JS<HTMLImageElement> = HTMLImageElementCast::to(&element);
-                    image_element.get_mut().update_image(image_cache_task.clone(), Some(url2.clone()));
-                }
                 _ => {}
             }
 

src/components/script/script_task.rs

@@ -722,7 +722,6 @@ impl ScriptTask {
                                                                  &mut document,
                                                                  url.clone(),
                                                                  self.resource_task.clone(),
-                                                                 self.image_cache_task.clone(),
                                                                  page.next_subpage_id.clone());
 
         let HtmlParserResult {

src/test/ref/basic.list

@@ -23,6 +23,7 @@
 == font_size_percentage.html font_size_em_ref.html
 == position_fixed_a.html position_fixed_b.html
 == img_size_a.html img_size_b.html
+== img_dynamic_remove.html img_dynamic_remove_ref.html
 == upper_id_attr.html upper_id_attr_ref.html
 # inline_border_a.html inline_border_b.html
 == anon_block_inherit_a.html anon_block_inherit_b.html

src/test/ref/img_dynamic_remove.html

@@ -0,0 +1,5 @@
+<!doctype html>
+<img src=400x400_green.png>
+<script>
+document.getElementsByTagName("img")[0].removeAttribute("src");
+</script>

src/test/ref/img_dynamic_remove_ref.html

@@ -0,0 +1,2 @@
+<!doctype html>
+<img>