clippy: fix warnings in components/gfx (#31560)

* clippy: fix warnings in components/gfx

* refactor: switched the order of impl so that its intent is clearer

* fix: add font context default in other platforms

components/gfx/font.rs

@@ -96,7 +96,7 @@ impl FontTableTagConversions for FontTableTag {
             (self >> 24) as u8,
             (self >> 16) as u8,
             (self >> 8) as u8,
-            (self >> 0) as u8,
+            *self as u8,
         ];
         str::from_utf8(&bytes).unwrap().to_owned()
     }
@@ -190,7 +190,7 @@ impl Font {
         let metrics = handle.metrics();
 
         Font {
-            handle: handle,
+            handle,
             shaper: None,
             descriptor,
             metrics,
@@ -403,7 +403,7 @@ impl FontGroup {
             .font_family
             .families
             .iter()
-            .map(|family| FontGroupFamily::new(descriptor.clone(), &family))
+            .map(|family| FontGroupFamily::new(descriptor.clone(), family))
             .collect();
 
         FontGroup {
@@ -419,7 +419,7 @@ impl FontGroup {
     /// found, returns None.
     pub fn find_by_codepoint<S: FontSource>(
         &mut self,
-        mut font_context: &mut FontContext<S>,
+        font_context: &mut FontContext<S>,
         codepoint: char,
     ) -> Option<FontRef> {
         let should_look_for_small_caps = self.descriptor.variant == font_variant_caps::T::SmallCaps &&
@@ -436,43 +436,40 @@ impl FontGroup {
 
         let has_glyph = |font: &FontRef| font.borrow().has_glyph_for(codepoint);
 
-        if let Some(font) = self.find(&mut font_context, |font| has_glyph(font)) {
+        if let Some(font) = self.find(font_context, has_glyph) {
             return font_or_synthesized_small_caps(font);
         }
 
         if let Some(ref last_matching_fallback) = self.last_matching_fallback {
-            if has_glyph(&last_matching_fallback) {
+            if has_glyph(last_matching_fallback) {
                 return font_or_synthesized_small_caps(last_matching_fallback.clone());
             }
         }
 
-        if let Some(font) = self.find_fallback(&mut font_context, Some(codepoint), has_glyph) {
+        if let Some(font) = self.find_fallback(font_context, Some(codepoint), has_glyph) {
             self.last_matching_fallback = Some(font.clone());
             return font_or_synthesized_small_caps(font);
         }
 
-        self.first(&mut font_context)
+        self.first(font_context)
     }
 
     /// Find the first available font in the group, or the first available fallback font.
-    pub fn first<S: FontSource>(
+    pub fn first<S: FontSource>(&mut self, font_context: &mut FontContext<S>) -> Option<FontRef> {
-        &mut self,
+        self.find(font_context, |_| true)
-        mut font_context: &mut FontContext<S>,
+            .or_else(|| self.find_fallback(font_context, None, |_| true))
-    ) -> Option<FontRef> {
-        self.find(&mut font_context, |_| true)
-            .or_else(|| self.find_fallback(&mut font_context, None, |_| true))
     }
 
     /// Find a font which returns true for `predicate`. This method mutates because we may need to
     /// load new font data in the process of finding a suitable font.
-    fn find<S, P>(&mut self, mut font_context: &mut FontContext<S>, predicate: P) -> Option<FontRef>
+    fn find<S, P>(&mut self, font_context: &mut FontContext<S>, predicate: P) -> Option<FontRef>
     where
         S: FontSource,
         P: FnMut(&FontRef) -> bool,
     {
         self.families
             .iter_mut()
-            .filter_map(|family| family.font(&mut font_context))
+            .filter_map(|family| family.font(font_context))
             .find(predicate)
     }
 
@@ -567,8 +564,7 @@ impl RunMetrics {
 }
 
 pub fn get_and_reset_text_shaping_performance_counter() -> usize {
-    let value = TEXT_SHAPING_PERFORMANCE_COUNTER.swap(0, Ordering::SeqCst);
+    TEXT_SHAPING_PERFORMANCE_COUNTER.swap(0, Ordering::SeqCst)
-    value
 }
 
 /// The scope within which we will look for a font.

components/gfx/font_cache_thread.rs

@@ -31,6 +31,7 @@ use crate::platform::font_list::{
 use crate::platform::font_template::FontTemplateData;
 
 /// A list of font templates that make up a given font family.
+#[derive(Default)]
 pub struct FontTemplates {
     templates: Vec<FontTemplate>,
 }
@@ -61,10 +62,6 @@ impl SerializedFontTemplate {
 }
 
 impl FontTemplates {
-    pub fn new() -> FontTemplates {
-        FontTemplates { templates: vec![] }
-    }
-
     /// Find a font in this family that matches a given descriptor.
     pub fn find_font_for_style(
         &mut self,
@@ -214,7 +211,7 @@ impl FontCache {
                                     font_key: font_template_info.font_key,
                                 },
                             )));
-                            let _ = bytes_sender.send(&*font_template_info.font_template.bytes());
+                            let _ = bytes_sender.send(&font_template_info.font_template.bytes());
                         },
                     };
                 },
@@ -262,7 +259,7 @@ impl FontCache {
         };
 
         if !self.web_families.contains_key(&family_name) {
-            let templates = FontTemplates::new();
+            let templates = FontTemplates::default();
             self.web_families.insert(family_name.clone(), templates);
         }
 
@@ -298,7 +295,7 @@ impl FontCache {
                         FetchResponseMsg::ProcessResponseChunk(new_bytes) => {
                             trace!("@font-face {} chunk={:?}", family_name, new_bytes);
                             if *response_valid.lock().unwrap() {
-                                bytes.lock().unwrap().extend(new_bytes.into_iter())
+                                bytes.lock().unwrap().extend(new_bytes)
                             }
                         },
                         FetchResponseMsg::ProcessResponseEOF(response) => {
@@ -312,7 +309,7 @@ impl FontCache {
                                 channel_to_self.send(msg).unwrap();
                                 return;
                             }
-                            let bytes = mem::replace(&mut *bytes.lock().unwrap(), vec![]);
+                            let bytes = mem::take(&mut *bytes.lock().unwrap());
                             trace!("@font-face {} data={:?}", family_name, bytes);
                             let bytes = match fontsan::process(&bytes) {
                                 Ok(san) => san,
@@ -365,10 +362,7 @@ impl FontCache {
         self.local_families.clear();
         for_each_available_family(|family_name| {
             let family_name = LowercaseString::new(&family_name);
-            if !self.local_families.contains_key(&family_name) {
+            self.local_families.entry(family_name).or_default();
-                let templates = FontTemplates::new();
-                self.local_families.insert(family_name, templates);
-            }
         });
     }
 
@@ -443,7 +437,7 @@ impl FontCache {
 
         FontTemplateInfo {
             font_template: template,
-            font_key: font_key,
+            font_key,
         }
     }
 
@@ -454,13 +448,13 @@ impl FontCache {
     ) -> Option<FontTemplateInfo> {
         match family_descriptor.scope {
             FontSearchScope::Any => self
-                .find_font_in_web_family(&template_descriptor, &family_descriptor.name)
+                .find_font_in_web_family(template_descriptor, &family_descriptor.name)
                 .or_else(|| {
-                    self.find_font_in_local_family(&template_descriptor, &family_descriptor.name)
+                    self.find_font_in_local_family(template_descriptor, &family_descriptor.name)
                 }),
 
             FontSearchScope::Local => {
-                self.find_font_in_local_family(&template_descriptor, &family_descriptor.name)
+                self.find_font_in_local_family(template_descriptor, &family_descriptor.name)
             },
         }
         .map(|t| self.get_font_template_info(t))
@@ -489,12 +483,12 @@ impl FontCacheThread {
                 let generic_fonts = populate_generic_fonts();
 
                 let mut cache = FontCache {
-                    port: port,
+                    port,
                     channel_to_self,
                     generic_fonts,
                     local_families: HashMap::new(),
                     web_families: HashMap::new(),
-                    font_context: FontContextHandle::new(),
+                    font_context: FontContextHandle::default(),
                     core_resource_thread,
                     webrender_api,
                     webrender_fonts: HashMap::new(),
@@ -506,7 +500,7 @@ impl FontCacheThread {
             })
             .expect("Thread spawning failed");
 
-        FontCacheThread { chan: chan }
+        FontCacheThread { chan }
     }
 
     pub fn add_web_font(
@@ -621,7 +615,7 @@ impl Deref for LowercaseString {
 
     #[inline]
     fn deref(&self) -> &str {
-        &*self.inner
+        &self.inner
     }
 }
 

components/gfx/font_context.rs

@@ -65,7 +65,7 @@ pub struct FontContext<S: FontSource> {
 
 impl<S: FontSource> FontContext<S> {
     pub fn new(font_source: S) -> FontContext<S> {
-        let handle = FontContextHandle::new();
+        let handle = FontContextHandle::default();
         FontContext {
             platform_handle: handle,
             font_source,
@@ -99,8 +99,8 @@ impl<S: FontSource> FontContext<S> {
             style,
         };
 
-        if let Some(ref font_group) = self.font_group_cache.get(&cache_key) {
+        if let Some(font_group) = self.font_group_cache.get(&cache_key) {
-            return (*font_group).clone();
+            return font_group.clone();
         }
 
         let font_group = Rc::new(RefCell::new(FontGroup::new(&cache_key.style)));
@@ -150,10 +150,7 @@ impl<S: FontSource> FontContext<S> {
             family_descriptor: family_descriptor.clone(),
         };
 
-        self.font_cache
+        self.font_cache.get(&cache_key).cloned().unwrap_or_else(|| {
-            .get(&cache_key)
-            .map(|v| v.clone())
-            .unwrap_or_else(|| {
             debug!(
                 "FontContext::font cache miss for font_descriptor={:?} family_descriptor={:?}",
                 font_descriptor, family_descriptor
@@ -182,11 +179,11 @@ impl<S: FontSource> FontContext<S> {
         family_descriptor: &FontFamilyDescriptor,
     ) -> Option<FontTemplateInfo> {
         let cache_key = FontTemplateCacheKey {
-            template_descriptor: template_descriptor.clone(),
+            template_descriptor: *template_descriptor,
             family_descriptor: family_descriptor.clone(),
         };
 
-        self.font_template_cache.get(&cache_key).map(|v| v.clone()).unwrap_or_else(|| {
+        self.font_template_cache.get(&cache_key).cloned().unwrap_or_else(|| {
             debug!(
                 "FontContext::font_template cache miss for template_descriptor={:?} family_descriptor={:?}",
                 template_descriptor,
@@ -194,7 +191,7 @@ impl<S: FontSource> FontContext<S> {
             );
 
             let template_info = self.font_source.font_template(
-                template_descriptor.clone(),
+                *template_descriptor,
                 family_descriptor.clone(),
             );
 

components/gfx/font_template.rs

@@ -109,18 +109,12 @@ impl FontTemplate {
             None => None,
         };
 
-        let maybe_strong_ref = match maybe_data {
+        let maybe_strong_ref = maybe_data.map(Arc::new);
-            Some(data) => Some(Arc::new(data)),
-            None => None,
-        };
 
-        let maybe_weak_ref = match maybe_strong_ref {
+        let maybe_weak_ref = maybe_strong_ref.as_ref().map(Arc::downgrade);
-            Some(ref strong_ref) => Some(Arc::downgrade(strong_ref)),
-            None => None,
-        };
 
         Ok(FontTemplate {
-            identifier: identifier,
+            identifier,
             descriptor: None,
             weak_ref: maybe_weak_ref,
             strong_ref: maybe_strong_ref,
@@ -161,7 +155,7 @@ impl FontTemplate {
         fctx: &FontContextHandle,
         requested_desc: &FontTemplateDescriptor,
     ) -> Option<Arc<FontTemplateData>> {
-        self.descriptor(&fctx).and_then(|descriptor| {
+        self.descriptor(fctx).and_then(|descriptor| {
             if *requested_desc == descriptor {
                 self.data().ok()
             } else {
@@ -177,7 +171,7 @@ impl FontTemplate {
         font_context: &FontContextHandle,
         requested_descriptor: &FontTemplateDescriptor,
     ) -> Option<(Arc<FontTemplateData>, f32)> {
-        self.descriptor(&font_context).and_then(|descriptor| {
+        self.descriptor(font_context).and_then(|descriptor| {
             self.data()
                 .ok()
                 .map(|data| (data, descriptor.distance_from(requested_descriptor)))

components/gfx/platform/freetype/font.rs

@@ -147,7 +147,7 @@ impl FontHandleMethods for FontHandle {
         let face = create_face(ft_ctx, &template, pt_size)?;
 
         let mut handle = FontHandle {
-            face: face,
+            face,
             font_data: template,
             context_handle: fctx.clone(),
             can_do_fast_shaping: false,
@@ -264,7 +264,7 @@ impl FontHandleMethods for FontHandle {
             let res = FT_Load_Glyph(self.face, glyph as FT_UInt, GLYPH_LOAD_FLAGS);
             if succeeded(res) {
                 let void_glyph = (*self.face).glyph;
-                let slot: FT_GlyphSlot = mem::transmute(void_glyph);
+                let slot: FT_GlyphSlot = void_glyph;
                 assert!(!slot.is_null());
                 let advance = (*slot).metrics.horiAdvance;
                 debug!("h_advance for {} is {}", glyph, advance);
@@ -313,17 +313,17 @@ impl FontHandleMethods for FontHandle {
             .map_or(max_advance, |advance| self.font_units_to_au(advance));
 
         let metrics = FontMetrics {
-            underline_size: underline_size,
+            underline_size,
-            underline_offset: underline_offset,
+            underline_offset,
-            strikeout_size: strikeout_size,
+            strikeout_size,
-            strikeout_offset: strikeout_offset,
+            strikeout_offset,
-            leading: leading,
+            leading,
-            x_height: x_height,
+            x_height,
-            em_size: em_size,
+            em_size,
-            ascent: ascent,
+            ascent,
             descent: -descent, // linux font's seem to use the opposite sign from mac
-            max_advance: max_advance,
+            max_advance,
-            average_advance: average_advance,
+            average_advance,
             line_gap: height,
         };
 
@@ -392,6 +392,7 @@ impl<'a> FontHandle {
         }
     }
 
+    #[allow(clippy::mut_from_ref)] // Intended for this function
     fn face_rec_mut(&'a self) -> &'a mut FT_FaceRec {
         unsafe { &mut (*self.face) }
     }
@@ -402,10 +403,10 @@ impl<'a> FontHandle {
         // face.size is a *c_void in the bindings, presumably to avoid
         // recursive structural types
         let size: &FT_SizeRec = unsafe { mem::transmute(&(*face.size)) };
-        let metrics: &FT_Size_Metrics = &(*size).metrics;
+        let metrics: &FT_Size_Metrics = &(size).metrics;
 
         let em_size = face.units_per_EM as f64;
-        let x_scale = (metrics.x_ppem as f64) / em_size as f64;
+        let x_scale = (metrics.x_ppem as f64) / em_size;
 
         // If this isn't true then we're scaling one of the axes wrong
         assert_eq!(metrics.x_ppem, metrics.y_ppem);

components/gfx/platform/freetype/font_context.rs

@@ -109,8 +109,8 @@ impl MallocSizeOf for FontContextHandle {
     }
 }
 
-impl FontContextHandle {
+impl Default for FontContextHandle {
-    pub fn new() -> FontContextHandle {
+    fn default() -> Self {
         let user = Box::into_raw(Box::new(User { size: 0 }));
         let mem = Box::into_raw(Box::new(FT_MemoryRec_ {
             user: user as *mut c_void,

components/gfx/platform/freetype/font_list.rs

@@ -17,10 +17,10 @@ use log::debug;
 use super::c_str_to_string;
 use crate::text::util::is_cjk;
 
-static FC_FAMILY: &'static [u8] = b"family\0";
+static FC_FAMILY: &[u8] = b"family\0";
-static FC_FILE: &'static [u8] = b"file\0";
+static FC_FILE: &[u8] = b"file\0";
-static FC_INDEX: &'static [u8] = b"index\0";
+static FC_INDEX: &[u8] = b"index\0";
-static FC_FONTFORMAT: &'static [u8] = b"fontformat\0";
+static FC_FONTFORMAT: &[u8] = b"fontformat\0";
 
 pub fn for_each_available_family<F>(mut callback: F)
 where
@@ -150,7 +150,7 @@ pub fn system_default_family(generic_name: &str) -> Option<String> {
     }
 }
 
-pub static SANS_SERIF_FONT_FAMILY: &'static str = "DejaVu Sans";
+pub static SANS_SERIF_FONT_FAMILY: &str = "DejaVu Sans";
 
 // Based on gfxPlatformGtk::GetCommonFallbackFonts() in Gecko
 pub fn fallback_font_families(codepoint: Option<char>) -> Vec<&'static str> {

components/gfx/platform/freetype/font_template.rs

@@ -36,10 +36,7 @@ impl fmt::Debug for FontTemplateData {
 
 impl FontTemplateData {
     pub fn new(identifier: Atom, bytes: Option<Vec<u8>>) -> Result<FontTemplateData, Error> {
-        Ok(FontTemplateData {
+        Ok(FontTemplateData { bytes, identifier })
-            bytes: bytes,
-            identifier: identifier,
-        })
     }
 
     /// Returns a clone of the data in this font. This may be a hugely expensive

components/gfx/platform/macos/font_context.rs

@@ -4,16 +4,10 @@
 
 use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
 
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, Default)]
 pub struct FontContextHandle {
-    _ctx: (),
-}
-
-impl FontContextHandle {
     // this is a placeholder until NSFontManager or whatever is bound in here.
-    pub fn new() -> FontContextHandle {
+    _ctx: (),
-        FontContextHandle { _ctx: () }
-    }
 }
 
 impl MallocSizeOf for FontContextHandle {

components/gfx/platform/windows/font_context.rs

@@ -4,14 +4,7 @@
 
 use malloc_size_of::malloc_size_of_is_0;
 
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, Default)]
 pub struct FontContextHandle;
 
-impl FontContextHandle {
-    // *shrug*
-    pub fn new() -> FontContextHandle {
-        FontContextHandle {}
-    }
-}
-
 malloc_size_of_is_0!(FontContextHandle);

components/gfx/rendering_context.rs

@@ -30,8 +30,8 @@ struct RenderingContextData {
 
 impl Drop for RenderingContextData {
     fn drop(&mut self) {
-        let ref mut device = self.device.borrow_mut();
+        let device = &mut self.device.borrow_mut();
-        let ref mut context = self.context.borrow_mut();
+        let context = &mut self.context.borrow_mut();
         if let Some(ref swap_chain) = self.swap_chain {
             let _ = swap_chain.destroy(device, context);
         }
@@ -45,7 +45,7 @@ impl RenderingContext {
         adapter: &Adapter,
         surface_type: SurfaceType<NativeWidget>,
     ) -> Result<Self, Error> {
-        let mut device = connection.create_device(&adapter)?;
+        let mut device = connection.create_device(adapter)?;
         let flags = ContextAttributeFlags::ALPHA |
             ContextAttributeFlags::DEPTH |
             ContextAttributeFlags::STENCIL;
@@ -94,8 +94,8 @@ impl RenderingContext {
         &self,
         surface: Surface,
     ) -> Result<SurfaceTexture, (Error, Surface)> {
-        let ref device = self.0.device.borrow();
+        let device = &self.0.device.borrow();
-        let ref mut context = self.0.context.borrow_mut();
+        let context = &mut self.0.context.borrow_mut();
         device.create_surface_texture(context, surface)
     }
 
@@ -103,14 +103,14 @@ impl RenderingContext {
         &self,
         surface_texture: SurfaceTexture,
     ) -> Result<Surface, (Error, SurfaceTexture)> {
-        let ref device = self.0.device.borrow();
+        let device = &self.0.device.borrow();
-        let ref mut context = self.0.context.borrow_mut();
+        let context = &mut self.0.context.borrow_mut();
         device.destroy_surface_texture(context, surface_texture)
     }
 
     pub fn make_gl_context_current(&self) -> Result<(), Error> {
-        let ref device = self.0.device.borrow();
+        let device = &self.0.device.borrow();
-        let ref context = self.0.context.borrow();
+        let context = &self.0.context.borrow();
         device.make_context_current(context)
     }
 
@@ -119,8 +119,8 @@ impl RenderingContext {
     }
 
     pub fn resize(&self, size: Size2D<i32>) -> Result<(), Error> {
-        let ref mut device = self.0.device.borrow_mut();
+        let device = &mut self.0.device.borrow_mut();
-        let ref mut context = self.0.context.borrow_mut();
+        let context = &mut self.0.context.borrow_mut();
         if let Some(swap_chain) = self.0.swap_chain.as_ref() {
             return swap_chain.resize(device, context, size);
         }
@@ -135,8 +135,8 @@ impl RenderingContext {
     }
 
     pub fn present(&self) -> Result<(), Error> {
-        let ref mut device = self.0.device.borrow_mut();
+        let device = &mut self.0.device.borrow_mut();
-        let ref mut context = self.0.context.borrow_mut();
+        let context = &mut self.0.context.borrow_mut();
         if let Some(ref swap_chain) = self.0.swap_chain {
             return swap_chain.swap_buffers(device, context, PreserveBuffer::No);
         }
@@ -153,8 +153,8 @@ impl RenderingContext {
     /// Invoke a closure with the surface associated with the current front buffer.
     /// This can be used to create a surfman::SurfaceTexture to blit elsewhere.
     pub fn with_front_buffer<F: FnMut(&Device, Surface) -> Surface>(&self, mut f: F) {
-        let ref mut device = self.0.device.borrow_mut();
+        let device = &mut self.0.device.borrow_mut();
-        let ref mut context = self.0.context.borrow_mut();
+        let context = &mut self.0.context.borrow_mut();
         let surface = device
             .unbind_surface_from_context(context)
             .unwrap()
@@ -168,52 +168,52 @@ impl RenderingContext {
     }
 
     pub fn connection(&self) -> Connection {
-        let ref device = self.0.device.borrow();
+        let device = &self.0.device.borrow();
         device.connection()
     }
 
     pub fn adapter(&self) -> Adapter {
-        let ref device = self.0.device.borrow();
+        let device = &self.0.device.borrow();
         device.adapter()
     }
 
     pub fn native_context(&self) -> NativeContext {
-        let ref device = self.0.device.borrow();
+        let device = &self.0.device.borrow();
-        let ref context = self.0.context.borrow();
+        let context = &self.0.context.borrow();
         device.native_context(context)
     }
 
     pub fn native_device(&self) -> NativeDevice {
-        let ref device = self.0.device.borrow();
+        let device = &self.0.device.borrow();
         device.native_device()
     }
 
     pub fn context_attributes(&self) -> ContextAttributes {
-        let ref device = self.0.device.borrow();
+        let device = &self.0.device.borrow();
-        let ref context = self.0.context.borrow();
+        let context = &self.0.context.borrow();
-        let ref descriptor = device.context_descriptor(context);
+        let descriptor = &device.context_descriptor(context);
         device.context_descriptor_attributes(descriptor)
     }
 
     pub fn context_surface_info(&self) -> Result<Option<SurfaceInfo>, Error> {
-        let ref device = self.0.device.borrow();
+        let device = &self.0.device.borrow();
-        let ref context = self.0.context.borrow();
+        let context = &self.0.context.borrow();
         device.context_surface_info(context)
     }
 
     pub fn surface_info(&self, surface: &Surface) -> SurfaceInfo {
-        let ref device = self.0.device.borrow();
+        let device = &self.0.device.borrow();
         device.surface_info(surface)
     }
 
     pub fn surface_texture_object(&self, surface: &SurfaceTexture) -> u32 {
-        let ref device = self.0.device.borrow();
+        let device = &self.0.device.borrow();
         device.surface_texture_object(surface)
     }
 
     pub fn get_proc_address(&self, name: &str) -> *const c_void {
-        let ref device = self.0.device.borrow();
+        let device = &self.0.device.borrow();
-        let ref context = self.0.context.borrow();
+        let context = &self.0.context.borrow();
         device.get_proc_address(context, name)
     }
 
@@ -221,7 +221,7 @@ impl RenderingContext {
         let device = self.0.device.borrow_mut();
         let mut context = self.0.context.borrow_mut();
         let mut surface = device.unbind_surface_from_context(&mut context)?.unwrap();
-        let _ = device.destroy_surface(&mut context, &mut surface)?;
+        device.destroy_surface(&mut context, &mut surface)?;
         Ok(())
     }
 

components/gfx/tests/font_context.rs

@@ -34,13 +34,13 @@ struct TestFontSource {
 
 impl TestFontSource {
     fn new() -> TestFontSource {
-        let mut csstest_ascii = FontTemplates::new();
+        let mut csstest_ascii = FontTemplates::default();
         Self::add_face(&mut csstest_ascii, "csstest-ascii", None);
 
-        let mut csstest_basic = FontTemplates::new();
+        let mut csstest_basic = FontTemplates::default();
         Self::add_face(&mut csstest_basic, "csstest-basic-regular", None);
 
-        let mut fallback = FontTemplates::new();
+        let mut fallback = FontTemplates::default();
         Self::add_face(&mut fallback, "csstest-basic-regular", Some("fallback"));
 
         let mut families = HashMap::new();
@@ -49,7 +49,7 @@ impl TestFontSource {
         families.insert(fallback_font_families(None)[0].to_owned(), fallback);
 
         TestFontSource {
-            handle: FontContextHandle::new(),
+            handle: FontContextHandle::default(),
             families,
             find_font_count: Rc::new(Cell::new(0)),
         }

components/gfx/tests/font_template.rs

@@ -35,7 +35,7 @@ fn test_font_template_descriptor() {
         )
         .unwrap();
 
-        let context = FontContextHandle::new();
+        let context = FontContextHandle::default();
 
         template.descriptor(&context).unwrap()
     }

components/gfx/text/glyph.rs

@@ -28,7 +28,7 @@ pub struct GlyphEntry {
 
 impl GlyphEntry {
     fn new(value: u32) -> GlyphEntry {
-        GlyphEntry { value: value }
+        GlyphEntry { value }
     }
 
     fn initial() -> GlyphEntry {
@@ -40,7 +40,7 @@ impl GlyphEntry {
         assert!(is_simple_glyph_id(id));
         assert!(is_simple_advance(advance));
 
-        let id_mask = id as u32;
+        let id_mask = id;
         let Au(advance) = advance;
         let advance_mask = (advance as u32) << GLYPH_ADVANCE_SHIFT;
 
@@ -89,7 +89,7 @@ const GLYPH_ID_MASK: u32 = 0x0000FFFF;
 const GLYPH_COUNT_MASK: u32 = 0x0000FFFF;
 
 fn is_simple_glyph_id(id: GlyphId) -> bool {
-    ((id as u32) & GLYPH_ID_MASK) == id
+    (id & GLYPH_ID_MASK) == id
 }
 
 fn is_simple_advance(advance: Au) -> bool {
@@ -157,9 +157,9 @@ struct DetailedGlyph {
 impl DetailedGlyph {
     fn new(id: GlyphId, advance: Au, offset: Point2D<Au>) -> DetailedGlyph {
         DetailedGlyph {
-            id: id,
+            id,
-            advance: advance,
+            advance,
-            offset: offset,
+            offset,
         }
     }
 }
@@ -172,18 +172,18 @@ struct DetailedGlyphRecord {
     detail_offset: usize,
 }
 
-impl PartialOrd for DetailedGlyphRecord {
-    fn partial_cmp(&self, other: &DetailedGlyphRecord) -> Option<Ordering> {
-        self.entry_offset.partial_cmp(&other.entry_offset)
-    }
-}
-
 impl Ord for DetailedGlyphRecord {
     fn cmp(&self, other: &DetailedGlyphRecord) -> Ordering {
         self.entry_offset.cmp(&other.entry_offset)
     }
 }
 
+impl PartialOrd for DetailedGlyphRecord {
+    fn partial_cmp(&self, other: &DetailedGlyphRecord) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
 // Manages the lookup table for detailed glyphs. Sorting is deferred
 // until a lookup is actually performed; this matches the expected
 // usage pattern of setting/appending all the detailed glyphs, and
@@ -210,7 +210,7 @@ impl<'a> DetailedGlyphStore {
 
     fn add_detailed_glyphs_for_entry(&mut self, entry_offset: ByteIndex, glyphs: &[DetailedGlyph]) {
         let entry = DetailedGlyphRecord {
-            entry_offset: entry_offset,
+            entry_offset,
             detail_offset: self.detail_buffer.len(),
         };
 
@@ -245,7 +245,7 @@ impl<'a> DetailedGlyphStore {
         assert!(self.lookup_is_sorted);
 
         let key = DetailedGlyphRecord {
-            entry_offset: entry_offset,
+            entry_offset,
             detail_offset: 0, // unused
         };
 
@@ -268,7 +268,7 @@ impl<'a> DetailedGlyphStore {
         assert!(self.lookup_is_sorted);
 
         let key = DetailedGlyphRecord {
-            entry_offset: entry_offset,
+            entry_offset,
             detail_offset: 0, // unused
         };
 
@@ -329,11 +329,11 @@ impl GlyphData {
         ligature_start: bool,
     ) -> GlyphData {
         GlyphData {
-            id: id,
+            id,
-            advance: advance,
+            advance,
             offset: offset.unwrap_or(Point2D::zero()),
-            cluster_start: cluster_start,
+            cluster_start,
-            ligature_start: ligature_start,
+            ligature_start,
         }
     }
 }
@@ -455,8 +455,8 @@ impl<'a> GlyphStore {
             total_advance: Au(0),
             total_word_separators: 0,
             has_detailed_glyphs: false,
-            is_whitespace: is_whitespace,
+            is_whitespace,
-            is_rtl: is_rtl,
+            is_rtl,
         }
     }
 
@@ -490,7 +490,7 @@ impl<'a> GlyphStore {
         let mut total_advance = Au(0);
         let mut total_word_separators = 0;
         for glyph in self.iter_glyphs_for_byte_range(&Range::new(ByteIndex(0), self.len())) {
-            total_advance = total_advance + glyph.advance();
+            total_advance += glyph.advance();
             if glyph.char_is_word_separator() {
                 total_word_separators += 1;
             }
@@ -539,7 +539,7 @@ impl<'a> GlyphStore {
 
     pub fn add_glyphs_for_byte_index(&mut self, i: ByteIndex, data_for_glyphs: &[GlyphData]) {
         assert!(i < self.len());
-        assert!(data_for_glyphs.len() > 0);
+        assert!(!data_for_glyphs.is_empty());
 
         let glyph_count = data_for_glyphs.len();
 
@@ -623,8 +623,6 @@ impl<'a> GlyphStore {
     pub fn advance_for_byte_range(&self, range: &Range<ByteIndex>, extra_word_spacing: Au) -> Au {
         if range.begin() == ByteIndex(0) && range.end() == self.len() {
             self.total_advance + extra_word_spacing * (self.total_word_separators as i32)
-        } else if !self.has_detailed_glyphs {
-            self.advance_for_byte_range_simple_glyphs(range, extra_word_spacing)
         } else {
             self.advance_for_byte_range_simple_glyphs(range, extra_word_spacing)
         }
@@ -664,13 +662,13 @@ impl<'a> GlyphStore {
 
 impl fmt::Debug for GlyphStore {
     fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
-        write!(formatter, "GlyphStore:\n")?;
+        writeln!(formatter, "GlyphStore:")?;
         let mut detailed_buffer = self.detail_store.detail_buffer.iter();
         for entry in self.entry_buffer.iter() {
             if entry.is_simple() {
-                write!(
+                writeln!(
                     formatter,
-                    "  simple id={:?} advance={:?}\n",
+                    "  simple id={:?} advance={:?}",
                     entry.id(),
                     entry.advance()
                 )?;
@@ -683,9 +681,9 @@ impl fmt::Debug for GlyphStore {
             if detailed_buffer.next().is_none() {
                 continue;
             }
-            write!(
+            writeln!(
                 formatter,
-                "  detailed id={:?} advance={:?}\n",
+                "  detailed id={:?} advance={:?}",
                 entry.id(),
                 entry.advance()
             )?;

components/gfx/text/shaping/harfbuzz.rs

@@ -47,8 +47,7 @@ pub struct ShapedGlyphEntry {
 }
 
 impl ShapedGlyphData {
-    pub fn new(buffer: *mut hb_buffer_t) -> ShapedGlyphData {
+    pub unsafe fn new(buffer: *mut hb_buffer_t) -> ShapedGlyphData {
-        unsafe {
         let mut glyph_count = 0;
         let glyph_infos = hb_buffer_get_glyph_infos(buffer, &mut glyph_count);
         assert!(!glyph_infos.is_null());
@@ -59,9 +58,8 @@ impl ShapedGlyphData {
 
         ShapedGlyphData {
             count: glyph_count as usize,
-                glyph_infos: glyph_infos,
+            glyph_infos,
-                pos_infos: pos_infos,
+            pos_infos,
-            }
         }
     }
 
@@ -70,7 +68,7 @@ impl ShapedGlyphData {
         assert!(i < self.count);
 
         unsafe {
-            let glyph_info_i = self.glyph_infos.offset(i as isize);
+            let glyph_info_i = self.glyph_infos.add(i);
             (*glyph_info_i).cluster
         }
     }
@@ -79,13 +77,17 @@ impl ShapedGlyphData {
         self.count
     }
 
+    pub fn is_empty(&self) -> bool {
+        self.count == 0
+    }
+
     /// Returns shaped glyph data for one glyph, and updates the y-position of the pen.
     pub fn entry_for_glyph(&self, i: usize, y_pos: &mut Au) -> ShapedGlyphEntry {
         assert!(i < self.count);
 
         unsafe {
-            let glyph_info_i = self.glyph_infos.offset(i as isize);
+            let glyph_info_i = self.glyph_infos.add(i);
-            let pos_info_i = self.pos_infos.offset(i as isize);
+            let pos_info_i = self.pos_infos.add(i);
             let x_offset = Shaper::fixed_to_float((*pos_info_i).x_offset);
             let y_offset = Shaper::fixed_to_float((*pos_info_i).y_offset);
             let x_advance = Shaper::fixed_to_float((*pos_info_i).x_advance);
@@ -101,7 +103,7 @@ impl ShapedGlyphData {
             } else {
                 // adjust the pen..
                 if y_advance > Au(0) {
-                    *y_pos = *y_pos - y_advance;
+                    *y_pos -= y_advance;
                 }
 
                 Some(Point2D::new(x_offset, *y_pos - y_offset))
@@ -110,7 +112,7 @@ impl ShapedGlyphData {
             ShapedGlyphEntry {
                 codepoint: (*glyph_info_i).codepoint as GlyphId,
                 advance: x_advance,
-                offset: offset,
+                offset,
             }
         }
     }
@@ -136,6 +138,7 @@ impl Drop for Shaper {
 }
 
 impl Shaper {
+    #[allow(clippy::not_unsafe_ptr_arg_deref)] // Has an unsafe block inside
     pub fn new(font: *const Font) -> Shaper {
         unsafe {
             let hb_face: *mut hb_face_t = hb_face_create_for_tables(
@@ -165,9 +168,9 @@ impl Shaper {
             );
 
             Shaper {
-                hb_face: hb_face,
+                hb_face,
-                hb_font: hb_font,
+                hb_font,
-                font: font,
+                font,
             }
         }
     }
@@ -415,7 +418,7 @@ impl Shaper {
         glyphs: &mut GlyphStore,
         buffer: *mut hb_buffer_t,
     ) {
-        let glyph_data = ShapedGlyphData::new(buffer);
+        let glyph_data = unsafe { ShapedGlyphData::new(buffer) };
         let glyph_count = glyph_data.len();
         let byte_max = text.len();
 
@@ -485,7 +488,7 @@ impl Shaper {
                 }
 
                 // if no glyphs were found yet, extend the char byte range more.
-                if glyph_span.len() == 0 {
+                if glyph_span.is_empty() {
                     continue;
                 }
 
@@ -508,8 +511,8 @@ impl Shaper {
                 // span or reach the end of the text.
             }
 
-            assert!(byte_range.len() > 0);
+            assert!(!byte_range.is_empty());
-            assert!(glyph_span.len() > 0);
+            assert!(!glyph_span.is_empty());
 
             // Now byte_range is the ligature clump formed by the glyphs in glyph_span.
             // We will save these glyphs to the glyph store at the index of the first byte.
@@ -580,7 +583,7 @@ impl Shaper {
         options: &ShapingOptions,
     ) -> Au {
         if let Some(letter_spacing) = options.letter_spacing {
-            advance = advance + letter_spacing;
+            advance += letter_spacing;
         };
 
         // CSS 2.1 § 16.4 states that "word spacing affects each space (U+0020) and non-breaking
@@ -654,13 +657,10 @@ extern "C" fn glyph_h_advance_func(
 
 fn glyph_space_advance(font: *const Font) -> (hb_codepoint_t, f64) {
     let space_unicode = ' ';
-    let space_glyph: hb_codepoint_t;
+    let space_glyph: hb_codepoint_t = match unsafe { (*font).glyph_index(space_unicode) } {
-    match unsafe { (*font).glyph_index(space_unicode) } {
+        Some(g) => g as hb_codepoint_t,
-        Some(g) => {
-            space_glyph = g as hb_codepoint_t;
-        },
         None => panic!("No space info"),
-    }
+    };
     let space_advance = unsafe { (*font).glyph_h_advance(space_glyph as GlyphId) };
     (space_glyph, space_advance)
 }

components/gfx/text/text_run.rs

@@ -129,7 +129,7 @@ impl<'a> Iterator for NaturalWordSliceIterator<'a> {
 
         if !byte_range.is_empty() {
             Some(TextRunSlice {
-                glyphs: &*slice_glyphs.glyph_store,
+                glyphs: &slice_glyphs.glyph_store,
                 offset: slice_range_begin,
                 range: byte_range,
             })
@@ -172,7 +172,7 @@ impl<'a> Iterator for CharacterSliceIterator<'a> {
 
         let index_within_glyph_run = byte_start - glyph_run.range.begin();
         Some(TextRunSlice {
-            glyphs: &*glyph_run.glyph_store,
+            glyphs: &glyph_run.glyph_store,
             offset: glyph_run.range.begin(),
             range: Range::new(index_within_glyph_run, byte_len),
         })
@@ -197,7 +197,7 @@ impl<'a> TextRun {
                 font_key: font.font_key,
                 pt_size: font.descriptor.pt_size,
                 glyphs: Arc::new(glyphs),
-                bidi_level: bidi_level,
+                bidi_level,
                 extra_word_spacing: Au(0),
             },
             break_at_zero,
@@ -217,7 +217,7 @@ impl<'a> TextRun {
         let mut break_at_zero = false;
 
         if breaker.is_none() {
-            if text.len() == 0 {
+            if text.is_empty() {
                 return (glyphs, true);
             }
             *breaker = Some(LineBreakLeafIter::new(text, 0));
@@ -253,7 +253,7 @@ impl<'a> TextRun {
                 // keep-all, try increasing the slice.
                 continue;
             }
-            if slice.len() > 0 {
+            if !slice.is_empty() {
                 glyphs.push(GlyphRun {
                     glyph_store: font.shape_text(&text[slice.clone()], options),
                     range: Range::new(
@@ -262,8 +262,8 @@ impl<'a> TextRun {
                     ),
                 });
             }
-            if whitespace.len() > 0 {
+            if !whitespace.is_empty() {
-                let mut options = options.clone();
+                let mut options = *options;
                 options
                     .flags
                     .insert(ShapingFlags::IS_WHITESPACE_SHAPING_FLAG);
@@ -348,7 +348,9 @@ impl<'a> TextRun {
                 }
             }
 
-            if let Ok(result) = (&**self.glyphs).binary_search_by(|current| current.compare(&index))
+            if let Ok(result) = self
+                .glyphs
+                .binary_search_by(|current| current.compare(&index))
             {
                 index_of_first_glyph_run_cache.set(Some((self_ptr, index, result)));
                 Some(result)
@@ -396,7 +398,7 @@ impl<'a> TextRun {
         };
         NaturalWordSliceIterator {
             glyphs: &self.glyphs[..],
-            index: index,
+            index,
             range: *range,
             reverse: false,
         }
@@ -424,9 +426,9 @@ impl<'a> TextRun {
         };
         NaturalWordSliceIterator {
             glyphs: &self.glyphs[..],
-            index: index,
+            index,
             range: *range,
-            reverse: reverse,
+            reverse,
         }
     }
 
@@ -445,7 +447,7 @@ impl<'a> TextRun {
         CharacterSliceIterator {
             text: &self.text,
             glyph_run: first_glyph_run,
-            glyph_run_iter: glyph_run_iter,
+            glyph_run_iter,
             range: *range,
         }
     }

components/gfx/text/util.rs

@@ -113,12 +113,7 @@ pub fn fixed_to_float(before: usize, f: i32) -> f64 {
 }
 
 pub fn is_bidi_control(c: char) -> bool {
-    match c {
+    matches!(c, '\u{202A}'..='\u{202E}' | '\u{2066}'..='\u{2069}' | '\u{200E}' | '\u{200F}' | '\u{061C}')
-        '\u{202A}'..='\u{202E}' => true,
-        '\u{2066}'..='\u{2069}' => true,
-        '\u{200E}' | '\u{200F}' | '\u{061C}' => true,
-        _ => false,
-    }
 }
 
 pub fn unicode_plane(codepoint: char) -> u32 {