Commit 932b85bb authored by Peter Boström's avatar Peter Boström Committed by Commit Bot

Prevent UAFs in GridLayout after Close()

BubbleDialogDelegateView::Close() removes all its children. GridLayout
wasn't built for dynamically removing children, so its ColumnSets holds
on to stale pointers to Views previously in the hierarchy. This causes
problems when asynchronous calls are made to get the bubble's preferred
size before Widget::Close() asynchronously finishes.

This is solved by removing the GridLayout as layout manager before the
children are removed. Future updates to GridLayout would preferably
accommodate removal of children.

Bug: 1106422, 1130111
Change-Id: I2181856ae9669fbce9e6f3dfa347c468c7c5d567
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425141
Auto-Submit: Peter Boström <pbos@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809760}
parent b2521ada
......@@ -186,6 +186,11 @@ void BubbleDialogModelHost::Close() {
// Widget::Close)
model_->OnWindowClosing(GetPassKey());
// TODO(pbos): Note that this is in place because GridLayout doesn't handle
// View removal correctly (keeps stale pointers). This is in place to prevent
// UAFs between Widget::Close() and destroying |this|.
SetLayoutManager(nullptr);
// TODO(pbos): Consider turning this into for-each-field remove field.
RemoveAllChildViews(true);
field_to_view_.clear();
......
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