Auto merge of #6289 - nnethercote:unleak-LOCAL_CONTEXT_KEY, r=pcwalton

`LOCAL_CONTEXT_KEY` is currently a `Cell<*mut LocalLayoutContext>`. The use
of the raw pointer means that the `LocalLayoutContext` is not dropped when
the thread dies; this leaks FreeType instances and probably other
things. There are also some unsafe getter functions in `LayoutContext`
(`font_context`, `applicable_declarations_cache` and
`style_sharing_candidate_cache`) that @eddyb says involve undefined
behaviour.

This changeset changes `LOCAL_CONTEXT_KEY` to
`RefCell<Option<Rc<LocalLayoutContext>>>`. This fixes the leak and also
results in safe getters.

(Fixes #6282.)

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

7 files changed, 38 insertions, 48 deletions

diff --git a/components/layout/construct.rs b/components/layout/construct.rs
index cbd77fbc211..d3d21007242 100644
--- a/components/layout/construct.rs
+++ b/components/layout/construct.rs
@@ -423,7 +423,7 @@ impl<'a> FlowConstructor<'a> {
         // for runs might collapse so much whitespace away that only hypothetical fragments
         // remain. In that case the inline flow will compute its ascent and descent to be zero.
         let scanned_fragments =
-            TextRunScanner::new().scan_for_runs(self.layout_context.font_context(),
+            TextRunScanner::new().scan_for_runs(&mut self.layout_context.font_context(),
                                                 fragments.fragments);
         let mut inline_flow_ref =
             FlowRef::new(box InlineFlow::from_fragments(scanned_fragments,
@@ -446,7 +446,7 @@ impl<'a> FlowConstructor<'a> {
 
 
             let (ascent, descent) =
-                inline_flow.compute_minimum_ascent_and_descent(self.layout_context.font_context(),
+                inline_flow.compute_minimum_ascent_and_descent(&mut self.layout_context.font_context(),
                                                                &**node.style());
             inline_flow.minimum_block_size_above_baseline = ascent;
             inline_flow.minimum_depth_below_baseline = descent;
@@ -1140,7 +1140,7 @@ impl<'a> FlowConstructor<'a> {
                             SpecificFragmentInfo::UnscannedText(
                                 UnscannedTextFragmentInfo::from_text(text))));
                         let marker_fragments = TextRunScanner::new().scan_for_runs(
-                            self.layout_context.font_context(),
+                            &mut self.layout_context.font_context(),
                             unscanned_marker_fragments);
                         debug_assert!(marker_fragments.len() == 1);
                         marker_fragments.fragments.into_iter().next()
diff --git a/components/layout/context.rs b/components/layout/context.rs
index deda786157d..c2bf3ad4db1 100644
--- a/components/layout/context.rs
+++ b/components/layout/context.rs
@@ -19,11 +19,10 @@ use net_traits::image::base::Image;
 use net_traits::image_cache_task::{ImageCacheChan, ImageCacheTask, ImageResponse, ImageState};
 use net_traits::image_cache_task::{UsePlaceholder};
 use script::layout_interface::{Animation, LayoutChan, ReflowGoal};
-use std::boxed;
-use std::cell::Cell;
+use std::cell::{RefCell, RefMut};
 use std::collections::HashMap;
 use std::collections::hash_state::DefaultState;
-use std::ptr;
+use std::rc::Rc;
 use std::sync::{Arc, Mutex};
 use std::sync::mpsc::{channel, Sender};
 use style::selector_matching::Stylist;
@@ -33,30 +32,31 @@ use util::geometry::Au;
 use util::opts;
 
 struct LocalLayoutContext {
-    font_context: FontContext,
-    applicable_declarations_cache: ApplicableDeclarationsCache,
-    style_sharing_candidate_cache: StyleSharingCandidateCache,
+    font_context: RefCell<FontContext>,
+    applicable_declarations_cache: RefCell<ApplicableDeclarationsCache>,
+    style_sharing_candidate_cache: RefCell<StyleSharingCandidateCache>,
 }
 
-thread_local!(static LOCAL_CONTEXT_KEY: Cell<*mut LocalLayoutContext> = Cell::new(ptr::null_mut()));
+thread_local!(static LOCAL_CONTEXT_KEY: RefCell<Option<Rc<LocalLayoutContext>>> = RefCell::new(None));
 
 fn create_or_get_local_context(shared_layout_context: &SharedLayoutContext)
-                               -> *mut LocalLayoutContext {
-    LOCAL_CONTEXT_KEY.with(|ref r| {
-        if r.get().is_null() {
-            let context = box LocalLayoutContext {
-                font_context: FontContext::new(shared_layout_context.font_cache_task.clone()),
-                applicable_declarations_cache: ApplicableDeclarationsCache::new(),
-                style_sharing_candidate_cache: StyleSharingCandidateCache::new(),
-            };
-            r.set(boxed::into_raw(context));
-        } else if shared_layout_context.screen_size_changed {
-            unsafe {
-                (*r.get()).applicable_declarations_cache.evict_all();
+                               -> Rc<LocalLayoutContext> {
+    LOCAL_CONTEXT_KEY.with(|r| {
+        let mut r = r.borrow_mut();
+        if let Some(context) = r.clone() {
+            if shared_layout_context.screen_size_changed {
+                context.applicable_declarations_cache.borrow_mut().evict_all();
             }
+            context
+        } else {
+            let context = Rc::new(LocalLayoutContext {
+                font_context: RefCell::new(FontContext::new(shared_layout_context.font_cache_task.clone())),
+                applicable_declarations_cache: RefCell::new(ApplicableDeclarationsCache::new()),
+                style_sharing_candidate_cache: RefCell::new(StyleSharingCandidateCache::new()),
+            });
+            *r = Some(context.clone());
+            context
         }
-
-        r.get()
     })
 }
 
@@ -121,7 +121,7 @@ unsafe impl Send for SharedLayoutContextWrapper {}
 
 pub struct LayoutContext<'a> {
     pub shared: &'a SharedLayoutContext,
-    cached_local_layout_context: *mut LocalLayoutContext,
+    cached_local_layout_context: Rc<LocalLayoutContext>,
 }
 
 impl<'a> LayoutContext<'a> {
@@ -136,27 +136,18 @@ impl<'a> LayoutContext<'a> {
     }
 
     #[inline(always)]
-    pub fn font_context<'b>(&'b self) -> &'b mut FontContext {
-        unsafe {
-            let cached_context = &mut *self.cached_local_layout_context;
-            &mut cached_context.font_context
-        }
+    pub fn font_context(&self) -> RefMut<FontContext> {
+        self.cached_local_layout_context.font_context.borrow_mut()
     }
 
     #[inline(always)]
-    pub fn applicable_declarations_cache<'b>(&'b self) -> &'b mut ApplicableDeclarationsCache {
-        unsafe {
-            let cached_context = &mut *self.cached_local_layout_context;
-            &mut cached_context.applicable_declarations_cache
-        }
+    pub fn applicable_declarations_cache(&self) -> RefMut<ApplicableDeclarationsCache> {
+        self.cached_local_layout_context.applicable_declarations_cache.borrow_mut()
     }
 
     #[inline(always)]
-    pub fn style_sharing_candidate_cache<'b>(&'b self) -> &'b mut StyleSharingCandidateCache {
-        unsafe {
-            let cached_context = &mut *self.cached_local_layout_context;
-            &mut cached_context.style_sharing_candidate_cache
-        }
+    pub fn style_sharing_candidate_cache(&self) -> RefMut<StyleSharingCandidateCache> {
+        self.cached_local_layout_context.style_sharing_candidate_cache.borrow_mut()
     }
 
     pub fn get_or_request_image(&self, url: Url, use_placeholder: UsePlaceholder)
diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs
index 82fab9d2ea1..2815eb88db6 100644
--- a/components/layout/fragment.rs
+++ b/components/layout/fragment.rs
@@ -856,7 +856,7 @@ impl Fragment {
                 self.border_box.size,
                 SpecificFragmentInfo::UnscannedText(UnscannedTextFragmentInfo::from_text(
                         "…".to_owned()))));
-        let ellipsis_fragments = TextRunScanner::new().scan_for_runs(layout_context.font_context(),
+        let ellipsis_fragments = TextRunScanner::new().scan_for_runs(&mut layout_context.font_context(),
                                                                      unscanned_ellipsis_fragments);
         debug_assert!(ellipsis_fragments.len() == 1);
         ellipsis_fragments.fragments.into_iter().next().unwrap()
@@ -1005,7 +1005,7 @@ impl Fragment {
 
     pub fn calculate_line_height(&self, layout_context: &LayoutContext) -> Au {
         let font_style = self.style.get_font_arc();
-        let font_metrics = text::font_metrics_for_style(layout_context.font_context(), font_style);
+        let font_metrics = text::font_metrics_for_style(&mut layout_context.font_context(), font_style);
         text::line_height_from_style(&*self.style, &font_metrics)
     }
 
@@ -1819,7 +1819,7 @@ impl Fragment {
                 // See CSS 2.1 § 10.8.1.
                 let block_flow = info.flow_ref.as_immutable_block();
                 let font_style = self.style.get_font_arc();
-                let font_metrics = text::font_metrics_for_style(layout_context.font_context(),
+                let font_metrics = text::font_metrics_for_style(&mut layout_context.font_context(),
                                                                 font_style);
                 InlineMetrics::from_block_height(&font_metrics,
                                                  block_flow.base.position.size.block,
diff --git a/components/layout/generated_content.rs b/components/layout/generated_content.rs
index 085b86457bb..fe08f9d3f85 100644
--- a/components/layout/generated_content.rs
+++ b/components/layout/generated_content.rs
@@ -429,7 +429,7 @@ fn render_text(layout_context: &LayoutContext,
                                                              info));
     // FIXME(pcwalton): This should properly handle multiple marker fragments. This could happen
     // due to text run splitting.
-    let fragments = TextRunScanner::new().scan_for_runs(layout_context.font_context(), fragments);
+    let fragments = TextRunScanner::new().scan_for_runs(&mut layout_context.font_context(), fragments);
     debug_assert!(fragments.len() >= 1);
     fragments.fragments.into_iter().next().unwrap().specific
 }
diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs
index 4dad51374c6..474b20c8001 100644
--- a/components/layout/layout_task.rs
+++ b/components/layout/layout_task.rs
@@ -312,7 +312,6 @@ impl LayoutTask {
         let reporter_name = format!("layout-reporter-{}", id.0);
         mem_profiler_chan.send(mem::ProfilerMsg::RegisterReporter(reporter_name.clone(), reporter));
 
-
         // Create the channel on which new animations can be sent.
         let (new_animations_sender, new_animations_receiver) = channel();
         let (image_cache_sender, image_cache_receiver) = channel();
diff --git a/components/layout/list_item.rs b/components/layout/list_item.rs
index 1840cc1fbde..d38b365f144 100644
--- a/components/layout/list_item.rs
+++ b/components/layout/list_item.rs
@@ -106,7 +106,7 @@ impl Flow for ListItemFlow {
             marker.assign_replaced_block_size_if_necessary(containing_block_block_size);
 
             let font_metrics =
-                text::font_metrics_for_style(layout_context.font_context(),
+                text::font_metrics_for_style(&mut layout_context.font_context(),
                                              marker.style.get_font_arc());
             let line_height = text::line_height_from_style(&*marker.style, &font_metrics);
             let item_inline_metrics = InlineMetrics::from_font_metrics(&font_metrics, line_height);
diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs
index 5ec9f490ada..a1e6011e3d8 100644
--- a/components/layout/traversal.rs
+++ b/components/layout/traversal.rs
@@ -154,7 +154,7 @@ impl<'a> PreorderDomTraversal for RecalcStyleForNode<'a> {
 
             // Check to see whether we can share a style with someone.
             let style_sharing_candidate_cache =
-                self.layout_context.style_sharing_candidate_cache();
+                &mut self.layout_context.style_sharing_candidate_cache();
             let sharing_result = unsafe {
                 node.share_style_if_possible(style_sharing_candidate_cache,
                                              parent_opt.clone())
@@ -181,7 +181,7 @@ impl<'a> PreorderDomTraversal for RecalcStyleForNode<'a> {
                         node.cascade_node(self.layout_context.shared,
                                           parent_opt,
                                           &applicable_declarations,
-                                          self.layout_context.applicable_declarations_cache(),
+                                          &mut self.layout_context.applicable_declarations_cache(),
                                           &self.layout_context.shared.new_animations_sender);
                     }