Repository: servo/servo
File: components/script/dom/svg/svgsvgelement.rs (Rust)

Code before:
    fn attribute_affects_presentational_hints(&self, attr: &Attr) -> bool {
        match attr.local_name() {
            &local_name!("width") | &local_name!("height") => true,
            _ => self
                .super_type()
                .unwrap()
                .attribute_affects_presentational_hints(attr),
        }
    }

    fn parse_plain_attribute(&self, name: &LocalName, value: DOMString) -> AttrValue {
        match *name {
            // TODO: This should accept lengths in arbitrary units instead of assuming px.
            local_name!("width") | local_name!("height") => AttrValue::from_i32(value.into(), -1),
            _ => self
                .super_type()
                .unwrap()
                .parse_plain_attribute(name, value),
        }
    }

    fn children_changed(&self, mutation: &ChildrenMutation, can_gc: CanGc) {
        if let Some(super_type) = self.super_type() {
            super_type.children_changed(mutation, can_gc);
        }

        self.invalidate_cached_serialized_subtree();
    }

    fn unbind_from_tree(&self, context: &UnbindContext<'_>, can_gc: CanGc) {
        if let Some(s) = self.super_type() {
            s.unbind_from_tree(context, can_gc);
        }
        let owner_window = self.owner_window();
        let data_url = self.cached_serialized_data_url.borrow().clone();
        if let Some(Ok(url)) = data_url {
            owner_window.layout_mut().remove_cached_image(&url);
            owner_window.image_cache().evict_completed_image(
                &url,
                owner_window.origin().immutable(),
                &None,
            );
        }
        self.invalidate_cached_serialized_subtree();
        self.owner_window()
            .image_cache()
            .evict_rasterized_image(&self.uuid);
    }
}


Review comment (performance):
Adding the image to the various members of `ImageCacheStore` (`completed_loads`, `vector_images`, `rasterized_vector_images`) is an asynchronous operation, so I think it can complete only after this is called, meaning the stale data would still be added to the cache later and never deleted. 

I think you can test this by adding a sleep to https://github.com/servo/servo/blob/21e58f3f8cbdca7d4e6174ec45df29a7a06f4212/components/compositing/paint.rs#L858 in order to delay to response. 

So the solution would be to take the async workflow of the `KeyCache` into account, maybe with a tombstone like logic by adding a `Deleted` variant to `PendingKey`(you should probably not just remove the entry from `images_pending_keys` because I think that will mess with the state machine of KeyCacheState). 

Code after:
                    None,
                    None,
                );
                let val = Length::parse_quirky(
                    &context,
                    parser,
                    style::values::specified::AllowQuirks::Always,
                );
                AttrValue::Length(value.to_string(), val.ok())
            },
            _ => self
                .super_type()
                .unwrap()
                .parse_plain_attribute(name, value),
        }
    }

    fn children_changed(&self, mutation: &ChildrenMutation, can_gc: CanGc) {
        if let Some(super_type) = self.super_type() {
            super_type.children_changed(mutation, can_gc);
        }

        self.invalidate_cached_serialized_subtree();
    }

    fn unbind_from_tree(&self, context: &UnbindContext<'_>, can_gc: CanGc) {
        if let Some(s) = self.super_type() {
            s.unbind_from_tree(context, can_gc);
        }
        let owner_window = self.owner_window();
        self.owner_window()
            .image_cache()
            .evict_rasterized_image(&self.uuid);
        let data_url = self.cached_serialized_data_url.borrow().clone();
        if let Some(Ok(url)) = data_url {
            owner_window.layout_mut().remove_cached_image(&url);
            owner_window.image_cache().evict_completed_image(
                &url,
                owner_window.origin().immutable(),
                &None,
            );
        }
        self.invalidate_cached_serialized_subtree();
    }
}