Skip to content

Commit

Permalink
[MSan] Use WeakPtr for done callback in DesktopMediaPickerViews tests
Browse files Browse the repository at this point in the history
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 <ellyjones@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1314136}
  • Loading branch information
zetafunction authored and pull[bot] committed Jun 14, 2024
1 parent 2c03ee7 commit 1125150
Showing 1 changed file with 3 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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<views::test::WidgetDestroyedWaiter>(
waiter.WaitIfNeededAndGet());
Expand Down Expand Up @@ -258,6 +258,8 @@ class DesktopMediaPickerViewsTestBase : public testing::Test {
base::RunLoop run_loop_;
std::optional<content::DesktopMediaID> picked_id_;
std::unique_ptr<views::test::WidgetDestroyedWaiter> widget_destroyed_waiter_;

base::WeakPtrFactory<DesktopMediaPickerViewsTestBase> weak_factory_{this};
};

class DesktopMediaPickerViewsTest
Expand Down

0 comments on commit 1125150

Please sign in to comment.