servo: Merge #12076 - Make font template data load fallible (from jdm:font-load); r=pcwalton

Remove a TODO around dealing with a failed file operation.

Can we write an automated test for this? I don't really know what font template data is, but this failure seems to be fontconfig-specific...

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #12037
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

Source-Repo: https://github.com/servo/servo
Source-Revision: a82d5106bd15d243c8552bf58472e3e9889d1ce2

servo/components/gfx/font_cache_thread.rs

@@ -96,8 +96,9 @@ impl FontTemplates {
             }
         }
 
-        let template = FontTemplate::new(identifier, maybe_data);
-        self.templates.push(template);
+        if let Ok(template) = FontTemplate::new(identifier, maybe_data) {
+            self.templates.push(template);
+        }
     }
 }
 

servo/components/gfx/font_template.rs

@@ -7,6 +7,7 @@ use platform::font::FontHandle;
 use platform::font_context::FontContextHandle;
 use platform::font_template::FontTemplateData;
 use std::fmt::{Debug, Error, Formatter};
+use std::io::Error as IoError;
 use std::sync::{Arc, Weak};
 use std::u32;
 use string_cache::Atom;
@@ -77,9 +78,9 @@ impl Debug for FontTemplate {
 /// is common, regardless of the number of instances of
 /// this font handle per thread.
 impl FontTemplate {
-    pub fn new(identifier: Atom, maybe_bytes: Option<Vec<u8>>) -> FontTemplate {
+    pub fn new(identifier: Atom, maybe_bytes: Option<Vec<u8>>) -> Result<FontTemplate, IoError> {
         let maybe_data = match maybe_bytes {
-            Some(_) => Some(FontTemplateData::new(identifier.clone(), maybe_bytes)),
+            Some(_) => Some(try!(FontTemplateData::new(identifier.clone(), maybe_bytes))),
             None => None,
         };
 
@@ -93,13 +94,13 @@ impl FontTemplate {
             None => None,
         };
 
-        FontTemplate {
+        Ok(FontTemplate {
             identifier: identifier,
             descriptor: None,
             weak_ref: maybe_weak_ref,
             strong_ref: maybe_strong_ref,
             is_valid: true,
-        }
+        })
     }
 
     pub fn identifier(&self) -> &Atom {
@@ -117,7 +118,7 @@ impl FontTemplate {
         // so that we can do font matching against it again in the future
         // without having to reload the font (unless it is an actual match).
         match self.descriptor {
-            Some(actual_desc) if *requested_desc == actual_desc => Some(self.data()),
+            Some(actual_desc) if *requested_desc == actual_desc => self.data().ok(),
             Some(_) => None,
             None => {
                 if self.instantiate(fctx).is_err() {
@@ -127,7 +128,7 @@ impl FontTemplate {
                 if self.descriptor
                        .as_ref()
                        .expect("Instantiation succeeded but no descriptor?") == requested_desc {
-                    Some(self.data())
+                    self.data().ok()
                 } else {
                     None
                 }
@@ -143,7 +144,9 @@ impl FontTemplate {
                                            -> Option<(Arc<FontTemplateData>, u32)> {
         match self.descriptor {
             Some(actual_descriptor) => {
-                Some((self.data(), actual_descriptor.distance_from(requested_descriptor)))
+                self.data().ok().map(|data| {
+                    (data, actual_descriptor.distance_from(requested_descriptor))
+                })
             }
             None => {
                 if self.instantiate(font_context).is_ok() {
@@ -151,7 +154,7 @@ impl FontTemplate {
                                        .as_ref()
                                        .expect("Instantiation successful but no descriptor?")
                                        .distance_from(requested_descriptor);
-                    Some((self.data(), distance))
+                    self.data().ok().map(|data| (data, distance))
                 } else {
                     None
                 }
@@ -164,7 +167,7 @@ impl FontTemplate {
             return Err(())
         }
 
-        let data = self.data();
+        let data = try!(self.data().map_err(|_| ()));
         let handle: Result<FontHandle, ()> = FontHandleMethods::new_from_template(font_context,
                                                                                   data,
                                                                                   None);
@@ -179,7 +182,7 @@ impl FontTemplate {
     /// Get the data for creating a font.
     pub fn get(&mut self) -> Option<Arc<FontTemplateData>> {
         if self.is_valid {
-            Some(self.data())
+            self.data().ok()
         } else {
             None
         }
@@ -188,19 +191,19 @@ impl FontTemplate {
     /// Get the font template data. If any strong references still
     /// exist, it will return a clone, otherwise it will load the
     /// font data and store a weak reference to it internally.
-    pub fn data(&mut self) -> Arc<FontTemplateData> {
+    pub fn data(&mut self) -> Result<Arc<FontTemplateData>, IoError> {
         let maybe_data = match self.weak_ref {
             Some(ref data) => data.upgrade(),
             None => None,
         };
 
         if let Some(data) = maybe_data {
-            return data
+            return Ok(data)
         }
 
         assert!(self.strong_ref.is_none());
-        let template_data = Arc::new(FontTemplateData::new(self.identifier.clone(), None));
+        let template_data = Arc::new(try!(FontTemplateData::new(self.identifier.clone(), None)));
         self.weak_ref = Some(Arc::downgrade(&template_data));
-        template_data
+        Ok(template_data)
     }
 }

servo/components/gfx/platform/freetype/font_template.rs

@@ -3,7 +3,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use std::fs::File;
-use std::io::Read;
+use std::io::{Read, Error};
 use string_cache::Atom;
 use webrender_traits::NativeFontHandle;
 
@@ -18,24 +18,24 @@ pub struct FontTemplateData {
 }
 
 impl FontTemplateData {
-    pub fn new(identifier: Atom, font_data: Option<Vec<u8>>) -> FontTemplateData {
+    pub fn new(identifier: Atom, font_data: Option<Vec<u8>>) -> Result<FontTemplateData, Error> {
         let bytes = match font_data {
             Some(bytes) => {
                 bytes
             },
             None => {
                 // TODO: Handle file load failure!
-                let mut file = File::open(&*identifier).unwrap();
+                let mut file = try!(File::open(&*identifier));
                 let mut buffer = vec![];
                 file.read_to_end(&mut buffer).unwrap();
                 buffer
             },
         };
 
-        FontTemplateData {
+        Ok(FontTemplateData {
             bytes: bytes,
             identifier: identifier,
-        }
+        })
     }
 
     /// Returns a clone of the data in this font. This may be a hugely expensive

servo/components/gfx/platform/macos/font_template.rs

@@ -12,7 +12,7 @@ use serde::de::{Error, Visitor};
 use std::borrow::ToOwned;
 use std::collections::HashMap;
 use std::fs::File;
-use std::io::Read;
+use std::io::{Read, Error as IoError};
 use std::ops::Deref;
 use std::sync::Mutex;
 use string_cache::Atom;
@@ -41,12 +41,12 @@ unsafe impl Send for FontTemplateData {}
 unsafe impl Sync for FontTemplateData {}
 
 impl FontTemplateData {
-    pub fn new(identifier: Atom, font_data: Option<Vec<u8>>) -> FontTemplateData {
-        FontTemplateData {
+    pub fn new(identifier: Atom, font_data: Option<Vec<u8>>) -> Result<FontTemplateData, IoError> {
+        Ok(FontTemplateData {
             ctfont: CachedCTFont(Mutex::new(HashMap::new())),
             identifier: identifier.to_owned(),
             font_data: font_data
-        }
+        })
     }
 
     /// Retrieves the Core Text font instance, instantiating it if necessary.