Assistant: Add notification setting

This patch adds a toggle to the Settings to enable/disable
notifications.

Bug: b/111505289
Test: VoiceInteractionControllerClientTest.PrefChangeSendsNotification
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: I7b1ba03cc326abb098f9f902f0c51d68f4938133
Reviewed-on: https://chromium-review.googlesource.com/1142503
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Yue Li <updowndota@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579841}

ash/assistant/assistant_notification_controller.cc

@@ -9,6 +9,7 @@
 #include "ash/resources/vector_icons/vector_icons.h"
 #include "ash/shell.h"
 #include "ash/strings/grit/ash_strings.h"
+#include "ash/voice_interaction/voice_interaction_controller.h"
 #include "base/bind.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/memory/weak_ptr.h"
@@ -121,6 +122,10 @@ void AssistantNotificationController::OnShowNotification(
     AssistantNotificationPtr notification) {
   DCHECK(assistant_);
 
+  // Do not show notification if the setting is false.
+  if (!Shell::Get()->voice_interaction_controller()->notification_enabled())
+    return;
+
   // Create the specified |notification| that should be rendered in the
   // |message_center| for the interaction.
   const base::string16 title = base::UTF8ToUTF16(notification->title);

ash/public/interfaces/voice_interaction_controller.mojom

@@ -89,6 +89,9 @@ interface VoiceInteractionController {
   // if disabled by policy.
   NotifyFeatureAllowed(AssistantAllowedState state);
 
+  // Called when the notification is enabled/disabled.
+  NotifyNotificationEnabled(bool enabled);
+
   // Return if the voice interaction setting is enabled/disabled.
   IsSettingEnabled() => (bool enabled);
 

ash/voice_interaction/voice_interaction_controller.cc

@@ -60,6 +60,10 @@ void VoiceInteractionController::NotifyFeatureAllowed(
   });
 }
 
