Fix TTS extension API language matching.

chrome/browser/speech/extension_api/tts_engine_extension_api.cc

Index: chrome/browser/speech/extension_api/tts_engine_extension_api.cc
diff --git a/chrome/browser/speech/extension_api/tts_engine_extension_api.cc b/chrome/browser/speech/extension_api/tts_engine_extension_api.cc
index 62a1c6eb9673505ae3817b7dd665c1364d2ed1ce..711880e877b94adef62980c433fe28ad3a0ec1ad 100644
--- a/chrome/browser/speech/extension_api/tts_engine_extension_api.cc
+++ b/chrome/browser/speech/extension_api/tts_engine_extension_api.cc
@@ -24,6 +24,17 @@ 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);
@@ -97,58 +108,69 @@ bool GetMatchingExtensionVoice(
   *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;
-    }
-
-    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()) {
+  // 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 (!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()) {
-        continue;
-      }
-      if (!voice.gender.empty() &&
-          !utterance->gender().empty() &&
-          voice.gender != utterance->gender()) {
+
+      if (!utterance->extension_id().empty() &&
+          utterance->extension_id() != extension->id()) {
         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;
+
+      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 (!has_all_required_event_types)
+        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;
+        *matching_extension = extension;
+        *voice_index = i;
+        return true;
+      }
     }
   }