Closed Bug 1408305 Opened 2 years ago Closed 2 years ago

make DOMIntersectionObserver use Servo for parsing the root margin value

Categories

(Core :: CSS Parsing and Computation, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: heycam, Assigned: heycam)

References

Details

Attachments

(2 files, 1 obsolete file)

Currently is uses nsCSSParser.
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #8918819 - Flags: review?(xidorn+moz)
Comment on attachment 8918819 [details]
style: Add FFI function for parsing IntersectionObserver rootMargin values.

https://reviewboard.mozilla.org/r/189676/#review195158

::: servo/ports/geckolib/glue.rs:4287
(Diff revision 1)
> +    let url_data = unsafe { dummy_url_data() };
> +    let context = ParserContext::new(
> +        Origin::Author,
> +        url_data,
> +        Some(CssRuleType::Style),
> +        PARSING_MODE_DEFAULT,
> +        QuirksMode::NoQuirks,
> +    );

I guess we can probably even have a dummy parser context so that we don't need to create it every time.
Attachment #8918819 - Flags: review?(xidorn+moz) → review+
Comment on attachment 8918820 [details]
Bug 1408305 - Use Servo to parse IntersectionObserver rootMargin values.

https://reviewboard.mozilla.org/r/189678/#review195162
Attachment #8918820 - Flags: review?(xidorn+moz) → review+
Comment on attachment 8918819 [details]
style: Add FFI function for parsing IntersectionObserver rootMargin values.

https://reviewboard.mozilla.org/r/189676/#review195278

::: servo/ports/geckolib/glue.rs:4287
(Diff revision 1)
> +    let url_data = unsafe { dummy_url_data() };
> +    let context = ParserContext::new(
> +        Origin::Author,
> +        url_data,
> +        Some(CssRuleType::Style),
> +        PARSING_MODE_DEFAULT,
> +        QuirksMode::NoQuirks,
> +    );

The ParserContext creation doesn't really do any meaningful work, just initializes those fields, so I don't think we really need a re-usable dummy parser context.  Though if I write another patch to do some single value parsing I might factor it out into a helper function.
Attached file Servo PR
Attachment #8918819 - Attachment is obsolete: true
Pushed by cmccormack@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3166813b276c
Use Servo to parse IntersectionObserver rootMargin values. r=xidorn
https://hg.mozilla.org/mozilla-central/rev/3166813b276c
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Looks like this bug brought a steady improvement:

== Change summary for alert #10033 (as of October 17 2017 07:20 UTC) ==

Improvements:

  7%  Strings PerfIsASCIIExample3 osx-10-10 opt      8,267.29 -> 7,654.67

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=10033
Maybe another bug caused that improvement.  My patch shouldn't be related to the performance of that benchmark.
You need to log in before you can comment on or make changes to this bug.