Skip to content

Commit

Permalink
Deprecates Gender from the TTS and TTS Engine APIs.
Browse files Browse the repository at this point in the history
Keeps the parameters hidden so that no current implementations will
crash.

Updates documentation to match.

BUG=863999,863998

Change-Id: Iebbdc6b40018909e806702654a23eb18d2087755
Reviewed-on: https://chromium-review.googlesource.com/1176356
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588552}
  • Loading branch information
Katie D authored and Commit Bot committed Sep 4, 2018
1 parent cad8420 commit 3e8c73c
Show file tree
Hide file tree
Showing 23 changed files with 97 additions and 180 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,6 @@ void TtsExtensionEngine::GetVoices(content::BrowserContext* browser_context,
result_voice.lang = voice.lang;
result_voice.remote = voice.remote;
result_voice.extension_id = extension->id();
if (voice.gender == constants::kGenderMale)
result_voice.gender = TTS_GENDER_MALE;
else if (voice.gender == constants::kGenderFemale)
result_voice.gender = TTS_GENDER_FEMALE;
else
result_voice.gender = TTS_GENDER_NONE;

for (std::set<std::string>::const_iterator iter =
voice.event_types.begin();
Expand Down Expand Up @@ -294,8 +288,6 @@ ExtensionTtsEngineUpdateVoicesFunction::Run() {
continue;
}
}
if (voice_data->HasKey(constants::kGenderKey))
voice_data->GetString(constants::kGenderKey, &voice.gender);
if (voice_data->HasKey(constants::kRemoteKey))
voice_data->GetBoolean(constants::kRemoteKey, &voice.remote);
if (voice_data->HasKey(constants::kExtensionIdKey)) {
Expand Down
22 changes: 6 additions & 16 deletions chrome/browser/speech/extension_api/tts_extension_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
#include <utility>

#include "base/lazy_instance.h"
#include "base/metrics/histogram_macros.h"
#include "base/metrics/user_metrics.h"
#include "base/values.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/speech/extension_api/tts_engine_extension_api.h"
Expand Down Expand Up @@ -180,21 +182,14 @@ bool TtsSpeakFunction::RunAsync() {
return false;
}

// TODO(katie): Remove this after M73. This is just used to track how the
// gender deprecation is progressing.
std::string gender_str;
TtsGenderType gender;
if (options->HasKey(constants::kGenderKey))
EXTENSION_FUNCTION_VALIDATE(
options->GetString(constants::kGenderKey, &gender_str));
if (gender_str == constants::kGenderMale) {
gender = TTS_GENDER_MALE;
} else if (gender_str == constants::kGenderFemale) {
gender = TTS_GENDER_FEMALE;
} else if (gender_str.empty()) {
gender = TTS_GENDER_NONE;
} else {
error_ = constants::kErrorInvalidGender;
return false;
}
UMA_HISTOGRAM_BOOLEAN("TextToSpeech.Utterance.HasGender",
!gender_str.empty());

double rate = blink::SpeechSynthesisConstants::kDoublePrefNotSet;
if (options->HasKey(constants::kRateKey)) {
Expand Down Expand Up @@ -280,7 +275,6 @@ bool TtsSpeakFunction::RunAsync() {
utterance->set_src_id(src_id);
utterance->set_src_url(source_url());
utterance->set_lang(lang);
utterance->set_gender(gender);
utterance->set_continuous_parameters(rate, pitch, volume);
utterance->set_can_enqueue(can_enqueue);
utterance->set_required_event_types(required_event_types);
Expand Down Expand Up @@ -327,10 +321,6 @@ ExtensionFunction::ResponseAction TtsGetVoicesFunction::Run() {
result_voice->SetBoolean(constants::kRemoteKey, voice.remote);
if (!voice.lang.empty())
result_voice->SetString(constants::kLangKey, voice.lang);
if (voice.gender == TTS_GENDER_MALE)
result_voice->SetString(constants::kGenderKey, constants::kGenderMale);
else if (voice.gender == TTS_GENDER_FEMALE)
result_voice->SetString(constants::kGenderKey, constants::kGenderFemale);
if (!voice.extension_id.empty())
result_voice->SetString(constants::kExtensionIdKey, voice.extension_id);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ const char kSrcIdKey[] = "srcId";
const char kVoiceNameKey[] = "voiceName";
const char kVolumeKey[] = "volume";

const char kGenderFemale[] = "female";
const char kGenderMale[] = "male";

const char kEventTypeCancelled[] = "cancelled";
const char kEventTypeEnd[] = "end";
const char kEventTypeError[] = "error";
Expand All @@ -40,7 +37,6 @@ const char kEventTypeStart[] = "start";
const char kEventTypeWord[] = "word";

const char kErrorExtensionIdMismatch[] = "Extension id mismatch.";
const char kErrorInvalidGender[] = "Invalid gender.";
const char kErrorInvalidLang[] = "Invalid lang.";
const char kErrorInvalidPitch[] = "Invalid pitch.";
const char kErrorInvalidRate[] = "Invalid rate.";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ extern const char kSrcIdKey[];
extern const char kVoiceNameKey[];
extern const char kVolumeKey[];

extern const char kGenderFemale[];
extern const char kGenderMale[];

extern const char kEventTypeCancelled[];
extern const char kEventTypeEnd[];
extern const char kEventTypeError[];
Expand All @@ -45,7 +42,6 @@ extern const char kEventTypeStart[];
extern const char kEventTypeWord[];

extern const char kErrorExtensionIdMismatch[];
extern const char kErrorInvalidGender[];
extern const char kErrorInvalidLang[];
extern const char kErrorInvalidPitch[];
extern const char kErrorInvalidRate[];
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/speech/tts_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ void TtsPlatformImplAndroid::GetVoices(
Java_TtsPlatformImpl_getVoiceName(env, java_ref_, i));
data.lang = base::android::ConvertJavaStringToUTF8(
Java_TtsPlatformImpl_getVoiceLanguage(env, java_ref_, i));
data.gender = TTS_GENDER_NONE;
data.events.insert(TTS_EVENT_START);
data.events.insert(TTS_EVENT_END);
data.events.insert(TTS_EVENT_ERROR);
Expand Down
13 changes: 0 additions & 13 deletions chrome/browser/speech/tts_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,6 @@ enum TtsEventType {
TTS_EVENT_RESUME
};

enum TtsGenderType {
TTS_GENDER_NONE,
TTS_GENDER_MALE,
TTS_GENDER_FEMALE
};

// Returns true if this event type is one that indicates an utterance
// is finished and can be destroyed.
bool IsFinalTtsEventType(TtsEventType event_type);
Expand All @@ -67,7 +61,6 @@ struct VoiceData {

std::string name;
std::string lang;
TtsGenderType gender;
std::string extension_id;
std::set<TtsEventType> events;

Expand Down Expand Up @@ -168,11 +161,6 @@ class Utterance {
}
const std::string& lang() const { return lang_; }

void set_gender(TtsGenderType gender) {
gender_ = gender;
}
TtsGenderType gender() const { return gender_; }

void set_continuous_parameters(const double rate,
const double pitch,
const double volume) {
Expand Down Expand Up @@ -254,7 +242,6 @@ class Utterance {
// The parsed options.
std::string voice_name_;
std::string lang_;
TtsGenderType gender_;
UtteranceContinuousParameters continuous_parameters_;
bool can_enqueue_;
std::set<TtsEventType> required_event_types_;
Expand Down
25 changes: 5 additions & 20 deletions chrome/browser/speech/tts_controller_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,7 @@ UtteranceContinuousParameters::UtteranceContinuousParameters()
// VoiceData
//


VoiceData::VoiceData()
: gender(TTS_GENDER_NONE),
remote(false),
native(false) {}
VoiceData::VoiceData() : remote(false), native(false) {}

VoiceData::VoiceData(const VoiceData& other) = default;

Expand All @@ -109,7 +105,6 @@ Utterance::Utterance(content::BrowserContext* browser_context)
: browser_context_(browser_context),
id_(next_utterance_id_++),
src_id_(-1),
gender_(TTS_GENDER_NONE),
can_enqueue_(false),
char_index_(0),
finished_(false) {
Expand Down Expand Up @@ -223,8 +218,6 @@ void TtsControllerImpl::SpeakNow(Utterance* utterance) {
!utterance->voice_name().empty());
UMA_HISTOGRAM_BOOLEAN("TextToSpeech.Utterance.HasLang",
!utterance->lang().empty());
UMA_HISTOGRAM_BOOLEAN("TextToSpeech.Utterance.HasGender",
utterance->gender() != TTS_GENDER_NONE);
UMA_HISTOGRAM_BOOLEAN("TextToSpeech.Utterance.HasRate",
utterance->continuous_parameters().rate != 1.0);
UMA_HISTOGRAM_BOOLEAN("TextToSpeech.Utterance.HasPitch",
Expand Down Expand Up @@ -470,7 +463,6 @@ int TtsControllerImpl::GetMatchingVoice(
// Utterange language (exact region preferred, then general language code)
// App/system language (exact region preferred, then general language code)
// Required event types
// Gender
// User-selected preference of voice given the general language code.

// TODO(gaochun): Replace the global variable g_browser_process with
Expand Down Expand Up @@ -509,18 +501,18 @@ int TtsControllerImpl::GetMatchingVoice(
if (!voice.lang.empty() && !utterance->lang().empty()) {
// An exact language match is worth more than a partial match.
if (voice.lang == utterance->lang()) {
score += 128;
score += 64;
} else if (l10n_util::GetLanguage(voice.lang) ==
l10n_util::GetLanguage(utterance->lang())) {
score += 64;
score += 32;
}
}

// Prefer the system language after that.
if (!voice.lang.empty()) {
if (l10n_util::GetLanguage(voice.lang) ==
l10n_util::GetLanguage(app_lang))
score += 32;
score += 16;
}

// Next, prefer required event types.
Expand All @@ -536,14 +528,7 @@ int TtsControllerImpl::GetMatchingVoice(
}
}
if (has_all_required_event_types)
score += 16;
}

// Prefer the requested gender.
if (voice.gender != TTS_GENDER_NONE &&
utterance->gender() != TTS_GENDER_NONE &&
voice.gender == utterance->gender()) {
score += 8;
score += 8;
}

#if defined(OS_CHROMEOS)
Expand Down
62 changes: 28 additions & 34 deletions chrome/browser/speech/tts_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ TEST_F(TtsControllerTest, TestGetMatchingVoice) {
std::unique_ptr<base::DictionaryValue> lang_to_voices =
std::make_unique<base::DictionaryValue>();
lang_to_voices->SetKey(
"es", base::Value("{\"name\":\"Voice8\",\"extension\":\"id8\"}"));
"es", base::Value("{\"name\":\"Voice7\",\"extension\":\"id7\"}"));
lang_to_voices->SetKey(
"noLanguage", base::Value("{\"name\":\"Android\",\"extension\":\"\"}"));
pref_service_.registry()->RegisterDictionaryPref(
Expand Down Expand Up @@ -132,82 +132,76 @@ TEST_F(TtsControllerTest, TestGetMatchingVoice) {
VoiceData voice0;
voices.push_back(voice0);
VoiceData voice1;
voice1.gender = TTS_GENDER_FEMALE;
voice1.events.insert(TTS_EVENT_WORD);
voices.push_back(voice1);
VoiceData voice2;
voice2.events.insert(TTS_EVENT_WORD);
voice2.lang = "de-DE";
voices.push_back(voice2);
VoiceData voice3;
voice3.lang = "de-DE";
voice3.lang = "fr-CA";
voices.push_back(voice3);
VoiceData voice4;
voice4.lang = "fr-CA";
voice4.name = "Voice4";
voices.push_back(voice4);
VoiceData voice5;
voice5.name = "Voice5";
voice5.extension_id = "id5";
voices.push_back(voice5);
VoiceData voice6;
voice6.extension_id = "id6";
voice6.extension_id = "id7";
voice6.name = "Voice6";
voice6.lang = "es-es";
voices.push_back(voice6);
VoiceData voice7;
voice7.extension_id = "id7";
voice7.name = "Voice7";
voice7.lang = "es-es";
voice7.lang = "es-mx";
voices.push_back(voice7);
VoiceData voice8;
voice8.extension_id = "id8";
voice8.name = "Voice8";
voice8.lang = "es-mx";
voice8.extension_id = "";
voice8.name = "Android";
voice8.lang = "";
voice8.native = true;
voices.push_back(voice8);
VoiceData voice9;
voice9.extension_id = "";
voice9.name = "Android";
voice9.lang = "";
voice9.native = true;
voices.push_back(voice9);

Utterance utterance(nullptr);
EXPECT_EQ(0, tts_controller->GetMatchingVoice(&utterance, voices));

utterance.set_gender(TTS_GENDER_FEMALE);
EXPECT_EQ(1, tts_controller->GetMatchingVoice(&utterance, voices));

std::set<TtsEventType> types;
types.insert(TTS_EVENT_WORD);
utterance.set_required_event_types(types);
EXPECT_EQ(2, tts_controller->GetMatchingVoice(&utterance, voices));
EXPECT_EQ(1, tts_controller->GetMatchingVoice(&utterance, voices));

utterance.set_lang("de-DE");
EXPECT_EQ(3, tts_controller->GetMatchingVoice(&utterance, voices));
EXPECT_EQ(2, tts_controller->GetMatchingVoice(&utterance, voices));

utterance.set_lang("fr-FR");
EXPECT_EQ(4, tts_controller->GetMatchingVoice(&utterance, voices));
EXPECT_EQ(3, tts_controller->GetMatchingVoice(&utterance, voices));

utterance.set_voice_name("Voice5");
EXPECT_EQ(5, tts_controller->GetMatchingVoice(&utterance, voices));
utterance.set_voice_name("Voice4");
EXPECT_EQ(4, tts_controller->GetMatchingVoice(&utterance, voices));

utterance.set_voice_name("");
utterance.set_extension_id("id6");
EXPECT_EQ(6, tts_controller->GetMatchingVoice(&utterance, voices));
utterance.set_extension_id("id5");
EXPECT_EQ(5, tts_controller->GetMatchingVoice(&utterance, voices));

#if defined(OS_CHROMEOS)
// Voice7 is matched when the utterance locale exactly matches its locale.
// Voice6 is matched when the utterance locale exactly matches its locale.
utterance.set_extension_id("");
utterance.set_lang("es-es");
EXPECT_EQ(7, tts_controller->GetMatchingVoice(&utterance, voices));
EXPECT_EQ(6, tts_controller->GetMatchingVoice(&utterance, voices));

// The 8th voice is the default for "es", even though the utterance is
// "es-ar". |voice7| is not matched because it is not the default.
// The 7th voice is the default for "es", even though the utterance is
// "es-ar". |voice6| is not matched because it is not the default.
utterance.set_extension_id("");
utterance.set_lang("es-ar");
EXPECT_EQ(8, tts_controller->GetMatchingVoice(&utterance, voices));
EXPECT_EQ(7, tts_controller->GetMatchingVoice(&utterance, voices));

// The 9th voice is like the built-in "Android" voice, it has no lang
// The 8th voice is like the built-in "Android" voice, it has no lang
// and no extension ID. Make sure it can still be matched.
utterance.set_voice_name("Android");
utterance.set_extension_id("");
utterance.set_lang("");
EXPECT_EQ(9, tts_controller->GetMatchingVoice(&utterance, voices));
EXPECT_EQ(8, tts_controller->GetMatchingVoice(&utterance, voices));
#endif // defined(OS_CHROMEOS)
}
}
Expand Down
7 changes: 0 additions & 7 deletions chrome/browser/speech/tts_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,6 @@ void OnSpeechEvent(NSSpeechSynthesizer* sender,
NSDictionary* attributes =
[NSSpeechSynthesizer attributesForVoice:voiceIdentifier];
NSString* name = [attributes objectForKey:NSVoiceName];
NSString* gender = [attributes objectForKey:NSVoiceGender];
NSString* localeIdentifier =
[attributes objectForKey:NSVoiceLocaleIdentifier];

Expand All @@ -242,12 +241,6 @@ void OnSpeechEvent(NSSpeechSynthesizer* sender,
} else {
data.lang = base::SysNSStringToUTF8(language);
}
if ([gender isEqualToString:NSVoiceGenderMale])
data.gender = TTS_GENDER_MALE;
else if ([gender isEqualToString:NSVoiceGenderFemale])
data.gender = TTS_GENDER_FEMALE;
else
data.gender = TTS_GENDER_NONE;
data.events.insert(TTS_EVENT_START);
data.events.insert(TTS_EVENT_END);
data.events.insert(TTS_EVENT_WORD);
Expand Down
9 changes: 0 additions & 9 deletions chrome/browser/speech/tts_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ namespace {

// ISpObjectToken key and value names.
const wchar_t kAttributesKey[] = L"Attributes";
const wchar_t kGenderValue[] = L"Gender";
const wchar_t kLanguageValue[] = L"Language";

} // anonymous namespace.
Expand Down Expand Up @@ -214,14 +213,6 @@ void TtsPlatformImplWin::GetVoices(
if (S_OK != voice_token->OpenKey(kAttributesKey, attributes.GetAddressOf()))
continue;

base::win::ScopedCoMem<WCHAR> gender;
if (S_OK == attributes->GetStringValue(kGenderValue, &gender)) {
if (0 == _wcsicmp(gender.get(), L"male"))
voice.gender = TTS_GENDER_MALE;
else if (0 == _wcsicmp(gender.get(), L"female"))
voice.gender = TTS_GENDER_FEMALE;
}

base::win::ScopedCoMem<WCHAR> language;
if (S_OK == attributes->GetStringValue(kLanguageValue, &language)) {
int lcid_value;
Expand Down
Loading

0 comments on commit 3e8c73c

Please sign in to comment.