Bug 1621773 - Gracefully handle errors creating shared memory UA style sheets. r=emilio

We still panic in a debug build, so that developers can notice when they
need to add a new static atom after modifying UA sheets.

We also add telemetry to note when this happens, add an app note to a
crash report, in case any crash later on occurs, and re-up the existing,
expired shared memory sheet telemetry probes so we can look at them
again.

Differential Revision: https://phabricator.services.mozilla.com/D73188

layout/style/GlobalStyleSheetCache.cpp

@@ -386,17 +386,33 @@ void GlobalStyleSheetCache::InitSharedSheetsInParent() {
       Servo_SharedMemoryBuilder_Create(
           header->mBuffer, kSharedMemorySize - offsetof(Header, mBuffer)));
 
+  nsCString message;
+
   // Copy each one into the shared memory, and record its pointer.
-#define STYLE_SHEET(identifier_, url_, shared_)            \
-  if (shared_) {                                           \
-    StyleSheet* sheet = identifier_##Sheet();              \
-    size_t i = size_t(UserAgentStyleSheetID::identifier_); \
-    URLExtraData::sShared[i] = sheet->URLData();           \
-    header->mSheets[i] = sheet->ToShared(builder.get());   \
+  //
+  // Normally calling ToShared on UA sheets should not fail.  It happens
+  // in practice in odd cases that seem like corrupted installations; see bug
+  // 1621773.  On failure, return early and fall back to non-shared sheets.
+#define STYLE_SHEET(identifier_, url_, shared_)                             \
+  if (shared_) {                                                            \
+    StyleSheet* sheet = identifier_##Sheet();                               \
+    size_t i = size_t(UserAgentStyleSheetID::identifier_);                  \
+    URLExtraData::sShared[i] = sheet->URLData();                            \
+    header->mSheets[i] = sheet->ToShared(builder.get(), message);           \
+    if (!header->mSheets[i]) {                                              \
+      CrashReporter::AppendAppNotesToCrashReport(NS_LITERAL_CSTRING("\n") + \
+                                                 message);                  \
+      Telemetry::Accumulate(                                                \
+          Telemetry::SHARED_MEMORY_UA_SHEETS_TOSHMEM_SUCCEEDED, false);     \
+      return;                                                               \
+    }                                                                       \
   }
 #include "mozilla/UserAgentStyleSheetList.h"
 #undef STYLE_SHEET
 
+  Telemetry::Accumulate(Telemetry::SHARED_MEMORY_UA_SHEETS_TOSHMEM_SUCCEEDED,
+                        true);
+
   // Finished writing into the shared memory.  Freeze it, so that a process
   // can't confuse other processes by changing the UA style sheet contents.
   if (NS_WARN_IF(!shm->Freeze())) {

layout/style/StyleSheet.cpp

@@ -1414,8 +1414,8 @@ void StyleSheet::SetSharedContents(const ServoCssRules* aSharedRules) {
   // which we don't have.
 }
 
-const ServoCssRules* StyleSheet::ToShared(
-    RawServoSharedMemoryBuilder* aBuilder) {
+const ServoCssRules* StyleSheet::ToShared(RawServoSharedMemoryBuilder* aBuilder,
+                                          nsCString& aErrorMessage) {
   // Assert some things we assume when creating a StyleSheet using shared
   // memory.
   MOZ_ASSERT(GetReferrerInfo()->ReferrerPolicy() == ReferrerPolicy::_empty);
@@ -1425,7 +1425,18 @@ const ServoCssRules* StyleSheet::ToShared(
   MOZ_ASSERT(Inner().mIntegrity.IsEmpty());
   MOZ_ASSERT(Principal()->IsSystemPrincipal());
 
-  return Servo_SharedMemoryBuilder_AddStylesheet(aBuilder, Inner().mContents);
+  const ServoCssRules* rules = Servo_SharedMemoryBuilder_AddStylesheet(
+      aBuilder, Inner().mContents, &aErrorMessage);
+
+#ifdef DEBUG
+  if (!rules) {
+    // Print the ToShmem error message so that developers know what to fix.
+    printf_stderr("%s\n", aErrorMessage.get());
+    MOZ_CRASH("UA style sheet contents failed shared memory requirements");
+  }
+#endif
+
+  return rules;
 }
 
 bool StyleSheet::IsReadOnly() const {

layout/style/StyleSheet.h

@@ -441,8 +441,10 @@ class StyleSheet final : public nsICSSLoaderObserver, public nsWrapperCache {
 
   // Copy the contents of this style sheet into the shared memory buffer managed
   // by aBuilder.  Returns the pointer into the buffer that the sheet contents
-  // were stored at.  (The returned pointer is to an Arc<Locked<Rules>> value.)
-  const ServoCssRules* ToShared(RawServoSharedMemoryBuilder* aBuilder);
+  // were stored at.  (The returned pointer is to an Arc<Locked<Rules>> value,
+  // or null, with a filled in aErrorMessage, on failure.)
+  const ServoCssRules* ToShared(RawServoSharedMemoryBuilder* aBuilder,
+                                nsCString& aErrorMessage);
 
   // Sets the contents of this style sheet to the specified aSharedRules
   // pointer, which must be a pointer somewhere in the aSharedMemory buffer

servo/components/derive_common/cg.rs

@@ -85,9 +85,22 @@ pub fn add_predicate(where_clause: &mut Option<syn::WhereClause>, pred: WherePre
         .push(pred);
 }
 
-pub fn fmap_match<F>(input: &DeriveInput, bind_style: BindStyle, mut f: F) -> TokenStream
+pub fn fmap_match<F>(input: &DeriveInput, bind_style: BindStyle, f: F) -> TokenStream
 where
-    F: FnMut(BindingInfo) -> TokenStream,
+    F: FnMut(&BindingInfo) -> TokenStream,
+{
+    fmap2_match(input, bind_style, f, |_| None)
+}
+
+pub fn fmap2_match<F, G>(
+    input: &DeriveInput,
+    bind_style: BindStyle,
+    mut f: F,
+    mut g: G,
+) -> TokenStream
+where
+    F: FnMut(&BindingInfo) -> TokenStream,
+    G: FnMut(&BindingInfo) -> Option<TokenStream>,
 {
     let mut s = synstructure::Structure::new(input);
     s.variants_mut().iter_mut().for_each(|v| {
@@ -95,12 +108,20 @@ where
     });
     s.each_variant(|variant| {
         let (mapped, mapped_fields) = value(variant, "mapped");
-        let fields_pairs = variant.bindings().iter().zip(mapped_fields);
+        let fields_pairs = variant.bindings().iter().zip(mapped_fields.iter());
         let mut computations = quote!();
         computations.append_all(fields_pairs.map(|(field, mapped_field)| {
-            let expr = f(field.clone());
+            let expr = f(field);
             quote! { let #mapped_field = #expr; }
         }));
+        computations.append_all(
+            mapped_fields
+                .iter()
+                .map(|mapped_field| match g(mapped_field) {
+                    Some(expr) => quote! { let #mapped_field = #expr; },
+                    None => quote!(),
+                }),
+        );
         computations.append_all(mapped);
         Some(computations)
     })

servo/components/style/gecko/url.rs

@@ -18,7 +18,7 @@ use std::fmt::{self, Write};
 use std::mem::ManuallyDrop;
 use std::sync::RwLock;
 use style_traits::{CssWriter, ParseError, ToCss};
-use to_shmem::{SharedMemoryBuilder, ToShmem};
+use to_shmem::{self, SharedMemoryBuilder, ToShmem};
 
 /// A CSS url() value for gecko.
 #[css(function = "url")]
@@ -241,11 +241,11 @@ impl LoadDataSource {
 }
 
 impl ToShmem for LoadDataSource {
-    fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
-        ManuallyDrop::new(match self {
+    fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> to_shmem::Result<Self> {
+        Ok(ManuallyDrop::new(match self {
             LoadDataSource::Owned(..) => LoadDataSource::Lazy,
             LoadDataSource::Lazy => LoadDataSource::Lazy,
-        })
+        }))
     }
 }
 

servo/components/style/gecko_string_cache/mod.rs

@@ -3,7 +3,6 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 #![allow(unsafe_code)]
-
 // This is needed for the constants in atom_macro.rs, because we have some
 // atoms whose names differ only by case, e.g. datetime and dateTime.
 #![allow(non_upper_case_globals)]
@@ -30,7 +29,7 @@ use std::num::NonZeroUsize;
 use std::ops::Deref;
 use std::{slice, str};
 use style_traits::SpecifiedValueInfo;
-use to_shmem::{SharedMemoryBuilder, ToShmem};
+use to_shmem::{self, SharedMemoryBuilder, ToShmem};
 
 #[macro_use]
 #[allow(improper_ctypes, non_camel_case_types, missing_docs)]
@@ -132,14 +131,15 @@ impl Borrow<WeakAtom> for Atom {
 }
 
 impl ToShmem for Atom {
-    fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
-        assert!(
-            self.is_static(),
-            "ToShmem failed for Atom: must be a static atom: {}",
-            self
-        );
+    fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> to_shmem::Result<Self> {
+        if !self.is_static() {
+            return Err(format!(
+                "ToShmem failed for Atom: must be a static atom: {}",
+                self
+            ));
+        }
 
-        ManuallyDrop::new(Atom(self.0))
+        Ok(ManuallyDrop::new(Atom(self.0)))
     }
 }
 

servo/components/style/shared_lock.rs

@@ -258,20 +258,20 @@ impl<T> Locked<T> {
 
 #[cfg(feature = "gecko")]
 impl<T: ToShmem> ToShmem for Locked<T> {
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> to_shmem::Result<Self> {
         let guard = self.shared_lock.read();
-        ManuallyDrop::new(Locked {
+        Ok(ManuallyDrop::new(Locked {
             shared_lock: SharedRwLock::read_only(),
             data: UnsafeCell::new(ManuallyDrop::into_inner(
-                self.read_with(&guard).to_shmem(builder),
+                self.read_with(&guard).to_shmem(builder)?,
             )),
-        })
+        }))
     }
 }
 
 #[cfg(feature = "servo")]
 impl<T: ToShmem> ToShmem for Locked<T> {
-    fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
+    fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> to_shmem::Result<Self> {
         panic!("ToShmem not supported in Servo currently")
     }
 }

servo/components/style/stylesheets/import_rule.rs

@@ -15,9 +15,8 @@ use crate::stylesheets::{CssRule, Origin, StylesheetInDocument};
 use crate::values::CssUrl;
 use cssparser::SourceLocation;
 use std::fmt::{self, Write};
-use std::mem::ManuallyDrop;
 use style_traits::{CssWriter, ToCss};
-use to_shmem::{SharedMemoryBuilder, ToShmem};
+use to_shmem::{self, SharedMemoryBuilder, ToShmem};
 
 /// With asynchronous stylesheet parsing, we can't synchronously create a
 /// GeckoStyleSheet. So we use this placeholder instead.
@@ -184,8 +183,10 @@ pub struct ImportRule {
 }
 
 impl ToShmem for ImportRule {
-    fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
-        panic!("ToShmem failed for ImportRule: cannot handle imported style sheets")
+    fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> to_shmem::Result<Self> {
+        Err(String::from(
+            "ToShmem failed for ImportRule: cannot handle imported style sheets",
+        ))
     }
 }
 

servo/components/style/stylesheets/mod.rs

@@ -40,7 +40,7 @@ use std::fmt;
 use std::mem::{self, ManuallyDrop};
 use style_traits::ParsingMode;
 #[cfg(feature = "gecko")]
-use to_shmem::{SharedMemoryBuilder, ToShmem};
+use to_shmem::{self, SharedMemoryBuilder, ToShmem};
 
 pub use self::counter_style_rule::CounterStyleRule;
 pub use self::document_rule::DocumentRule;
@@ -117,20 +117,25 @@ impl Drop for UrlExtraData {
 
 #[cfg(feature = "gecko")]
 impl ToShmem for UrlExtraData {
-    fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
+    fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> to_shmem::Result<Self> {
         if self.0 & 1 == 0 {
             let shared_extra_datas = unsafe { &structs::URLExtraData_sShared };
             let self_ptr = self.as_ref() as *const _ as *mut _;
             let sheet_id = shared_extra_datas
                 .iter()
-                .position(|r| r.mRawPtr == self_ptr)
-                .expect(
-                    "ToShmem failed for UrlExtraData: expected sheet's URLExtraData to be in \
-                     URLExtraData::sShared",
-                );
-            ManuallyDrop::new(UrlExtraData((sheet_id << 1) | 1))
+                .position(|r| r.mRawPtr == self_ptr);
+            let sheet_id = match sheet_id {
+                Some(id) => id,
+                None => {
+                    return Err(String::from(
+                        "ToShmem failed for UrlExtraData: expected sheet's URLExtraData to be in \
+                         URLExtraData::sShared",
+                    ));
+                },
+            };
+            Ok(ManuallyDrop::new(UrlExtraData((sheet_id << 1) | 1)))
         } else {
-            ManuallyDrop::new(UrlExtraData(self.0))
+            Ok(ManuallyDrop::new(UrlExtraData(self.0)))
         }
     }
 }

servo/components/style/values/computed/font.rs

@@ -32,7 +32,7 @@ use std::mem::{self, ManuallyDrop};
 use std::slice;
 use style_traits::{CssWriter, ParseError, ToCss};
 #[cfg(feature = "gecko")]
-use to_shmem::{SharedMemoryBuilder, ToShmem};
+use to_shmem::{self, SharedMemoryBuilder, ToShmem};
 
 pub use crate::values::computed::Length as MozScriptMinSize;
 pub use crate::values::specified::font::{FontSynthesis, MozScriptSizeMultiplier};
@@ -466,19 +466,20 @@ pub enum FontFamilyList {
 
 #[cfg(feature = "gecko")]
 impl ToShmem for FontFamilyList {
-    fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
+    fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> to_shmem::Result<Self> {
         // In practice, the only SharedFontList objects we create from shared
         // style sheets are ones with a single generic entry.
-        ManuallyDrop::new(match *self {
+        Ok(ManuallyDrop::new(match *self {
             FontFamilyList::SharedFontList(ref r) => {
-                assert!(
-                    r.mNames.len() == 1 && r.mNames[0].mName.mRawPtr.is_null(),
-                    "ToShmem failed for FontFamilyList: cannot handle non-generic families",
-                );
+                if !(r.mNames.len() == 1 && r.mNames[0].mName.mRawPtr.is_null()) {
+                    return Err(String::from(
+                        "ToShmem failed for FontFamilyList: cannot handle non-generic families",
+                    ));
+                }
                 FontFamilyList::Generic(r.mNames[0].mGeneric)
             },
             FontFamilyList::Generic(t) => FontFamilyList::Generic(t),
-        })
+        }))
     }
 }
 

servo/components/style_traits/owned_slice.rs

@@ -13,7 +13,7 @@ use std::marker::PhantomData;
 use std::ops::{Deref, DerefMut};
 use std::ptr::NonNull;
 use std::{fmt, iter, mem, slice};
-use to_shmem::{SharedMemoryBuilder, ToShmem};
+use to_shmem::{self, SharedMemoryBuilder, ToShmem};
 
 /// A struct that basically replaces a `Box<[T]>`, but which cbindgen can
 /// understand.
@@ -159,10 +159,10 @@ impl<T: MallocSizeOf + Sized> MallocSizeOf for OwnedSlice<T> {
 }
 
 impl<T: ToShmem + Sized> ToShmem for OwnedSlice<T> {
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> mem::ManuallyDrop<Self> {
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> to_shmem::Result<Self> {
         unsafe {
-            let dest = to_shmem::to_shmem_slice(self.iter(), builder);
-            mem::ManuallyDrop::new(Self::from(Box::from_raw(dest)))
+            let dest = to_shmem::to_shmem_slice(self.iter(), builder)?;
+            Ok(mem::ManuallyDrop::new(Self::from(Box::from_raw(dest))))
         }
     }
 }

servo/components/to_shmem/lib.rs

@@ -42,6 +42,11 @@ use std::slice;
 use std::str;
 use thin_slice::ThinBoxedSlice;
 
+/// Result type for ToShmem::to_shmem.
+///
+/// The String is an error message describing why the call failed.
+pub type Result<T> = std::result::Result<ManuallyDrop<T>, String>;
+
 // Various pointer arithmetic functions in this file can be replaced with
 // functions on `Layout` once they have stabilized:
 //
@@ -114,13 +119,13 @@ impl SharedMemoryBuilder {
     /// a shared memory buffer by calling ToShmem::to_shmem on it.
     ///
     /// Panics if there is insufficient space in the buffer.
-    pub fn write<T: ToShmem>(&mut self, value: &T) -> *mut T {
+    pub fn write<T: ToShmem>(&mut self, value: &T) -> std::result::Result<*mut T, String> {
         // Reserve space for the value.
         let dest: *mut T = self.alloc_value();
 
         // Make a clone of the value with all of its heap allocations
         // placed in the shared memory buffer.
-        let value = value.to_shmem(self);
+        let value = value.to_shmem(self)?;
 
         unsafe {
             // Copy the value into the buffer.
@@ -128,7 +133,7 @@ impl SharedMemoryBuilder {
         }
 
         // Return a pointer to the shared value.
-        dest
+        Ok(dest)
     }
 
     /// Reserves space in the shared memory buffer to fit a value of type T,
@@ -190,7 +195,10 @@ pub trait ToShmem: Sized {
     ///
     /// The return type is wrapped in ManuallyDrop to make it harder to
     /// accidentally invoke the destructor of the value that is produced.
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self>;
+    ///
+    /// Returns a Result so that we can gracefully recover from unexpected
+    /// content.
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result<Self>;
 }
 
 #[macro_export]
@@ -201,8 +209,8 @@ macro_rules! impl_trivial_to_shmem {
                 fn to_shmem(
                     &self,
                     _builder: &mut $crate::SharedMemoryBuilder,
-                ) -> ::std::mem::ManuallyDrop<Self> {
-                    ::std::mem::ManuallyDrop::new(*self)
+                ) -> $crate::Result<Self> {
+                    $crate::Result::Ok(::std::mem::ManuallyDrop::new(*self))
                 }
             }
         )*
@@ -231,58 +239,60 @@ impl_trivial_to_shmem!(cssparser::SourceLocation);
 impl_trivial_to_shmem!(cssparser::TokenSerializationType);
 
 impl<T> ToShmem for PhantomData<T> {
-    fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
-        ManuallyDrop::new(*self)
+    fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> Result<Self> {
+        Ok(ManuallyDrop::new(*self))
     }
 }
 
 impl<T: ToShmem> ToShmem for Range<T> {
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
-        ManuallyDrop::new(Range {
-            start: ManuallyDrop::into_inner(self.start.to_shmem(builder)),
-            end: ManuallyDrop::into_inner(self.end.to_shmem(builder)),
-        })
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result<Self> {
+        Ok(ManuallyDrop::new(Range {
+            start: ManuallyDrop::into_inner(self.start.to_shmem(builder)?),
+            end: ManuallyDrop::into_inner(self.end.to_shmem(builder)?),
+        }))
     }
 }
 
 impl ToShmem for cssparser::UnicodeRange {
-    fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
-        ManuallyDrop::new(cssparser::UnicodeRange {
+    fn to_shmem(&self, _builder: &mut SharedMemoryBuilder) -> Result<Self> {
+        Ok(ManuallyDrop::new(cssparser::UnicodeRange {
             start: self.start,
             end: self.end,
-        })
+        }))
     }
 }
 
 impl<T: ToShmem, U: ToShmem> ToShmem for (T, U) {
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
-        ManuallyDrop::new((
-            ManuallyDrop::into_inner(self.0.to_shmem(builder)),
-            ManuallyDrop::into_inner(self.1.to_shmem(builder)),
-        ))
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result<Self> {
+        Ok(ManuallyDrop::new((
+            ManuallyDrop::into_inner(self.0.to_shmem(builder)?),
+            ManuallyDrop::into_inner(self.1.to_shmem(builder)?),
+        )))
     }
 }
 
 impl<T: ToShmem> ToShmem for Wrapping<T> {
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
-        ManuallyDrop::new(Wrapping(ManuallyDrop::into_inner(self.0.to_shmem(builder))))
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result<Self> {
+        Ok(ManuallyDrop::new(Wrapping(ManuallyDrop::into_inner(
+            self.0.to_shmem(builder)?,
+        ))))
     }
 }
 
 impl<T: ToShmem> ToShmem for Box<T> {
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result<Self> {
         // Reserve space for the boxed value.
         let dest: *mut T = builder.alloc_value();
 
         // Make a clone of the boxed value with all of its heap allocations
         // placed in the shared memory buffer.
-        let value = (**self).to_shmem(builder);
+        let value = (**self).to_shmem(builder)?;
 
         unsafe {
             // Copy the value into the buffer.
             ptr::write(dest, ManuallyDrop::into_inner(value));
 
-            ManuallyDrop::new(Box::from_raw(dest))
+            Ok(ManuallyDrop::new(Box::from_raw(dest)))
         }
     }
 }
@@ -293,7 +303,7 @@ unsafe fn to_shmem_slice_ptr<'a, T, I>(
     src: I,
     dest: *mut T,
     builder: &mut SharedMemoryBuilder,
-) -> *mut [T]
+) -> std::result::Result<*mut [T], String>
 where
     T: 'a + ToShmem,
     I: ExactSizeIterator<Item = &'a T>,
@@ -303,15 +313,18 @@ where
     // Make a clone of each element from the iterator with its own heap
     // allocations placed in the buffer, and copy that clone into the buffer.
     for (src, dest) in src.zip(dest.iter_mut()) {
-        ptr::write(dest, ManuallyDrop::into_inner(src.to_shmem(builder)));
+        ptr::write(dest, ManuallyDrop::into_inner(src.to_shmem(builder)?));
     }
 
-    dest
+    Ok(dest)
 }
 
 /// Writes all the items in `src` into a slice in the shared memory buffer and
 /// returns a pointer to the slice.
-pub unsafe fn to_shmem_slice<'a, T, I>(src: I, builder: &mut SharedMemoryBuilder) -> *mut [T]
+pub unsafe fn to_shmem_slice<'a, T, I>(
+    src: I,
+    builder: &mut SharedMemoryBuilder,
+) -> std::result::Result<*mut [T], String>
 where
     T: 'a + ToShmem,
     I: ExactSizeIterator<Item = &'a T>,
@@ -321,16 +334,16 @@ where
 }
 
 impl<T: ToShmem> ToShmem for Box<[T]> {
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result<Self> {
         unsafe {
-            let dest = to_shmem_slice(self.iter(), builder);
-            ManuallyDrop::new(Box::from_raw(dest))
+            let dest = to_shmem_slice(self.iter(), builder)?;
+            Ok(ManuallyDrop::new(Box::from_raw(dest)))
         }
     }
 }
 
 impl<T: ToShmem> ToShmem for ThinBoxedSlice<T> {
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result<Self> {
         // We could support this if we needed but in practice we will never
         // need to handle such big ThinBoxedSlices.
         assert!(
@@ -340,14 +353,14 @@ impl<T: ToShmem> ToShmem for ThinBoxedSlice<T> {
         );
 
         unsafe {
-            let dest = to_shmem_slice(self.iter(), builder);
-            ManuallyDrop::new(ThinBoxedSlice::from_raw(dest))
+            let dest = to_shmem_slice(self.iter(), builder)?;
+            Ok(ManuallyDrop::new(ThinBoxedSlice::from_raw(dest)))
         }
     }
 }
 
 impl ToShmem for Box<str> {
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result<Self> {
         // Reserve space for the string bytes.
         let dest: *mut u8 = builder.alloc_array(self.len());
 
@@ -355,15 +368,15 @@ impl ToShmem for Box<str> {
             // Copy the value into the buffer.
             ptr::copy(self.as_ptr(), dest, self.len());
 
-            ManuallyDrop::new(Box::from_raw(str::from_utf8_unchecked_mut(
-                slice::from_raw_parts_mut(dest, self.len()),
+            Ok(ManuallyDrop::new(Box::from_raw(
+                str::from_utf8_unchecked_mut(slice::from_raw_parts_mut(dest, self.len())),
             )))
         }
     }
 }
 
 impl ToShmem for String {
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result<Self> {
         // Reserve space for the string bytes.
         let dest: *mut u8 = builder.alloc_array(self.len());
 
@@ -371,13 +384,17 @@ impl ToShmem for String {
             // Copy the value into the buffer.
             ptr::copy(self.as_ptr(), dest, self.len());
 
-            ManuallyDrop::new(String::from_raw_parts(dest, self.len(), self.len()))
+            Ok(ManuallyDrop::new(String::from_raw_parts(
+                dest,
+                self.len(),
+                self.len(),
+            )))
         }
     }
 }
 
 impl ToShmem for CString {
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result<Self> {
         let len = self.as_bytes_with_nul().len();
 
         // Reserve space for the string bytes.
@@ -387,53 +404,55 @@ impl ToShmem for CString {
             // Copy the value into the buffer.
             ptr::copy(self.as_ptr(), dest, len);
 
-            ManuallyDrop::new(CString::from_raw(dest))
+            Ok(ManuallyDrop::new(CString::from_raw(dest)))
         }
     }
 }
 
 impl<T: ToShmem> ToShmem for Vec<T> {
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result<Self> {
         unsafe {
-            let dest = to_shmem_slice(self.iter(), builder) as *mut T;
+            let dest = to_shmem_slice(self.iter(), builder)? as *mut T;
             let dest_vec = Vec::from_raw_parts(dest, self.len(), self.len());
-            ManuallyDrop::new(dest_vec)
+            Ok(ManuallyDrop::new(dest_vec))
         }
     }
 }
 
 impl<T: ToShmem, A: Array<Item = T>> ToShmem for SmallVec<A> {
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result<Self> {
         let dest_vec = unsafe {
             if self.spilled() {
                 // Place the items in a separate allocation in the shared memory
                 // buffer.
-                let dest = to_shmem_slice(self.iter(), builder) as *mut T;
+                let dest = to_shmem_slice(self.iter(), builder)? as *mut T;
                 SmallVec::from_raw_parts(dest, self.len(), self.len())
             } else {
                 // Place the items inline.
                 let mut s = SmallVec::new();
-                to_shmem_slice_ptr(self.iter(), s.as_mut_ptr(), builder);
+                to_shmem_slice_ptr(self.iter(), s.as_mut_ptr(), builder)?;
                 s.set_len(self.len());
                 s
             }
         };
 
-        ManuallyDrop::new(dest_vec)
+        Ok(ManuallyDrop::new(dest_vec))
     }
 }
 
 impl<T: ToShmem> ToShmem for Option<T> {
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
-        ManuallyDrop::new(
-            self.as_ref()
-                .map(|v| ManuallyDrop::into_inner(v.to_shmem(builder))),
-        )
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result<Self> {
+        let v = match self {
+            Some(v) => Some(ManuallyDrop::into_inner(v.to_shmem(builder)?)),
+            None => None,
+        };
+
+        Ok(ManuallyDrop::new(v))
     }
 }
 
 impl<T: 'static + ToShmem> ToShmem for Arc<T> {
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result<Self> {
         // Assert that we don't encounter any shared references to values we
         // don't expect.  Those we expect are those noted by calling
         // add_allowed_duplication_type, and should be types where we're fine
@@ -453,7 +472,7 @@ impl<T: 'static + ToShmem> ToShmem for Arc<T> {
 
         // Make a clone of the Arc-owned value with all of its heap allocations
         // placed in the shared memory buffer.
-        let value = (**self).to_shmem(builder);
+        let value = (**self).to_shmem(builder)?;
 
         // Create a new Arc with the shared value and have it place its
         // ArcInner in the shared memory buffer.
@@ -466,13 +485,13 @@ impl<T: 'static + ToShmem> ToShmem for Arc<T> {
             #[cfg(debug_assertions)]
             builder.shared_values.insert(self.heap_ptr());
 
-            ManuallyDrop::new(static_arc)
+            Ok(ManuallyDrop::new(static_arc))
         }
     }
 }
 
 impl<H: 'static + ToShmem, T: 'static + ToShmem> ToShmem for ThinArc<H, T> {
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result<Self> {
         // We don't currently have any shared ThinArc values in stylesheets,
         // so don't support them for now.
         #[cfg(debug_assertions)]
@@ -484,8 +503,11 @@ impl<H: 'static + ToShmem, T: 'static + ToShmem> ToShmem for ThinArc<H, T> {
 
         // Make a clone of the Arc-owned header and slice values with all of
         // their heap allocations placed in the shared memory buffer.
-        let header = self.header.header.to_shmem(builder);
-        let values: Vec<ManuallyDrop<T>> = self.slice.iter().map(|v| v.to_shmem(builder)).collect();
+        let header = self.header.header.to_shmem(builder)?;
+        let mut values = Vec::with_capacity(self.slice.len());
+        for v in self.slice.iter() {
+            values.push(v.to_shmem(builder)?);
+        }
 
         // Create a new ThinArc with the shared value and have it place
         // its ArcInner in the shared memory buffer.
@@ -499,13 +521,13 @@ impl<H: 'static + ToShmem, T: 'static + ToShmem> ToShmem for ThinArc<H, T> {
             #[cfg(debug_assertions)]
             builder.shared_values.insert(self.heap_ptr());
 
-            ManuallyDrop::new(static_arc)
+            Ok(ManuallyDrop::new(static_arc))
         }
     }
 }
 
 impl ToShmem for SmallBitVec {
-    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
+    fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> Result<Self> {
         let storage = match self.clone().into_storage() {
             InternalStorage::Spilled(vs) => {
                 // Reserve space for the boxed slice values.
@@ -524,13 +546,15 @@ impl ToShmem for SmallBitVec {
             },
             InternalStorage::Inline(x) => InternalStorage::Inline(x),
         };
-        ManuallyDrop::new(unsafe { SmallBitVec::from_storage(storage) })
+        Ok(ManuallyDrop::new(unsafe {
+            SmallBitVec::from_storage(storage)
+        }))
     }
 }
 
 #[cfg(feature = "string_cache")]
 impl<Static: string_cache::StaticAtomSet> ToShmem for string_cache::Atom<Static> {
-    fn to_shmem(&self, _: &mut SharedMemoryBuilder) -> ManuallyDrop<Self> {
+    fn to_shmem(&self, _: &mut SharedMemoryBuilder) -> Result<Self> {
         // NOTE(emilio): In practice, this can be implemented trivially if
         // string_cache could expose the implementation detail of static atoms
         // being an index into the static table (and panicking in the

servo/components/to_shmem_derive/to_shmem.rs

@@ -27,13 +27,23 @@ pub fn derive(mut input: syn::DeriveInput) -> TokenStream {
 
     input.generics.where_clause = where_clause;
 
-    let match_body = cg::fmap_match(&input, BindStyle::Ref, |binding| {
-        quote! {
-            ::std::mem::ManuallyDrop::into_inner(
-                ::to_shmem::ToShmem::to_shmem(#binding, builder)
-            )
-        }
-    });
+    // Do all of the `to_shmem()?` calls before the `ManuallyDrop::into_inner()`
+    // calls, so that we don't drop a value in the shared memory buffer if one
+    // of the `to_shmem`s fails.
+    let match_body = cg::fmap2_match(
+        &input,
+        BindStyle::Ref,
+        |binding| {
+            quote! {
+                ::to_shmem::ToShmem::to_shmem(#binding, builder)?
+            }
+        },
+        |binding| {
+            Some(quote! {
+                ::std::mem::ManuallyDrop::into_inner(#binding)
+            })
+        },
+    );
 
     let name = &input.ident;
     let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl();
@@ -44,12 +54,12 @@ pub fn derive(mut input: syn::DeriveInput) -> TokenStream {
             fn to_shmem(
                 &self,
                 builder: &mut ::to_shmem::SharedMemoryBuilder,
-            ) -> ::std::mem::ManuallyDrop<Self> {
-                ::std::mem::ManuallyDrop::new(
+            ) -> ::to_shmem::Result<Self> {
+                Ok(::std::mem::ManuallyDrop::new(
                     match *self {
                         #match_body
                     }
-                )
+                ))
             }
         }
     }

servo/ports/geckolib/glue.rs

@@ -6799,6 +6799,7 @@ pub unsafe extern "C" fn Servo_SharedMemoryBuilder_Create(
 pub unsafe extern "C" fn Servo_SharedMemoryBuilder_AddStylesheet(
     builder: &mut RawServoSharedMemoryBuilder,
     raw_contents: &RawServoStyleSheetContents,
+    error_message: &mut nsACString,
 ) -> *const ServoCssRules {
     let builder = SharedMemoryBuilder::from_ffi_mut(builder);
     let contents = StylesheetContents::as_arc(&raw_contents);
@@ -6810,10 +6811,15 @@ pub unsafe extern "C" fn Servo_SharedMemoryBuilder_AddStylesheet(
     debug_assert!(contents.source_map_url.read().is_none());
     debug_assert!(contents.source_url.read().is_none());
 
-    let rules = &contents.rules;
-    let shared_rules: &Arc<Locked<CssRules>> = &*builder.write(rules);
-    (&*shared_rules)
-        .with_raw_offset_arc(|arc| *Locked::<CssRules>::arc_as_borrowed(arc) as *const _)
+    match builder.write(&contents.rules) {
+        Ok(rules_ptr) => {
+            (*rules_ptr).with_raw_offset_arc(|arc| *Locked::arc_as_borrowed(arc) as *const _)
+        },
+        Err(message) => {
+            error_message.assign(&message);
+            ptr::null()
+        },
+    }
 }
 
 #[no_mangle]

toolkit/components/telemetry/Histograms.json

@@ -14863,28 +14863,38 @@
     "record_in_processes": ["main"],
     "products": ["firefox", "geckoview"],
     "alert_emails": ["cmccormack@mozilla.com"],
-    "bug_numbers": [1562060],
-    "expires_in_version": "76",
+    "bug_numbers": [1562060, 1621773],
+    "expires_in_version": "83",
     "kind": "boolean",
     "releaseChannelCollection": "opt-out",
     "description": "Whether the parent process successfully mapped the User Agent style sheet shared memory buffer at the desired address"
   },
+  "SHARED_MEMORY_UA_SHEETS_TOSHMEM_SUCCEEDED": {
+    "record_in_processes": ["main"],
+    "products": ["firefox", "geckoview"],
+    "alert_emails": ["layout-telemetry-alerts@mozilla.com"],
+    "bug_numbers": [1621773],
+    "expires_in_version": "83",
+    "kind": "boolean",
+    "releaseChannelCollection": "opt-out",
+    "description": "Whether the parent process failed to convert User Agent style sheets into a form suitable for storing in shared memory"
+  },
   "SHARED_MEMORY_UA_SHEETS_MAPPED_PARENT_AFTER_FREEZE": {
     "record_in_processes": ["main"],
     "products": ["firefox", "geckoview"],
-    "alert_emails": ["cmccormack@mozilla.com"],
-    "bug_numbers": [1562060],
-    "expires_in_version": "76",
+    "alert_emails": ["layout-telemetry-alerts@mozilla.com"],
+    "bug_numbers": [1562060, 1621773],
+    "expires_in_version": "83",
     "kind": "boolean",
     "releaseChannelCollection": "opt-out",
-    "description": "Whether the parent process successfully re-mapped the User Agent style sheet shared memory buffer at the desired address after freezing it"
+    "description": "Whether the parent process successfully mapped the User Agent style sheet shared memory buffer at the desired address"
   },
   "SHARED_MEMORY_UA_SHEETS_MAPPED_CHILD": {
     "record_in_processes": ["content"],
     "products": ["firefox", "geckoview"],
-    "alert_emails": ["cmccormack@mozilla.com"],
-    "bug_numbers": [1562060],
-    "expires_in_version": "76",
+    "alert_emails": ["layout-telemetry-alerts@mozilla.com"],
+    "bug_numbers": [1562060, 1621773],
+    "expires_in_version": "83",
     "kind": "boolean",
     "releaseChannelCollection": "opt-out",
     "description": "Whether a child process successfully mapped the User Agent style sheet shared memory buffer at the desired address"