Fix UI jank caused by visibility change.

Previously, when animating the footer view and/or its children, we
modified visibility so as not to receive focus/accessibility events on
hidden views.

Unfortunately, changing the visibility of the footer view has a side
effect of freeing up space for UiElementContainerView. When this
happens, there is a bit of UI jank when the UiElementContainerView
takes that available space when really that space should permanently be
reserved for the footer. This becomes more obvious in a follow up CL in
which we add a shadow above the footer.

Now, we don't modify view visibility. Instead, we update focus behavior
as appropriate before animations start and after they end.

See bug for repro steps.

Bug: b:113871610
Change-Id: Ic8aa509c722ace44675a3ab7c7e0453c0aa4a352
Reviewed-on: https://chromium-review.googlesource.com/1197906
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588926}

ash/assistant/ui/main_stage/assistant_footer_view.cc

@@ -75,6 +75,24 @@ void AssistantFooterView::ChildVisibilityChanged(views::View* child) {
   PreferredSizeChanged();
 }
 
+void AssistantFooterView::SetFocusBehavior(FocusBehavior focus_behavior) {
+  switch (focus_behavior) {
+    case FocusBehavior::NEVER:
+      // When focus behavior is NEVER, we update both views.
+      suggestion_container_->SetFocusBehavior(FocusBehavior::NEVER);
+      opt_in_view_->SetFocusBehavior(FocusBehavior::NEVER);
+      break;
+    case FocusBehavior::ALWAYS:
+      // When focus behavior is ALWAYS, we need to respect opt in state.
+      UpdateFocusBehavior();
+      break;
+    case FocusBehavior::ACCESSIBLE_ONLY:
+      // We don't currently use ACCESSIBLE_ONLY focus behavior.
+      NOTREACHED();
+      break;
+  }
+}
+
 void AssistantFooterView::InitLayout() {
   SetLayoutManager(std::make_unique<views::FillLayout>());
 
@@ -82,8 +100,6 @@ void AssistantFooterView::InitLayout() {
   const bool setup_completed =
       Shell::Get()->voice_interaction_controller()->setup_completed();
 
-  LOG(ERROR) << "eyor: setup_completed: " << setup_completed;
-
   // Suggestion container.
   suggestion_container_ = new SuggestionContainerView(assistant_controller_);
   suggestion_container_->set_can_process_events_within_subtree(setup_completed);
@@ -92,7 +108,6 @@ void AssistantFooterView::InitLayout() {
   suggestion_container_->SetPaintToLayer();
   suggestion_container_->layer()->SetFillsBoundsOpaquely(false);
   suggestion_container_->layer()->SetOpacity(setup_completed ? 1.f : 0.f);
-  suggestion_container_->SetVisible(setup_completed);
 
   AddChildView(suggestion_container_);
 
@@ -105,9 +120,11 @@ void AssistantFooterView::InitLayout() {
   opt_in_view_->SetPaintToLayer();
   opt_in_view_->layer()->SetFillsBoundsOpaquely(false);
   opt_in_view_->layer()->SetOpacity(setup_completed ? 0.f : 1.f);
-  opt_in_view_->SetVisible(!setup_completed);
 
   AddChildView(opt_in_view_);
+
+  // Initialize focus behavior.
+  UpdateFocusBehavior(setup_completed);
 }
 
 void AssistantFooterView::OnVoiceInteractionSetupCompleted(bool completed) {
@@ -115,8 +132,6 @@ void AssistantFooterView::OnVoiceInteractionSetupCompleted(bool completed) {
   using assistant::util::CreateOpacityElement;
   using assistant::util::StartLayerAnimationSequence;
 
-  LOG(ERROR) << "eyor: completed: " << completed;
-
   // When the consent state changes, we need to hide/show the appropriate views.
   views::View* hide_view =
       completed ? static_cast<views::View*>(opt_in_view_)
@@ -126,14 +141,15 @@ void AssistantFooterView::OnVoiceInteractionSetupCompleted(bool completed) {
       completed ? static_cast<views::View*>(suggestion_container_)
                 : static_cast<views::View*>(opt_in_view_);
 
-  // Reset visibility to enable animation.
-  hide_view->SetVisible(true);
-  show_view->SetVisible(true);
+  // Views should not be focusable while they're animating.
+  suggestion_container_->SetFocusBehavior(FocusBehavior::NEVER);
+  opt_in_view_->SetFocusBehavior(FocusBehavior::NEVER);
 
   // Hide the view for the previous consent state by fading to 0% opacity.
   StartLayerAnimationSequence(hide_view->layer()->GetAnimator(),
                               CreateLayerAnimationSequence(CreateOpacityElement(
                                   0.f, kAnimationFadeOutDuration)),
+                              // Observe the animation.
                               animation_observer_.get());
 
   // Show the view for the next consent state by fading to 100% opacity with
@@ -166,12 +182,25 @@ bool AssistantFooterView::OnAnimationEnded(
 
   // Only the view relevant to our consent state should process events.
   suggestion_container_->set_can_process_events_within_subtree(setup_completed);
-  suggestion_container_->SetVisible(setup_completed);
   opt_in_view_->set_can_process_events_within_subtree(!setup_completed);
-  opt_in_view_->SetVisible(!setup_completed);
+
+  UpdateFocusBehavior(setup_completed);
 
   // Return false to prevent the observer from destroying itself.
   return false;
 }
 
+void AssistantFooterView::UpdateFocusBehavior(
+    base::Optional<bool> setup_completed) {
+  if (!setup_completed.has_value()) {
+    setup_completed =
+        Shell::Get()->voice_interaction_controller()->setup_completed();
+  }
+
+  suggestion_container_->SetFocusBehavior(
+      setup_completed.value() ? FocusBehavior::ALWAYS : FocusBehavior::NEVER);
+  opt_in_view_->SetFocusBehavior(
+      setup_completed.value() ? FocusBehavior::NEVER : FocusBehavior::ALWAYS);
+}
+
 }  // namespace ash

ash/assistant/ui/main_stage/assistant_footer_view.h

@@ -9,6 +9,7 @@
 
 #include "ash/public/interfaces/voice_interaction_controller.mojom.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "mojo/public/cpp/bindings/binding_set.h"
 #include "ui/views/view.h"
 
@@ -45,12 +46,17 @@ class AssistantFooterView : public views::View,
       mojom::AssistantAllowedState state) override {}
   void OnLocaleChanged(const std::string& locale) override {}
 
+  void SetFocusBehavior(FocusBehavior focus_behavior);
+
  private:
   void InitLayout();
 
   void OnAnimationStarted(const ui::CallbackLayerAnimationObserver& observer);
   bool OnAnimationEnded(const ui::CallbackLayerAnimationObserver& observer);
 
+  void UpdateFocusBehavior(
+      base::Optional<bool> setup_completed = base::nullopt);
+
   AssistantController* const assistant_controller_;  // Owned by Shell.
 
   SuggestionContainerView* suggestion_container_;  // Owned by view hierarchy.

ash/assistant/ui/main_stage/assistant_main_stage.cc

@@ -648,8 +648,8 @@ void AssistantMainStage::UpdateFooter() {
   // When it is not visible, it should not process events.
   bool visible = !committed_query_view_ && !pending_query_view_;
 
-  // Reset visibility to enable animation.
-  footer_->SetVisible(true);
+  // The footer should not be focusable while it's animating.
+  footer_->SetFocusBehavior(FocusBehavior::NEVER);
 
   if (visible) {
     // The footer will animate up into position so we need to set an initial
@@ -704,7 +704,8 @@ bool AssistantMainStage::OnFooterAnimationEnded(
   // there is no committed or pending query view.
   bool visible = !committed_query_view_ && !pending_query_view_;
   footer_->set_can_process_events_within_subtree(visible);
-  footer_->SetVisible(visible);
+  footer_->SetFocusBehavior(visible ? FocusBehavior::ALWAYS
+                                    : FocusBehavior::NEVER);
 
   // Return false so that the observer does not destroy itself.
   return false;

ash/assistant/ui/main_stage/assistant_opt_in_view.cc

@@ -91,6 +91,16 @@ void AssistantOptInView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
   label_->SizeToFit(width());
 }
 
+void AssistantOptInView::ButtonPressed(views::Button* sender,
+                                       const ui::Event& event) {
+  if (delegate_)
+    delegate_->OnOptInButtonPressed();
+}
+
+void AssistantOptInView::SetFocusBehavior(FocusBehavior focus_behavior) {
+  container_->SetFocusBehavior(focus_behavior);
+}
+
 void AssistantOptInView::InitLayout() {
   views::BoxLayout* layout_manager =
       SetLayoutManager(std::make_unique<views::BoxLayout>(
@@ -103,18 +113,17 @@ void AssistantOptInView::InitLayout() {
       views::BoxLayout::MainAxisAlignment::MAIN_AXIS_ALIGNMENT_CENTER);
 
   // Container.
-  AssistantOptInContainer* container =
-      new AssistantOptInContainer(/*listener=*/this);
+  container_ = new AssistantOptInContainer(/*listener=*/this);
 
   layout_manager =
-      container->SetLayoutManager(std::make_unique<views::BoxLayout>(
+      container_->SetLayoutManager(std::make_unique<views::BoxLayout>(
           views::BoxLayout::Orientation::kHorizontal,
           gfx::Insets(0, kPaddingDip)));
 
   layout_manager->set_cross_axis_alignment(
       views::BoxLayout::CrossAxisAlignment::CROSS_AXIS_ALIGNMENT_CENTER);
 
-  AddChildView(container);
+  AddChildView(container_);
 
   // Label.
   label_ = new views::StyledLabel(base::string16(), /*listener=*/nullptr);
@@ -146,16 +155,10 @@ void AssistantOptInView::InitLayout() {
       gfx::Range(offsets.at(1), offsets.at(1) + get_started.length()),
       CreateStyleInfo(gfx::Font::Weight::BOLD));
 
-  container->AddChildView(label_);
-
-  container->SetFocusForPlatform();
-  container->SetAccessibleName(label_text);
-}
+  container_->AddChildView(label_);
 
-void AssistantOptInView::ButtonPressed(views::Button* sender,
-                                       const ui::Event& event) {
-  if (delegate_)
-    delegate_->OnOptInButtonPressed();
+  container_->SetFocusForPlatform();
+  container_->SetAccessibleName(label_text);
 }
 
 }  // namespace ash

ash/assistant/ui/main_stage/assistant_opt_in_view.h

@@ -41,9 +41,12 @@ class AssistantOptInView : public views::View, public views::ButtonListener {
 
   void set_delegate(AssistantOptInDelegate* delegate) { delegate_ = delegate; }
 
+  void SetFocusBehavior(FocusBehavior focus_behavior);
+
  private:
   void InitLayout();
 
+  views::Button* container_;   // Owned by view hierarchy.
   views::StyledLabel* label_;  // Owned by view hierarchy.
 
   AssistantOptInDelegate* delegate_ = nullptr;

ash/assistant/ui/main_stage/suggestion_container_view.cc

@@ -137,6 +137,7 @@ void SuggestionContainerView::OnSuggestionsChanged(
     app_list::SuggestionChipView* suggestion_chip_view =
         new app_list::SuggestionChipView(params, /*listener=*/this);
     suggestion_chip_view->SetAccessibleName(params.text);
+    suggestion_chip_view->SetFocusBehavior(focus_behavior_);
 
     // Given a suggestion chip view, we need to be able to look up the id of
     // the underlying suggestion. This is used for handling press events.
@@ -189,6 +190,12 @@ void SuggestionContainerView::ButtonPressed(views::Button* sender,
       suggestion);
 }
 
+void SuggestionContainerView::SetFocusBehavior(FocusBehavior focus_behavior) {
+  focus_behavior_ = focus_behavior;
+  for (int i = 0; i < content_view()->child_count(); ++i)
+    content_view()->child_at(i)->SetFocusBehavior(focus_behavior);
+}
+
 void SuggestionContainerView::OnUiVisibilityChanged(
     AssistantVisibility new_visibility,
     AssistantVisibility old_visibility,

ash/assistant/ui/main_stage/suggestion_container_view.h

@@ -62,6 +62,8 @@ class SuggestionContainerView : public AssistantScrollView,
   // views::ButtonListener:
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
+  void SetFocusBehavior(FocusBehavior focus_behavior);
+
  private:
   void InitLayout();
 
@@ -74,6 +76,8 @@ class SuggestionContainerView : public AssistantScrollView,
 
   AssistantController* const assistant_controller_;  // Owned by Shell.
 
+  FocusBehavior focus_behavior_ = FocusBehavior::NEVER;
+
   views::BoxLayout* layout_manager_;  // Owned by view hierarchy.
 
   // Cache of suggestion chip views owned by the view hierarchy. The key for the