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

Fix memory leak when using webFrame and spell checker #16770

Merged
merged 2 commits into from Feb 8, 2019

Conversation

zcbenz
Copy link
Member

@zcbenz zcbenz commented Feb 6, 2019

Description of Change

Close #15459.

When using spell checker in pages with sandbox, there are two sources of memory leak:

  1. Every time when page is reloaded, the code would create a new api::WebFrame, and the old one would be leaked since the renderer process is not restarted.
  2. The spell check client is only destroyed when web frame is destroyed, but reloading a frame does not destroy the corresponding blink::WebFrame.

This PR fixes the leak with 2 methods:

  1. Refactor the webFrame module so it does not create api::WebFrame instances, the native methods become simple getters without a global state. So no memory will be leaked on reload, and the code now is cleaner and uses less memory.
  2. The spell check client is now destroyed when the page context is destroyed, so it won't hold a reference to code in the old context, which prevents garbage collection of V8.

Note that this PR should be rebased instead of squashed as the commits have clear responsibilities.

Checklist

Release Notes

Notes: Fix memory leak when using webFrame and spell checker.

@zcbenz zcbenz requested a review from a team February 6, 2019 07:44
@zcbenz zcbenz force-pushed the web-frame-refactor branch 3 times, most recently from 6ada33d to 7703b05 Compare February 6, 2019 08:34
When reloading a page without restarting renderer process (for example
sandbox mode), the blink::WebFrame is not destroyed, but api::WebFrame
is always recreated for the new page context. This leaves a leak of
api::WebFrame.
Copy link
Contributor

@nitsakh nitsakh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍 Thanks!

@MarshallOfSound MarshallOfSound merged commit d16b581 into master Feb 8, 2019
@release-clerk
Copy link

release-clerk bot commented Feb 8, 2019

Release Notes Persisted

Fix memory leak when using webFrame and spell checker.

@trop
Copy link
Contributor

trop bot commented Feb 8, 2019

I have automatically backported this PR to "5-0-x", please check out #16851

@sofianguy sofianguy added this to Fixed in 5.0.0-beta.3 in 5.0.x Feb 18, 2019
@sofianguy sofianguy added this to Fixed in 3.1.4 in 3.1.x Feb 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
3.1.x
Fixed in 3.1.4
5.0.x
Fixed in 5.0.0-beta.3
Development

Successfully merging this pull request may close these issues.

Memory leak with use sandbox true and webFrame.setSpellCheckProvider
4 participants