Fix crash when closing and reopening the Assistant UI.

This crash is hard to reproduce, but what happened was:
    - User sent a query ('volume up'). The response for this is
      rendered.
    - User sent another query (click in suggestion chip). This means we
      start fading out the response of the first query.
    - The response of the new query arrives before the fade-out is
      complete,so the response is postponed until the fade out is
      complete.
    - Now close the Assistant UI. Due to the bug, the previous response
      was not cleared but it remained queued.
    - Open the UI again, and send another query ('volume down').
    - This fades out the initial text. If that completes before the new
      response arrives, the previous queued response was animated in.
    - When this animate in completed, the ui_element_container_view
      tried to access the current response, which is a nullptr and
      caused a crash.

Note that this is not the crash described in the bug report.
However I failed to reproduce that crash, and because it is a related
area I am hoping that both have the same root cause.

This will need to be monitored to ensure the bug is actually fixed.

       complexity involved I abandoned the effort.

Bug: b:145136105
Change-Id: Ia512eb370c2b9f8ab506575f2252dc520471793a
Tests: Manually tested. I tried adding unittests but given the
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1993114
Commit-Queue: Jeroen Dhollander <jeroendh@google.com>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#730233}

ash/assistant/ui/main_stage/animated_container_view.cc

@@ -58,6 +58,7 @@ void AnimatedContainerView::OnResponseChanged(
 
 void AnimatedContainerView::OnResponseCleared() {
   RemoveAllViews();
+  queued_response_ = nullptr;
 }
 
 void AnimatedContainerView::RemoveAllViews() {
@@ -137,13 +138,13 @@ void AnimatedContainerView::FadeOutViews() {
 
 void AnimatedContainerView::ChangeResponse(
     const scoped_refptr<const AssistantResponse>& response) {
-  // We may have to pend the response while we animate the previous response off
-  // stage. We use a shared pointer to ensure that any views we add to the view
-  // hierarchy can be removed before the underlying views are destroyed.
-  pending_response_ = response;
+  // We may have to postpone the response while we animate the previous response
+  // off stage. We use a shared pointer to ensure that any views we add to the
+  // view hierarchy can be removed before the underlying views are destroyed.
+  queued_response_ = response;
 
   // If we are currently fading out the old content, don't interrupt it.
-  // When the fading out is completed, it will detect we've got a pending
+  // When the fading out is completed, it will detect we've got a queued
   // response and animate it in.
   if (fade_out_in_progress_)
     return;
@@ -151,7 +152,7 @@ void AnimatedContainerView::ChangeResponse(
   // If we don't have any pre-existing content, there is nothing to animate off
   // stage so we can proceed to add the new response.
   if (content_view()->children().empty()) {
-    AddResponse(std::move(pending_response_));
+    AddResponse(std::move(queued_response_));
     return;
   }
 
@@ -242,10 +243,10 @@ bool AnimatedContainerView::AnimateOutObserverCallback(
   // clearing of their views and managed resources.
   weak_ptr->RemoveAllViews();
 
-  // It is safe to add our pending response, if one exists, to the view
+  // It is safe to add our queued response, if one exists, to the view
   // hierarchy now that we've cleared the previous response from the stage.
-  if (weak_ptr->pending_response_)
-    weak_ptr->AddResponse(std::move(weak_ptr->pending_response_));
+  if (weak_ptr->queued_response_)
+    weak_ptr->AddResponse(std::move(weak_ptr->queued_response_));
 
   // We return true to delete our observer.
   return true;
@@ -268,8 +269,8 @@ bool AnimatedContainerView::FadeOutObserverCallback(
 
   // If the new response arrived while the fade-out was in progress, we will
   // start handling it now.
-  if (weak_ptr->pending_response_)
-    weak_ptr->ChangeResponse(std::move(weak_ptr->pending_response_));
+  if (weak_ptr->queued_response_)
+    weak_ptr->ChangeResponse(std::move(weak_ptr->queued_response_));
 
   // We return true to delete our observer.
   return true;

ash/assistant/ui/main_stage/animated_container_view.h

@@ -130,11 +130,11 @@ class COMPONENT_EXPORT(ASSISTANT_UI) AnimatedContainerView
   bool fade_out_in_progress_ = false;
 
   // Shared pointers to the response that is currently on stage as well as the
-  // pending response to be presented following the former's animated exit. We
+  // queued response to be presented following the former's animated exit. We
   // use shared pointers to ensure that underlying views are not destroyed
   // before we have an opportunity to remove their associated views.
   scoped_refptr<const AssistantResponse> response_;
-  scoped_refptr<const AssistantResponse> pending_response_;
+  scoped_refptr<const AssistantResponse> queued_response_;
 
   base::WeakPtrFactory<AnimatedContainerView> weak_factory_{this};
 

ash/assistant/ui/main_stage/ui_element_container_view.cc

@@ -183,6 +183,7 @@ void UiElementContainerView::OnAllViewsAnimatedIn() {
   // We don't read when there is TTS to avoid speaking over the server response.
   const AssistantResponse* response =
       delegate()->GetInteractionModel()->response();
+  DCHECK(response);
   if (!response->has_tts())
     NotifyAccessibilityEvent(ax::mojom::Event::kAlert, true);
 }