Merge pull request #2707 from glennw/image-cache-handle

Remove LocalImageCacheHandle hack and associated unsafe code.

src/components/main/layout/context.rs

@@ -14,9 +14,10 @@ use gfx::font_context::{FontContext, FontContextInfo};
 use green::task::GreenTask;
 use script::layout_interface::LayoutChan;
 use servo_msg::constellation_msg::ConstellationChan;
-use servo_net::image::holder::LocalImageCacheHandle;
+use servo_net::local_image_cache::LocalImageCache;
 use servo_util::geometry::Au;
 use servo_util::opts::Opts;
+use sync::{Arc, Mutex};
 use std::mem;
 #[cfg(not(target_os="android"))]
 use std::ptr;
@@ -54,7 +55,7 @@ local_data_key!(style_sharing_candidate_cache: *mut StyleSharingCandidateCache)
 #[deriving(Clone)]
 pub struct LayoutContext {
     /// The local image cache.
-    pub image_cache: LocalImageCacheHandle,
+    pub image_cache: Arc<Mutex<LocalImageCache>>,
 
     /// The current screen size.
     pub screen_size: Size2D<Au>,

src/components/main/layout/fragment.rs

@@ -33,7 +33,8 @@ use gfx::font::FontStyle;
 use gfx::text::glyph::CharIndex;
 use gfx::text::text_run::TextRun;
 use servo_msg::constellation_msg::{ConstellationChan, FrameRectMsg, PipelineId, SubpageId};
-use servo_net::image::holder::{ImageHolder, LocalImageCacheHandle};
+use servo_net::image::holder::ImageHolder;
+use servo_net::local_image_cache::LocalImageCache;
 use servo_util::geometry::Au;
 use servo_util::geometry;
 use servo_util::range::*;
@@ -49,7 +50,7 @@ use style::{ComputedValues, TElement, TNode, cascade_anonymous};
 use style::computed_values::{LengthOrPercentageOrAuto, overflow, LPA_Auto, background_attachment};
 use style::computed_values::{background_repeat, border_style, clear, position, text_align};
 use style::computed_values::{text_decoration, vertical_align, visibility, white_space};
-use sync::Arc;
+use sync::{Arc, Mutex};
 use url::Url;
 
 /// Fragments (`struct Fragment`) are the leaves of the layout tree. They cannot position themselves. In
@@ -133,7 +134,7 @@ impl ImageFragmentInfo {
     /// me.
     pub fn new(node: &ThreadSafeLayoutNode,
                image_url: Url,
-               local_image_cache: LocalImageCacheHandle)
+               local_image_cache: Arc<Mutex<LocalImageCache>>)
                -> ImageFragmentInfo {
         fn convert_length(node: &ThreadSafeLayoutNode, name: &str) -> Option<Au> {
             let element = node.as_element();

src/components/main/layout/layout_task.rs

@@ -43,7 +43,6 @@ use script::layout_interface::{ReflowForDisplay, ReflowMsg};
 use script::script_task::{ReflowCompleteMsg, ScriptChan, SendEventMsg};
 use servo_msg::compositor_msg::Scrollable;
 use servo_msg::constellation_msg::{ConstellationChan, PipelineId, Failure, FailureMsg};
-use servo_net::image::holder::LocalImageCacheHandle;
 use servo_net::image_cache_task::{ImageCacheTask, ImageResponseMsg};
 use servo_net::local_image_cache::{ImageResponder, LocalImageCache};
 use servo_util::geometry::Au;
@@ -86,7 +85,7 @@ pub struct LayoutTask {
     pub image_cache_task: ImageCacheTask,
 
     /// The local image cache.
-    pub local_image_cache: LocalImageCacheHandle,
+    pub local_image_cache: Arc<Mutex<LocalImageCache>>,
 
     /// The size of the viewport.
     pub screen_size: Size2D<Au>,
@@ -313,12 +312,7 @@ impl LayoutTask {
            opts: &Opts,
            profiler_chan: ProfilerChan)
            -> LayoutTask {
-        let local_image_cache = box LocalImageCache(image_cache_task.clone());
+        let local_image_cache = Arc::new(Mutex::new(LocalImageCache(image_cache_task.clone())));
-        let local_image_cache = unsafe {
-            let cache = Arc::new(Mutex::new(
-                mem::transmute::<Box<LocalImageCache>, *()>(local_image_cache)));
-            LocalImageCacheHandle::new(mem::transmute::<Arc<Mutex<*()>>,Arc<*()>>(cache))
-        };
         let screen_size = Size2D(Au(0), Au(0));
         let parallel_traversal = if opts.layout_threads != 1 {
             Some(WorkQueue::new("LayoutWorker", opts.layout_threads, ptr::mut_null()))
@@ -570,7 +564,7 @@ impl LayoutTask {
 
         {
             // Reset the image cache.
-            let mut local_image_cache = self.local_image_cache.get().lock();
+            let mut local_image_cache = self.local_image_cache.lock();
             local_image_cache.next_round(self.make_on_image_available_cb());
         }
 

src/components/net/image/holder.rs

@@ -8,7 +8,6 @@ use local_image_cache::LocalImageCache;
 
 use geom::size::Size2D;
 use std::mem;
-use std::ptr;
 use sync::{Arc, Mutex};
 use url::Url;
 
@@ -16,44 +15,6 @@ use url::Url;
 // the network stack. This should probably be factored out into an interface and use dependency
 // injection.
 
-/// An unfortunate hack to make this `Arc<Mutex>` `Share`.
-pub struct LocalImageCacheHandle {
-    data: *uint,
-}
-
-impl Drop for LocalImageCacheHandle {
-    fn drop(&mut self) {
-        unsafe {
-            let _: Box<Arc<Mutex<Box<LocalImageCache>>>> =
-                mem::transmute(mem::replace(&mut self.data, ptr::null()));
-        }
-    }
-}
-
-impl Clone for LocalImageCacheHandle {
-    fn clone(&self) -> LocalImageCacheHandle {
-        unsafe {
-            let handle = mem::transmute::<&Arc<Mutex<Box<LocalImageCache>>>,&Arc<*()>>(self.get());
-            let new_handle = (*handle).clone();
-            LocalImageCacheHandle::new(new_handle)
-        }
-    }
-}
-
-impl LocalImageCacheHandle {
-    pub unsafe fn new(cache: Arc<*()>) -> LocalImageCacheHandle {
-        LocalImageCacheHandle {
-            data: mem::transmute(box cache),
-        }
-    }
-
-    pub fn get<'a>(&'a self) -> &'a Arc<Mutex<Box<LocalImageCache>>> {
-        unsafe {
-            mem::transmute::<*uint,&'a Arc<Mutex<Box<LocalImageCache>>>>(self.data)
-        }
-    }
-}
-
 /// A struct to store image data. The image will be loaded once the first time it is requested,
 /// and an Arc will be stored.  Clones of this Arc are given out on demand.
 #[deriving(Clone)]
@@ -61,11 +22,11 @@ pub struct ImageHolder {
     url: Url,
     image: Option<Arc<Box<Image>>>,
     cached_size: Size2D<int>,
-    local_image_cache: LocalImageCacheHandle,
+    local_image_cache: Arc<Mutex<LocalImageCache>>,
 }
 
 impl ImageHolder {
-    pub fn new(url: Url, local_image_cache: LocalImageCacheHandle) -> ImageHolder {
+    pub fn new(url: Url, local_image_cache: Arc<Mutex<LocalImageCache>>) -> ImageHolder {
         debug!("ImageHolder::new() {}", url.to_str());
         let holder = ImageHolder {
             url: url,
@@ -80,7 +41,7 @@ impl ImageHolder {
         // should be done as early as possible and decode only once we
         // are sure that the image will be used.
         {
-            let val = holder.local_image_cache.get().lock();
+            let val = holder.local_image_cache.lock();
             let mut local_image_cache = val;
             local_image_cache.prefetch(&holder.url);
             local_image_cache.decode(&holder.url);
@@ -120,7 +81,7 @@ impl ImageHolder {
         // the image and store it for the future
         if self.image.is_none() {
             let port = {
-                let val = self.local_image_cache.get().lock();
+                let val = self.local_image_cache.lock();
                 let mut local_image_cache = val;
                 local_image_cache.get_image(&self.url)
             };