Skip to content

Commit

Permalink
style: Gracefully handle errors creating shared memory UA style sheets.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
heycam authored and emilio committed Jun 3, 2020
1 parent 709c52f commit 6bae0b9
Show file tree
Hide file tree
Showing 10 changed files with 185 additions and 123 deletions.
29 changes: 25 additions & 4 deletions components/derive_common/cg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,22 +85,43 @@ 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| {
v.bind_with(|_| bind_style);
});
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)
})
Expand Down
8 changes: 4 additions & 4 deletions components/style/gecko/url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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,
})
}))
}
}

Expand Down
20 changes: 10 additions & 10 deletions components/style/gecko_string_cache/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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)]
Expand Down Expand Up @@ -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
);

ManuallyDrop::new(Atom(self.0))
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
));
}

Ok(ManuallyDrop::new(Atom(self.0)))
}
}

Expand Down
10 changes: 5 additions & 5 deletions components/style/shared_lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}
Expand Down
9 changes: 5 additions & 4 deletions components/style/stylesheets/import_rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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",
))
}
}

Expand Down
23 changes: 14 additions & 9 deletions components/style/stylesheets/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)))
}
}
}
Expand Down
17 changes: 9 additions & 8 deletions components/style/values/computed/font.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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),
})
}))
}
}

Expand Down
8 changes: 4 additions & 4 deletions components/style_traits/owned_slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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))))
}
}
}
Expand Down
Loading

0 comments on commit 6bae0b9

Please sign in to comment.