search

chromiumembedded / cef

Chromium Embedded Framework (CEF). A simple framework for embedding Chromium-based browsers in other applications.
https://bitbucket.org/chromiumembedded/cef/
Other
3.38k stars 467 forks source link

chrome: Crash using some WebUI pages in Alloy-style browser #3763

Closed WizardCM closed 3 months ago

WizardCM commented 3 months ago

Describe the bug

Rather than opening a new tab/window, the entire application crashes.

To Reproduce Steps to reproduce the behavior:

  1. Launch with Chrome Runtime and Alloy Style (see cefclient flags below)
  2. Navigate to chrome://settings
  3. Click on 'Autofill and passwords'
  4. Click 'Password Manager'
  5. Crash
  6. Repeat steps 1 and 2
  7. Click 'About Chromium'
  8. Click 'Get help with Chromium'

Expected behavior

Either a new window should open, or nothing should happen.

Password Manager does work fine if opened directly chrome://password-manager

Screenshots

image

image

image

image

Versions (please complete the following information):

Additional context Does the problem reproduce with the cefclient or cefsimple sample application at the same version?

Yes.

cefclient.exe --enable-chrome-runtime --use-alloy-style

Does the problem reproduce with Google Chrome at the same version?

No.

Add any other context about the problem here.

The same crash does not occur if a Chrome UI window is opened and then chrome://settings is used from there.

If preferred, I can split these two crashes into separate bug reports.

magreenblatt commented 3 months ago

Here's the error in the password manager case:

[61616:61612:0806/163110.874:FATAL:password_manager_handler.cc(42)] Check failed: current_broswer.

From PasswordManagerHandler::HandleShowPasswordManager.

magreenblatt commented 3 months ago

In the "Get help with Chromium" case |browser| is nullptr in AboutHandler::HandleOpenHelpPage.

magreenblatt commented 3 months ago

The solution in both cases is to just exit early after chrome::FindBrowserWithTab returns nullptr.

magreenblatt commented 3 months ago

The solution in both cases is to just exit early after chrome::FindBrowserWithTab returns nullptr.

We'll need to audit the callers of chrome::FindBrowserWithTab. Many check that |browser| is non-nullptr, but some don't.

In some cases it might be better to create a new Chrome-style browser to contain the command. For example, showing the password manager in a new window instead of simply ignoring the link click.

Alternately, we could force chrome://settings (and possibly other WebUI pages) to open in a Chrome-style browser in the first place.

magreenblatt commented 3 months ago

we could force chrome://settings (and possibly other WebUI pages) to open in a Chrome-style browser in the first place.

This is likely the best option, similar to the WebUI allowlist that we had with Alloy bootstrap.