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

Memory leak fix #38

Merged
merged 5 commits into from
May 22, 2017
Merged

Memory leak fix #38

merged 5 commits into from
May 22, 2017

Conversation

Llorx
Copy link
Contributor

@Llorx Llorx commented May 22, 2017

When an inline style element is deleted, the reference is kept inside processedSheets, creating a memory leak.

Also added a small optimization to avoid a try catch, as they are a bit expensive

EDIT: Removed try/catch thingy, as tests failed because of that.

Llorx and others added 5 commits May 22, 2017 15:24
When an inline style element (without cssRules attached as it is inline and not coming from a file) is deleted, the reference is kept inside `processedSheets`, creating a memory leak.

Also added a small optimization to avoid a try catch, as [they are a bit expensive](https://stackoverflow.com/questions/38121208/why-cant-v8-optimize-try-catch-finally)
@ausi
Copy link
Owner

ausi commented May 22, 2017

Thank you for the pull request!

The try catch is needed because browsers throw a SecurityError as soon as you access cssRules on a cross origin stylesheet. Fortunately try catch is no longer slow in Chrome’s V8 due to the new TurboFan compiler.

I added a unit test for your fix in 6992479. Once the cross browser tests on BrowserStack run through I will merge it. 🎉

@ausi ausi self-assigned this May 22, 2017
@ausi ausi added the bug label May 22, 2017
@ausi ausi merged commit 2eab487 into ausi:master May 22, 2017
@Llorx Llorx deleted the patch-1 branch May 22, 2017 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants