Refactor AnimatedContainerView for response processing v2.

This CL modifies AnimatedContainerView to:
- Split HandleResponse() into HandleSuggestion() and HandleUiElement().
- In this CL, these new functions are invoked at the same time as
  HandleResponse(), but a follow up CL will add additional invocations
  in response processing v2 in reaction to AssistantResponseObserver
  events.
- Modifies existing paradigm of derivative classes explicitly having to
  call AddElementAnimator() to instead have HandleSuggestion() and
  HandleUiElement() return any new animators that should be owned and
  acted upon. This will be convenient in response processing v2 when
  animations are most often *not* performed in unison.

Other changes in this CL are trivial and should be straightforward to
follow :)

Note that this CL has *no effect* on existing behavior.


Bug: b:129411551
Change-Id: Ibbcd7654c5a2b6dfe7ca488dc559738531085b09
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2067473
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Jeroen Dhollander <jeroendh@google.com>
Cr-Commit-Position: refs/heads/master@{#745742}

ash/ambient/ui/assistant_response_container_view.cc

@@ -4,8 +4,6 @@
 
 #include "ash/ambient/ui/assistant_response_container_view.h"
 
-#include <memory>
-
 #include "ash/assistant/model/assistant_interaction_model_observer.h"
 #include "ash/assistant/model/assistant_response.h"
 #include "ash/assistant/model/ui/assistant_card_element.h"
@@ -13,6 +11,7 @@
 #include "ash/assistant/model/ui/assistant_ui_element.h"
 #include "ash/assistant/ui/assistant_view_delegate.h"
 #include "ash/assistant/ui/main_stage/assistant_text_element_view.h"
+#include "ash/assistant/ui/main_stage/element_animator.h"
 #include "ui/views/layout/box_layout.h"
 
 namespace ash {
@@ -51,23 +50,23 @@ void AssistantResponseContainerView::InitLayout() {
       views::BoxLayout::Orientation::kVertical));
 }
 
