commit 1036273e484dae035407b8efca1667dd4d88e866
author Yu Zhong <yuzhong@google.com> Wed Jun 12 21:34:35 2024
committer Chromium LUCI CQ <chromium-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Jun 12 21:34:35 2024
tree 71fee433ffd942bbf7590ffcd3474213b966830d
parent e95ac012fea6bfc240dba4c3ecead9c783eb546f

[Read Aloud] Do not show toast when first opening reading mode

Original bug fix had some side effects causing toast not showing up
for requested downloads, addressing them by assigning an unknown
status in local storage when a language download is requested.

Fixed: 344764908
Change-Id: I1adabf4ae5cdef6e24f39721de8c4cacf16e5995
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5623694
Reviewed-by: Kristi Saney <kristislee@google.com>
Reviewed-by: Lauren Winston <lwinston@google.com>
Commit-Queue: Yu Zhong <yuzhong@google.com>
Cr-Commit-Position: refs/heads/main@{#1314258}

chrome/browser/resources/side_panel/read_anything/app.ts

diff --git a/chrome/browser/resources/side_panel/read_anything/app.ts b/chrome/browser/resources/side_panel/read_anything/app.ts
index 9bb1c043..13402a27 100644
--- a/chrome/browser/resources/side_panel/read_anything/app.ts
+++ b/chrome/browser/resources/side_panel/read_anything/app.ts
@@ -24,7 +24,7 @@
 import {minOverflowLengthToScroll, playFromSelectionTimeout, toastDurationMs, validatedFontName} from './common.js';
 import type {ReadAnythingToolbarElement} from './read_anything_toolbar.js';
 import type {VoicePackStatus} from './voice_language_util.js';
-import {areVoicesEqual, AVAILABLE_GOOGLE_TTS_LOCALES, convertLangOrLocaleForVoicePackManager, convertLangOrLocaleToExactVoicePackLocale, convertLangToAnAvailableLangIfPresent, createInitialListOfEnabledLanguages, doesLanguageHaveNaturalVoices, getVoicePackConvertedLangIfExists, isEspeak, isNatural, isVoicePackStatusError, isVoicePackStatusSuccess, mojoVoicePackStatusToVoicePackStatusEnum, VoiceClientSideStatusCode, VoicePackServerStatusErrorCode, VoicePackServerStatusSuccessCode} from './voice_language_util.js';
+import {areVoicesEqual, AVAILABLE_GOOGLE_TTS_LOCALES, convertLangOrLocaleForVoicePackManager, convertLangOrLocaleToExactVoicePackLocale, convertLangToAnAvailableLangIfPresent, createInitialListOfEnabledLanguages, doesLanguageHaveNaturalVoices, getVoicePackConvertedLangIfExists, isEspeak, isNatural, isVoicePackStatusError, isVoicePackStatusSuccess, isWaitingForInstallLocally, mojoVoicePackStatusToVoicePackStatusEnum, VoiceClientSideStatusCode, VoicePackServerStatusErrorCode, VoicePackServerStatusSuccessCode} from './voice_language_util.js';
 
 const ReadAnythingElementBase = WebUiListenerMixin(PolymerElement);
 
@@ -1014,9 +1014,16 @@
         case VoicePackServerStatusSuccessCode.INSTALLED:
           // See if voice is newly downloaded and should have a toast notifying
           // the user.
-          if (oldVoicePackStatus &&
-              oldVoicePackStatus.code !==
-                  VoicePackServerStatusSuccessCode.INSTALLED) {
+          // If the old voice pack status is undefined, it means we haven't
+          // received a server status yet. If we are now receiving an installed
+          // status, and we were locally waiting for an install, then we know
+          // the language is newly downloaded.
+          if ((!oldVoicePackStatus &&
+               isWaitingForInstallLocally(
+                   this.getVoicePackLocalStatus_(lang))) ||
+              (oldVoicePackStatus &&
+               oldVoicePackStatus.code !==
+                   VoicePackServerStatusSuccessCode.INSTALLED)) {
             // TODO (b/346619236): some language codes miss locale and are not
             // translated into display names
             this.lastDownloadedLang = getVoicePackConvertedLangIfExists(lang);

chrome/browser/resources/side_panel/read_anything/voice_language_util.ts

diff --git a/chrome/browser/resources/side_panel/read_anything/voice_language_util.ts b/chrome/browser/resources/side_panel/read_anything/voice_language_util.ts
index 0ae921a6..faa6fde 100644
--- a/chrome/browser/resources/side_panel/read_anything/voice_language_util.ts
+++ b/chrome/browser/resources/side_panel/read_anything/voice_language_util.ts
@@ -255,6 +255,12 @@
       locale => locale.startsWith(possibleConvertedLang.toLowerCase()));
 }
 
+export function isWaitingForInstallLocally(status: VoiceClientSideStatusCode|
+                                           undefined) {
+  return status === VoiceClientSideStatusCode.SENT_INSTALL_REQUEST ||
+      status === VoiceClientSideStatusCode.SENT_INSTALL_REQUEST_ERROR_RETRY;
+}
+
 function convertUnsupportedBaseLangToSupportedLocale(baseLang: string): string|
     undefined {
   // Check if it's a base lang that supports a locale. These are the only

chrome/test/data/webui/side_panel/read_anything/update_voice_pack_test.ts

diff --git a/chrome/test/data/webui/side_panel/read_anything/update_voice_pack_test.ts b/chrome/test/data/webui/side_panel/read_anything/update_voice_pack_test.ts
index 2c8af1f..27702b0 100644
--- a/chrome/test/data/webui/side_panel/read_anything/update_voice_pack_test.ts
+++ b/chrome/test/data/webui/side_panel/read_anything/update_voice_pack_test.ts
@@ -107,7 +107,7 @@
 
       // The first call to update status should be the existing status from
       // the server.
-      app.updateVoicePackStatus(lang, 'kNotInstalled');
+      app.updateVoicePackStatus(lang, 'kInstalled');
 
       assertFalse(toast.open);
     });
@@ -140,8 +140,13 @@
       const lang = 'en';
 
       // existing status
-      app.updateVoicePackStatus(lang, 'kNotInstalled');
+      app.updateVoicePackStatus(lang, 'kInstalled');
       // then we request install
+      // Bypass Typescript compiler to allow us to set a private readonly
+      // property
+      // @ts-ignore
+      app.setVoicePackLocalStatus_(
+          lang, VoiceClientSideStatusCode.SENT_INSTALL_REQUEST);
       app.updateVoicePackStatus(lang, 'kInstalling');
       // install completes
       app.updateVoicePackStatus(lang, 'kInstalled');