Commit d0372afe authored by Tom Anderson's avatar Tom Anderson Committed by Commit Bot

Fix heap-use-after-free in SelectFileDialogImplGTK

This fixes a double-free of SelectFileDialogImplGTK:

~SelectFileDialogImplGTK chrome/browser/ui/libgtkui/select_file_dialog_impl_gtk.cc:90:5
libgtkui::SelectFileDialogImplGTK::~SelectFileDialogImplGTK() chrome/browser/ui/libgtkui/select_file_dialog_impl_gtk.cc:88
DeleteInternal<ui::SelectFileDialog> base/memory/ref_counted.h:414:5
Destruct base/memory/ref_counted.h:369
Release base/memory/ref_counted.h:403
Release base/memory/scoped_refptr.h:284
~scoped_refptr base/memory/scoped_refptr.h:208
libgtkui::SelectFileDialogImplGTK::OnFileChooserDestroy(_GtkWidget*) chrome/browser/ui/libgtkui/select_file_dialog_impl_gtk.cc:556
~SelectFileDialogImplGTK chrome/browser/ui/libgtkui/select_file_dialog_impl_gtk.cc:90:5
libgtkui::SelectFileDialogImplGTK::~SelectFileDialogImplGTK() chrome/browser/ui/libgtkui/select_file_dialog_impl_gtk.cc:88
DeleteInternal<ui::SelectFileDialog> base/memory/ref_counted.h:414:5

BUG=897999,880073
R=sky

Change-Id: If69e2d857e03e8bca472ebd396e9217af3fc74bb
Reviewed-on: https://chromium-review.googlesource.com/c/1297080Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602170}
parent 6ae09b90
......@@ -452,7 +452,18 @@ void SelectFileDialogImplGTK::DestroyDialog(GtkWidget* dialog) {
}
gtk_widget_destroy(dialog);
OnFileChooserDestroy(dialog);
OnFileChooserDestroyInternal(dialog);
}
void SelectFileDialogImplGTK::OnFileChooserDestroyInternal(GtkWidget* dialog) {
CHECK(dialog);
// |parent| can be nullptr when closing the host window
// while opening the file-picker.
aura::Window* parent = dialogs_[dialog]->parent;
if (parent)
parent->RemoveObserver(this);
std::move(*dialogs_[dialog]->enable_event_listening).Run();
dialogs_.erase(dialog);
}
void* SelectFileDialogImplGTK::GetParamsForDialog(GtkWidget* dialog) {
......@@ -545,14 +556,7 @@ void SelectFileDialogImplGTK::OnSelectMultiFileDialogResponse(GtkWidget* dialog,
void SelectFileDialogImplGTK::OnFileChooserDestroy(GtkWidget* dialog) {
scoped_refptr<SelectFileDialogImplGTK> keep_alive{this};
CHECK(dialog);
// |parent| can be nullptr when closing the host window
// while opening the file-picker.
aura::Window* parent = dialogs_[dialog]->parent;
if (parent)
parent->RemoveObserver(this);
std::move(*dialogs_[dialog]->enable_event_listening).Run();
dialogs_.erase(dialog);
OnFileChooserDestroyInternal(dialog);
}
void SelectFileDialogImplGTK::OnUpdatePreview(GtkWidget* chooser) {
......
......@@ -109,9 +109,16 @@ class SelectFileDialogImplGTK : public SelectFileDialogImpl,
const base::FilePath& default_path,
gfx::NativeWindow parent);
// Deallocates all resources for dialogs_[dialog].
// Destroys the widget and deallocates all resources for dialogs_[dialog].
void DestroyDialog(GtkWidget* dialog);
// Deallocates all resources for dialogs_[dialog].
void OnFileChooserDestroyInternal(GtkWidget* dialog);
// The below callbacks may only be called from GTK, otherwise it's possible
// that the keep_alive scoped_refptr's will double-destruct |this|
// (https://crbug.com/897999).
// Callback for when the user responds to a Save As or Open File dialog.
CHROMEG_CALLBACK_1(SelectFileDialogImplGTK,
void,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment