Commit 1de6cac5 authored by David Black's avatar David Black Committed by Commit Bot

Attempt to fix crash in Assistant UI close sequence.

I was unable to reproduce the crash, but based on the stack trace in
crbug/952996, my understanding of the cause is:

(1) User attempts to close Assistant UI via caption bar while animation
    is in progress.
(2) This triggers an animation abort to occur which fires an animation
    ended callback in UiElementContainerView.
(3) UiElementContainerView currently only handles the case where an
    animation end occurs due to a new Assistant response being queued
    for display.
(4) We attempt to access the queued Assistant response and crash.

Since (1) will ultimately result in the entire Assistant view hierarchy
being destroyed, the cleanest way to handle this is to just prevent
(3) - (4) from accessing invalid state.

Bug: 952996
Change-Id: I056df562a21f18ae032beaeb46fc637ecef21c1b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1574743Reviewed-by: default avatarTao Wu <wutao@chromium.org>
Commit-Queue: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/master@{#652668}
parent 7a281343
......@@ -338,9 +338,14 @@ bool UiElementContainerView::OnAllUiElementsExitAnimationEnded(
// clearing of their views and managed resources.
OnResponseCleared();
// It is safe to add our pending response to the view hierarchy now that we've
// cleared the previous response from the stage.
OnResponseAdded(std::move(pending_response_));
// It is safe to add our pending response, if one exists, to the view
// hierarchy now that we've cleared the previous response from the stage. The
// only known case where a pending response may not exist is if the animation
// sequence was aborted due to the associated view hierarchy being destroyed.
// When that occurs, the entire UiElementContainerView hierarchy will soon be
// destroyed so we can simply take no action here. (See crbug/952996).
if (pending_response_)
OnResponseAdded(std::move(pending_response_));
// Return false to prevent the observer from destroying itself.
return false;
......
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