From 83904edbec58618f217d2ec936fa8ddfa8a82626 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 6 Sep 2017 17:31:45 +0200 Subject: [PATCH] Make get_property_value unsafe This takes raw pointers and does things with the things they point to, and circumvent a lock on a hash table. --- ports/geckolib/glue.rs | 113 ++++++++++++++++++++++++++++------------- 1 file changed, 77 insertions(+), 36 deletions(-) diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 3241e32bee1..c52fe8a12a1 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -2267,7 +2267,7 @@ pub extern "C" fn Servo_DeclarationBlock_GetNthProperty(declarations: RawServoDe macro_rules! get_property_id_from_property { ($property: ident, $ret: expr) => {{ - let property = unsafe { $property.as_ref().unwrap().as_str_unchecked() }; + let property = $property.as_ref().unwrap().as_str_unchecked(); match PropertyId::parse(property.into(), None) { Ok(property_id) => property_id, Err(_) => { return $ret; } @@ -2275,33 +2275,46 @@ macro_rules! get_property_id_from_property { }} } -fn get_property_value(declarations: RawServoDeclarationBlockBorrowed, - property_id: PropertyId, value: *mut nsAString) { +unsafe fn get_property_value( + declarations: RawServoDeclarationBlockBorrowed, + property_id: PropertyId, + value: *mut nsAString, +) { // This callsite is hot enough that the lock acquisition shows up in profiles. // Using an unchecked read here improves our performance by ~10% on the // microbenchmark in bug 1355599. - unsafe { - read_locked_arc_unchecked(declarations, |decls: &PropertyDeclarationBlock| { - decls.property_value_to_css(&property_id, value.as_mut().unwrap()).unwrap(); - }) - } + read_locked_arc_unchecked(declarations, |decls: &PropertyDeclarationBlock| { + decls.property_value_to_css(&property_id, value.as_mut().unwrap()).unwrap(); + }) } #[no_mangle] -pub extern "C" fn Servo_DeclarationBlock_GetPropertyValue(declarations: RawServoDeclarationBlockBorrowed, - property: *const nsACString, value: *mut nsAString) { - get_property_value(declarations, get_property_id_from_property!(property, ()), value) +pub unsafe extern "C" fn Servo_DeclarationBlock_GetPropertyValue( + declarations: RawServoDeclarationBlockBorrowed, + property: *const nsACString, + value: *mut nsAString, +) { + get_property_value( + declarations, + get_property_id_from_property!(property, ()), + value, + ) } #[no_mangle] -pub extern "C" fn Servo_DeclarationBlock_GetPropertyValueById(declarations: RawServoDeclarationBlockBorrowed, - property: nsCSSPropertyID, value: *mut nsAString) { +pub unsafe extern "C" fn Servo_DeclarationBlock_GetPropertyValueById( + declarations: RawServoDeclarationBlockBorrowed, + property: nsCSSPropertyID, + value: *mut nsAString, +) { get_property_value(declarations, get_property_id_from_nscsspropertyid!(property, ()), value) } #[no_mangle] -pub extern "C" fn Servo_DeclarationBlock_GetPropertyIsImportant(declarations: RawServoDeclarationBlockBorrowed, - property: *const nsACString) -> bool { +pub unsafe extern "C" fn Servo_DeclarationBlock_GetPropertyIsImportant( + declarations: RawServoDeclarationBlockBorrowed, + property: *const nsACString, +) -> bool { let property_id = get_property_id_from_property!(property, false); read_locked_arc(declarations, |decls: &PropertyDeclarationBlock| { decls.property_priority(&property_id).important() @@ -2328,25 +2341,49 @@ fn set_property(declarations: RawServoDeclarationBlockBorrowed, property_id: Pro } #[no_mangle] -pub extern "C" fn Servo_DeclarationBlock_SetProperty(declarations: RawServoDeclarationBlockBorrowed, - property: *const nsACString, value: *const nsACString, - is_important: bool, data: *mut URLExtraData, - parsing_mode: structs::ParsingMode, - quirks_mode: nsCompatibility, - loader: *mut Loader) -> bool { - set_property(declarations, get_property_id_from_property!(property, false), - value, is_important, data, parsing_mode, quirks_mode.into(), loader) +pub unsafe extern "C" fn Servo_DeclarationBlock_SetProperty( + declarations: RawServoDeclarationBlockBorrowed, + property: *const nsACString, + value: *const nsACString, + is_important: bool, + data: *mut URLExtraData, + parsing_mode: structs::ParsingMode, + quirks_mode: nsCompatibility, + loader: *mut Loader, +) -> bool { + set_property( + declarations, + get_property_id_from_property!(property, false), + value, + is_important, + data, + parsing_mode, + quirks_mode.into(), + loader, + ) } #[no_mangle] -pub extern "C" fn Servo_DeclarationBlock_SetPropertyById(declarations: RawServoDeclarationBlockBorrowed, - property: nsCSSPropertyID, value: *const nsACString, - is_important: bool, data: *mut URLExtraData, - parsing_mode: structs::ParsingMode, - quirks_mode: nsCompatibility, - loader: *mut Loader) -> bool { - set_property(declarations, get_property_id_from_nscsspropertyid!(property, false), - value, is_important, data, parsing_mode, quirks_mode.into(), loader) +pub unsafe extern "C" fn Servo_DeclarationBlock_SetPropertyById( + declarations: RawServoDeclarationBlockBorrowed, + property: nsCSSPropertyID, + value: *const nsACString, + is_important: bool, + data: *mut URLExtraData, + parsing_mode: structs::ParsingMode, + quirks_mode: nsCompatibility, + loader: *mut Loader, +) -> bool { + set_property( + declarations, + get_property_id_from_nscsspropertyid!(property, false), + value, + is_important, + data, + parsing_mode, + quirks_mode.into(), + loader, + ) } fn remove_property(declarations: RawServoDeclarationBlockBorrowed, property_id: PropertyId) { @@ -2356,8 +2393,10 @@ fn remove_property(declarations: RawServoDeclarationBlockBorrowed, property_id: } #[no_mangle] -pub extern "C" fn Servo_DeclarationBlock_RemoveProperty(declarations: RawServoDeclarationBlockBorrowed, - property: *const nsACString) { +pub unsafe extern "C" fn Servo_DeclarationBlock_RemoveProperty( + declarations: RawServoDeclarationBlockBorrowed, + property: *const nsACString, +) { remove_property(declarations, get_property_id_from_property!(property, ())) } @@ -2853,8 +2892,10 @@ pub extern "C" fn Servo_DeclarationBlock_SetTextDecorationColorOverride(declarat } #[no_mangle] -pub extern "C" fn Servo_CSSSupports2(property: *const nsACString, - value: *const nsACString) -> bool { +pub unsafe extern "C" fn Servo_CSSSupports2( + property: *const nsACString, + value: *const nsACString, +) -> bool { let id = get_property_id_from_property!(property, false); let mut declarations = SourcePropertyDeclaration::new(); @@ -2862,7 +2903,7 @@ pub extern "C" fn Servo_CSSSupports2(property: *const nsACString, &mut declarations, id, value, - unsafe { DUMMY_URL_DATA }, + DUMMY_URL_DATA, structs::ParsingMode_Default, QuirksMode::NoQuirks, &NullReporter,