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

components/translate/core/browser/translate_prefs.cc

 // Copyright 2014 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 "components/translate/core/browser/translate_prefs.h"
 
 #include <set>
 
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "build/build_config.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/translate/core/browser/translate_accept_languages.h"
 #include "components/translate/core/browser/translate_download_manager.h"
 #include "components/translate/core/browser/translate_experiment.h"
 #include "components/translate/core/common/translate_util.h"
 
 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";
 const char TranslatePrefs::kPrefTranslateDeniedCount[] =
     "translate_denied_count_for_language";
 const char TranslatePrefs::kPrefTranslateIgnoredCount[] =
     "translate_ignored_count_for_language";
 const char TranslatePrefs::kPrefTranslateAcceptedCount[] =
     "translate_accepted_count";
 const char TranslatePrefs::kPrefTranslateBlockedLanguages[] =
     "translate_blocked_languages";
 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.
 //
 // * translate_last_denied_time
 // * translate_too_often_denied
 // * translate_language_blacklist
 
 namespace {
 
 // Expands language codes to make these more suitable for Accept-Language.
@@ skipping 415 matching lines @@
       user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
   registry->RegisterDictionaryPref(
       kPrefTranslateAcceptedCount,
       user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
   registry->RegisterListPref(kPrefTranslateBlockedLanguages,
                              user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
   registry->RegisterDictionaryPref(kPrefTranslateLastDeniedTimeForLanguage);
   registry->RegisterDictionaryPref(
       kPrefTranslateTooOftenDeniedForLanguage,
       user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
+  registry->RegisterDictionaryPref(
+      kPrefLanguageProfile, user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
 }
 
 // static
 void TranslatePrefs::MigrateUserPrefs(PrefService* user_prefs,
                                       const char* accept_languages_pref) {
   // Old format of kPrefTranslateWhitelists
   // - original language -> list of target langs to auto-translate
   // - list of langs is in order of being enabled i.e. last in list is the
   //   most recent language that user enabled via
   //   Always translate |source_lang| to |target_lang|"
@@ skipping 75 matching lines @@
 bool TranslatePrefs::IsListEmpty(const char* pref_id) const {
   const base::ListValue* blacklist = prefs_->GetList(pref_id);
   return (blacklist == NULL || blacklist->empty());
 }
 
 bool TranslatePrefs::IsDictionaryEmpty(const char* pref_id) const {
   const base::DictionaryValue* dict = prefs_->GetDictionary(pref_id);
   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