Hook up Assistant Voice Match screen

Bug: b/116166191 Test: Manual Test Change-Id: I5611a49d124f75ff535724224594e3f098ed31be Reviewed-on: https://chromium-review.googlesource.com/c/1355520 Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org> Commit-Queue: Yue Li <updowndota@chromium.org> Cr-Commit-Position: refs/heads/master@{#613649}
nwjs · Dec 4, 2018 · b027ce5 · b027ce5
1 parent 93b7c08
commit b027ce5
Show file tree

Hide file tree

Showing 10 changed files with 216 additions and 58 deletions.
diff --git a/chrome/browser/resources/chromeos/assistant_optin/assistant_get_more.js b/chrome/browser/resources/chromeos/assistant_optin/assistant_get_more.js
@@ -46,6 +46,13 @@ Polymer({
    */
   screenShown_: false,
 
+  /**
+   * Whether the voice match feature has been enabled.
+   * @type {boolean}
+   * @private
+   */
+  voiceMatchFeatureEnabled_: false,
+
   /**
    * On-tap event handler for next button.
    *
@@ -56,13 +63,17 @@ Polymer({
       return;
     }
     this.buttonsDisabled = true;
-    var hotword = this.$$('#toggle0').hasAttribute('checked');
-    var screenContext = this.$$('#toggle1').hasAttribute('checked');
-    var toggle2 = this.$$('#toggle2');
-    var emailOptedIn = toggle2 != null && toggle2.hasAttribute('checked');
+
+    if (!this.voiceMatchFeatureEnabled_) {
+      var hotword = this.$$('#toggle-hotword').hasAttribute('checked');
+      chrome.send('login.AssistantOptInFlowScreen.hotwordResult', [hotword]);
+    }
+    var screenContext = this.$$('#toggle-context').hasAttribute('checked');
+    var toggleEmail = this.$$('#toggle-email');
+    var emailOptedIn =
+        toggleEmail != null && toggleEmail.hasAttribute('checked');
 
     // TODO(updowndota): Wrap chrome.send() calls with a proxy object.
-    chrome.send('login.AssistantOptInFlowScreen.hotwordResult', [hotword]);
     chrome.send(
         'login.AssistantOptInFlowScreen.GetMoreScreen.userActed',
         [screenContext, emailOptedIn]);
@@ -83,6 +94,7 @@ Polymer({
     this.$['title-text'].textContent = data['getMoreTitle'];
     this.$['intro-text'].textContent = data['getMoreIntro'];
     this.$['next-button-text'].textContent = data['getMoreContinueButton'];
+    this.voiceMatchFeatureEnabled_ = data['voiceMatchFeatureEnabled'];
 
     this.consentStringLoaded_ = true;
     if (this.settingZippyLoaded_) {
@@ -103,15 +115,15 @@ Polymer({
           'data:text/html;charset=utf-8,' +
               encodeURIComponent(zippy.getWrappedIcon(data['iconUri'])));
       zippy.setAttribute('toggle-style', true);
-      zippy.id = 'zippy' + i;
+      zippy.id = 'zippy-' + data['id'];
       var title = document.createElement('div');
       title.className = 'zippy-title';
       title.textContent = data['title'];
       zippy.appendChild(title);
 
       var toggle = document.createElement('cr-toggle');
       toggle.className = 'zippy-toggle';
-      toggle.id = 'toggle' + i;
+      toggle.id = 'toggle-' + data['id'];
       if (data['defaultEnabled']) {
         toggle.setAttribute('checked', '');
       }

diff --git a/chrome/browser/resources/chromeos/assistant_optin/assistant_optin_flow.js b/chrome/browser/resources/chromeos/assistant_optin/assistant_optin_flow.js
@@ -42,9 +42,7 @@ Polymer({
    * @param {!Object} data New dictionary with i18n values.
    */
   reloadContent: function(data) {
-    // Reload global local strings, process DOM tree again.
-    loadTimeData.overrideValues(data);
-    i18nTemplate.process(document, loadTimeData);
+    this.voiceMatchFeatureEnabled = data['voiceMatchFeatureEnabled'];
     this.$['value-prop'].reloadContent(data);
     this.$['third-party'].reloadContent(data);
     this.$['get-more'].reloadContent(data);
@@ -80,7 +78,11 @@ Polymer({
         this.showScreen(this.$['third-party']);
         break;
       case this.$['third-party']:
-        this.showScreen(this.$['voice-match']);
+        if (this.voiceMatchFeatureEnabled) {
+          this.showScreen(this.$['voice-match']);
+        } else {
+          this.showScreen(this.$['get-more']);
+        }
         break;
       case this.$['voice-match']:
         this.showScreen(this.$['get-more']);

diff --git a/chrome/browser/resources/chromeos/assistant_optin/assistant_voice_match.css b/chrome/browser/resources/chromeos/assistant_optin/assistant_voice_match.css
@@ -24,11 +24,17 @@
   padding: 0 8px 0 8px;
 }
 
-.intro #recording-container,
-.recording #intro-container,
-.completed #intro-container {
-  position: absolute;
-  visibility: hidden;
+#intro-container,
+#recording-container,
+#already-setup-container {
+  display: none;
+}
+
+.intro #intro-container,
+.recording #recording-container,
+.completed #recording-container,
+.already-setup #already-setup-container {
+  display: block;
 }
 
 .recording #title-completed,
@@ -37,9 +43,10 @@
 }
 
 .intro #skip-button,
-.intro #next-button,
+.intro #agree-button,
 .recording #later-button,
-.completed #done-button {
+.completed #done-button,
+.already-setup #next-button {
   display: block;
 }
 

diff --git a/chrome/browser/resources/chromeos/assistant_optin/assistant_voice_match.html b/chrome/browser/resources/chromeos/assistant_optin/assistant_voice_match.html
@@ -48,25 +48,35 @@
             <div class="content" id="footer-text"
                 i18n-content="assistantVoiceMatchFooter"></div>
           </div>
+          <div id="already-setup-container">
+            <div class="title" id="title-already-setup"
+                i18n-content="assistantVoiceMatchAlreadySetupTitle"></div>
+            <div class="content"
+                i18n-content="assistantVoiceMatchAlreadySetupMessage"></div>
+          </div>
         </div>
       </div>
       <div slot="bottom-buttons" class="flex layout horizontal">
         <div class="flex"></div>
         <oobe-text-button id="skip-button" on-tap="onSkipTap_"
             disabled="[[buttonsDisabled]]">
-          <div id="skip-button-text">No thanks</div>
+          <div i18n-content="assistantOptinNoThanksButton"></div>
         </oobe-text-button>
         <oobe-text-button id="later-button" on-tap="onSkipTap_"
             disabled="[[buttonsDisabled]]">
-          <div id="skip-button-text">Do it later</div>
+          <div i18n-content="assistantOptinLaterButton"></div>
         </oobe-text-button>
-        <oobe-text-button id="next-button" inverse on-tap="onNextTap_"
+        <oobe-text-button id="agree-button" inverse on-tap="onAgreeTap_"
             disabled="[[buttonsDisabled]]">
-          <div id="next-button-text">I agree</div>
+          <div i18n-content="assistantOptinAgreeButton"></div>
         </oobe-text-button>
         <oobe-text-button id="done-button" inverse on-tap="onDoneTap_"
             disabled="[[buttonsDisabled]]">
-          <div id="done-button-text">Save</div>
+          <div i18n-content="assistantOptinSaveButton"></div>
+        </oobe-text-button>
+        <oobe-text-button id="next-button" inverse on-tap="onDoneTap_"
+            disabled="[[buttonsDisabled]]">
+          <div i18n-content="next"></div>
         </oobe-text-button>
       </div>
     </oobe-dialog>

