Skip to content

Commit

Permalink
Fix assertions with Save As dialog
Browse files Browse the repository at this point in the history
Fixes some issues introduced by 8e79307 (see #3314) and
6354d8d (see #3239).

To test:
- Run `cefclient --url=chrome://net-export`
- Click the "Start Logging to Disk" button
- Exit cefclient; get no debug assertions
  • Loading branch information
magreenblatt committed Jun 20, 2024
1 parent 7d739b3 commit 5ab3234
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 7 deletions.
20 changes: 14 additions & 6 deletions libcef/browser/file_dialog_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ FileChooserParams SelectFileToFileChooserParams(
// |extensions| is a list of allowed extensions. For example, it might be
// { { "htm", "html" }, { "txt" } }
for (size_t i = 0; i < file_types->extensions.size(); ++i) {
if (!file_types->extension_mimetypes[i].empty()) {
if (file_types->extension_mimetypes.size() > i &&
!file_types->extension_mimetypes[i].empty()) {
// Use the original mime type.
params.accept_types.push_back(file_types->extension_mimetypes[i]);
} else if (file_types->extensions[i].size() == 1) {
Expand Down Expand Up @@ -273,7 +274,8 @@ CefFileDialogManager::~CefFileDialogManager() = default;
void CefFileDialogManager::Destroy() {
if (dialog_listener_) {
// Cancel the listener and delete related objects.
SelectFileDoneByListenerCallback(/*listener_destroyed=*/false);
SelectFileDoneByListenerCallback(/*listener=*/nullptr,
/*listener_destroyed=*/false);
}
DCHECK(!dialog_);
DCHECK(!dialog_listener_);
Expand Down Expand Up @@ -424,7 +426,8 @@ void CefFileDialogManager::RunSelectFile(
listener, params,
base::BindOnce(&CefFileDialogManager::SelectFileDoneByListenerCallback,
weak_ptr_factory_.GetWeakPtr(),
/*listener_destroyed=*/false));
/*listener=*/listener,
/*listener_destroyed=*/true));

// This call will not be intercepted by CefSelectFileDialogFactory due to the
// |run_from_cef=true| flag.
Expand All @@ -449,9 +452,9 @@ void CefFileDialogManager::SelectFileListenerDestroyed(

// This notification will arrive from whomever owns |listener|, so we don't
// want to execute any |listener| methods after this point.
if (dialog_listener_ && listener == dialog_listener_->listener()) {
if (dialog_listener_) {
// Cancel the currently active dialog.
SelectFileDoneByListenerCallback(/*listener_destroyed=*/true);
SelectFileDoneByListenerCallback(listener, /*listener_destroyed=*/true);
} else {
// Any future SelectFileDoneByDelegateCallback call for |listener| becomes a
// no-op.
Expand Down Expand Up @@ -570,9 +573,14 @@ void CefFileDialogManager::SelectFileDoneByDelegateCallback(
}

void CefFileDialogManager::SelectFileDoneByListenerCallback(
ui::SelectFileDialog::Listener* listener,
bool listener_destroyed) {
CEF_REQUIRE_UIT();

// |listener| will be provided iff |listener_destroyed=true|, as
// |dialog_listener_->listener()| will return nullptr at this point.
DCHECK(!listener || listener_destroyed);

// Avoid re-entrancy of this method. CefSelectFileDialogListener callbacks to
// the delegated listener may result in an immediate call to
// SelectFileListenerDestroyed() while |dialog_listener_| is still on the
Expand All @@ -587,7 +595,7 @@ void CefFileDialogManager::SelectFileDoneByListenerCallback(
DCHECK(dialog_);
DCHECK(dialog_listener_);

active_listeners_.erase(dialog_listener_->listener());
active_listeners_.erase(listener ? listener : dialog_listener_->listener());

// Clear |dialog_listener_| before calling Cancel() to avoid re-entrancy.
auto dialog_listener = dialog_listener_;
Expand Down
4 changes: 3 additions & 1 deletion libcef/browser/file_dialog_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ class CefFileDialogManager {
ui::SelectFileDialog::Listener* listener,
void* params,
const std::vector<base::FilePath>& paths);
void SelectFileDoneByListenerCallback(bool listener_destroyed);
void SelectFileDoneByListenerCallback(
ui::SelectFileDialog::Listener* listener,
bool listener_destroyed);

// CefBrowserHostBase pointer is guaranteed to outlive this object.
const raw_ptr<CefBrowserHostBase> browser_;
Expand Down
2 changes: 2 additions & 0 deletions libcef/browser/file_dialog_runner.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ class CefSelectFileDialogFactory final : public ui::SelectFileDialogFactory {
// Delegates the running of the dialog to CefFileDialogManager.
class CefSelectFileDialog final : public ui::SelectFileDialog {
public:
// |listener| is not owned by this object. It will remain valid until
// ListenerDestroyed() is called.
CefSelectFileDialog(ui::SelectFileDialog::Listener* listener,
std::unique_ptr<ui::SelectFilePolicy> policy)
: ui::SelectFileDialog(listener, std::move(policy)) {
Expand Down

0 comments on commit 5ab3234

Please sign in to comment.