Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove unnecessary rooting from CSSStyleDeclaration #8126

Closed
Ms2ger opened this issue Oct 21, 2015 · 3 comments
Closed

Remove unnecessary rooting from CSSStyleDeclaration #8126

Ms2ger opened this issue Oct 21, 2015 · 3 comments
Labels
A-content/dom Interacting with the DOM from web content C-assigned There is someone working on resolving the issue E-less-complex Straightforward. Recommended for a new contributor. I-perf-slow Unnecessary performance degredation. I-refactor No impact; the issue is one of maintainability or tidiness. Proposed solution requires refactoring.

Comments

@Ms2ger
Copy link
Contributor

Ms2ger commented Oct 21, 2015

Most methods on CSSStyleDeclaration start with

        let owner = self.owner.root();

This is no longer necessary; the code below can use self.owner directly.

Code: components/script/dom/cssstyledeclaration.rs
@Ms2ger Ms2ger added A-content/dom Interacting with the DOM from web content E-less-complex Straightforward. Recommended for a new contributor. I-perf-slow Unnecessary performance degredation. I-refactor No impact; the issue is one of maintainability or tidiness. Proposed solution requires refactoring. labels Oct 21, 2015
@r-52
Copy link
Contributor

r-52 commented Oct 21, 2015

Could I take a look at this one?

@frewsxcv
Copy link
Contributor

Go for it!

@frewsxcv frewsxcv added C-assigned There is someone working on resolving the issue I-refactor No impact; the issue is one of maintainability or tidiness. Proposed solution requires refactoring. and removed I-refactor No impact; the issue is one of maintainability or tidiness. Proposed solution requires refactoring. labels Oct 21, 2015
@r-52 r-52 mentioned this issue Oct 21, 2015
Merged
bors-servo pushed a commit that referenced this issue Oct 23, 2015
Auto merge of #8137 - romankl:gh/8126, r=nox
ff2c7bb 
Remove unnecessary rooting from `CSSStyleDeclaration`

This commit will replace calls to `self.owner.root()` with `self.owner` to avoid unnecessary rooting of JS elements objects.

Ref.- Issue: #8126

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/8137)
<!-- Reviewable:end -->
@r-52
Copy link
Contributor

r-52 commented Oct 24, 2015

PR #8137 landed and fixed this issue

@jdm jdm closed this as completed Oct 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-content/dom Interacting with the DOM from web content C-assigned There is someone working on resolving the issue E-less-complex Straightforward. Recommended for a new contributor. I-perf-slow Unnecessary performance degredation. I-refactor No impact; the issue is one of maintainability or tidiness. Proposed solution requires refactoring.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@jdm @Ms2ger @frewsxcv @r-52