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]: BrowserWindow.removeBrowserView will crash the app if the browserView is destroyed #37642

Closed
3 tasks done
guoqikai opened this issue Mar 22, 2023 · 8 comments · Fixed by #38842
Closed
3 tasks done

Comments

@guoqikai
Copy link

guoqikai commented Mar 22, 2023

Preflight Checklist

Electron Version

23.1.4

What operating system are you using?

macOS

Operating System Version

13.0

What arch are you using?

x64

Last Known Working Electron version

No response

Expected Behavior

BrowserWindow.removeBrowserView should remove the browserView regardless of whether browserView is destroyed or not.

Actual Behavior

The app will exit with code SIGSEGV.

Testcase Gist URL

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

async function createWindow () {
  // Create the browser window.
  const mainWindow = new BrowserWindow({
    width: 800,
    height: 600,
    webPreferences: {
      preload: path.join(__dirname, 'preload.js')
    }
  })
  const view = new BrowserView({
    preload: path.join(__dirname, 'preload.js')
  });

  view.webContents.on('destroyed', () => {
    mainWindow.removeBrowserView(view)
  });

  mainWindow.addBrowserView(view);
  view.setBounds({x:0, y:0, width:300, height:300});

  // and load the index.html of the app.
  await mainWindow.loadFile('index.html')

  await view.webContents.loadFile('index.html')

  view.webContents.close();


  // Open the DevTools.
  // mainWindow.webContents.openDevTools()
}

// This method will be called when Electron has finished
// initialization and is ready to create browser windows.
// Some APIs can only be used after this event occurs.
app.whenReady().then(() => {
  createWindow()

  app.on('activate', function () {
    // On macOS it's common to re-create a window in the app when the
    // dock icon is clicked and there are no other windows open.
    if (BrowserWindow.getAllWindows().length === 0) createWindow()
  })
})

// Quit when all windows are closed, except on macOS. There, it's common
// for applications and their menu bar to stay active until the user quits
// explicitly with Cmd + Q.
app.on('window-all-closed', function () {
  if (process.platform !== 'darwin') app.quit()
})

Additional Information

No response

@jkleinsc jkleinsc added the blocked/need-repro Needs a test case to reproduce the bug label Mar 22, 2023
@github-actions
Copy link
Contributor

Hello @guoqikai. 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 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.

@guoqikai
Copy link
Author

guoqikai commented Mar 22, 2023

@codebytere
Copy link
Member

codebytere commented Apr 4, 2023

@guoqikai that is not an applicable Fiddle Gist - please provide one in the expected format. The one provided cannot be loaded into Fiddle.

@guoqikai
Copy link
Author

guoqikai commented Apr 4, 2023

@codebytere do you mean you can't open the link I provided or the code itself cannot be loaded? I've fixed the link if you meant the later one

@oold
Copy link

oold commented Apr 13, 2023

Currently also struggling with this issue, although I'm trying to close my window and therefore quit the app, when the view is destroyed. I'm suspecting this might have the same root cause.

@oold
Copy link

oold commented Apr 13, 2023

Fiddle gist for the original issue: https://gist.github.com/oold/87e07c1ac296bff2c01ac03d702fa2a6

@oold
Copy link

oold commented Apr 13, 2023

@guoqikai

@codebytere do you mean you can't open the link I provided or the code itself cannot be loaded? I've fixed the link if you meant the later one

It needs to be loadable by pasting the link here:
image

That means you need to export your code from the Electron Fiddle app.

@BABA983
Copy link

BABA983 commented Apr 24, 2023

you can try remove the browserview first, and then destory the webcontents with a private method which not documentd (browserview.webContents as any).destroy?.();

jkleinsc pushed a commit that referenced this issue Jun 21, 2023
… `webContents` (#38842)

fix: crash calling removeBrowserView() with destroyed webContents

#37642
trop bot added a commit that referenced this issue Jun 21, 2023
#37642

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
trop bot added a commit that referenced this issue Jun 21, 2023
#37642

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
trop bot added a commit that referenced this issue Jun 21, 2023
#37642

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
codebytere added a commit that referenced this issue Jun 22, 2023
… `webContents` (#38883)

fix: crash calling removeBrowserView() with destroyed webContents

#37642

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
codebytere added a commit that referenced this issue Jun 22, 2023
… `webContents` (#38884)

fix: crash calling removeBrowserView() with destroyed webContents

#37642

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
codebytere added a commit that referenced this issue Jun 22, 2023
… `webContents` (#38885)

fix: crash calling removeBrowserView() with destroyed webContents

#37642

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
MrHuangJser pushed a commit to MrHuangJser/electron that referenced this issue Dec 11, 2023
… `webContents` (electron#38842)

fix: crash calling removeBrowserView() with destroyed webContents

electron#37642
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants