From 1125150659c2e12dc823d0ca5523887d2b8ad2af Mon Sep 17 00:00:00 2001 From: Daniel Cheng Date: Wed, 12 Jun 2024 18:25:18 +0000 Subject: [PATCH] [MSan] Use WeakPtr for done callback in DesktopMediaPickerViews tests The test fixture uses base::Unretained() for the done callback, but DesktopMediaPickerViews::NotifyDialogResult() asynchronously invokes this callback when the picker is done. The test fixture also embeds a BrowserTaskEnvironment, which runs tasks until idle at destruction time. This ensures the callback runs before the test fixture is completely destroyed. Despite this, the current tests trigger MSan use-after-dtor errors, since the bound callback uses other fields of the test fixture which are already destroyed: fields are destroyed in reverse order of declaration, and BrowserTaskEnvironment is the first field. As it's unlikely that the test cares about the callback anymore when the fixture is destroyed, just use a WeakPtr. Bug: 40222690 Change-Id: If7c05f4e8bae01d4f5950776643b0273eab6a8ac Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5626676 Reviewed-by: Elly FJ Commit-Queue: Daniel Cheng Cr-Commit-Position: refs/heads/main@{#1314136} --- .../desktop_capture/desktop_media_picker_views_unittest.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views_unittest.cc b/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views_unittest.cc index 1f12c0dc7def89..455021daecc443 100644 --- a/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views_unittest.cc +++ b/chrome/browser/ui/views/desktop_capture/desktop_media_picker_views_unittest.cc @@ -216,7 +216,7 @@ class DesktopMediaPickerViewsTestBase : public testing::Test { picker_views_->Show( picker_params, std::move(source_lists), base::BindOnce(&DesktopMediaPickerViewsTestBase::OnPickerDone, - base::Unretained(this))); + weak_factory_.GetWeakPtr())); widget_destroyed_waiter_ = std::make_unique( waiter.WaitIfNeededAndGet()); @@ -258,6 +258,8 @@ class DesktopMediaPickerViewsTestBase : public testing::Test { base::RunLoop run_loop_; std::optional picked_id_; std::unique_ptr widget_destroyed_waiter_; + + base::WeakPtrFactory weak_factory_{this}; }; class DesktopMediaPickerViewsTest