diff --git a/chrome/browser/resources/chromeos/assistant_optin/assistant_voice_match.js b/chrome/browser/resources/chromeos/assistant_optin/assistant_voice_match.js
@@ -31,11 +31,11 @@ Polymer({
   },
 
   /**
-   * On-tap event handler for next button.
+   * On-tap event handler for agree button.
    *
    * @private
    */
-  onNextTap_: function() {
+  onAgreeTap_: function() {
     this.removeClass_('intro');
     this.addClass_('recording');
     chrome.send(
@@ -44,7 +44,7 @@ Polymer({
   },
 
   /**
-   * On-tap event handler for next button.
+   * On-tap event handler for done button.
    *
    * @private
    */
@@ -107,6 +107,6 @@ Polymer({
    * Signal from host to show the screen.
    */
   onShow: function() {
-    this.$['next-button'].focus();
+    this.$['agree-button'].focus();
   },
 });
diff --git a/chrome/browser/ui/webui/chromeos/assistant_optin/assistant_optin_utils.cc b/chrome/browser/ui/webui/chromeos/assistant_optin/assistant_optin_utils.cc
@@ -10,6 +10,7 @@
 #include "chrome/browser/ui/webui/chromeos/user_image_source.h"
 #include "chrome/grit/browser_resources.h"
 #include "chrome/grit/generated_resources.h"
+#include "chromeos/services/assistant/public/features.h"
 #include "components/arc/arc_prefs.h"
 #include "components/consent_auditor/consent_auditor.h"
 #include "components/signin/core/browser/signin_manager_base.h"
@@ -108,23 +109,28 @@ base::Value CreateGetMoreData(bool email_optin_needed,
                               const assistant::EmailOptInUi& email_optin_ui) {
   base::Value get_more_data(base::Value::Type::LIST);
 
-  // Process hotword data.
-  base::Value hotword_data(base::Value::Type::DICTIONARY);
-  hotword_data.SetKey(
-      "title",
-      base::Value(l10n_util::GetStringUTF16(IDS_ASSISTANT_HOTWORD_TITLE)));
-  hotword_data.SetKey(
-      "description",
-      base::Value(l10n_util::GetStringUTF16(IDS_ASSISTANT_HOTWORD_DESC)));
-  hotword_data.SetKey("defaultEnabled", base::Value(true));
-  hotword_data.SetKey(
-      "iconUri",
-      base::Value("https://www.gstatic.com/images/icons/material/system/"
-                  "2x/mic_none_grey600_48dp.png"));
-  get_more_data.GetList().push_back(std::move(hotword_data));
+  if (!base::FeatureList::IsEnabled(
+          assistant::features::kAssistantVoiceMatch)) {
+    // Process hotword data.
+    base::Value hotword_data(base::Value::Type::DICTIONARY);
+    hotword_data.SetKey("id", base::Value("hotword"));
+    hotword_data.SetKey(
+        "title",
+        base::Value(l10n_util::GetStringUTF16(IDS_ASSISTANT_HOTWORD_TITLE)));
+    hotword_data.SetKey(
+        "description",
+        base::Value(l10n_util::GetStringUTF16(IDS_ASSISTANT_HOTWORD_DESC)));
+    hotword_data.SetKey("defaultEnabled", base::Value(true));
+    hotword_data.SetKey(
+        "iconUri",
+        base::Value("https://www.gstatic.com/images/icons/material/system/"
+                    "2x/mic_none_grey600_48dp.png"));
+    get_more_data.GetList().push_back(std::move(hotword_data));
+  }
 
   // Process screen context data.
   base::Value context_data(base::Value::Type::DICTIONARY);
+  context_data.SetKey("id", base::Value("context"));
   context_data.SetKey("title", base::Value(l10n_util::GetStringUTF16(
                                    IDS_ASSISTANT_SCREEN_CONTEXT_TITLE)));
   context_data.SetKey("description", base::Value(l10n_util::GetStringUTF16(
@@ -139,6 +145,7 @@ base::Value CreateGetMoreData(bool email_optin_needed,
   // Process email optin data.
   if (email_optin_needed) {
     base::Value data(base::Value::Type::DICTIONARY);
+    data.SetKey("id", base::Value("email"));
     data.SetKey("title", base::Value(email_optin_ui.title()));
     data.SetKey("description", base::Value(email_optin_ui.description()));
     data.SetKey("defaultEnabled",

diff --git a/chrome/browser/ui/webui/chromeos/login/assistant_optin_flow_screen_handler.cc b/chrome/browser/ui/webui/chromeos/login/assistant_optin_flow_screen_handler.cc
@@ -79,7 +79,16 @@ void AssistantOptInFlowScreenHandler::DeclareLocalizedValues(
                IDS_ASSISTANT_VOICE_MATCH_COMPLETE);
   builder->Add("assistantVoiceMatchUploading",
                IDS_ASSISTANT_VOICE_MATCH_UPLOADING);
+  builder->Add("assistantVoiceMatchAlreadySetupTitle",
+               IDS_ASSISTANT_VOICE_MATCH_ALREADY_SETUP_TITLE);
+  builder->Add("assistantVoiceMatchAlreadySetupMessage",
+               IDS_ASSISTANT_VOICE_MATCH_ALREADY_SETUP_MESSAGE);
   builder->Add("assistantOptinOKButton", IDS_OOBE_OK_BUTTON_TEXT);
+  builder->Add("assistantOptinNoThanksButton", IDS_ASSISTANT_NO_THANKS_BUTTON);
+  builder->Add("assistantOptinLaterButton", IDS_ASSISTANT_LATER_BUTTON);
+  builder->Add("assistantOptinAgreeButton", IDS_ASSISTANT_AGREE_BUTTON);
+  builder->Add("assistantOptinSaveButton", IDS_ASSISTANT_SAVE_BUTTON);
+  builder->Add("assistantOptinWaitMessage", IDS_ASSISTANT_WAIT_MESSAGE);
   builder->Add("assistantReadyTitle", IDS_ASSISTANT_READY_SCREEN_TITLE);
   builder->Add("assistantReadyMessage", IDS_ASSISTANT_READY_SCREEN_MESSAGE);
   builder->Add("assistantReadyButton", IDS_ASSISTANT_DONE_BUTTON);
@@ -166,6 +175,7 @@ void AssistantOptInFlowScreenHandler::OnProcessingHotword() {
 }
 
 void AssistantOptInFlowScreenHandler::OnSpeakerIdEnrollmentDone() {
+  settings_manager_->StopSpeakerIdEnrollment(base::DoNothing());
   CallJSWithPrefix("onVoiceMatchUpdate", base::Value("done"));
 }
 
@@ -227,7 +237,15 @@ void AssistantOptInFlowScreenHandler::OnEmailOptInResult(bool opted_in) {
 void AssistantOptInFlowScreenHandler::OnStateChanged(
     ash::mojom::VoiceInteractionState state) {
   if (state != ash::mojom::VoiceInteractionState::NOT_READY) {
-    BindAssistantSettingsManager();
+    if (voice_enrollment_pending) {
+      voice_enrollment_pending = false;
+      DCHECK(settings_manager_.is_bound() &&
+             base::FeatureList::IsEnabled(
+                 assistant::features::kAssistantVoiceMatch));
+      settings_manager_->StartSpeakerIdEnrollment(true, std::move(client_ptr_));
+    } else {
+      BindAssistantSettingsManager();
+    }
     arc::VoiceInteractionControllerClient::Get()->RemoveObserver(this);
   }
 }
@@ -244,7 +262,9 @@ void AssistantOptInFlowScreenHandler::BindAssistantSettingsManager() {
                            mojo::MakeRequest(&settings_manager_));
   client_binding_.Bind(mojo::MakeRequest(&client_ptr_));
 
-  SendGetSettingsRequest();
+  if (initialized_) {
+    SendGetSettingsRequest();
+  }
 }
 
 void AssistantOptInFlowScreenHandler::SendGetSettingsRequest() {
@@ -337,7 +357,11 @@ void AssistantOptInFlowScreenHandler::OnGetSettingsResponse(
   }
 
   // Pass string constants dictionary.
-  ReloadContent(GetSettingsUiStrings(settings_ui, activity_control_needed_));
+  auto dictionary = GetSettingsUiStrings(settings_ui, activity_control_needed_);
+  dictionary.SetKey("voiceMatchFeatureEnabled",
+                    base::Value(base::FeatureList::IsEnabled(
+                        assistant::features::kAssistantVoiceMatch)));
+  ReloadContent(dictionary);
 }
 
 void AssistantOptInFlowScreenHandler::OnUpdateSettingsResponse(
@@ -395,11 +419,6 @@ void AssistantOptInFlowScreenHandler::HandleThirdPartyScreenUserAction(
   if (action == kNextPressed) {
     RecordAssistantOptInStatus(THIRD_PARTY_CONTINUED);
     ShowNextScreen();
-    if (!base::FeatureList::IsEnabled(
-            assistant::features::kAssistantVoiceMatch)) {
-      // Skip the voice match enrollment if feature is disabled.
-      ShowNextScreen();
-    }
   }
 }
 
@@ -409,10 +428,24 @@ void AssistantOptInFlowScreenHandler::HandleVoiceMatchScreenUserAction(
           assistant::features::kAssistantVoiceMatch)) {
     return;
   }
-  if (action == kNextPressed || action == kSkipPressed) {
+  PrefService* prefs = ProfileManager::GetActiveUserProfile()->GetPrefs();
+
+  if (action == kNextPressed) {
+    ShowNextScreen();
+  } else if (action == kSkipPressed) {
+    prefs->SetBoolean(arc::prefs::kVoiceInteractionHotwordEnabled, false);
+    settings_manager_->StopSpeakerIdEnrollment(base::DoNothing());
     ShowNextScreen();
   } else if (action == kRecordPressed) {
-    settings_manager_->StartSpeakerIdEnrollment(true, std::move(client_ptr_));
+    if (!prefs->GetBoolean(arc::prefs::kVoiceInteractionHotwordEnabled)) {
+      // Turn on hotword will restart the Assistant service. Thus the enrollment
+      // request should be sent after the service restart complete.
+      voice_enrollment_pending = true;
+      prefs->SetBoolean(arc::prefs::kVoiceInteractionHotwordEnabled, true);
+      arc::VoiceInteractionControllerClient::Get()->AddObserver(this);
+    } else {
+      settings_manager_->StartSpeakerIdEnrollment(true, std::move(client_ptr_));
+    }
   }
 }
 
@@ -475,6 +508,12 @@ void AssistantOptInFlowScreenHandler::HandleFlowFinished() {
     CallJSWithPrefix("closeDialog");
 }
 
-void AssistantOptInFlowScreenHandler::HandleFlowInitialized() {}
+void AssistantOptInFlowScreenHandler::HandleFlowInitialized() {
+  initialized_ = true;
+
+  if (settings_manager_.is_bound()) {
+    SendGetSettingsRequest();
+  }
+}
 
 }  // namespace chromeos
diff --git a/chrome/browser/ui/webui/chromeos/login/assistant_optin_flow_screen_handler.h b/chrome/browser/ui/webui/chromeos/login/assistant_optin_flow_screen_handler.h
@@ -118,6 +118,12 @@ class AssistantOptInFlowScreenHandler
   // Counter for the number of loading timeout happens.
   int loading_timeout_counter_ = 0;
 
+  // Whether the screen has been initialized.
+  bool initialized_ = false;
+
+  // Whether there is a pending voice match enrollment request.
+  bool voice_enrollment_pending = false;
+
   mojo::Binding<assistant::mojom::SpeakerIdEnrollmentClient> client_binding_;
   assistant::mojom::SpeakerIdEnrollmentClientPtr client_ptr_;
   assistant::mojom::AssistantSettingsManagerPtr settings_manager_;