Commit 971d2ba1 authored by David Black's avatar David Black Committed by Commit Bot

Fix crash caused by failure to detach NativeViewHosts.

With ChromeVox enabled, queries containing cards after the first
would result in a crash during clean up of the previous response's
cards. This occurred due to failure to detach the card's NativeViewHost
before removing from the view hierarchy and destroying.

Bug: b:116173111
Change-Id: Icb115ab1d3a5afad23a897b1f5a4dae9734210ff
Reviewed-on: https://chromium-review.googlesource.com/1237973
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594799}
parent 479dc64d
......@@ -323,6 +323,14 @@ void UiElementContainerView::OnResponseCleared() {
// Prevent any in-flight card rendering requests from returning.
render_request_weak_factory_.InvalidateWeakPtrs();
// We need to detach native view hosts before they are removed from the view
// hierarchy and destroyed.
if (!native_view_hosts_.empty()) {
for (views::NativeViewHost* native_view_host : native_view_hosts_)
native_view_host->Detach();
native_view_hosts_.clear();
}
// We can prevent over-propagation of the PreferredSizeChanged event by
// stopping propagation during batched view hierarchy add/remove operations.
SetPropagatePreferredSizeChanged(false);
......@@ -498,6 +506,10 @@ void UiElementContainerView::OnCardReady(
content_view()->AddChildView(view_holder);
view_holder->Attach();
// Cache a reference to the attached native view host so that it can be
// detached prior to being removed from the view hierarchy and destroyed.
native_view_hosts_.push_back(view_holder);
// The view will be animated on its own layer, so we need to do some initial
// layer setup. We're going to fade the view in, so hide it. Note that we
// approximate 0% opacity by actually using 0.01%. We do this to workaround
......
......@@ -19,6 +19,10 @@ namespace ui {
class CallbackLayerAnimationObserver;
} // namespace ui
namespace views {
class NativeViewHost;
} // namespace views
namespace ash {
class AssistantCardElement;
......@@ -92,6 +96,11 @@ class UiElementContainerView : public AssistantScrollView,
// been added/removed before propagating. This reduces layout passes.
bool propagate_preferred_size_changed_ = true;
// Cached references to the native view hosts associated with card elements.
// We maintain a reference so long as the native view host is attached so that
// we can detach before removal from the view hierarchy and destruction.
std::vector<views::NativeViewHost*> native_view_hosts_;
// UI elements will be animated on their own layers. We track the desired
// opacity to which each layer should be animated when processing the next
// query response.
......
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