Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-06 14:10:11 +01:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #18365 - chenpighead:stylo-keyframe-location, r=upsuper,emilio

Browse source 
stylo: Store location information for keyframe rules.

So far, we only store location info for the whole Keyframes block, not for each
of the keyframe rule. Without this info, the devtool can't present the rules
on the devtool panel properly.

In this patch, we collect the source location info while parsing keyframe selector.
The binding function, Servo_KeyframesRule_GetKeyframe, is also fixed (and renamed
to Servo_KeyframesRule_GetKeyframeAt to match the fix) to accept line and column
parameters from Gecko, so we can pass/set them with the source location from Servo.

This is the servo part of [Bug 1394994](https://bugzilla.mozilla.org/show_bug.cgi?id=1394994).

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix [Bug 1394994](https://bugzilla.mozilla.org/show_bug.cgi?id=1394994)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18365)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-09-05 11:22:20 -05:00 • committed by GitHub
commit f648e12935
6 changed files with 59 additions and 16 deletions
Download patch file Download diff file Expand all files Collapse all files

7
components/style/gecko/generated/bindings.rs
View file

@ -2342,9 +2342,10 @@ extern "C" {
-> u32;
}
extern "C" {
pub fn Servo_KeyframesRule_GetKeyframe(rule:
RawServoKeyframesRuleBorrowed,
index: u32)
pub fn Servo_KeyframesRule_GetKeyframeAt(rule:
RawServoKeyframesRuleBorrowed,
index: u32, line: *mut u32,
column: *mut u32)
-> RawServoKeyframeStrong;
}
extern "C" {

25
components/style/stylesheets/keyframes_rule.rs
View file

@ -20,7 +20,7 @@ use std::fmt;
use style_traits::{PARSING_MODE_DEFAULT, ToCss, ParseError, StyleParseError};
use style_traits::PropertyDeclarationParseError;
use stylesheets::{CssRuleType, StylesheetContents};
use stylesheets::rule_parser::VendorPrefix;
use stylesheets::rule_parser::{VendorPrefix, get_location_with_offset};
use values::{KeyframesName, serialize_percentage};
/// A [`@keyframes`][keyframes] rule.
@ -192,6 +192,9 @@ pub struct Keyframe {
/// Note that `!important` rules in keyframes don't apply, but we keep this
/// `Arc` just for convenience.
pub block: Arc<Locked<PropertyDeclarationBlock>>,
/// The line and column of the rule's source code.
pub source_location: SourceLocation,
}
impl ToCssWithGuard for Keyframe {
@ -249,6 +252,7 @@ impl DeepCloneWithLock for Keyframe {
Keyframe {
selector: self.selector.clone(),
block: Arc::new(lock.wrap(self.block.read_with(guard).clone())),
source_location: self.source_location.clone(),
}
}
}
@ -483,16 +487,28 @@ impl<'a, 'i, R> AtRuleParser<'i> for KeyframeListParser<'a, R> {
type Error = SelectorParseError<'i, StyleParseError<'i>>;
}
/// A wrapper to wraps the KeyframeSelector with its source location
struct KeyframeSelectorParserPrelude {
selector: KeyframeSelector,
source_location: SourceLocation,
}
impl<'a, 'i, R: ParseErrorReporter> QualifiedRuleParser<'i> for KeyframeListParser<'a, R> {
type Prelude = KeyframeSelector;
type Prelude = KeyframeSelectorParserPrelude;
type QualifiedRule = Arc<Locked<Keyframe>>;
type Error = SelectorParseError<'i, StyleParseError<'i>>;
fn parse_prelude<'t>(&mut self, input: &mut Parser<'i, 't>) -> Result<Self::Prelude, ParseError<'i>> {
let start_position = input.position();
let start_location = input.current_source_location();
let location = get_location_with_offset(start_location);
match KeyframeSelector::parse(input) {
Ok(sel) => Ok(sel),
Ok(sel) => {
Ok(KeyframeSelectorParserPrelude {
selector: sel,
source_location: location,
})
},
Err(e) => {
let error = ContextualParseError::InvalidKeyframeRule(input.slice_from(start_position), e.clone());
self.context.log_css_error(self.error_context, start_location, error);
@ -530,8 +546,9 @@ impl<'a, 'i, R: ParseErrorReporter> QualifiedRuleParser<'i> for KeyframeListPars
// `parse_important` is not called here, `!important` is not allowed in keyframe blocks.
}
Ok(Arc::new(self.shared_lock.wrap(Keyframe {
selector: prelude,
selector: prelude.selector,
block: Arc::new(self.shared_lock.wrap(block)),
source_location: prelude.source_location,
})))
}
}

2
components/style/stylesheets/rule_parser.rs
View file

@ -587,7 +587,7 @@ impl<'a, 'b, 'i, R: ParseErrorReporter> QualifiedRuleParser<'i> for NestedRulePa
}
/// Adjust a location's column to accommodate DevTools.
fn get_location_with_offset(location: SourceLocation) -> SourceLocation {
pub fn get_location_with_offset(location: SourceLocation) -> SourceLocation {
SourceLocation {
line: location.line,
// Column offsets are not yet supported, but Gecko devtools expect 1-based columns.

15
ports/geckolib/glue.rs
View file

@ -1528,11 +1528,16 @@ pub extern "C" fn Servo_KeyframesRule_GetCount(rule: RawServoKeyframesRuleBorrow
}
#[no_mangle]
pub extern "C" fn Servo_KeyframesRule_GetKeyframe(rule: RawServoKeyframesRuleBorrowed, index: u32)
-> RawServoKeyframeStrong {
read_locked_arc(rule, |rule: &KeyframesRule| {
rule.keyframes[index as usize].clone().into_strong()
})
pub extern "C" fn Servo_KeyframesRule_GetKeyframeAt(rule: RawServoKeyframesRuleBorrowed, index: u32,
line: *mut u32, column: *mut u32) -> RawServoKeyframeStrong {
let global_style_data = &*GLOBAL_STYLE_DATA;
let guard = global_style_data.shared_lock.read();
let key = Locked::<KeyframesRule>::as_arc(&rule).read_with(&guard)
.keyframes[index as usize].clone();
let location = key.read_with(&guard).source_location;
*unsafe { line.as_mut().unwrap() } = location.line as u32;
*unsafe { column.as_mut().unwrap() } = location.column as u32;
key.into_strong()
}
#[no_mangle]

16
tests/unit/style/keyframes.rs
View file

@ -2,6 +2,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
use cssparser::SourceLocation;
use servo_arc::Arc;
use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, Importance};
use style::properties::animated_properties::AnimatableLonghand;
@ -29,10 +30,12 @@ fn test_empty_keyframe() {
#[test]
fn test_no_property_in_keyframe() {
let shared_lock = SharedRwLock::new();
let dummy_location = SourceLocation { line: 0, column: 0 };
let keyframes = vec![
Arc::new(shared_lock.wrap(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(1.)]),
block: Arc::new(shared_lock.wrap(PropertyDeclarationBlock::new()))
block: Arc::new(shared_lock.wrap(PropertyDeclarationBlock::new())),
source_location: dummy_location,
})),
];
let animation = KeyframesAnimation::from_keyframes(&keyframes,
@ -73,15 +76,18 @@ fn test_missing_property_in_initial_keyframe() {
block
}));
let dummy_location = SourceLocation { line: 0, column: 0 };
let keyframes = vec![
Arc::new(shared_lock.wrap(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.)]),
block: declarations_on_initial_keyframe.clone(),
source_location: dummy_location,
})),
Arc::new(shared_lock.wrap(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(1.)]),
block: declarations_on_final_keyframe.clone(),
source_location: dummy_location,
})),
];
let animation = KeyframesAnimation::from_keyframes(&keyframes,
@ -133,15 +139,18 @@ fn test_missing_property_in_final_keyframe() {
Importance::Normal,
)));
let dummy_location = SourceLocation { line: 0, column: 0 };
let keyframes = vec![
Arc::new(shared_lock.wrap(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.)]),
block: declarations_on_initial_keyframe.clone(),
source_location: dummy_location,
})),
Arc::new(shared_lock.wrap(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(1.)]),
block: declarations_on_final_keyframe.clone(),
source_location: dummy_location,
})),
];
let animation = KeyframesAnimation::from_keyframes(&keyframes,
@ -186,14 +195,17 @@ fn test_missing_keyframe_in_both_of_initial_and_final_keyframe() {
block
}));
let dummy_location = SourceLocation { line: 0, column: 0 };
let keyframes = vec![
Arc::new(shared_lock.wrap(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.)]),
block: Arc::new(shared_lock.wrap(PropertyDeclarationBlock::new()))
block: Arc::new(shared_lock.wrap(PropertyDeclarationBlock::new())),
source_location: dummy_location,
})),
Arc::new(shared_lock.wrap(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(vec![KeyframePercentage::new(0.5)]),
block: declarations.clone(),
source_location: dummy_location,
})),
];
let animation = KeyframesAnimation::from_keyframes(&keyframes,

10
tests/unit/style/stylesheets.rs
View file

@ -217,7 +217,11 @@ fn test_parse_stylesheet() {
(PropertyDeclaration::Width(
LengthOrPercentageOrAuto::Percentage(Percentage(0.))),
Importance::Normal),
])))
]))),
source_location: SourceLocation {
line: 17,
column: 13,
},
})),
Arc::new(stylesheet.shared_lock.wrap(Keyframe {
selector: KeyframeSelector::new_for_unit_testing(
@ -231,6 +235,10 @@ fn test_parse_stylesheet() {
vec![TimingFunction::ease()])),
Importance::Normal),
]))),
source_location: SourceLocation {
line: 18,
column: 13,
},
})),
],
vendor_prefix: None,