Add show-on-scroll FeatureParam to Assistant proactive suggestions.

Previously, the entry point view for the Assistant proactive suggestions
feature would be presented to the user as soon as possible.

Now, with the addition of |show-on-scroll|, which is
|ENABLED_BY_DEFAULT|, the entry point view for the feature will show/
hide in response to the user's vertical scroll activity in the source
web contents.

This will reduce overall triggering of the feature but hypothetically
increase click through as the entry point is now being presented at a
more helpful and appropriate time.

Bug: b:142144655
Change-Id: I538e618af55d7ed3f0ab0480c300d9529893d03f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1918662Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/master@{#715920}

ash/assistant/assistant_proactive_suggestions_controller.cc

@@ -11,6 +11,7 @@
 #include "ash/public/cpp/assistant/proactive_suggestions.h"
 #include "ash/public/cpp/assistant/util/histogram_util.h"
 #include "chromeos/services/assistant/public/features.h"
+#include "components/viz/common/vertical_scroll_direction.h"
 #include "ui/views/widget/widget.h"
 
 namespace ash {
@@ -70,6 +71,39 @@ void AssistantProactiveSuggestionsController::OnProactiveSuggestionsChanged(
       ->OnProactiveSuggestionsChanged(std::move(proactive_suggestions));
 }
 
+void AssistantProactiveSuggestionsController::
+    OnSourceVerticalScrollDirectionChanged(
+        viz::VerticalScrollDirection scroll_direction) {
+  if (!chromeos::assistant::features::
+          IsProactiveSuggestionsShowOnScrollEnabled()) {
+    // If show-on-scroll is disabled, presentation logic of the view executes
+    // immediately as a result of changes in the active set of proactive
+    // suggestions.
+    return;
+  }
+
+  switch (scroll_direction) {
+    case viz::VerticalScrollDirection::kDown:
+      // When the user scrolls down, the proactive suggestions widget is hidden
+      // as this is a strong signal that the user is engaging with the page.
+      HideUi();
+      return;
+    case viz::VerticalScrollDirection::kUp:
+      // When the user scrolls up, the proactive suggestions widget will be
+      // shown to the user as this is a signal that the user may be interested
+      // in engaging with related content as part of a deeper dive. Note that
+      // this method will no-op if there is no active set of proactive
+      // suggestions or if the set has been blacklisted from being shown.
+      MaybeShowUi();
+      return;
+    case viz::VerticalScrollDirection::kNull:
+      // The value |kNull| only exists to indicate the absence of a vertical
+      // scroll direction and should never be propagated in a change event.
+      NOTREACHED();
+      return;
+  }
+}
+
 void AssistantProactiveSuggestionsController::OnProactiveSuggestionsChanged(
     scoped_refptr<const ProactiveSuggestions> proactive_suggestions,
     scoped_refptr<const ProactiveSuggestions> old_proactive_suggestions) {
@@ -78,22 +112,18 @@ void AssistantProactiveSuggestionsController::OnProactiveSuggestionsChanged(
   // will no-op if the proactive suggestions view does not exist.
   CloseUi(ProactiveSuggestionsShowResult::kCloseByContextChange);
 
-  if (!proactive_suggestions)
-    return;
-
-  // If this set of proactive suggestions is blacklisted, it should not be shown
-  // to the user. A set of proactive suggestions may be blacklisted as a result
-  // of duplicate suppression or as a result of the user explicitly closing the
-  // proactive suggestions view.
-  if (base::Contains(proactive_suggestions_blacklist_,
-                     proactive_suggestions->hash())) {
-    RecordProactiveSuggestionsShowAttempt(
-        proactive_suggestions->category(),
-        ProactiveSuggestionsShowAttempt::kAbortedByDuplicateSuppression);
+  if (chromeos::assistant::features::
+          IsProactiveSuggestionsShowOnScrollEnabled()) {
+    // When show-on-scroll is enabled, presentation of the proactive suggestions
+    // view is handled in response to changes in vertical scroll direction.
     return;
   }
 
-  ShowUi();
+  // We'll now attempt to show the proactive suggestions view to the user. Note
+  // that this method will no-op if there is no active set of proactive
+  // suggestions or if the set of proactive suggestions is blacklisted from
+  // being shown to the user.
+  MaybeShowUi();
 }
 
 void AssistantProactiveSuggestionsController::
@@ -148,8 +178,36 @@ void AssistantProactiveSuggestionsController::
       AssistantEntryPoint::kProactiveSuggestions);
 }
 
-void AssistantProactiveSuggestionsController::ShowUi() {
-  DCHECK(!view_);
+void AssistantProactiveSuggestionsController::MaybeShowUi() {
+  if (view_) {
+    // If the |view_| already exists, calling MaybeShowUi() will just ensure
+    // that its widget is showing to the user.
+    view_->GetWidget()->ShowInactive();
+    return;
+  }
+
+  // Retrieve the cached set of proactive suggestions.
+  scoped_refptr<const ProactiveSuggestions> proactive_suggestions =
+      assistant_controller_->suggestions_controller()
+          ->model()
+          ->GetProactiveSuggestions();
+
+  // There's nothing to show if there are no proactive suggestions in the cache.
+  if (!proactive_suggestions)
+    return;
+
+  // If the cached set of proactive suggestions is blacklisted, it should not be
+  // shown to the user. A set of proactive suggestions may be blacklisted as a
+  // result of duplicate suppression or as a result of the user explicitly
+  // closing the proactive suggestions view.
+  if (base::Contains(proactive_suggestions_blacklist_,
+                     proactive_suggestions->hash())) {
+    RecordProactiveSuggestionsShowAttempt(
+        proactive_suggestions->category(),
+        ProactiveSuggestionsShowAttempt::kAbortedByDuplicateSuppression);
+    return;
+  }
+
   view_ = new ProactiveSuggestionsView(assistant_controller_->view_delegate());
   view_->GetWidget()->ShowInactive();
 
@@ -189,4 +247,9 @@ void AssistantProactiveSuggestionsController::CloseUi(
   view_ = nullptr;
 }
 
+void AssistantProactiveSuggestionsController::HideUi() {
+  if (view_)
+    view_->GetWidget()->Hide();
+}
+
 }  // namespace ash

ash/assistant/assistant_proactive_suggestions_controller.h

@@ -53,6 +53,8 @@ class AssistantProactiveSuggestionsController
   void OnProactiveSuggestionsClientDestroying() override;
   void OnProactiveSuggestionsChanged(
       scoped_refptr<const ProactiveSuggestions> proactive_suggestions) override;
+  void OnSourceVerticalScrollDirectionChanged(
+      viz::VerticalScrollDirection scroll_direction) override;
 
   // AssistantSuggestionsModelObserver:
   void OnProactiveSuggestionsChanged(
@@ -66,8 +68,9 @@ class AssistantProactiveSuggestionsController
   void OnProactiveSuggestionsViewPressed() override;
 
  private:
-  void ShowUi();
+  void MaybeShowUi();
   void CloseUi(ProactiveSuggestionsShowResult result);
+  void HideUi();
 
   AssistantController* const assistant_controller_;  // Owned by Shell.
 

ash/assistant/ui/proactive_suggestions_view.cc

@@ -137,21 +137,28 @@ void ProactiveSuggestionsView::ButtonPressed(views::Button* sender,
   delegate_->OnProactiveSuggestionsViewPressed();
 }
 
+void ProactiveSuggestionsView::OnWidgetClosing(views::Widget* widget) {
+  widget->RemoveObserver(this);
+
+  // When closing, the proactive suggestions window fades out.
+  wm::SetWindowVisibilityAnimationType(
+      widget->GetNativeWindow(),
+      wm::WindowVisibilityAnimationType::WINDOW_VISIBILITY_ANIMATION_TYPE_FADE);
+}
+
 void ProactiveSuggestionsView::OnWindowDestroying(aura::Window* window) {
   window->RemoveObserver(this);
 }
 
 void ProactiveSuggestionsView::OnWindowVisibilityChanging(aura::Window* window,
                                                           bool visible) {
-  if (!visible) {
-    // When exiting, the proactive suggestions window fades out.
-    wm::SetWindowVisibilityAnimationType(
-        window, wm::WindowVisibilityAnimationType::
-                    WINDOW_VISIBILITY_ANIMATION_TYPE_FADE);
+  // If the widget is marked as closed, we are in a closing sequence and we
+  // don't need to update the window animation as the appropriate animation for
+  // close has already been set in OnWidgetClosing().
+  if (GetWidget()->IsClosed())
     return;
-  }
 
-  // When entering, the proactive suggestions window translates in vertically.
+  // If showing/hiding, the proactive suggestions window translates vertically.
   wm::SetWindowVisibilityAnimationType(
       window, wm::WindowVisibilityAnimationType::
                   WINDOW_VISIBILITY_ANIMATION_TYPE_VERTICAL);
@@ -271,6 +278,11 @@ void ProactiveSuggestionsView::InitWidget() {
   views::Widget* widget = new views::Widget();
   widget->Init(std::move(params));
   widget->SetContentsView(this);
+
+  // We observe the |widget| in order to modify animation behavior of its window
+  // prior to closing. This is necessary due to the fact that we utilize a
+  // different animation on widget close than we do for widget show and hide.
+  widget->AddObserver(this);
 }
 
 void ProactiveSuggestionsView::InitWindow() {
@@ -288,8 +300,7 @@ void ProactiveSuggestionsView::InitWindow() {
 
   // We observe the window in order to modify animation behavior prior to window
   // visibility changes. This needs to be done dynamically as bounds are not
-  // fully initialized yet for calculating offset position and the animation
-  // behavior for exit should only be set once the enter animation is completed.
+  // fully initialized yet for calculating offset position.
   window->AddObserver(this);
 }
 

ash/assistant/ui/proactive_suggestions_view.h

@@ -9,6 +9,7 @@
 #include "ui/aura/window_observer.h"
 #include "ui/display/display_observer.h"
 #include "ui/views/controls/button/button.h"
+#include "ui/views/widget/widget_observer.h"
 
 namespace views {
 class ImageButton;
@@ -23,6 +24,7 @@ class ProactiveSuggestions;
 class COMPONENT_EXPORT(ASSISTANT_UI) ProactiveSuggestionsView
     : public views::Button,
       public views::ButtonListener,
+      public views::WidgetObserver,
       public aura::WindowObserver,
       public display::DisplayObserver,
       public KeyboardControllerObserver {
@@ -42,6 +44,9 @@ class COMPONENT_EXPORT(ASSISTANT_UI) ProactiveSuggestionsView
   // views::ButtonListener:
   void ButtonPressed(views::Button* sender, const ui::Event& event) override;
 
+  // views::WidgetObserver:
+  void OnWidgetClosing(views::Widget* widget) override;
+
   // aura::WindowObserver:
   void OnWindowDestroying(aura::Window* window) override;
   void OnWindowVisibilityChanging(aura::Window* window, bool visible) override;

ash/public/cpp/assistant/proactive_suggestions_client.h

@@ -9,6 +9,10 @@
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
 
+namespace viz {
+enum class VerticalScrollDirection;
+}  // namespace viz
+
 namespace ash {
 
 class ProactiveSuggestions;
@@ -31,6 +35,11 @@ class ASH_PUBLIC_EXPORT ProactiveSuggestionsClient {
     virtual void OnProactiveSuggestionsChanged(
         scoped_refptr<const ProactiveSuggestions> proactive_suggestions) {}
 
+    // Invoked when the vertical |scroll_direction| is changed in the source
+    // web contents associated with the active set of proactive suggestions.
+    virtual void OnSourceVerticalScrollDirectionChanged(
+        viz::VerticalScrollDirection scroll_direction) {}
+
    protected:
     Delegate() = default;
     virtual ~Delegate() = default;

chrome/browser/ui/ash/assistant/proactive_suggestions_client_impl.cc

@@ -118,6 +118,12 @@ void ProactiveSuggestionsClientImpl::DidStartNavigation(
   SetActiveUrl(active_contents_->GetURL());
 }
 
+void ProactiveSuggestionsClientImpl::DidChangeVerticalScrollDirection(
+    viz::VerticalScrollDirection scroll_direction) {
+  if (delegate_)
+    delegate_->OnSourceVerticalScrollDirectionChanged(scroll_direction);
+}
+
 void ProactiveSuggestionsClientImpl::OnAssistantFeatureAllowedChanged(
     ash::mojom::AssistantAllowedState state) {
   // When the Assistant feature is allowed/disallowed we may need to resume/

chrome/browser/ui/ash/assistant/proactive_suggestions_client_impl.h

@@ -49,6 +49,8 @@ class ProactiveSuggestionsClientImpl : public ash::ProactiveSuggestionsClient,
   // content::WebContentsObserver:
   void DidStartNavigation(
       content::NavigationHandle* navigation_handle) override;
+  void DidChangeVerticalScrollDirection(
+      viz::VerticalScrollDirection scroll_direction) override;
 
   // AssistantStateObserver:
   void OnAssistantFeatureAllowedChanged(

chromeos/services/assistant/public/features.cc

@@ -34,8 +34,14 @@ const base::FeatureParam<std::string>
     kAssistantProactiveSuggestionsServerExperimentIds{
         &kAssistantProactiveSuggestions, "server-experiment-ids", ""};
 
+// When enabled, the proactive suggestions view will show only after the user
+// scrolls up in the source web contents. When disabled, the view will be shown
+// immediately once the set of proactive suggestions are available.
+const base::FeatureParam<bool> kAssistantProactiveSuggestionsShowOnScroll{
+    &kAssistantProactiveSuggestions, "show-on-scroll", true};
+
 const base::FeatureParam<bool> kAssistantProactiveSuggestionsSuppressDuplicates{
-    &kAssistantProactiveSuggestions, "suppress-duplicates", true};
+    &kAssistantProactiveSuggestions, "suppress-duplicates", false};
 
 const base::FeatureParam<int>
     kAssistantProactiveSuggestionsTimeoutThresholdMillis{
@@ -127,6 +133,10 @@ bool IsProactiveSuggestionsEnabled() {
   return base::FeatureList::IsEnabled(kAssistantProactiveSuggestions);
 }
 
+bool IsProactiveSuggestionsShowOnScrollEnabled() {
+  return kAssistantProactiveSuggestionsShowOnScroll.Get();
+}
+
 bool IsProactiveSuggestionsSuppressDuplicatesEnabled() {
   return kAssistantProactiveSuggestionsSuppressDuplicates.Get();
 }

chromeos/services/assistant/public/features.h

@@ -116,6 +116,9 @@ COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) bool IsPowerManagerEnabled();
 
 COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) bool IsProactiveSuggestionsEnabled();
 
+COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC)
+bool IsProactiveSuggestionsShowOnScrollEnabled();
+
 COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC)
 bool IsProactiveSuggestionsSuppressDuplicatesEnabled();