Implement more visibility states.

Previously, Assistant was visibility was either true || false.
Now, Assistant is one of { kClosed, kHidden, kVisible  }.

These visibility states are:
- kClosed: Assistant is not visible. Launching Assistant will start
           a new session with a clean stage.
- kHidden: Assistant is not visible. Launching Assistant will resume
           a paused session with the stage in its previous state.
- kVisible: Assistant is visible and a session is in progress.

Per product requirement:
- We become kClosed when explicitly dismissed by the user.
- We become kHidden when dismissed by auto-navigation.

Still TODO: Implement timer to auto-close if hidden for 5min.

See bug for demo.

Bug: b:113129833
Change-Id: I2f0f5dba3c2d58d55e5e494b69a93cbb8a667d66
Reviewed-on: https://chromium-review.googlesource.com/1188748
Commit-Queue: David Black <dmblack@google.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586398}

ash/BUILD.gn

@@ -230,6 +230,8 @@ component("ash") {
     "assistant/ui/main_stage/ui_element_container_view.h",
     "assistant/util/animation_util.cc",
     "assistant/util/animation_util.h",
+    "assistant/util/assistant_util.cc",
+    "assistant/util/assistant_util.h",
     "assistant/util/deep_link_util.cc",
     "assistant/util/deep_link_util.h",
     "assistant/util/views_util.cc",

ash/assistant/assistant_cache_controller.cc

@@ -8,6 +8,7 @@
 
 #include "ash/assistant/assistant_controller.h"
 #include "ash/assistant/assistant_ui_controller.h"
+#include "ash/assistant/util/assistant_util.h"
 #include "ash/assistant/util/deep_link_util.h"
 #include "ash/shell.h"
 #include "ash/strings/grit/ash_strings.h"
@@ -61,11 +62,13 @@ void AssistantCacheController::OnAssistantControllerDestroying() {
   assistant_controller_->ui_controller()->RemoveModelObserver(this);
 }
 
-void AssistantCacheController::OnUiVisibilityChanged(bool visible,
-                                                     AssistantSource source) {
-  // When hiding the UI we update our cache of conversation starters so that
-  // they're fresh for the next session.
-  if (!visible)
+void AssistantCacheController::OnUiVisibilityChanged(
+    AssistantVisibility new_visibility,
+    AssistantVisibility old_visibility,
+    AssistantSource source) {
+  // When Assistant is finishing a session, we update our cache of conversation
+  // starters so that they're fresh for the next launch.
+  if (assistant::util::IsFinishingSession(new_visibility))
     UpdateConversationStarters();
 }
 

ash/assistant/assistant_cache_controller.h

@@ -36,7 +36,9 @@ class AssistantCacheController : public AssistantControllerObserver,
   void OnAssistantControllerDestroying() override;
 
   // AssistantUiModelObserver:
-  void OnUiVisibilityChanged(bool visible, AssistantSource source) override;
+  void OnUiVisibilityChanged(AssistantVisibility new_visibility,
+                             AssistantVisibility old_visibility,
+                             AssistantSource source) override;
 
  private:
   // mojom::VoiceInteractionObserver:

ash/assistant/assistant_controller_unittest.cc

@@ -57,12 +57,6 @@ class AssistantControllerTest : public AshTestBase {
     chromeos::assistant::mojom::AssistantPtr assistant;
     assistant_binding_->Bind(mojo::MakeRequest(&assistant));
     controller_->SetAssistant(std::move(assistant));
-
-    ASSERT_FALSE(IsAssistantUiVisible());
-  }
-
-  bool IsAssistantUiVisible() {
-    return controller_->ui_controller()->model()->visible();
   }
 
   ash::AssistantController* controller() { return controller_; }

ash/assistant/assistant_interaction_controller.cc

@@ -97,21 +97,34 @@ void AssistantInteractionController::OnDeepLinkReceived(
 }
 
 void AssistantInteractionController::OnUiVisibilityChanged(
-    bool visible,
+    AssistantVisibility new_visibility,
+    AssistantVisibility old_visibility,
     AssistantSource source) {
-  if (visible) {
-    // TODO(dmblack): When the UI becomes visible, we may need to immediately
-    // start a voice interaction depending on |source| and user preference.
-    if (source == AssistantSource::kStylus)
-      assistant_interaction_model_.SetInputModality(InputModality::kStylus);
-    return;
+  switch (new_visibility) {
+    case AssistantVisibility::kClosed:
+      // When the UI is closed we need to stop any active interaction. We also
+      // reset the interaction state and restore the default input modality.
+      StopActiveInteraction();
+      assistant_interaction_model_.ClearInteraction();
+      assistant_interaction_model_.SetInputModality(InputModality::kKeyboard);
+      break;
+    case AssistantVisibility::kHidden:
+      // When the UI is hidden we stop any voice query in progress so that we
+      // don't listen to the user while not visible. We also restore the default
+      // input modality for the next launch.
+      if (assistant_interaction_model_.pending_query().type() ==
+          AssistantQueryType::kVoice) {
+        StopActiveInteraction();
+      }
+      assistant_interaction_model_.SetInputModality(InputModality::kKeyboard);
+      break;
+    case AssistantVisibility::kVisible:
+      // TODO(dmblack): When the UI becomes visible, we may need to immediately
+      // start a voice interaction depending on |source| and user preference.
+      if (source == AssistantSource::kStylus)
+        assistant_interaction_model_.SetInputModality(InputModality::kStylus);
+      break;
   }
-
-  // When the UI is hidden, we need to stop any active interaction. We also
-  // reset the interaction state and restore the default input modality.
-  StopActiveInteraction();
-  assistant_interaction_model_.ClearInteraction();
-  assistant_interaction_model_.SetInputModality(InputModality::kKeyboard);
 }
 
 void AssistantInteractionController::OnHighlighterEnabledChanged(
@@ -150,6 +163,11 @@ void AssistantInteractionController::OnInteractionStateChanged(
 
 void AssistantInteractionController::OnInputModalityChanged(
     InputModality input_modality) {
+  if (assistant_controller_->ui_controller()->model()->visibility() !=
+      AssistantVisibility::kVisible) {
+    return;
+  }
+
   if (input_modality == InputModality::kVoice)
     return;
 

ash/assistant/assistant_interaction_controller.h

@@ -66,7 +66,9 @@ class AssistantInteractionController
   void OnInputModalityChanged(InputModality input_modality) override;
 
   // AssistantUiModelObserver:
-  void OnUiVisibilityChanged(bool visible, AssistantSource source) override;
+  void OnUiVisibilityChanged(AssistantVisibility new_visibility,
+                             AssistantVisibility old_visibility,
+                             AssistantSource source) override;
 
   // HighlighterController::Observer:
   void OnHighlighterEnabledChanged(HighlighterEnabledState state) override;

ash/assistant/assistant_screen_context_controller.cc

@@ -188,11 +188,12 @@ void AssistantScreenContextController::OnAssistantControllerDestroying() {
 }
 
 void AssistantScreenContextController::OnUiVisibilityChanged(
-    bool visible,
+    AssistantVisibility new_visibility,
+    AssistantVisibility old_visibility,
     AssistantSource source) {
-  // We don't initiate a cache request if the UI is being hidden. Instead, we
-  // abort any requests in progress and reset state.
-  if (!visible) {
+  // We only initiate a contextual query for caching if the UI is being shown.
+  // Otherwise, we abort any requests in progress and reset state.
+  if (new_visibility != AssistantVisibility::kVisible) {
     screen_context_request_factory_.InvalidateWeakPtrs();
     assistant_screen_context_model_.SetRequestState(
         ScreenContextRequestState::kIdle);

ash/assistant/assistant_screen_context_controller.h

@@ -56,7 +56,9 @@ class ASH_EXPORT AssistantScreenContextController
   void OnAssistantControllerDestroying() override;
 
   // AssistantUiModelObserver:
-  void OnUiVisibilityChanged(bool visible, AssistantSource source) override;
+  void OnUiVisibilityChanged(AssistantVisibility new_visibility,
+                             AssistantVisibility old_visibility,
+                             AssistantSource source) override;
 
   // Invoked on screen context request finished event.
   void OnScreenContextRequestFinished();

ash/assistant/assistant_ui_controller.cc

@@ -79,10 +79,11 @@ void AssistantUiController::OnWidgetVisibilityChanged(views::Widget* widget,
 }
 
 void AssistantUiController::OnWidgetDestroying(views::Widget* widget) {
-  // We need to update the model when the widget is destroyed as this may not
-  // have occurred due to a call to HideUi/ToggleUi. This can occur as the
-  // result of pressing the ESC key.
-  assistant_ui_model_.SetVisible(false, AssistantSource::kUnspecified);
+  // We need to update the model when the widget is destroyed as this may have
+  // happened outside our control. This can occur as the result of pressing the
+  // ESC key, for example.
+  assistant_ui_model_.SetVisibility(AssistantVisibility::kClosed,
+                                    AssistantSource::kUnspecified);
 
   container_view_->GetWidget()->RemoveObserver(this);
   container_view_ = nullptr;
@@ -110,7 +111,7 @@ void AssistantUiController::OnMicStateChanged(MicState mic_state) {
 
 void AssistantUiController::OnScreenContextRequestStateChanged(
     ScreenContextRequestState request_state) {
-  if (!assistant_ui_model_.visible())
+  if (assistant_ui_model_.visibility() != AssistantVisibility::kVisible)
     return;
 
   // Once screen context request state has become idle, it is safe to activate
@@ -159,11 +160,11 @@ void AssistantUiController::OnHighlighterEnabledChanged(
     HighlighterEnabledState state) {
   switch (state) {
     case HighlighterEnabledState::kEnabled:
-      if (!assistant_ui_model_.visible())
+      if (assistant_ui_model_.visibility() != AssistantVisibility::kVisible)
         ShowUi(AssistantSource::kStylus);
       break;
     case HighlighterEnabledState::kDisabledByUser:
-      if (assistant_ui_model_.visible())
+      if (assistant_ui_model_.visibility() == AssistantVisibility::kVisible)
         HideUi(AssistantSource::kStylus);
       break;
     case HighlighterEnabledState::kDisabledBySessionComplete:
@@ -200,22 +201,23 @@ void AssistantUiController::OnDeepLinkReceived(
 }
 
 void AssistantUiController::OnUrlOpened(const GURL& url) {
-  // We go into mini UI mode when opening a URL in a new tab.
-  if (assistant_ui_model_.visible())
-    UpdateUiMode(AssistantUiMode::kMiniUi);
+  // We hide Assistant UI when opening a URL in a new tab.
+  if (assistant_ui_model_.visibility() == AssistantVisibility::kVisible)
+    HideUi(AssistantSource::kUnspecified);
 }
 
-void AssistantUiController::OnUiVisibilityChanged(bool visible,
-                                                  AssistantSource source) {
+void AssistantUiController::OnUiVisibilityChanged(
+    AssistantVisibility new_visibility,
+    AssistantVisibility old_visibility,
+    AssistantSource source) {
   Shell::Get()->voice_interaction_controller()->NotifyStatusChanged(
-      visible ? mojom::VoiceInteractionState::RUNNING
-              : mojom::VoiceInteractionState::STOPPED);
+      new_visibility == AssistantVisibility::kVisible
+          ? mojom::VoiceInteractionState::RUNNING
+          : mojom::VoiceInteractionState::STOPPED);
 
-  if (visible)
-    return;
-
-  // Metalayer mode should not be sticky. Disable it when hiding UI.
-  Shell::Get()->highlighter_controller()->AbortSession();
+  // Metalayer should not be sticky. Disable when the UI is no longer visible.
+  if (old_visibility == AssistantVisibility::kVisible)
+    Shell::Get()->highlighter_controller()->AbortSession();
 }
 
 void AssistantUiController::ShowUi(AssistantSource source) {
@@ -234,7 +236,7 @@ void AssistantUiController::ShowUi(AssistantSource source) {
     return;
   }
 
-  if (assistant_ui_model_.visible())
+  if (assistant_ui_model_.visibility() == AssistantVisibility::kVisible)
     return;
 
   if (!container_view_) {
@@ -246,22 +248,34 @@ void AssistantUiController::ShowUi(AssistantSource source) {
   // necessary due to limitations imposed by retrieving screen context. Once we
   // have finished retrieving screen context, the Assistant widget is activated.
   container_view_->GetWidget()->ShowInactive();
-  assistant_ui_model_.SetVisible(true, source);
+  assistant_ui_model_.SetVisibility(AssistantVisibility::kVisible, source);
 }
 
 void AssistantUiController::HideUi(AssistantSource source) {
-  if (!assistant_ui_model_.visible())
+  if (assistant_ui_model_.visibility() == AssistantVisibility::kHidden)
     return;
 
   if (container_view_)
     container_view_->GetWidget()->Hide();
 
-  assistant_ui_model_.SetVisible(false, source);
+  assistant_ui_model_.SetVisibility(AssistantVisibility::kHidden, source);
+}
+
+void AssistantUiController::CloseUi(AssistantSource source) {
+  if (assistant_ui_model_.visibility() == AssistantVisibility::kClosed)
+    return;
+
+  assistant_ui_model_.SetVisibility(AssistantVisibility::kClosed, source);
+
+  if (container_view_) {
+    container_view_->GetWidget()->CloseNow();
+    DCHECK_EQ(nullptr, container_view_);
+  }
 }
 
 void AssistantUiController::ToggleUi(AssistantSource source) {
-  if (assistant_ui_model_.visible())
-    HideUi(source);
+  if (assistant_ui_model_.visibility() == AssistantVisibility::kVisible)
+    CloseUi(source);
   else
     ShowUi(source);
 }

ash/assistant/assistant_ui_controller.h

@@ -94,10 +94,13 @@ class ASH_EXPORT AssistantUiController
   void OnUrlOpened(const GURL& url) override;
 
   // AssistantUiModelObserver:
-  void OnUiVisibilityChanged(bool visible, AssistantSource source) override;
+  void OnUiVisibilityChanged(AssistantVisibility new_visibility,
+                             AssistantVisibility old_visibility,
+                             AssistantSource source) override;
 
   void ShowUi(AssistantSource source);
   void HideUi(AssistantSource source);
+  void CloseUi(AssistantSource source);
   void ToggleUi(AssistantSource source);
 
   AssistantContainerView* GetViewForTest();

ash/assistant/model/assistant_ui_model.cc

@@ -28,12 +28,15 @@ void AssistantUiModel::SetUiMode(AssistantUiMode ui_mode) {
   NotifyUiModeChanged();
 }
 
-void AssistantUiModel::SetVisible(bool visible, AssistantSource source) {
-  if (visible == visible_)
+void AssistantUiModel::SetVisibility(AssistantVisibility visibility,
+                                     AssistantSource source) {
+  if (visibility == visibility_)
     return;
 
-  visible_ = visible;
-  NotifyUiVisibilityChanged(source);
+  const AssistantVisibility old_visibility = visibility_;
+  visibility_ = visibility;
+
+  NotifyUiVisibilityChanged(old_visibility, source);
 }
 
 void AssistantUiModel::NotifyUiModeChanged() {
@@ -41,9 +44,11 @@ void AssistantUiModel::NotifyUiModeChanged() {
     observer.OnUiModeChanged(ui_mode_);
 }
 
-void AssistantUiModel::NotifyUiVisibilityChanged(AssistantSource source) {
+void AssistantUiModel::NotifyUiVisibilityChanged(
+    AssistantVisibility old_visibility,
+    AssistantSource source) {
   for (AssistantUiModelObserver& observer : observers_)
-    observer.OnUiVisibilityChanged(visible_, source);
+    observer.OnUiVisibilityChanged(visibility_, old_visibility, source);
 }
 
 }  // namespace ash

ash/assistant/model/assistant_ui_model.h

@@ -31,6 +31,13 @@ enum class AssistantUiMode {
   kWebUi,
 };
 
+// Enumeration of Assistant visibility states.
+enum class AssistantVisibility {
+  kClosed,   // Assistant UI is hidden and the previous session has finished.
+  kHidden,   // Assistant UI is hidden and the previous session is paused.
+  kVisible,  // Assistant UI is visible and a session is in progress.
+};
+
 // Models the Assistant UI.
 class AssistantUiModel {
  public:
@@ -48,17 +55,18 @@ class AssistantUiModel {
   AssistantUiMode ui_mode() const { return ui_mode_; }
 
   // Sets the UI visibility.
-  void SetVisible(bool visible, AssistantSource source);
+  void SetVisibility(AssistantVisibility visibility, AssistantSource source);
 
-  bool visible() const { return visible_; }
+  AssistantVisibility visibility() const { return visibility_; }
 
  private:
   void NotifyUiModeChanged();
-  void NotifyUiVisibilityChanged(AssistantSource source);
+  void NotifyUiVisibilityChanged(AssistantVisibility old_visibility,
+                                 AssistantSource source);
 
   AssistantUiMode ui_mode_ = AssistantUiMode::kMainUi;
 
-  bool visible_ = false;
+  AssistantVisibility visibility_ = AssistantVisibility::kClosed;
 
   base::ObserverList<AssistantUiModelObserver>::Unchecked observers_;
 

ash/assistant/model/assistant_ui_model_observer.h

@@ -11,6 +11,7 @@ namespace ash {
 
 enum class AssistantSource;
 enum class AssistantUiMode;
+enum class AssistantVisibility;
 
 // An observer which receives notification of changes to the Assistant UI model.
 class AssistantUiModelObserver {
@@ -18,9 +19,12 @@ class AssistantUiModelObserver {
   // Invoked when the UI mode is changed.
   virtual void OnUiModeChanged(AssistantUiMode ui_mode) {}
 
-  // Invoked when the UI visibility is changed. The |source| of the visibility
-  // change event is provided for interested observers.
-  virtual void OnUiVisibilityChanged(bool visible, AssistantSource source) {}
+  // Invoked when the UI visibility is changed from |old_visibility| to
+  // |new_visibility|. The |source| of the visibility change event is provided
+  // for interested observers.
+  virtual void OnUiVisibilityChanged(AssistantVisibility new_visibility,
+                                     AssistantVisibility old_visibility,
+                                     AssistantSource source) {}
 
  protected:
   AssistantUiModelObserver() = default;

ash/assistant/ui/assistant_container_view.cc

@@ -188,7 +188,8 @@ void AssistantContainerView::PreferredSizeChanged() {
     return;
 
   const bool visible =
-      assistant_controller_->ui_controller()->model()->visible();
+      assistant_controller_->ui_controller()->model()->visibility() ==
+      AssistantVisibility::kVisible;
 
   // When visible, size changes are animated.
   if (visible) {

ash/assistant/ui/assistant_main_view.cc

@@ -15,6 +15,7 @@
 #include "ash/assistant/ui/dialog_plate/dialog_plate.h"
 #include "ash/assistant/ui/main_stage/assistant_main_stage.h"
 #include "ash/assistant/util/animation_util.h"
+#include "ash/assistant/util/assistant_util.h"
 #include "base/time/time.h"
 #include "ui/compositor/layer_animation_element.h"
 #include "ui/compositor/layer_animator.h"
@@ -129,11 +130,13 @@ void AssistantMainView::InitLayout() {
   AddChildView(dialog_plate_);
 }
 
-void AssistantMainView::OnUiVisibilityChanged(bool visible,
-                                              AssistantSource source) {
-  if (visible) {
-    // When Assistant UI is shown, we animate in the appearance of the caption
-    // bar and dialog plate.
+void AssistantMainView::OnUiVisibilityChanged(
+    AssistantVisibility new_visibility,
+    AssistantVisibility old_visibility,
+    AssistantSource source) {
+  if (assistant::util::IsStartingSession(new_visibility, old_visibility)) {
+    // When Assistant is starting a new session, we animate in the appearance of
+    // the caption bar and dialog plate.
     using assistant::util::CreateLayerAnimationSequence;
     using assistant::util::CreateOpacityElement;
 
@@ -158,11 +161,11 @@ void AssistantMainView::OnUiVisibilityChanged(bool visible,
     return;
   }
 
-  // When the Assistant UI is being hidden we need to reset our minimum height
-  // restriction so that the default restrictions are restored for the next
-  // time the view is shown.
-  min_height_dip_ = kMinHeightDip;
-  PreferredSizeChanged();
+  if (assistant::util::IsFinishingSession(new_visibility)) {
+    // When Assistant is finishing a session, we need to reset view state.
+    min_height_dip_ = kMinHeightDip;
+    PreferredSizeChanged();
+  }
 }
 
 void AssistantMainView::RequestFocus() {

ash/assistant/ui/assistant_main_view.h

@@ -30,7 +30,9 @@ class AssistantMainView : public views::View, public AssistantUiModelObserver {
   void RequestFocus() override;
 
   // AssistantUiModelObserver:
-  void OnUiVisibilityChanged(bool visible, AssistantSource source) override;
+  void OnUiVisibilityChanged(AssistantVisibility new_visibility,
+                             AssistantVisibility old_visibility,
+                             AssistantSource source) override;
 
  private:
   void InitLayout();

ash/assistant/ui/assistant_mini_view.cc

@@ -13,6 +13,7 @@
 #include "ash/assistant/model/assistant_query.h"
 #include "ash/assistant/ui/assistant_ui_constants.h"
 #include "ash/assistant/ui/logo_view/base_logo_view.h"
+#include "ash/assistant/util/assistant_util.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "base/strings/utf_string_conversions.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -136,12 +137,14 @@ void AssistantMiniView::OnResponseChanged(const AssistantResponse& response) {
   UpdatePrompt();
 }
 
-void AssistantMiniView::OnUiVisibilityChanged(bool visible,
-                                              AssistantSource source) {
-  if (visible)
+void AssistantMiniView::OnUiVisibilityChanged(
+    AssistantVisibility new_visibility,
+    AssistantVisibility old_visibility,
+    AssistantSource source) {
+  if (!assistant::util::IsFinishingSession(new_visibility))
     return;
 
-  // Reset state for the next Assistant UI session.
+  // When Assistant is finishing a session, we need to reset view state.
   last_active_query_.reset();
   UpdatePrompt();
 }

ash/assistant/ui/assistant_mini_view.h

@@ -55,7 +55,9 @@ class AssistantMiniView : public views::Button,
   void OnResponseChanged(const AssistantResponse& response) override;
 
   // AssistantUiModelObserver:
-  void OnUiVisibilityChanged(bool visible, AssistantSource source) override;
+  void OnUiVisibilityChanged(AssistantVisibility new_visibility,
+                             AssistantVisibility old_visibility,
+                             AssistantSource source) override;
 
   void set_delegate(AssistantMiniViewDelegate* delegate) {
     delegate_ = delegate;

ash/assistant/ui/dialog_plate/dialog_plate.cc

@@ -224,10 +224,12 @@ void DialogPlate::OnInputModalityChanged(InputModality input_modality) {
   }
 }
 
-void DialogPlate::OnUiVisibilityChanged(bool visible, AssistantSource source) {
-  // When the Assistant UI is hidden we need to clear the dialog plate so that
-  // text does not persist across Assistant entries.
-  if (!visible)
+void DialogPlate::OnUiVisibilityChanged(AssistantVisibility new_visibility,
+                                        AssistantVisibility old_visibility,
+                                        AssistantSource source) {
+  // When the Assistant UI is no longer visible we need to clear the dialog
+  // plate so that text does not persist across Assistant launches.
+  if (old_visibility == AssistantVisibility::kVisible)
     textfield_->SetText(base::string16());
 }
 

ash/assistant/ui/dialog_plate/dialog_plate.h

@@ -93,7 +93,9 @@ class DialogPlate : public views::View,
   void OnInputModalityChanged(InputModality input_modality) override;
 
   // AssistantUiModelObserver:
-  void OnUiVisibilityChanged(bool visible, AssistantSource source) override;
+  void OnUiVisibilityChanged(AssistantVisibility new_visibility,
+                             AssistantVisibility old_visibility,
+                             AssistantSource source) override;
 
  private:
   void InitLayout();

ash/assistant/ui/main_stage/assistant_header_view.cc

@@ -14,6 +14,7 @@
 #include "ash/assistant/ui/assistant_ui_constants.h"
 #include "ash/assistant/ui/logo_view/base_logo_view.h"
 #include "ash/assistant/util/animation_util.h"
+#include "ash/assistant/util/assistant_util.h"
 #include "base/time/time.h"
 #include "ui/compositor/layer_animation_sequence.h"
 #include "ui/compositor/layer_animator.h"
@@ -134,11 +135,13 @@ void AssistantHeaderView::OnResponseChanged(const AssistantResponse& response) {
            CreateOpacityElement(1.f, kResponseAnimationFadeInDuration))});
 }
 
-void AssistantHeaderView::OnUiVisibilityChanged(bool visible,
-                                                AssistantSource source) {
-  if (visible) {
-    // When Assistant UI is shown, we animate in the appearance of the molecule
-    // icon.
+void AssistantHeaderView::OnUiVisibilityChanged(
+    AssistantVisibility new_visibility,
+    AssistantVisibility old_visibility,
+    AssistantSource source) {
+  if (assistant::util::IsStartingSession(new_visibility, old_visibility)) {
+    // When Assistant is starting a new session, we animate in the appearance of
+    // the molecule icon.
     using assistant::util::CreateLayerAnimationSequence;
     using assistant::util::CreateOpacityElement;
     using assistant::util::CreateTransformElement;
@@ -168,7 +171,10 @@ void AssistantHeaderView::OnUiVisibilityChanged(bool visible,
     return;
   }
 
-  // When Assistant UI is hidden, we restore initial state for the next session.
+  if (!assistant::util::IsFinishingSession(new_visibility))
+    return;
+
+  // When Assistant is finishing a session, we need to reset view state.
   is_first_response_ = true;
 
   molecule_icon_->layer()->SetTransform(gfx::Transform());

ash/assistant/ui/main_stage/assistant_header_view.h

@@ -36,7 +36,9 @@ class AssistantHeaderView : public views::View,
   void OnResponseChanged(const AssistantResponse& response) override;
 
   // AssistantUiModelObserver:
-  void OnUiVisibilityChanged(bool visible, AssistantSource source) override;
+  void OnUiVisibilityChanged(AssistantVisibility new_visibility,
+                             AssistantVisibility old_visibility,
+                             AssistantSource source) override;
 
  private:
   void InitLayout();

ash/assistant/ui/main_stage/assistant_main_stage.cc

@@ -14,6 +14,7 @@
 #include "ash/assistant/ui/main_stage/assistant_query_view.h"
 #include "ash/assistant/ui/main_stage/ui_element_container_view.h"
 #include "ash/assistant/util/animation_util.h"
+#include "ash/assistant/util/assistant_util.h"
 #include "ash/strings/grit/ash_strings.h"
 #include "base/bind.h"
 #include "base/time/time.h"
@@ -510,11 +511,13 @@ void AssistantMainStage::OnResponseChanged(const AssistantResponse& response) {
   OnActivateQuery();
 }
 
-void AssistantMainStage::OnUiVisibilityChanged(bool visible,
-                                               AssistantSource source) {
-  if (visible) {
-    // When Assistant UI is shown, we animate in the appearance of the greeting
-    // label and footer.
+void AssistantMainStage::OnUiVisibilityChanged(
+    AssistantVisibility new_visibility,
+    AssistantVisibility old_visibility,
+    AssistantSource source) {
+  if (assistant::util::IsStartingSession(new_visibility, old_visibility)) {
+    // When Assistant is starting a new session, we animate in the appearance of
+    // the greeting label and footer.
     using assistant::util::CreateLayerAnimationSequence;
     using assistant::util::CreateOpacityElement;
     using assistant::util::CreateTransformElement;
@@ -553,7 +556,10 @@ void AssistantMainStage::OnUiVisibilityChanged(bool visible,
     return;
   }
 
-  // When Assistant UI is hidden, we restore initial state for the next session.
+  if (!assistant::util::IsFinishingSession(new_visibility))
+    return;
+
+  // When Assistant is finishing a session, we need to reset view state.
   is_first_query_ = true;
 
   delete active_query_view_;

ash/assistant/ui/main_stage/assistant_main_stage.h

@@ -57,7 +57,9 @@ class AssistantMainStage : public views::View,
   void OnResponseChanged(const AssistantResponse& response) override;
 
   // AssistantUiModelObserver:
-  void OnUiVisibilityChanged(bool visible, AssistantSource source) override;
+  void OnUiVisibilityChanged(AssistantVisibility new_visibility,
+                             AssistantVisibility old_visibility,
+                             AssistantSource source) override;
 
  private:
   void InitLayout();

ash/assistant/ui/main_stage/suggestion_container_view.cc

@@ -12,6 +12,7 @@
 #include "ash/assistant/assistant_ui_controller.h"
 #include "ash/assistant/model/assistant_response.h"
 #include "ash/assistant/ui/assistant_ui_constants.h"
+#include "ash/assistant/util/assistant_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "ui/views/layout/box_layout.h"
 
@@ -98,8 +99,8 @@ void SuggestionContainerView::OnResponseChanged(
 
 void SuggestionContainerView::OnResponseCleared() {
   // Note that we don't reset |has_received_response_| here because that refers
-  // to whether we've received a response during the current Assistant UI
-  // session, not whether we are currently displaying a response.
+  // to whether we've received a response during the current Assistant session,
+  // not whether we are currently displaying a response.
   OnSuggestionsCleared();
 }
 
@@ -185,22 +186,28 @@ void SuggestionContainerView::ButtonPressed(views::Button* sender,
       suggestion);
 }
 
-void SuggestionContainerView::OnUiVisibilityChanged(bool visible,
-                                                    AssistantSource source) {
-  if (visible) {
-    // Show conversation starters at the beginning of an Assistant session.
+void SuggestionContainerView::OnUiVisibilityChanged(
+    AssistantVisibility new_visibility,
+    AssistantVisibility old_visibility,
+    AssistantSource source) {
+  if (assistant::util::IsStartingSession(new_visibility, old_visibility)) {
+    // Show conversation starters at the start of a new Assistant session.
     OnConversationStartersChanged(assistant_controller_->cache_controller()
                                       ->model()
                                       ->GetConversationStarters());
-  } else {
-    // Reset view state.
-    has_received_response_ = false;
-
-    // When we become visible again we will be showing conversation starters so
-    // we need to center align our content.
-    layout_manager_->set_main_axis_alignment(
-        views::BoxLayout::MainAxisAlignment::MAIN_AXIS_ALIGNMENT_CENTER);
+    return;
   }
+
+  if (!assistant::util::IsFinishingSession(new_visibility))
+    return;
+
+  // When Assistant is finishing a session, we need to reset view state.
+  has_received_response_ = false;
+
+  // When we start a new session we will be showing conversation starters so
+  // we need to center align our content.
+  layout_manager_->set_main_axis_alignment(
+      views::BoxLayout::MainAxisAlignment::MAIN_AXIS_ALIGNMENT_CENTER);
 }
 
 }  // namespace ash

ash/assistant/ui/main_stage/suggestion_container_view.h

@@ -55,7 +55,9 @@ class SuggestionContainerView : public AssistantScrollView,
   void OnResponseCleared() override;
 
   // AssistantUiModelObserver:
-  void OnUiVisibilityChanged(bool visible, AssistantSource source) override;
+  void OnUiVisibilityChanged(AssistantVisibility new_visibility,
+                             AssistantVisibility old_visibility,
+                             AssistantSource source) override;
 
   // views::ButtonListener:
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;

ash/assistant/ui/main_stage/ui_element_container_view.cc

@@ -254,8 +254,8 @@ void UiElementContainerView::OnCommittedQueryChanged(
 
   // When a query is committed, we fade out the views for the previous response
   // until the next Assistant response has been received.
-  for (const std::pair<ui::Layer*, float>& pair : ui_element_layers_) {
-    pair.first->GetAnimator()->StartAnimation(
+  for (const std::pair<ui::LayerOwner*, float>& pair : ui_element_views_) {
+    pair.first->layer()->GetAnimator()->StartAnimation(
         CreateLayerAnimationSequence(CreateOpacityElement(
             /*opacity=*/pair.second, kUiElementAnimationFadeOutDuration)));
   }
@@ -263,9 +263,9 @@ void UiElementContainerView::OnCommittedQueryChanged(
 
 void UiElementContainerView::OnResponseChanged(
     const AssistantResponse& response) {
-  // If we haven't cached any layers, there is nothing to animate off stage so
-  // we can proceed to add the new response.
-  if (ui_element_layers_.empty()) {
+  // If we don't have any pre-existing content, there is nothing to animate off
+  // stage so we we can proceed to add the new response.
+  if (!content_view()->has_children()) {
     OnResponseAdded(response);
     return;
   }
@@ -277,9 +277,9 @@ void UiElementContainerView::OnResponseChanged(
   // There is a previous response on stage, so we'll animate it off before
   // adding the new response. The new response will be added upon invocation of
   // the exit animation ended callback.
-  for (const std::pair<ui::Layer*, float> pair : ui_element_layers_) {
+  for (const std::pair<ui::LayerOwner*, float>& pair : ui_element_views_) {
     StartLayerAnimationSequence(
-        pair.first->GetAnimator(),
+        pair.first->layer()->GetAnimator(),
         // Fade out the opacity to 0%.
         CreateLayerAnimationSequence(
             CreateOpacityElement(0.f, kUiElementAnimationFadeOutDuration)),
@@ -296,7 +296,7 @@ void UiElementContainerView::OnResponseCleared() {
   render_request_weak_factory_.InvalidateWeakPtrs();
 
   content_view()->RemoveAllChildViews(/*delete_children=*/true);
-  ui_element_layers_.clear();
+  ui_element_views_.clear();
 
   ReleaseAllCards();
 
@@ -336,13 +336,14 @@ void UiElementContainerView::OnAllUiElementsAdded() {
 
   // Now that we've received and added all UI elements for the current query
   // response, we can animate them in.
-  for (const std::pair<ui::Layer*, float>& pair : ui_element_layers_) {
+  for (const std::pair<ui::LayerOwner*, float>& pair : ui_element_views_) {
     // We fade in the views to full opacity after a slight delay.
-    pair.first->GetAnimator()->StartAnimation(CreateLayerAnimationSequence(
-        ui::LayerAnimationElement::CreatePauseElement(
-            ui::LayerAnimationElement::AnimatableProperty::OPACITY,
-            kUiElementAnimationFadeInDelay),
-        CreateOpacityElement(1.f, kUiElementAnimationFadeInDuration)));
+    pair.first->layer()->GetAnimator()->StartAnimation(
+        CreateLayerAnimationSequence(
+            ui::LayerAnimationElement::CreatePauseElement(
+                ui::LayerAnimationElement::AnimatableProperty::OPACITY,
+                kUiElementAnimationFadeInDelay),
+            CreateOpacityElement(1.f, kUiElementAnimationFadeInDuration)));
   }
 }
 
@@ -440,12 +441,10 @@ void UiElementContainerView::OnCardReady(
     view_holder->native_view()->layer()->SetFillsBoundsOpaquely(false);
     view_holder->native_view()->layer()->SetOpacity(0.f);
 
-    // We cache the layer for the view for use during animations and cache its
-    // desired opacity that we'll animate to while processing the next query
-    // response.
-    ui_element_layers_.push_back(
-        std::pair<ui::Layer*, float>(view_holder->native_view()->layer(),
-                                     kCardElementAnimationFadeOutOpacity));
+    // We cache the native view for use during animations and its desired
+    // opacity that we'll animate to while processing the next query response.
+    ui_element_views_.push_back(std::pair<ui::LayerOwner*, float>(
+        view_holder->native_view(), kCardElementAnimationFadeOutOpacity));
   }
 
   // TODO(dmblack): Handle Mash case.
@@ -467,11 +466,10 @@ void UiElementContainerView::OnTextElementAdded(
   text_element_view->layer()->SetFillsBoundsOpaquely(false);
   text_element_view->layer()->SetOpacity(0.f);
 
-  // We cache the layer for the view for use during animations and cache its
-  // desired opacity that we'll animate to while processing the next query
-  // response.
-  ui_element_layers_.push_back(std::pair<ui::Layer*, float>(
-      text_element_view->layer(), kTextElementAnimationFadeOutOpacity));
+  // We cache the view for use during animations and its desired opacity that
+  // we'll animate to while processing the next query response.
+  ui_element_views_.push_back(std::pair<ui::LayerOwner*, float>(
+      text_element_view, kTextElementAnimationFadeOutOpacity));
 
   content_view()->AddChildView(text_element_view);
 }

ash/assistant/ui/main_stage/ui_element_container_view.h

@@ -83,7 +83,7 @@ class UiElementContainerView : public AssistantScrollView,
   // 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.
-  std::vector<std::pair<ui::Layer*, float>> ui_element_layers_;
+  std::vector<std::pair<ui::LayerOwner*, float>> ui_element_views_;
 
   std::unique_ptr<ui::CallbackLayerAnimationObserver>
       ui_elements_exit_animation_observer_;

ash/assistant/util/assistant_util.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/assistant/util/assistant_util.h"
+
+#include "ash/assistant/model/assistant_ui_model.h"
+
+namespace ash {
+namespace assistant {
+namespace util {
+
+bool IsStartingSession(AssistantVisibility new_visibility,
+                       AssistantVisibility old_visibility) {
+  return old_visibility == AssistantVisibility::kClosed &&
+         new_visibility == AssistantVisibility::kVisible;
+}
+
+bool IsFinishingSession(AssistantVisibility new_visibility) {
+  return new_visibility == AssistantVisibility::kClosed;
+}
+
+}  // namespace util
+}  // namespace assistant
+}  // namespace ash

ash/assistant/util/assistant_util.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef ASH_ASSISTANT_UTIL_ASSISTANT_UTIL_H_
+#define ASH_ASSISTANT_UTIL_ASSISTANT_UTIL_H_
+
+namespace ash {
+
+enum class AssistantVisibility;
+
+namespace assistant {
+namespace util {
+
+// Returns true if Assistant is starting a new session, false otherwise.
+bool IsStartingSession(AssistantVisibility new_visibility,
+                       AssistantVisibility old_visibility);
+
+// Returns true if Assistant is finishing a session, false otherwise.
+bool IsFinishingSession(AssistantVisibility new_visibility);
+
+}  // namespace util
+}  // namespace assistant
+}  // namespace ash
+
+#endif  // ASH_ASSISTANT_UTIL_ASSISTANT_UTIL_H_