using ulp to improve TranslateManager GetTargetLanguage() and InitiateTranslation()

components/translate/core/browser/translate_prefs.cc

Index: components/translate/core/browser/translate_prefs.cc
diff --git a/components/translate/core/browser/translate_prefs.cc b/components/translate/core/browser/translate_prefs.cc
index 98419c0c91474d127ccef774ead2cf8f0f123a91..eeee4e3bf2c49adafd5f77b147b501c733e4a245 100644
--- a/components/translate/core/browser/translate_prefs.cc
+++ b/components/translate/core/browser/translate_prefs.cc
@@ -20,6 +20,7 @@
 
 namespace translate {
 
+const char TranslatePrefs::kPrefLanguageProfile[] = "language_profile";

[groby-ooo-7-16, 2016/08/02 00:38:21]

If ULP is translate specific, it should be translate_language_profile. If it's
not, there should probably be a follow-up bug to separate it out into its own
component.

 const char TranslatePrefs::kPrefTranslateSiteBlacklist[] =
     "translate_site_blacklist";
 const char TranslatePrefs::kPrefTranslateWhitelists[] = "translate_whitelists";
@@ -35,10 +36,20 @@ const char TranslatePrefs::kPrefTranslateLastDeniedTimeForLanguage[] =
     "translate_last_denied_time_for_language";
 const char TranslatePrefs::kPrefTranslateTooOftenDeniedForLanguage[] =
     "translate_too_often_denied_for_language";
+
 const char kTranslateUI2016Q2TrialName[] = "TranslateUI2016Q2";
 const char kAlwaysTranslateOfferThreshold[] =
     "always_translate_offer_threshold";
 
+// For reading ULP prefs.
+const char kConfidence[] = "confidence";
+const char kItems[] = "items";
+const char kLanguage[] = "language";
+const char kProbability[] = "probability";
+const char kReading[] = "reading";
+const char kSettings[] = "settings";
+const char kWriting[] = "writing";
+
 // The below properties used to be used but now are deprecated. Don't use them
 // since an old profile might have some values there.
 //
@@ -474,6 +485,8 @@ void TranslatePrefs::RegisterProfilePrefs(
   registry->RegisterDictionaryPref(
       kPrefTranslateTooOftenDeniedForLanguage,
       user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
+  registry->RegisterDictionaryPref(
+      kPrefLanguageProfile, user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
 }
 
 // static
@@ -569,4 +582,91 @@ bool TranslatePrefs::IsDictionaryEmpty(const char* pref_id) const {
   return (dict == NULL || dict->empty());
 }
 
+// Functions to read from User Language Profile

[groby-ooo-7-16, 2016/08/02 00:38:21]

Probably might want to document that via a test instead.

[ftang, 2016/08/03 02:01:18]

Acknowledged.

+// The structure of User Language Profile looks like below:
+// language_profile {
+//   reading: {
+//     confidence: 0.7,
+//     items: [
+//       {
+//         language: 'en',
+//         probability: 0.45
+//       }, {
+//         language: 'zh-TW',
+//         probability: 0.33
+//       }, {
+//         language: 'pt-BR',
+//         probability: 0.22
+//       }
+//     ]
+//   },
+//   writing: {
+//     confidence: 0.35,
+//     items: [
+//       {
+//         language: 'en',
+//         probability: 0.75
+//       }, {
+//         language: 'pt-BR',
+//         probability: 0.25
+//       }
+//     ]
+//   },
+//   settings: ["pt-BR", "en", "zh-TW" ]
+// }
+double TranslatePrefs::GetFromUserLanguageProfile(
+    base::StringPiece path,
+    LanguageProbabilityList* out_value) const {

[groby-ooo-7-16, 2016/08/02 00:38:21]

That's a lot of ad-hoc work - a JSONValueConverter might be a better idea.

If you want to keep manual parsing, a bunch of comments below - otherwise, you
can ignore those.

+  const base::DictionaryValue* dict =
+      prefs_->GetDictionary(kPrefLanguageProfile);
+  const base::DictionaryValue* entries = nullptr;
+  if (dict == nullptr || dict->empty() ||

[groby-ooo-7-16, 2016/08/02 00:38:21]

1) No need to check dict->empty, because GetDictionary will then return a
nullptr

2) Please don't do a bunch of work in if statements. This would probably cleaner
as

    if (!dict)
      return 0.0;
    if (!dict->GetDictionary(path, &entries))
      return 0.0;

[ftang, 2016/08/03 02:01:18]

Done.

+      !dict->GetDictionary(path, &entries) || entries == nullptr ||
+      entries->empty())
+    return 0.0;
+  double confidence = 0.0;
+  const base::ListValue* list = nullptr;
+  if (!entries->GetDouble(kConfidence, &confidence) ||
+      !entries->GetList(kItems, &list) || list == nullptr || list->empty())

[groby-ooo-7-16, 2016/08/02 00:38:21]

Same here - if GetList is true, list is not null.

[ftang, 2016/08/03 02:01:18]

Done.

+    return confidence;
+  std::string language;
+  double probability;
+  for (size_t i = 0; i < list->GetSize(); i++) {

[groby-ooo-7-16, 2016/08/02 00:38:21]

ListValue is iterable, so C++11 iteration works
  for (const auto& entry : list) 

+    const base::DictionaryValue* item = nullptr;
+    if (list->GetDictionary(i, &item) && item != nullptr && !item->empty() &&

[groby-ooo-7-16, 2016/08/02 00:38:21]

See above :)

[ftang, 2016/08/03 02:01:18]

Done.

+        item->GetString(kLanguage, &language) &&
+        item->GetDouble(kProbability, &probability)) {
+      out_value->push_back(std::make_pair(language, probability));
+    }
+  }
+  return confidence;
+}
+
+double TranslatePrefs::GetReadingFromUserLanguageProfile(
+    LanguageProbabilityList* list) const {
+  return GetFromUserLanguageProfile(kReading, list);
+}
+
+double TranslatePrefs::GetWritingFromUserLanguageProfile(
+    LanguageProbabilityList* list) const {
+  return GetFromUserLanguageProfile(kWriting, list);
+}
+
+int TranslatePrefs::GetUniversialLanguageSettings(

[groby-ooo-7-16, 2016/08/02 00:38:21]

I didn't see any consumer of this function yet?

[ftang, 2016/08/03 02:01:18]

you are right. Removed. We should keep the ULP prefs in the prefs but no need to
get the information out for TranslatePrefs.

+    std::vector<std::string>* out_value) const {

[groby-ooo-7-16, 2016/08/02 00:38:21]

Why use an outvalue? Just return std::vector<std::string> (There is no cost,
since it is eligible for NRVO)

[ftang, 2016/08/03 02:01:18]

since I remove this function. It does not matter any more.

+  const base::DictionaryValue* dict =
+      prefs_->GetDictionary(kPrefLanguageProfile);
+  const base::ListValue* list = nullptr;
+  if (dict == nullptr || dict->empty() || !dict->GetList(kSettings, &list) ||

[groby-ooo-7-16, 2016/08/02 00:38:21]

See above.

[ftang, 2016/08/03 02:01:18]

Acknowledged.

+      list == nullptr || list->empty())
+    return 0;
+  std::string language;
+  for (size_t i = 0; i < list->GetSize(); i++) {
+    if (list->GetString(i, &language)) {
+      out_value->push_back(language);
+    }
+  }
+  return out_value->size();
+}
+
 }  // namespace translate