0

Fix AcrossTasksDangling in select_file_dialog.h

Overriding classes need to reset `listener_` to nullptr on
`ListenerDestroyed()` calls to avoid holding a dangling ptr.

Change-Id: I16bed9d1b14d86394bf206ef2e17bdf4306d21cc
Bug: 1474236
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4794134
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Ali Hijazi <ahijazi@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1190276}
This commit is contained in:
Ali Hijazi
2023-08-30 19:48:26 +00:00
committed by Chromium LUCI CQ
parent 56b557cfe2
commit 41c119a54a
15 changed files with 29 additions and 15 deletions

@ -132,7 +132,7 @@ class TestSelectFileDialog : public ui::SelectFileDialog {
bool IsRunning(gfx::NativeWindow owning_window) const override {
return true;
}
void ListenerDestroyed() override {}
void ListenerDestroyed() override { listener_ = nullptr; }
bool HasMultipleFileTypeChoicesImpl() override { return false; }
private:

@ -83,7 +83,7 @@ class TestSelectFileDialog : public ui::SelectFileDialog {
bool IsRunning(gfx::NativeWindow owning_window) const override {
return true;
}
void ListenerDestroyed() override {}
void ListenerDestroyed() override { listener_ = nullptr; }
bool HasMultipleFileTypeChoicesImpl() override { return false; }
private:

@ -41,7 +41,9 @@ FileEntryPicker::FileEntryPicker(
base::FilePath::StringType(), owning_window, /*params=*/nullptr, caller);
}
FileEntryPicker::~FileEntryPicker() = default;
FileEntryPicker::~FileEntryPicker() {
select_file_dialog_->ListenerDestroyed();
}
void FileEntryPicker::FileSelected(const base::FilePath& path,
int index,

@ -78,7 +78,7 @@ class TestSelectFileDialog : public ui::SelectFileDialog {
bool IsRunning(gfx::NativeWindow owning_window) const override {
return false;
}
void ListenerDestroyed() override {}
void ListenerDestroyed() override { listener_ = nullptr; }
bool HasMultipleFileTypeChoicesImpl() override { return false; }
private:
@ -143,7 +143,7 @@ class FakeCancellingSelectFileDialog : public ui::SelectFileDialog {
bool IsRunning(gfx::NativeWindow owning_window) const override {
return false;
}
void ListenerDestroyed() override {}
void ListenerDestroyed() override { listener_ = nullptr; }
bool HasMultipleFileTypeChoicesImpl() override { return false; }
private:

@ -36,7 +36,9 @@ bool SelectPredeterminedFileDialog::IsRunning(
return false;
}
void SelectPredeterminedFileDialog::ListenerDestroyed() {}
void SelectPredeterminedFileDialog::ListenerDestroyed() {
listener_ = nullptr;
}
bool SelectPredeterminedFileDialog::HasMultipleFileTypeChoicesImpl() {
return false;

@ -404,6 +404,12 @@ IN_PROC_BROWSER_TEST_P(SelectFileDialogExtensionBrowserTest, CanResize) {
// The dialog should be resizable.
ASSERT_EQ(!GetParam().tablet_mode, OpenDialogIsResizable());
// Click the "Cancel" button. This closes the dialog thus removing it from
// `PendingDialog::map_`. `PendingDialog::map_` otherwise prevents the dialog
// from being destroyed on `reset()` in test TearDown and the
// `SelectFileDialog::listener_` becomes dangling.
CloseDialog(DIALOG_BTN_CANCEL, owning_window);
}

@ -280,7 +280,7 @@ class TestSelectFileDialog : public ui::SelectFileDialog {
return false;
}
void ListenerDestroyed() override {}
void ListenerDestroyed() override { listener_ = nullptr; }
bool HasMultipleFileTypeChoicesImpl() override { return false; }

@ -126,7 +126,7 @@ class FakeSelectFileDialog : public ui::SelectFileDialog {
bool IsRunning(gfx::NativeWindow owning_window) const override {
return true;
}
void ListenerDestroyed() override {}
void ListenerDestroyed() override { listener_ = nullptr; }
bool HasMultipleFileTypeChoicesImpl() override { return false; }
void VerifyExtensions(const FileTypeInfo* file_types) {

@ -96,7 +96,7 @@ class TestSelectFileDialog : public ui::SelectFileDialog {
bool IsRunning(gfx::NativeWindow owning_window) const override {
return false;
}
void ListenerDestroyed() override {}
void ListenerDestroyed() override { listener_ = nullptr; }
bool HasMultipleFileTypeChoicesImpl() override { return false; }
private:

@ -70,7 +70,7 @@ class PPAPITestSelectFileDialog : public ui::SelectFileDialog {
bool IsRunning(gfx::NativeWindow owning_window) const override {
return false;
}
void ListenerDestroyed() override {}
void ListenerDestroyed() override { listener_ = nullptr; }
private:
void RespondToFileSelectionRequest(void* params) {

@ -53,7 +53,7 @@ class CancellingSelectFileDialog : public ui::SelectFileDialog {
bool IsRunning(gfx::NativeWindow owning_window) const override {
return false;
}
void ListenerDestroyed() override {}
void ListenerDestroyed() override { listener_ = nullptr; }
bool HasMultipleFileTypeChoicesImpl() override { return false; }
private:
@ -112,7 +112,7 @@ class FakeSelectFileDialog : public ui::SelectFileDialog {
bool IsRunning(gfx::NativeWindow owning_window) const override {
return false;
}
void ListenerDestroyed() override {}
void ListenerDestroyed() override { listener_ = nullptr; }
bool HasMultipleFileTypeChoicesImpl() override { return false; }
private:

@ -1544,7 +1544,7 @@ class FakeSelectFileDialog : public ui::SelectFileDialog {
bool IsRunning(gfx::NativeWindow owning_window) const override {
return false;
}
void ListenerDestroyed() override {}
void ListenerDestroyed() override { listener_ = nullptr; }
bool HasMultipleFileTypeChoicesImpl() override { return false; }
private:

@ -93,4 +93,8 @@ void FakeSelectFileDialog::CallMultiFilesSelected(
listener_->MultiFilesSelected(files, params_);
}
void FakeSelectFileDialog::ListenerDestroyed() {
listener_ = nullptr;
}
} // namespace ui

@ -79,7 +79,7 @@ class FakeSelectFileDialog : public SelectFileDialog {
const GURL* caller) override;
bool HasMultipleFileTypeChoicesImpl() override;
bool IsRunning(gfx::NativeWindow owning_window) const override;
void ListenerDestroyed() override {}
void ListenerDestroyed() override;
// Returns the file title provided to the dialog.
const std::u16string& title() const { return title_; }

@ -234,7 +234,7 @@ class SHELL_DIALOGS_EXPORT SelectFileDialog
const GURL* caller) = 0;
// The listener to be notified of selection completion.
raw_ptr<Listener, AcrossTasksDanglingUntriaged> listener_;
raw_ptr<Listener> listener_;
private:
// Tests if the file selection dialog can be displayed by