From 5ab32347b8e453a052d708bc5ee86daa75cb4b4e Mon Sep 17 00:00:00 2001
From: Marshall Greenblatt <magreenblatt@gmail.com>
Date: Thu, 20 Jun 2024 13:55:23 -0400
Subject: [PATCH] Fix assertions with Save As dialog

Fixes some issues introduced by 8e79307a62 (see #3314) and
6354d8daf1 (see #3239).

To test:
- Run `cefclient --url=chrome://net-export`
- Click the "Start Logging to Disk" button
- Exit cefclient; get no debug assertions
---
 libcef/browser/file_dialog_manager.cc | 20 ++++++++++++++------
 libcef/browser/file_dialog_manager.h  |  4 +++-
 libcef/browser/file_dialog_runner.cc  |  2 ++
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/libcef/browser/file_dialog_manager.cc b/libcef/browser/file_dialog_manager.cc
index a7ad14278..5ae29e534 100644
--- a/libcef/browser/file_dialog_manager.cc
+++ b/libcef/browser/file_dialog_manager.cc
@@ -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) {
@@ -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_);
@@ -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.
@@ -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.
@@ -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
@@ -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_;
diff --git a/libcef/browser/file_dialog_manager.h b/libcef/browser/file_dialog_manager.h
index 504136ba0..25128ee72 100644
--- a/libcef/browser/file_dialog_manager.h
+++ b/libcef/browser/file_dialog_manager.h
@@ -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_;
diff --git a/libcef/browser/file_dialog_runner.cc b/libcef/browser/file_dialog_runner.cc
index aed901b02..a793fb1ad 100644
--- a/libcef/browser/file_dialog_runner.cc
+++ b/libcef/browser/file_dialog_runner.cc
@@ -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)) {