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.

Sign up for GitHub

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

Avoid synchronous WebContents calls in webview editor #81783

Closed
Tyriar opened this issue Oct 1, 2019 · 3 comments
Closed

Avoid synchronous WebContents calls in webview editor #81783

Tyriar opened this issue Oct 1, 2019 · 3 comments
Assignees
@mjbvz
Labels
debt Code quality issues webview Webview issues
Milestone
October 2019

Comments

@Tyriar
Copy link
Member

Tyriar commented Oct 1, 2019 •
edited

#81698

Related: #81780

getOwnerBrowserWindow and isDestroyed are 2 synchronous calls that happen on layout that can be expensive (seems around 0.5-8ms on my Windows box). I think we can cache the owner browser window object and listen to on('destroyed') instead.

vscode/src/vs/workbench/contrib/webview/electron-browser/webviewElement.ts

Lines 578 to 581 in 9c1fa6f

const window = (contents as any).getOwnerBrowserWindow();
if (!window || !window.webContents || window.webContents.isDestroyed()) {
return;
}

@Tyriar Tyriar changed the title Avoid synchronous WebContents calls in custom editor Avoid synchronous WebContents calls in webview editor Oct 1, 2019
@mjbvz mjbvz added this to the October 2019 milestone Oct 1, 2019
@mjbvz mjbvz added debt Code quality issues webview Webview issues labels Oct 1, 2019
@mjbvz
Copy link
Contributor

mjbvz commented Oct 1, 2019

Marking for october since it is not a regression

These checks were added to fix some crashes we were hitting so we need to be careful here. If you call certain webview methods without knowing if the webview was destroyed (which we used not to be able to determine reliably) things can go very wrong

@Tyriar
Copy link
Member Author

Tyriar commented Oct 1, 2019

@mjbvz tracking whether it's destroyed via the destroyed event and remembering it will likely be faster for that though.

@mjbvz mjbvz closed this as completed in 953d193 Oct 7, 2019
@mjbvz
Copy link
Contributor

mjbvz commented Oct 7, 2019

Seems like this code isn't even required anymore...

Let me know if you notice anything weird with webview scaling in the 1.40 insiders

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@mjbvz mjbvz

Labels
debt Code quality issues webview Webview issues
Projects
None yet
Milestone
October 2019
Development

No branches or pull requests
2 participants
@Tyriar @mjbvz