Fix TTS extension API language matching.

chrome/browser/speech/extension_api/tts_engine_extension_api.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/speech/extension_api/tts_engine_extension_api.h"
 
 #include <string>
 
 #include "base/json/json_writer.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/extension_event_router.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/speech/extension_api/tts_extension_api_constants.h"
 #include "chrome/browser/speech/extension_api/tts_extension_api_controller.h"
 #include "chrome/common/extensions/extension.h"
 
 using extensions::Extension;
 
 namespace constants = tts_extension_api_constants;
 
 namespace events {
 const char kOnSpeak[] = "ttsEngine.onSpeak";
 const char kOnStop[] = "ttsEngine.onStop";
 };  // namespace events
 
+namespace {
+// Given a language/region code of the form 'fr-FR', returns just the basic
+// language portion, e.g. 'fr'.
+std::string TrimLanguageCode(std::string lang) {
+  if (lang.size() >= 5 && lang[2] == '-')
+    return lang.substr(0, 2);
+  else
+    return lang;
+}
+}
+
 void GetExtensionVoices(Profile* profile, ListValue* result_voices) {
   ExtensionService* service = profile->GetExtensionService();
   DCHECK(service);
   ExtensionEventRouter* event_router = profile->GetExtensionEventRouter();
   DCHECK(event_router);
 
   const ExtensionSet* extensions = service->extensions();
   ExtensionSet::const_iterator iter;
   for (iter = extensions->begin(); iter != extensions->end(); ++iter) {
     const Extension* extension = *iter;
@@ skipping 53 matching lines @@
   ExtensionService* service = utterance->profile()->GetExtensionService();
   DCHECK(service);
   ExtensionEventRouter* event_router =
       utterance->profile()->GetExtensionEventRouter();
   DCHECK(event_router);
 
   *matching_extension = NULL;
   *voice_index = -1;
   const ExtensionSet* extensions = service->extensions();
   ExtensionSet::const_iterator iter;
-  for (iter = extensions->begin(); iter != extensions->end(); ++iter) {
-    const Extension* extension = *iter;
 
-    if (!event_router->ExtensionHasEventListener(
-            extension->id(), events::kOnSpeak) ||
-        !event_router->ExtensionHasEventListener(
-            extension->id(), events::kOnStop)) {
-      continue;
-    }
+  // Make two passes: the first time, do strict language matching
+  // ('fr-FR' does not match 'fr-CA'). The second time, do prefix
+  // language matching ('fr-FR' matches 'fr' and 'fr-CA')
+  for (int pass = 0; pass < 2; ++pass) {
+    for (iter = extensions->begin(); iter != extensions->end(); ++iter) {
+      const Extension* extension = *iter;
 
-    if (!utterance->extension_id().empty() &&
-        utterance->extension_id() != extension->id()) {
-      continue;
-    }
-
-    const std::vector<Extension::TtsVoice>& tts_voices =
-        extension->tts_voices();
-    for (size_t i = 0; i < tts_voices.size(); ++i) {
-      const Extension::TtsVoice& voice = tts_voices[i];
-      if (!voice.voice_name.empty() &&
-          !utterance->voice_name().empty() &&
-          voice.voice_name != utterance->voice_name()) {
+      if (!event_router->ExtensionHasEventListener(
+              extension->id(), events::kOnSpeak) ||
+          !event_router->ExtensionHasEventListener(
+              extension->id(), events::kOnStop)) {
         continue;
       }
-      if (!voice.lang.empty() &&
-          !utterance->lang().empty() &&
-          voice.lang != utterance->lang()) {
+
+      if (!utterance->extension_id().empty() &&
+          utterance->extension_id() != extension->id()) {
         continue;
       }
-      if (!voice.gender.empty() &&
-          !utterance->gender().empty() &&
-          voice.gender != utterance->gender()) {
-        continue;
+
+      const std::vector<Extension::TtsVoice>& tts_voices =
+          extension->tts_voices();
+      for (size_t i = 0; i < tts_voices.size(); ++i) {

[David Tseng, 2012/06/20 17:18:30]

Use a const iterator here.

[dmazzoni, 2012/06/20 22:51:12]

We're using *i* below - we need to return the voice index. Switching to an
iterator would mean we'd need to keep a separate index variable.

[David Tseng, 2012/06/20 23:38:41]

On 2012/06/20 22:51:12, Dominic Mazzoni wrote:

std::distance

But, I can go either way.

+        const Extension::TtsVoice& voice = tts_voices[i];
+        if (!voice.voice_name.empty() &&
+            !utterance->voice_name().empty() &&
+            voice.voice_name != utterance->voice_name()) {
+          continue;
+        }
+        if (!voice.lang.empty() && !utterance->lang().empty()) {

[David Tseng, 2012/06/20 17:18:30]

Can you only make two passes to resolve the lang code (and not iterate through
all voices again)?

[dmazzoni, 2012/06/20 22:51:12]

I think it's necessary to iterate over all extensions twice, because I want to
make sure that an exact match is always preferred over a fuzzy match.

It would be possible to iterate over all voices just once and keep track of just
possible matches, then do two quick passes over that shorter list to pick the
one with the best language match, but I don't think that would improve
readability, and the number of voices is small so performance shouldn't matter
here.

Does that make sense? Or do you see another approach?

[David Tseng, 2012/06/20 23:38:41]

On 2012/06/20 22:51:12, Dominic Mazzoni wrote:

We perform strict matching via voice_name right? Are there extensions that have
no voice_name? I was thinking only of the case when we match against only one
voice name (and so we would only need to strict/fuzzy match against its lang).

But, if some extensions have no voice name, then I see why you need to do this.

+          std::string voice_lang = voice.lang;
+          std::string utterance_lang = utterance->lang();
+          if (pass == 1) {
+            voice_lang = TrimLanguageCode(voice_lang);
+            utterance_lang = TrimLanguageCode(utterance_lang);
+          }
+          if (voice_lang != utterance_lang)
+            continue;
+        }
+        if (!voice.gender.empty() &&
+            !utterance->gender().empty() &&
+            voice.gender != utterance->gender()) {
+          continue;
+        }
+        if (utterance->required_event_types().size() > 0) {
+          bool has_all_required_event_types = true;
+          for (std::set<std::string>::const_iterator iter =
+                   utterance->required_event_types().begin();
+               iter != utterance->required_event_types().end();
+               ++iter) {
+            if (voice.event_types.find(*iter) == voice.event_types.end()) {
+              has_all_required_event_types = false;
+              break;
+            }
+          }
+          if (!has_all_required_event_types)
+            continue;
+        }
+
+        *matching_extension = extension;
+        *voice_index = i;
+        return true;
       }
-      if (utterance->required_event_types().size() > 0) {
-        bool has_all_required_event_types = true;
-        for (std::set<std::string>::const_iterator iter =
-                 utterance->required_event_types().begin();
-             iter != utterance->required_event_types().end();
-             ++iter) {
-          if (voice.event_types.find(*iter) == voice.event_types.end()) {
-            has_all_required_event_types = false;
-            break;
-          }
-        }
-        if (!has_all_required_event_types)
-          continue;
-      }
-
-      *matching_extension = extension;
-      *voice_index = i;
-      return true;
     }
   }
 
   return false;
 }
 
 void ExtensionTtsEngineSpeak(Utterance* utterance,
                              const Extension* extension,
                              size_t voice_index) {
   // See if the engine supports the "end" event; if so, we can keep the
@@ skipping 99 matching lines @@
     std::string error_message;
     event->GetString(constants::kErrorMessageKey, &error_message);
     controller->OnTtsEvent(
         utterance_id, TTS_EVENT_ERROR, char_index, error_message);
   } else {
     EXTENSION_FUNCTION_VALIDATE(false);
   }
 
   return true;
 }