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

[Bug]: Crash when adding child view #41974

Open
3 tasks done
t57ser opened this issue Apr 26, 2024 · 8 comments · May be fixed by #42085
Open
3 tasks done

[Bug]: Crash when adding child view #41974

t57ser opened this issue Apr 26, 2024 · 8 comments · May be fixed by #42085
Assignees

Comments

@t57ser
Copy link
Contributor

t57ser commented Apr 26, 2024

Preflight Checklist

Electron Version

30.0.0

What operating system are you using?

Windows

Operating System Version

10.0.19042

What arch are you using?

x64

Last Known Working Electron version

No response

Expected Behavior

No crash

Actual Behavior

Electron crashes with:
/c/Program Files/nodejs/npm: line 64: 11105 Segmentation fault "$NODE_EXE" "$NPM_CLI_JS" "$@"

image

image

I am not sure what exactly causes this, but it happened when I switched the implementation from browserView to webContentsView

Testcase Gist URL

No response

Additional Information

No response

@VerteDinde VerteDinde added the blocked/need-repro Needs a test case to reproduce the bug label Apr 26, 2024
@electron-issue-triage
Copy link

Hello @t57ser. Thanks for reporting this and helping to make Electron better!

Would it be possible for you to make a standalone testcase with only the code necessary to reproduce the issue? For example, Electron Fiddle is a great tool for making small test cases and makes it easy to publish your test case to a gist that Electron maintainers can use.

Stand-alone test cases make fixing issues go more smoothly: it ensure everyone's looking at the same issue, it removes all unnecessary variables from the equation, and it can also provide the basis for automated regression tests.

Now adding the blocked/need-repro Needs a test case to reproduce the bug label for this reason. After you make a test case, please link to it in a followup comment. This issue will be closed in 10 days if the above is not addressed.

@VerteDinde
Copy link
Member

Hey @t57ser, I appreciate the report and I know it might be tricky to make an isolated repro case for this one. If you happen to be able to share any code that can repro the issue, that may help us better understand what's going on - the stack traces alone unfortunately don't give us much to go on here.

If a repro is too hard to make, could you please attach a crash dump to help us get more information? That might give us just a bit more than the stack trace screenshot. You can collect them by adding the following snippet to your main process code, before app.whenReady:

const { app, crashReporter } = require('electron')

console.log(app.getPath('crashDumps'))
crashReporter.start({ submitURL: '', uploadToServer: false })

Then reproduce the crash, zip up the crash dumps directory and attach it here.

@t57ser
Copy link
Contributor Author

t57ser commented Apr 28, 2024

It seems like this was an issue on my side.
The app had the following code:

const parentWindow = electron.BrowserWindow.fromBrowserView(view)
parentWindow.removeBrowserView(child);

in order to get the parent and detach the view. I later changed this to

const parentWindow = electron.BrowserWindow.fromWebContents(view.webContents)
parentWindow.contentView.removeChildView(child);

This is wrong though as it does not get the parent.
After fixing these occurrances, the crash disappeared. It probably still should not crash if such a mistake is made but I do not see this as a critical issue.

@electron-issue-triage electron-issue-triage bot removed the blocked/need-repro Needs a test case to reproduce the bug label Apr 28, 2024
@t57ser
Copy link
Contributor Author

t57ser commented Apr 28, 2024

I think I know how to reproduce it:

window1.contentView.addChildView(view);
window2.contentView.addChildView(view);

Attach the same view to 2 different windows and it crashes

@codebytere
Copy link
Member

@t57ser thanks! agree it should never crash - will investigate soon :)

@codebytere codebytere added the status/confirmed A maintainer reproduced the bug or agreed with the feature label May 5, 2024
@codebytere codebytere self-assigned this May 5, 2024
@codebytere
Copy link
Member

@t57ser couldn't repro with the following:

sample

const { app, BrowserWindow, WebContentsView } = require('electron')

function createWindow () {
  const win1 = new BrowserWindow({ title: 'win1' })
  const win2 = new BrowserWindow({ title: 'win2' })

  const view1 = new WebContentsView()
  win1.contentView.addChildView(view1)

  setTimeout(() => {
   win2.contentView.addChildView(view1)
  }, 5000)

  view1.webContents.loadURL('https://electronjs.org')
  view1.setBounds({ x: 0, y: 0, width: 400, height: 400 })
}

app.whenReady().then(() => {
  createWindow()

  app.on('activate', function () {
    if (BrowserWindow.getAllWindows().length === 0) createWindow()
  })
})

app.on('window-all-closed', function () {
  if (process.platform !== 'darwin') app.quit()
})

what happens instead is the webcontentsview is detached from win1 and attached to win2.

@t57ser
Copy link
Contributor Author

t57ser commented May 7, 2024

@codebytere This seems to crash for me each time:

const {app, BrowserWindow, ipcMain} = require('electron')
const electron =  require('electron')
const path = require('path')

let mainWindow

function createWindow () {

	const mainWindow1 = new BrowserWindow({
		x: 0,
		y: 0,
		width: 1000,
		height: 600,
		show: true,
	})

	mainWindow = new BrowserWindow({
		x: 0,
		y: 0,
		width: 1000,
		height: 600,
		show: true,
	})

	mainWindow.loadFile("index.html")

	const view = new electron.WebContentsView({
		x:0,
		y: 0,
		width: 600,
		height: 600,
		show: true,
	})

	view.setBounds({
		x:0,
		y: 0,
		width: 600,
		height: 600,
	})

	mainWindow.contentView.addChildView(view);
	mainWindow.contentView.addChildView(view);
	mainWindow.contentView.addChildView(view);
	mainWindow.contentView.addChildView(view);
	mainWindow1.contentView.addChildView(view);
	mainWindow1.contentView.addChildView(view);
	mainWindow1.contentView.addChildView(view);
	mainWindow.contentView.addChildView(view);

}

app.on('ready', createWindow)
app.on('window-all-closed', function () {
	if (process.platform !== 'darwin') {
		app.quit()
	}
})
app.on('activate', function () {
	if (mainWindow === null) {
		createWindow()
	}
})

@codebytere
Copy link
Member

codebytere commented May 7, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🛑 Blocks Stable
Status: 🛑 Blocks Stable
Development

Successfully merging a pull request may close this issue.

3 participants