+void VoiceInteractionController::NotifyNotificationEnabled(bool enabled) {
+  notification_enabled_ = enabled;
+}
+
 void VoiceInteractionController::IsSettingEnabled(
     IsSettingEnabledCallback callback) {
   std::move(callback).Run(settings_enabled_);

ash/voice_interaction/voice_interaction_controller.h

@@ -29,6 +29,7 @@ class ASH_EXPORT VoiceInteractionController
   void NotifyHotwordEnabled(bool enabled) override;
   void NotifySetupCompleted(bool completed) override;
   void NotifyFeatureAllowed(mojom::AssistantAllowedState state) override;
+  void NotifyNotificationEnabled(bool enabled) override;
   void IsSettingEnabled(IsSettingEnabledCallback callback) override;
   void IsSetupCompleted(IsSetupCompletedCallback callback) override;
   void IsHotwordEnabled(IsHotwordEnabledCallback callback) override;
@@ -44,6 +45,8 @@ class ASH_EXPORT VoiceInteractionController
 
   mojom::AssistantAllowedState allowed_state() const { return allowed_state_; }
 
+  bool notification_enabled() const { return notification_enabled_; }
+
   void FlushForTesting();
 
  private:
@@ -55,13 +58,16 @@ class ASH_EXPORT VoiceInteractionController
   // Whether voice interaction is enabled in system settings.
   bool settings_enabled_ = false;
 
-  // Whether voice intearction setup flow has completed.
+  // Whether voice interaction setup flow has completed.
   bool setup_completed_ = false;
 
   // Whether hotword listening is enabled.
   bool hotword_enabled_ = false;
 
-  // Whether voice intearction feature is allowed or disallowed for what reason.
+  // Whether notification is enabled.
+  bool notification_enabled_ = false;
+
+  // Whether voice interaction feature is allowed or disallowed for what reason.
   mojom::AssistantAllowedState allowed_state_ =
       mojom::AssistantAllowedState::ALLOWED;
 

chrome/app/settings_strings.grdp

@@ -2654,6 +2654,12 @@
   <message name="IDS_SETTINGS_GOOGLE_ASSISTANT_ENABLE_HOTWORD_DESCRIPTION" desc="Sub label for hotword-enable toggle.">
     Access your Assistant any time you say "OK Google" when your screen is on.
   </message>
+  <message name="IDS_SETTINGS_GOOGLE_ASSISTANT_ENABLE_NOTIFICATION" desc="Title for a toggle that lets the assistant show you notifications.">
+    Let Assistant show you notifications
+  </message>
+  <message name="IDS_SETTINGS_GOOGLE_ASSISTANT_ENABLE_NOTIFICATION_DESCRIPTION" desc="Sub label for notification-enable toggle.">
+    Enables the Assistant to show you notifications.
+  </message>
   <message name="IDS_SETTINGS_GOOGLE_ASSISTANT_SETTINGS" desc="Title for a button that opens the Google Assistant app settings.">
     Google Assistant settings
   </message>

chrome/browser/chromeos/arc/voice_interaction/fake_voice_interaction_controller.cc

@@ -44,6 +44,10 @@ void FakeVoiceInteractionController::NotifyFeatureAllowed(
   assistant_allowed_state_ = state;
 }
 
+void FakeVoiceInteractionController::NotifyNotificationEnabled(bool enabled) {
+  voice_interaction_notification_enabled_ = enabled;
+}
+
 void FakeVoiceInteractionController::IsSettingEnabled(
     IsSettingEnabledCallback callback) {
   std::move(callback).Run(voice_interaction_settings_enabled_);

chrome/browser/chromeos/arc/voice_interaction/fake_voice_interaction_controller.h

@@ -25,6 +25,7 @@ class FakeVoiceInteractionController
   void NotifyHotwordEnabled(bool enabled) override;
   void NotifySetupCompleted(bool completed) override;
   void NotifyFeatureAllowed(ash::mojom::AssistantAllowedState state) override;
+  void NotifyNotificationEnabled(bool enabled) override;
   void IsSettingEnabled(IsSettingEnabledCallback callback) override;
   void IsSetupCompleted(IsSetupCompletedCallback callback) override;
   void IsHotwordEnabled(IsHotwordEnabledCallback callback) override;
@@ -48,6 +49,9 @@ class FakeVoiceInteractionController
   ash::mojom::AssistantAllowedState assistant_allowed_state() const {
     return assistant_allowed_state_;
   }
+  bool voice_interaction_notification_enabled() const {
+    return voice_interaction_notification_enabled_;
+  }
 
  private:
   ash::mojom::VoiceInteractionState voice_interaction_state_ =
@@ -56,6 +60,7 @@ class FakeVoiceInteractionController
   bool voice_interaction_context_enabled_ = false;
   bool voice_interaction_hotword_enabled_ = false;
   bool voice_interaction_setup_completed_ = false;
+  bool voice_interaction_notification_enabled_ = false;
   ash::mojom::AssistantAllowedState assistant_allowed_state_ =
       ash::mojom::AssistantAllowedState::DISALLOWED_BY_INCOGNITO;
 

chrome/browser/chromeos/arc/voice_interaction/voice_interaction_controller_client.cc

@@ -122,6 +122,15 @@ void VoiceInteractionControllerClient::NotifyFeatureAllowed() {
   voice_interaction_controller_->NotifyFeatureAllowed(state);
 }
 
+void VoiceInteractionControllerClient::NotifyNotificationEnabled() {
+  DCHECK(profile_);
+  PrefService* prefs = profile_->GetPrefs();
+  // Make sure voice interaction is enabled.
+  DCHECK(prefs->GetBoolean(prefs::kVoiceInteractionEnabled));
+  bool enabled = prefs->GetBoolean(prefs::kVoiceInteractionNotificationEnabled);
+  voice_interaction_controller_->NotifyNotificationEnabled(enabled);
+}
+
 void VoiceInteractionControllerClient::ActiveUserChanged(
     const user_manager::User* active_user) {
   if (active_user && active_user->is_profile_created())
@@ -178,12 +187,19 @@ void VoiceInteractionControllerClient::SetProfile(Profile* profile) {
       base::BindRepeating(
           &VoiceInteractionControllerClient::NotifyHotwordEnabled,
           base::Unretained(this)));
+  pref_change_registrar_->Add(
+      prefs::kVoiceInteractionNotificationEnabled,
+      base::BindRepeating(
+          &VoiceInteractionControllerClient::NotifyNotificationEnabled,
+          base::Unretained(this)));
 
   NotifySetupCompleted();
   NotifySettingsEnabled();
   NotifyContextEnabled();
-  if (prefs->GetBoolean(prefs::kVoiceInteractionEnabled))
+  if (prefs->GetBoolean(prefs::kVoiceInteractionEnabled)) {
     NotifyHotwordEnabled();
+    NotifyNotificationEnabled();
+  }
   NotifyFeatureAllowed();
 }
 

chrome/browser/chromeos/arc/voice_interaction/voice_interaction_controller_client.h

@@ -58,6 +58,7 @@ class VoiceInteractionControllerClient
   void NotifyHotwordEnabled();
   void NotifySetupCompleted();
   void NotifyFeatureAllowed();
+  void NotifyNotificationEnabled();
 
   // user_manager::UserManager::UserSessionStateObserver overrides:
   void ActiveUserChanged(const user_manager::User* active_user) override;

chrome/browser/chromeos/arc/voice_interaction/voice_interaction_controller_client_unittest.cc

@@ -126,6 +126,17 @@ TEST_F(VoiceInteractionControllerClientTest, PrefChangeSendsNotification) {
       true,
       voice_interaction_controller()->voice_interaction_hotword_enabled());
 
+  // Default setting is true.
+  ASSERT_EQ(true,
+            prefs->GetBoolean(prefs::kVoiceInteractionNotificationEnabled));
+  prefs->SetBoolean(prefs::kVoiceInteractionNotificationEnabled, false);
+  ASSERT_EQ(false,
+            prefs->GetBoolean(prefs::kVoiceInteractionNotificationEnabled));
+  voice_interaction_controller_client()->FlushMojoForTesting();
+  EXPECT_EQ(
+      false,
+      voice_interaction_controller()->voice_interaction_notification_enabled());
+
   ASSERT_EQ(false,
             prefs->GetBoolean(prefs::kArcVoiceInteractionValuePropAccepted));
   prefs->SetBoolean(prefs::kArcVoiceInteractionValuePropAccepted, true);

chrome/browser/extensions/api/settings_private/prefs_util.cc

@@ -364,6 +364,8 @@ const PrefsUtil::TypedPrefMap& PrefsUtil::GetWhitelistedKeys() {
       settings_api::PrefType::PREF_TYPE_BOOLEAN;
   (*s_whitelist)[arc::prefs::kVoiceInteractionHotwordEnabled] =
       settings_api::PrefType::PREF_TYPE_BOOLEAN;
+  (*s_whitelist)[arc::prefs::kVoiceInteractionNotificationEnabled] =
+      settings_api::PrefType::PREF_TYPE_BOOLEAN;
 
   // Misc.
   (*s_whitelist)[::prefs::kUse24HourClock] =

chrome/browser/resources/settings/google_assistant_page/google_assistant_page.html

@@ -37,6 +37,14 @@
             sub-label="$i18n{googleAssistantEnableHotwordDescription}">
         </settings-toggle-button>
       </template>
+      <template is="dom-if" if="[[assistantFeatureEnabled_]]">
+        <settings-toggle-button id="googleAssistantNotificationEnable"
+            class="continuation indented"
+            pref="{{prefs.settings.voice_interaction.notification.enabled}}"
+            label="$i18n{googleAssistantEnableNotification}"
+            sub-label="$i18n{googleAssistantEnableNotificationDescription}">
+        </settings-toggle-button>
+      </template>
       <div id="googleAssistantSettings" class="settings-box"
           on-click="onGoogleAssistantSettingsTapped_" actionable>
         <div class="start">

chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc

@@ -2124,6 +2124,10 @@ void AddGoogleAssistantStrings(content::WebUIDataSource* html_source) {
        IDS_SETTINGS_GOOGLE_ASSISTANT_ENABLE_HOTWORD},
       {"googleAssistantEnableHotwordDescription",
        IDS_SETTINGS_GOOGLE_ASSISTANT_ENABLE_HOTWORD_DESCRIPTION},
+      {"googleAssistantEnableNotification",
+       IDS_SETTINGS_GOOGLE_ASSISTANT_ENABLE_NOTIFICATION},
+      {"googleAssistantEnableNotificationDescription",
+       IDS_SETTINGS_GOOGLE_ASSISTANT_ENABLE_NOTIFICATION_DESCRIPTION},
       {"googleAssistantSettings", IDS_SETTINGS_GOOGLE_ASSISTANT_SETTINGS},
   };
   AddLocalizedStringsBulk(html_source, localized_strings,

components/arc/arc_prefs.cc

@@ -104,6 +104,10 @@ const char kVoiceInteractionContextEnabled[] =
 // to use hotword listening.
 const char kVoiceInteractionHotwordEnabled[] =
     "settings.voice_interaction.hotword.enabled";
+// A preference that indicates the user has allowed voice interaction services
+// to send notification.
+const char kVoiceInteractionNotificationEnabled[] =
+    "settings.voice_interaction.notification.enabled";
 
 void RegisterProfilePrefs(PrefRegistrySimple* registry) {
   // TODO(dspaid): Implement a mechanism to allow this to sync on first boot
@@ -147,6 +151,7 @@ void RegisterProfilePrefs(PrefRegistrySimple* registry) {
   registry->RegisterBooleanPref(kVoiceInteractionContextEnabled, false);
   registry->RegisterBooleanPref(kVoiceInteractionEnabled, false);
   registry->RegisterBooleanPref(kVoiceInteractionHotwordEnabled, false);
+  registry->RegisterBooleanPref(kVoiceInteractionNotificationEnabled, true);
 }
 
 }  // namespace prefs

components/arc/arc_prefs.h

@@ -44,6 +44,7 @@ ARC_EXPORT extern const char kVoiceInteractionActivityControlAccepted[];
 ARC_EXPORT extern const char kVoiceInteractionEnabled[];
 ARC_EXPORT extern const char kVoiceInteractionContextEnabled[];
 ARC_EXPORT extern const char kVoiceInteractionHotwordEnabled[];
+ARC_EXPORT extern const char kVoiceInteractionNotificationEnabled[];
 
 void RegisterProfilePrefs(PrefRegistrySimple* registry);