-void AssistantResponseContainerView::HandleResponse(
-    const AssistantResponse& response) {
-  for (const auto& ui_element : response.GetUiElements()) {
+std::unique_ptr<ElementAnimator>
+AssistantResponseContainerView::HandleUiElement(
+    const AssistantUiElement* ui_element) {
   switch (ui_element->type()) {
     case AssistantUiElementType::kCard:
       // For card elements, we instead use the "fallback" message for HTML
       // card rendering as the text response.
       AddTextElementView(new AssistantTextElement(
-            static_cast<const AssistantCardElement*>(ui_element.get())
-                ->fallback()));
+          static_cast<const AssistantCardElement*>(ui_element)->fallback()));
       break;
     case AssistantUiElementType::kText:
-        AddTextElementView(
-            static_cast<const AssistantTextElement*>(ui_element.get()));
+      AddTextElementView(static_cast<const AssistantTextElement*>(ui_element));
       break;
   }
-  }
+
+  // Return |nullptr| to prevent animations.
+  return nullptr;
 }
 
 void AssistantResponseContainerView::AddTextElementView(

ash/ambient/ui/assistant_response_container_view.h

@@ -5,12 +5,13 @@
 #ifndef ASH_AMBIENT_UI_ASSISTANT_RESPONSE_CONTAINER_VIEW_H_
 #define ASH_AMBIENT_UI_ASSISTANT_RESPONSE_CONTAINER_VIEW_H_
 
+#include <memory>
+
 #include "ash/assistant/ui/main_stage/animated_container_view.h"
 #include "base/macros.h"
 
 namespace ash {
 
-class AssistantResponse;
 class AssistantTextElement;
 class AssistantViewDelegate;
 
@@ -29,7 +30,8 @@ class AssistantResponseContainerView : public AnimatedContainerView {
   void AddTextElementView(const AssistantTextElement* text_element);
 
   // AnimatedContainerView:
-  void HandleResponse(const AssistantResponse& response) override;
+  std::unique_ptr<ElementAnimator> HandleUiElement(
+      const AssistantUiElement* ui_element) override;
 
   DISALLOW_COPY_AND_ASSIGN(AssistantResponseContainerView);
 };

ash/assistant/assistant_interaction_controller.cc

@@ -386,6 +386,14 @@ void AssistantInteractionController::OnInteractionStarted(
 
 void AssistantInteractionController::OnInteractionFinished(
     AssistantInteractionResolution resolution) {
+  // If we don't have an active interaction, that indicates that this
+  // interaction was explicitly stopped outside of LibAssistant. In this case,
+  // we ensure that the mic is closed but otherwise ignore this event.
+  if (IsResponseProcessingV2Enabled() && !HasActiveInteraction()) {
+    model_.SetMicState(MicState::kClosed);
+    return;
+  }
+
   model_.SetInteractionState(InteractionState::kInactive);
   model_.SetMicState(MicState::kClosed);
 

ash/assistant/model/assistant_response.cc

@@ -165,18 +165,22 @@ AssistantResponse::GetUiElements() const {
   return ui_elements_;
 }
 
+// TODO(b/112034793): Migrate |id| into AssistantSuggestion.
 void AssistantResponse::AddSuggestions(
     std::vector<AssistantSuggestionPtr> suggestions) {
-  std::vector<AssistantSuggestion*> ptrs;
+  // A mapping of raw suggestion pointers to their respective ids. Note that we
+  // use the index of each suggestion within our backing vector to represent id.
+  std::map<int, const AssistantSuggestion*> ptrs;
 
   for (AssistantSuggestionPtr& suggestion : suggestions) {
     suggestions_.push_back(std::move(suggestion));
-    ptrs.push_back(suggestions_.back().get());
+    ptrs.insert({suggestions_.size() - 1, suggestions_.back().get()});
   }
 
   NotifySuggestionsAdded(ptrs);
 }
 
+// TODO(b/112034793): Migrate |id| into AssistantSuggestion.
 const chromeos::assistant::mojom::AssistantSuggestion*
 AssistantResponse::GetSuggestionById(int id) const {
   // We consider the index of a suggestion within our backing vector to be its
@@ -186,12 +190,13 @@ AssistantResponse::GetSuggestionById(int id) const {
   return suggestions_.at(id).get();
 }
 
+// TODO(b/112034793): Migrate |id| into AssistantSuggestion.
 std::map<int, const chromeos::assistant::mojom::AssistantSuggestion*>
 AssistantResponse::GetSuggestions() const {
+  // A mapping of raw suggestion pointers to their respective ids. Note that we
+  // use the index of each suggestion within our backing vector to represent id.
   std::map<int, const AssistantSuggestion*> suggestions;
 
-  // We use index within our backing vector to represent the unique identifier
-  // for a suggestion.
   int id = 0;
   for (const AssistantSuggestionPtr& suggestion : suggestions_)
     suggestions[id++] = suggestion.get();
@@ -211,7 +216,7 @@ void AssistantResponse::NotifyUiElementAdded(
 }
 
 void AssistantResponse::NotifySuggestionsAdded(
-    const std::vector<AssistantSuggestion*>& suggestions) {
+    const std::map<int, const AssistantSuggestion*>& suggestions) {
   for (auto& observer : observers_)
     observer.OnSuggestionsAdded(suggestions);
 }

ash/assistant/model/assistant_response.h

@@ -82,7 +82,7 @@ class COMPONENT_EXPORT(ASSISTANT_MODEL) AssistantResponse
 
  private:
   void NotifyUiElementAdded(const AssistantUiElement* ui_element);
-  void NotifySuggestionsAdded(const std::vector<AssistantSuggestion*>&);
+  void NotifySuggestionsAdded(const std::map<int, const AssistantSuggestion*>&);
 
   struct PendingUiElement;
   class Processor;

ash/assistant/model/assistant_response_observer.h

@@ -5,7 +5,7 @@
 #ifndef ASH_ASSISTANT_MODEL_ASSISTANT_RESPONSE_OBSERVER_H_
 #define ASH_ASSISTANT_MODEL_ASSISTANT_RESPONSE_OBSERVER_H_
 
-#include <vector>
+#include <map>
 
 #include "base/component_export.h"
 #include "base/observer_list_types.h"
@@ -25,7 +25,9 @@ class COMPONENT_EXPORT(ASSISTANT_MODEL) AssistantResponseObserver
   virtual void OnUiElementAdded(const AssistantUiElement* ui_element) {}
 
   // Invoked when the specified |suggestions| are added to the response.
-  virtual void OnSuggestionsAdded(const std::vector<AssistantSuggestion*>&) {}
+  // Note that the provided map is keyed by id.
+  virtual void OnSuggestionsAdded(
+      const std::map<int, const AssistantSuggestion*>& suggestions) {}
 
  protected:
   AssistantResponseObserver() = default;

ash/assistant/ui/main_stage/animated_container_view.cc

@@ -101,6 +101,17 @@ void AnimatedContainerView::AnimateIn() {
   animation_observer->SetActive();
 }
 
+std::unique_ptr<ElementAnimator> AnimatedContainerView::HandleUiElement(
+    const AssistantUiElement* ui_element) {
+  return nullptr;
+}
+
+std::unique_ptr<ElementAnimator> AnimatedContainerView::HandleSuggestion(
+    int id,
+    const AssistantSuggestion* suggestion) {
+  return nullptr;
+}
+
 void AnimatedContainerView::SetPropagatePreferredSizeChanged(bool propagate) {
   if (propagate == propagate_preferred_size_changed_)
     return;
@@ -189,7 +200,25 @@ void AnimatedContainerView::AddResponse(
   // to the view hierarchy to reduce layout passes.
   SetPropagatePreferredSizeChanged(false);
 
-  HandleResponse(*response_);
+  std::vector<std::unique_ptr<ElementAnimator>> animators;
+
+  // Create views (and animators) for the suggestions belonging to the response.
+  for (const auto& pair : response_->GetSuggestions()) {
+    auto animator =
+        HandleSuggestion(/*id=*/pair.first, /*suggestion=*/pair.second);
+    if (animator)
+      animators.push_back(std::move(animator));
+  }
+
+  // Create views (and animators) for the UI elements belonging to the response.
+  for (const auto& ui_element : response_->GetUiElements()) {
+    auto animator = HandleUiElement(ui_element.get());
+    if (animator)
+      animators.push_back(std::move(animator));
+  }
+
+  // Cache the animators that were just created.
+  std::move(animators.begin(), animators.end(), std::back_inserter(animators_));
 
   // Now that the response for the current query has been added to the view
   // hierarchy, we can restart propagation of PreferredSizeChanged events since

ash/assistant/ui/main_stage/animated_container_view.h

@@ -5,11 +5,13 @@
 #ifndef ASH_ASSISTANT_UI_MAIN_STAGE_ANIMATED_CONTAINER_VIEW_H_
 #define ASH_ASSISTANT_UI_MAIN_STAGE_ANIMATED_CONTAINER_VIEW_H_
 
+#include <map>
 #include <memory>
 #include <vector>
 
 #include "ash/assistant/model/assistant_interaction_model_observer.h"
 #include "ash/assistant/ui/base/assistant_scroll_view.h"
+#include "chromeos/services/assistant/public/mojom/assistant.mojom-forward.h"
 
 namespace ui {
 class CallbackLayerAnimationObserver;
@@ -17,40 +19,43 @@ class CallbackLayerAnimationObserver;
 
 namespace ash {
 
-class AssistantViewDelegate;
 class AssistantResponse;
+class AssistantUiElement;
+class AssistantViewDelegate;
 class ElementAnimator;
 
-// A view that will observe the |AssistantResponse| and which will use
-// |ElementAnimator| to animate each child view.
+// A view that will observe the AssistantResponse and which will use
+// ElementAnimator to animate each child view.
 //
-// To use this you must implement |HandleResponse| and in there
-//    - Add the new child views for the given |AssistantResponse|.
-//    - Add animators for the new view by calling |AddElementAnimator|.
+// To use this you should implement HandleUiElement() and/or HandleSuggestion()
+// to:
+//    - Add new child views as appropriate.
+//    - Return animators for any newly created views.
 //
 // More in detail, this is what will happen:
-//    1) When |AssistantInteractionModelObserver::OnCommittedQueryChanged| is
-//       observed, |FadeOut| is called on all |ElementAnimator| instances.
+//    1) When AssistantInteractionModelObserver::OnCommittedQueryChanged() is
+//       observed, FadeOut() is called on all ElementAnimator instances.
 //       Furthermore all views will stop processing events (like click).
-//    2) When |AssistantInteractionModelObserver::OnResponseChanged| is
-//       observed, we wait until the |FadeOut| animations are complete.
-//    3) Next |AnimateOut| is invoked on all |ElementAnimator| instances.
+//    2) When AssistantInteractionModelObserver::OnResponseChanged() is
+//       observed, we wait until the FadeOut() animations are complete.
+//    3) Next AnimateOut() is invoked on all ElementAnimator instances.
 //    4) When these animations are complete, all child views are removed.
-//    5) |AnimatedContainerView::OnAllViewsRemoved| is invoked to inform
-//       the derived class all child views are removed.
-//    6) Next |AnimatedContainerView::HandleResponse| is called with the new
-//       |AssistantResponse|.
-//       In here the derived class should add the child views for the new
-//       response, as well as adding |ElementAnimator| instances by calling
-//       |AnimatedContainerView::AddElementAnimator|.
-//    7) When all new child views have been added, |AnimateIn| is invoked on
-//       all |ElementAnimator| instances.
+//    5) OnAllViewsRemoved() is invoked to inform the derived class all child
+//       views are removed.
+//    6) Next HandleSuggestion() and HandleUiElement() is called for the new
+//       AssistantResponse. In those methods, the derived class should add the
+//       child views for the new response, as well as return ElementAnimator
+//       instances.
+//    7) When all new child views have been added, AnimateIn() is invoked on
+//       all ElementAnimator instances.
 //    8) Finally when this animation is complete the derived class is informed
-//       through |AnimatedContainerView::OnAllViewsAnimatedIn|.
+//       through OnAllViewsAnimatedIn().
 class COMPONENT_EXPORT(ASSISTANT_UI) AnimatedContainerView
     : public AssistantScrollView,
       public AssistantInteractionModelObserver {
  public:
+  using AssistantSuggestion = chromeos::assistant::mojom::AssistantSuggestion;
+
   explicit AnimatedContainerView(AssistantViewDelegate* delegate);
   ~AnimatedContainerView() override;
 
@@ -65,8 +70,7 @@ class COMPONENT_EXPORT(ASSISTANT_UI) AnimatedContainerView
 
   // AssistantInteractionModelObserver:
   void OnCommittedQueryChanged(const AssistantQuery& query) override;
-  void OnResponseChanged(
-      const scoped_refptr<AssistantResponse>& response) override;
+  void OnResponseChanged(const scoped_refptr<AssistantResponse>&) override;
   void OnResponseCleared() override;
 
   // Remove all current responses/views.
@@ -88,12 +92,22 @@ class COMPONENT_EXPORT(ASSISTANT_UI) AnimatedContainerView
   // This is called when the exit animations are done.
   virtual void OnAllViewsRemoved() {}
 
-  // Callback called to create the new views.
-  // For each new views it should
-  //    - Create and add the |views::View|.
-  //    - Call |AddElementAnimator| and pass it the |ElementAnimator| to
-  //      animate the view.
-  virtual void HandleResponse(const AssistantResponse& response) = 0;
+  // Callback called to create a view for a UI element.
+  // The implementer should:
+  //    - Create and add the appropriate views::View.
+  //    - Return an ElementAnimator to animate the view. Note that it is
+  //      permissible to return |nullptr| if no managed animation is desired.
+  virtual std::unique_ptr<ElementAnimator> HandleUiElement(
+      const AssistantUiElement* ui_element);
+
+  // Callback called to create a view for a suggestion.
+  // The implementer should:
+  //    - Create and add the appropriate views::View.
+  //    - Return an ElementAnimator to animate the view. Note that it is
+  //      permissible to return |nullptr| if no managed animation is desired.
+  virtual std::unique_ptr<ElementAnimator> HandleSuggestion(
+      int id,
+      const AssistantSuggestion* suggestion);
 
   AssistantViewDelegate* delegate() { return delegate_; }
 

ash/assistant/ui/main_stage/assistant_ui_element_view_factory.cc

@@ -21,14 +21,14 @@ AssistantUiElementViewFactory::AssistantUiElementViewFactory(
 AssistantUiElementViewFactory::~AssistantUiElementViewFactory() = default;
 
 std::unique_ptr<AssistantUiElementView> AssistantUiElementViewFactory::Create(
-    AssistantUiElement* ui_element) const {
+    const AssistantUiElement* ui_element) const {
   switch (ui_element->type()) {
     case AssistantUiElementType::kCard:
       return std::make_unique<AssistantCardElementView>(
-          delegate_, static_cast<AssistantCardElement*>(ui_element));
+          delegate_, static_cast<const AssistantCardElement*>(ui_element));
     case AssistantUiElementType::kText:
       return std::make_unique<AssistantTextElementView>(
-          static_cast<AssistantTextElement*>(ui_element));
+          static_cast<const AssistantTextElement*>(ui_element));
   }
 }
 

ash/assistant/ui/main_stage/assistant_ui_element_view_factory.h

@@ -26,7 +26,7 @@ class COMPONENT_EXPORT(ASSISTANT_UI) AssistantUiElementViewFactory {
 
   // Creates a view for the specified |ui_element|.
   std::unique_ptr<AssistantUiElementView> Create(
-      AssistantUiElement* ui_element) const;
+      const AssistantUiElement* ui_element) const;
 
  private:
   // Owned by AssistantController.

ash/assistant/ui/main_stage/suggestion_container_view.cc

@@ -183,19 +183,28 @@ void SuggestionContainerView::OnConversationStartersChanged(
     return;
 
   RemoveAllViews();
-  OnSuggestionsChanged(conversation_starters);
+
+  std::vector<std::unique_ptr<ElementAnimator>> animators;
+  for (const auto& pair : conversation_starters) {
+    auto animator =
+        AddSuggestionChip(/*id=*/pair.first, /*suggestion=*/pair.second);
+    if (animator)
+      AddElementAnimator(std::move(animator));
+  }
+
   AnimateIn();
 }
 
-void SuggestionContainerView::HandleResponse(
-    const AssistantResponse& response) {
+std::unique_ptr<ElementAnimator> SuggestionContainerView::HandleSuggestion(
+    int id,
+    const AssistantSuggestion* suggestion) {
   has_received_response_ = true;
 
   // When no longer showing conversation starters, we start align our content.
   layout_manager_->set_main_axis_alignment(
       views::BoxLayout::MainAxisAlignment::kStart);
 
-  OnSuggestionsChanged(response.GetSuggestions());
+  return AddSuggestionChip(id, suggestion);
 }
 
 void SuggestionContainerView::OnAllViewsRemoved() {
@@ -213,28 +222,17 @@ void SuggestionContainerView::OnAllViewsRemoved() {
   // not whether we are currently displaying a response.
 }
 
-void SuggestionContainerView::OnSuggestionsChanged(
-    const std::map<int, const AssistantSuggestion*>& suggestions) {
-  for (const auto& suggestion : suggestions) {
-    // We will use the same identifier by which the Assistant interaction model
-    // uniquely identifies a suggestion to uniquely identify its corresponding
-    // suggestion chip view.
-    AddSuggestionChip(/*suggestion=*/*suggestion.second,
-                      /*id=*/suggestion.first);
-  }
-}
-
-void SuggestionContainerView::AddSuggestionChip(
-    const AssistantSuggestion& suggestion,
-    int id) {
+std::unique_ptr<ElementAnimator> SuggestionContainerView::AddSuggestionChip(
+    int id,
+    const AssistantSuggestion* suggestion) {
   SuggestionChipView::Params params;
-  params.text = base::UTF8ToUTF16(suggestion.text);
+  params.text = base::UTF8ToUTF16(suggestion->text);
 
-  if (!suggestion.icon_url.is_empty()) {
+  if (!suggestion->icon_url.is_empty()) {
     // Initiate a request to download the image for the suggestion chip icon.
     // Note that the request is identified by the suggestion id.
     delegate()->DownloadImage(
-        suggestion.icon_url,
+        suggestion->icon_url,
         base::BindOnce(&SuggestionContainerView::OnSuggestionChipIconDownloaded,
                        download_request_weak_factory_.GetWeakPtr(), id));
 
@@ -261,9 +259,9 @@ void SuggestionContainerView::AddSuggestionChip(
   suggestion_chip_views_[id] =
       content_view()->AddChildView(std::move(suggestion_chip_view));
 
-  // Set the animations for the suggestion chip.
-  AddElementAnimator(std::make_unique<SuggestionChipAnimator>(
-      suggestion_chip_views_[id], this));
+  // Return the animator for the suggestion chip.
+  return std::make_unique<SuggestionChipAnimator>(suggestion_chip_views_[id],
+                                                  this);
 }
 
 void SuggestionContainerView::OnSuggestionChipIconDownloaded(

ash/assistant/ui/main_stage/suggestion_container_view.h

@@ -7,6 +7,7 @@
 
 #include <map>
 #include <memory>
+#include <vector>
 
 #include "ash/assistant/model/assistant_suggestions_model_observer.h"
 #include "ash/assistant/model/assistant_ui_model_observer.h"
@@ -71,12 +72,14 @@ class COMPONENT_EXPORT(ASSISTANT_UI) SuggestionContainerView
   void InitLayout();
 
   // AnimatedContainerView:
-  void HandleResponse(const AssistantResponse& response) override;
+  std::unique_ptr<ElementAnimator> HandleSuggestion(
+      int id,
+      const AssistantSuggestion* suggestion) override;
   void OnAllViewsRemoved() override;
 
-  void OnSuggestionsChanged(
-      const std::map<int, const AssistantSuggestion*>& suggestions);
-  void AddSuggestionChip(const AssistantSuggestion& suggestion, int id);
+  std::unique_ptr<ElementAnimator> AddSuggestionChip(
+      int id,
+      const AssistantSuggestion* suggestion);
 
   // Invoked on suggestion chip icon downloaded event.
   void OnSuggestionChipIconDownloaded(int id, const gfx::ImageSkia& icon);

ash/assistant/ui/main_stage/ui_element_container_view.cc

@@ -121,10 +121,10 @@ void UiElementContainerView::OnCommittedQueryChanged(
   AnimatedContainerView::OnCommittedQueryChanged(query);
 }
 
-void UiElementContainerView::HandleResponse(const AssistantResponse& response) {
-  for (const auto& ui_element : response.GetUiElements()) {
+std::unique_ptr<ElementAnimator> UiElementContainerView::HandleUiElement(
+    const AssistantUiElement* ui_element) {
   // Create a new view for the |ui_element|.
-    auto view = view_factory_->Create(ui_element.get());
+  auto view = view_factory_->Create(ui_element);
 
   // If the first UI element is a card, it has a unique margin requirement.
   const bool is_card = ui_element->type() == AssistantUiElementType::kCard;
@@ -134,12 +134,12 @@ void UiElementContainerView::HandleResponse(const AssistantResponse& response) {
     view->SetBorder(views::CreateEmptyBorder(kMarginTopDip, 0, 0, 0));
   }
 
-    // Add the view to the view hierarchy and bind an animator to handle all of
-    // its animations. Note that we prepare its animation layer for entry.
+  // Add the view to the hierarchy and prepare its animation layer for entry.
   auto* view_ptr = content_view()->AddChildView(std::move(view));
-    AddElementAnimator(view_ptr->CreateAnimator());
   view_ptr->GetLayerForAnimating()->SetOpacity(0.f);
-  }
+
+  // Return the animator that will be used to animate the view.
+  return view_ptr->CreateAnimator();
 }
 
 void UiElementContainerView::OnAllViewsAnimatedIn() {

ash/assistant/ui/main_stage/ui_element_container_view.h

@@ -18,7 +18,6 @@
 
 namespace ash {
 
-class AssistantResponse;
 class AssistantUiElementViewFactory;
 class AssistantViewDelegate;
 
@@ -44,7 +43,8 @@ class COMPONENT_EXPORT(ASSISTANT_UI) UiElementContainerView
   void InitLayout();
 
   // AnimatedContainerView:
-  void HandleResponse(const AssistantResponse& response) override;
+  std::unique_ptr<ElementAnimator> HandleUiElement(
+      const AssistantUiElement* ui_element) override;
   void OnAllViewsAnimatedIn() override;
   void OnScrollBarUpdated(views::ScrollBar* scroll_bar,
                           int viewport_size,