Skip to content

Commit

Permalink
[Files SWA]: Use WeakPtr to prevent a possible UAR bug.
Browse files Browse the repository at this point in the history
UAR = use after release.

Bug: 1290008

Change-Id: I8ec1cab58830a6e12468523b737ee589cfe8e23a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3426095
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Bo Majewski <majewski@chromium.org>
Cr-Commit-Position: refs/heads/main@{#965077}
  • Loading branch information
Bo Majewski authored and Chromium LUCI CQ committed Jan 31, 2022
1 parent 56102b6 commit 0a4ede6
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 9 deletions.
21 changes: 12 additions & 9 deletions chrome/browser/ui/views/select_file_dialog_extension.cc
Original file line number Diff line number Diff line change
Expand Up @@ -194,13 +194,13 @@ SelectFileDialogExtension::RoutingID GetRoutingID(
class SystemFilesAppDialogDelegate : public chromeos::SystemWebDialogDelegate,
public ash::ColorModeObserver {
public:
SystemFilesAppDialogDelegate(SelectFileDialogExtension* parent,
SystemFilesAppDialogDelegate(base::WeakPtr<SelectFileDialogExtension> parent,
const std::string& id,
GURL url,
std::u16string title)
: chromeos::SystemWebDialogDelegate(url, title),
id_(id),
parent_(parent) {
parent_(std::move(parent)) {
ash::ColorProvider::Get()->AddObserver(this);
}
~SystemFilesAppDialogDelegate() override {
Expand Down Expand Up @@ -240,11 +240,17 @@ class SystemFilesAppDialogDelegate : public chromeos::SystemWebDialogDelegate,
}

void OnDialogShown(content::WebUI* webui) override {
parent_->OnSystemDialogShown(webui->GetWebContents(), id_);
if (parent_) {
parent_->OnSystemDialogShown(webui->GetWebContents(), id_);
}
chromeos::SystemWebDialogDelegate::OnDialogShown(webui);
}

void OnDialogWillClose() override { parent_->OnSystemDialogWillClose(); }
void OnDialogWillClose() override {
if (parent_) {
parent_->OnSystemDialogWillClose();
}
}

void OnColorModeChanged(bool dark_mode_enabled) override {
auto* color_provider = ash::ColorProvider::Get();
Expand All @@ -263,7 +269,7 @@ class SystemFilesAppDialogDelegate : public chromeos::SystemWebDialogDelegate,
const std::string id_;

// The parent of this delegate.
SelectFileDialogExtension* parent_;
base::WeakPtr<SelectFileDialogExtension> parent_;
};

/////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -536,11 +542,8 @@ void SelectFileDialogExtension::SelectFileWithFileManagerParams(
base_window ? base_window->GetNativeWindow() : owner.window;

if (ash::features::IsFileManagerSwaEnabled()) {
// SystemFilesAppDialogDelegate is a self-deleting class that calls the
// delete operator once the dialog for which it was created has been closed.
// Hence this memory-leak looking code pattern.
auto* dialog_delegate = new SystemFilesAppDialogDelegate(
this, routing_id, file_manager_url, dialog_title);
weak_factory_.GetWeakPtr(), routing_id, file_manager_url, dialog_title);
dialog_delegate->SetModal(owner.window != nullptr);
dialog_delegate->set_can_resize(can_resize_);
dialog_delegate->ShowSystemDialogForBrowserContext(profile_, parent_window);
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ui/views/select_file_dialog_extension.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

#include "ash/public/cpp/style/color_mode_observer.h"
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/ui/views/extensions/extension_dialog_observer.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "ui/gfx/native_widget_types.h" // gfx::NativeWindow
Expand Down Expand Up @@ -185,6 +186,7 @@ class SelectFileDialogExtension : public ui::SelectFileDialog,
int selection_index_ = 0;
bool can_resize_ = true;
void* params_ = nullptr;
base::WeakPtrFactory<SelectFileDialogExtension> weak_factory_{this};
};

#endif // CHROME_BROWSER_UI_VIEWS_SELECT_FILE_DIALOG_EXTENSION_H_

0 comments on commit 0a4ede6

Please sign in to comment.