Add support for duplicate suppression in proactive suggestions.

Per UX request, we shouldn't show proactive suggestions during the same
login session if we've already shown them to the user. Duplicates are
defined as having the same content description.

Note that this has been implemented as a FeatureParam to give us a lever
to disable this behavior if needed.

Bug: b:139199754
Change-Id: I39e78529159ffd697383b98b23b4986f0d3a10ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1774968Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/master@{#691740}

ash/assistant/assistant_ui_controller.cc

@@ -25,6 +25,7 @@
 #include "ash/system/toast/toast_manager_impl.h"
 #include "base/bind.h"
 #include "base/optional.h"
+#include "chromeos/services/assistant/public/features.h"
 #include "chromeos/services/assistant/public/mojom/assistant.mojom.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -152,13 +153,39 @@ void AssistantUiController::OnMicStateChanged(MicState mic_state) {
 
 void AssistantUiController::OnProactiveSuggestionsChanged(
     scoped_refptr<const ProactiveSuggestions> proactive_suggestions) {
-  // When proactive suggestions are present, we show the associated view if it
-  // isn't showing already. If it's already showing, no action need be taken.
-  if (proactive_suggestions) {
+  const bool should_suppress_duplicates = chromeos::assistant::features::
+      IsProactiveSuggestionsSuppressDuplicatesEnabled();
+
+  // When proactive suggestions duplicate suppression is enabled, we'll need to
+  // check if a set of proactive suggestions has already been shown before
+  // showing it to the user. Per UX requirement, proactive suggestions are
+  // considered duplicates for purposes of suppression if they share the same
+  // content |description|, regardless of whether or not their respective |html|
+  // matches. Note that we only calculate the hash here if needed.
+  const size_t proactive_suggestions_hash =
+      proactive_suggestions && should_suppress_duplicates
+          ? base::FastHash(proactive_suggestions->description())
+          : static_cast<size_t>(0);
+
+  bool should_show = !!proactive_suggestions;
+  if (should_show && should_suppress_duplicates) {
+    should_show = !base::Contains(shown_proactive_suggestions_,
+                                  proactive_suggestions_hash);
+  }
+
+  // When proactive suggestions need to be shown, we show the associated view if
+  // it isn't showing already. If it's already showing, no action need be taken.
+  if (should_show) {
     if (!proactive_suggestions_view_) {
       CreateProactiveSuggestionsView();
       proactive_suggestions_view_->GetWidget()->ShowInactive();
     }
+
+    // When suppressing duplicates, we need to cache the hash for the proactive
+    // suggestions that are being shown so that we don't show them again.
+    if (should_suppress_duplicates)
+      shown_proactive_suggestions_.emplace(proactive_suggestions_hash);
+
     // The proactive suggestions widget will automatically be closed if the user
     // doesn't interact with it within a fixed interval.
     auto_close_proactive_suggestions_timer_.Start(
@@ -168,7 +195,7 @@ void AssistantUiController::OnProactiveSuggestionsChanged(
             weak_factory_.GetWeakPtr()));
     return;
   }
-  // When proactive suggestions are absent, we need to ensure that the
+  // When proactive suggestions should not be shown, we need to ensure that the
   // associated view is absent if it isn't already.
   if (proactive_suggestions_view_) {
     ResetProactiveSuggestionsView();

ash/assistant/assistant_ui_controller.h

@@ -7,6 +7,7 @@
 
 #include <map>
 #include <memory>
+#include <set>
 #include <string>
 
 #include "ash/ash_export.h"
@@ -191,6 +192,12 @@ class ASH_EXPORT AssistantUiController
   // Whether the UI controller is observing changes to the usable work area.
   bool is_observing_usable_work_area_ = false;
 
+  // When proactive suggestions duplicate suppression is enabled, we won't show
+  // proactive suggestions that have been shown before. To accomplish this, we
+  // must cache proactive suggestions that have already been shown to the user.
+  // We cache a hash representation of the proactive suggestions to save space.
+  std::set<size_t> shown_proactive_suggestions_;
+
   base::WeakPtrFactory<AssistantUiController> weak_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(AssistantUiController);

chromeos/services/assistant/public/features.cc

@@ -25,6 +25,9 @@ const base::Feature kAssistantAppSupport{"AssistantAppSupport",
 const base::Feature kAssistantProactiveSuggestions{
     "AssistantProactiveSuggestions", base::FEATURE_DISABLED_BY_DEFAULT};
 
+const base::FeatureParam<bool> kAssistantProactiveSuggestionsSuppressDuplicates{
+    &kAssistantProactiveSuggestions, "suppress-duplicates", true};
+
 const base::Feature kAssistantRoutines{"AssistantRoutines",
                                        base::FEATURE_DISABLED_BY_DEFAULT};
 
@@ -107,6 +110,10 @@ bool IsProactiveSuggestionsEnabled() {
   return base::FeatureList::IsEnabled(kAssistantProactiveSuggestions);
 }
 
+bool IsProactiveSuggestionsSuppressDuplicatesEnabled() {
+  return kAssistantProactiveSuggestionsSuppressDuplicates.Get();
+}
+
 bool IsRoutinesEnabled() {
   return base::FeatureList::IsEnabled(kAssistantRoutines);
 }

chromeos/services/assistant/public/features.h

@@ -7,6 +7,7 @@
 
 #include "base/component_export.h"
 #include "base/feature_list.h"
+#include "base/metrics/field_trial_params.h"
 
 namespace chromeos {
 namespace assistant {
@@ -32,6 +33,12 @@ extern const base::Feature kAssistantAppSupport;
 COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC)
 extern const base::Feature kAssistantProactiveSuggestions;
 
+// Enables suppression of Assistant proactive suggestions that have already been
+// shown to the user.
+COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC)
+extern const base::FeatureParam<bool>
+    kAssistantProactiveSuggestionsSuppressDuplicates;
+
 // Enables Assistant routines.
 COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC)
 extern const base::Feature kAssistantRoutines;
@@ -104,6 +111,9 @@ COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) bool IsPowerManagerEnabled();
 
 COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) bool IsProactiveSuggestionsEnabled();
 
+COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC)
+bool IsProactiveSuggestionsSuppressDuplicatesEnabled();
+
 COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) bool IsRoutinesEnabled();
 
 COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) bool IsScreenContextQueryEnabled();