Translate: Refactoring: TranslateLanguageList's URL fetchers

Reduce TranslateLanguageList's URL fetchers into only one URL fetcher to fetch the language list including alpha languages.

TEST=manual (seeing chrome://translate-internals/)

Review URL: https://chromiumcodereview.appspot.com/18669002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@211362 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 139 insertions, 143 deletions

diff --git a/chrome/browser/translate/translate_language_list.cc b/chrome/browser/translate/translate_language_list.cc
index 88f0701..d5b13c0 100644
--- a/chrome/browser/translate/translate_language_list.cc
+++ b/chrome/browser/translate/translate_language_list.cc
@@ -103,10 +103,7 @@ const char kLanguageListFetchURL[] =
 const char kAlphaLanguageQueryName[] = "alpha";
 const char kAlphaLanguageQueryValue[] = "1";
 
-// Assign following IDs to URLFetchers so that tests can distinguish each
-// request in order to simiulate respectively.
-const int kFetcherIdForLanguageList = 1;
-const int kFetcherIdForAlphaLanguageList = 2;
+const int kFetcherId = 1;
 
 // Represent if the language list updater is disabled.
 bool update_is_disabled = false;
@@ -126,12 +123,19 @@ void NotifyEvent(int line, const std::string& message) {
 // Parses |language_list| containing the list of languages that the translate
 // server can translate to and from, and fills |set| with them.
 void SetSupportedLanguages(const std::string& language_list,
-                           std::set<std::string>* set) {
-  DCHECK(set);
+                           std::set<std::string>* target_language_set,
+                           std::set<std::string>* alpha_language_set) {
+  DCHECK(target_language_set);
+  DCHECK(alpha_language_set);
+
   // The format is:
-  // sl({"sl": {"XX": "LanguageName", ...}, "tl": {"XX": "LanguageName", ...}})
-  // Where "sl(" is set in kLanguageListCallbackName
-  // and "tl" is kTargetLanguagesKey
+  // sl({
+  //   "sl": {"XX": "LanguageName", ...},
+  //   "tl": {"XX": "LanguageName", ...},
+  //   "al": {"XX": 1, ...}
+  // })
+  // Where "sl(" is set in kLanguageListCallbackName, "tl" is
+  // kTargetLanguagesKey and "al" kAlphaLanguagesKey.
   if (!StartsWithASCII(language_list,
                        TranslateLanguageList::kLanguageListCallbackName,
                        false) ||
@@ -152,8 +156,9 @@ void SetSupportedLanguages(const std::string& language_list,
     NOTREACHED();
     return;
   }
-  // The first level dictionary contains two sub-dict, one for source languages
-  // and the other for target languages, we want to use the target languages.
+  // The first level dictionary contains three sub-dict, first for source
+  // languages and second for target languages, we want to use the target
+  // languages. The last is for alpha languages.
   DictionaryValue* language_dict =
       static_cast<DictionaryValue*>(json_value.get());
   DictionaryValue* target_languages = NULL;
@@ -167,7 +172,7 @@ void SetSupportedLanguages(const std::string& language_list,
   const std::string& locale = g_browser_process->GetApplicationLocale();
 
   // Now we can clear language list.
-  set->clear();
+  target_language_set->clear();
   std::string message;
   // ... and replace it with the values we just fetched from the server.
   for (DictionaryValue::Iterator iter(*target_languages);
@@ -178,28 +183,32 @@ void SetSupportedLanguages(const std::string& language_list,
       TranslateBrowserMetrics::ReportUndisplayableLanguage(lang);
       continue;
     }
-    set->insert(lang);
+    target_language_set->insert(lang);
     if (message.empty())
       message += lang;
     else
       message += ", " + lang;
   }
   NotifyEvent(__LINE__, message);
-}
-
-// Returns URL to fetch the language list.
-GURL GetLanguageListFetchURL(bool include_alpha_languages) {
-  GURL url = GURL(kLanguageListFetchURL);
-  url = TranslateURLUtil::AddHostLocaleToUrl(url);
-  url = TranslateURLUtil::AddApiKeyToUrl(url);
 
-  if (include_alpha_languages) {
-    url = net::AppendQueryParameter(url,
-                                    kAlphaLanguageQueryName,
-                                    kAlphaLanguageQueryValue);
+  // Get the alpha languages. The "al" parameter could be abandoned.
+  DictionaryValue* alpha_languages = NULL;
+  if (!language_dict->GetDictionary(TranslateLanguageList::kAlphaLanguagesKey,
+                                    &alpha_languages) ||
+      alpha_languages == NULL) {
+    return;
   }
 
-  return url;
+  // We assume that the alpha languages are included in the above target
+  // languages, and don't use UMA or NotifyEvent.
+  alpha_language_set->clear();
+  for (DictionaryValue::Iterator iter(*alpha_languages);
+       !iter.IsAtEnd(); iter.Advance()) {
+    const std::string& lang = iter.key();
+    if (!l10n_util::IsLocaleNameTranslated(lang.c_str(), locale))
+      continue;
+    alpha_language_set->insert(lang);
+  }
 }
 
 }  // namespace
@@ -207,25 +216,20 @@ GURL GetLanguageListFetchURL(bool include_alpha_languages) {
 // This must be kept in sync with the &cb= value in the kLanguageListFetchURL.
 const char TranslateLanguageList::kLanguageListCallbackName[] = "sl(";
 const char TranslateLanguageList::kTargetLanguagesKey[] = "tl";
+const char TranslateLanguageList::kAlphaLanguagesKey[] = "al";
 
 TranslateLanguageList::TranslateLanguageList() {
   // We default to our hard coded list of languages in
   // |kDefaultSupportedLanguages|. This list will be overriden by a server
   // providing supported langauges list.
   for (size_t i = 0; i < arraysize(kDefaultSupportedLanguages); ++i)
-    supported_languages_.insert(kDefaultSupportedLanguages[i]);
-  UpdateSupportedLanguages();
+    all_supported_languages_.insert(kDefaultSupportedLanguages[i]);
 
   if (update_is_disabled)
     return;
 
-  language_list_fetcher_.reset(
-      new TranslateURLFetcher(kFetcherIdForLanguageList));
+  language_list_fetcher_.reset(new TranslateURLFetcher(kFetcherId));
   language_list_fetcher_->set_max_retry_on_5xx(kMaxRetryOn5xx);
-
-  alpha_language_list_fetcher_.reset(
-      new TranslateURLFetcher(kFetcherIdForAlphaLanguageList));
-  alpha_language_list_fetcher_->set_max_retry_on_5xx(kMaxRetryOn5xx);
 }
 
 TranslateLanguageList::~TranslateLanguageList() {
@@ -240,7 +244,7 @@ void TranslateLanguageList::GetSupportedLanguages(
 
   // Update language lists if they are not updated after Chrome was launched
   // for later requests.
-  if (language_list_fetcher_.get() || alpha_language_list_fetcher_.get())
+  if (language_list_fetcher_.get())
     RequestLanguageList();
 }
 
@@ -262,9 +266,7 @@ bool TranslateLanguageList::IsSupportedLanguage(const std::string& language) {
 }
 
 bool TranslateLanguageList::IsAlphaLanguage(const std::string& language) {
-  // |language| should exist only in the alpha language list.
-  return supported_alpha_languages_.count(language) != 0 &&
-         supported_languages_.count(language) == 0;
+  return alpha_languages_.count(language) != 0;
 }
 
 void TranslateLanguageList::RequestLanguageList() {
@@ -277,10 +279,15 @@ void TranslateLanguageList::OnResourceRequestsAllowed() {
   if (language_list_fetcher_.get() &&
       (language_list_fetcher_->state() == TranslateURLFetcher::IDLE ||
        language_list_fetcher_->state() == TranslateURLFetcher::FAILED)) {
-    GURL url = GetLanguageListFetchURL(false);
+    GURL url = GURL(kLanguageListFetchURL);
+    url = TranslateURLUtil::AddHostLocaleToUrl(url);
+    url = TranslateURLUtil::AddApiKeyToUrl(url);
+    url = net::AppendQueryParameter(url,
+                                    kAlphaLanguageQueryName,
+                                    kAlphaLanguageQueryValue);
 
     std::string message = base::StringPrintf(
-        "Language list fetch starts (URL: %s)",
+        "Language list includeing alpha languages fetch starts (URL: %s)",
         url.spec().c_str());
     NotifyEvent(__LINE__, message);
 
@@ -291,24 +298,6 @@ void TranslateLanguageList::OnResourceRequestsAllowed() {
     if (!result)
       NotifyEvent(__LINE__, "Request is omitted due to retry limit");
   }
-
-  if (alpha_language_list_fetcher_.get() &&
-      (alpha_language_list_fetcher_->state() == TranslateURLFetcher::IDLE ||
-       alpha_language_list_fetcher_->state() == TranslateURLFetcher::FAILED)) {
-    GURL url = GetLanguageListFetchURL(true);
-
-    std::string message = base::StringPrintf(
-        "Alpha language list fetch starts (URL: %s)",
-        url.spec().c_str());
-    NotifyEvent(__LINE__, message);
-
-    bool result = alpha_language_list_fetcher_->Request(
-        url,
-        base::Bind(&TranslateLanguageList::OnLanguageListFetchComplete,
-                   base::Unretained(this)));
-    if (!result)
-      NotifyEvent(__LINE__, "Request is omitted due to retry limit");
-  }
 }
 
 // static
@@ -327,47 +316,16 @@ void TranslateLanguageList::OnLanguageListFetchComplete(
     // The TranslateURLFetcher has a limit for retried requests and aborts
     // re-try not to invoke OnLanguageListFetchComplete anymore if it's asked to
     // re-try too many times.
-    GURL url = GetLanguageListFetchURL(id == kFetcherIdForAlphaLanguageList);
-    std::string message = base::StringPrintf(
-        "Failed to Fetch languages from: %s", url.spec().c_str());
-    NotifyEvent(__LINE__, message);
+    NotifyEvent(__LINE__, "Failed to fetch languages");
     return;
   }
 
-  std::string message = base::StringPrintf(
-      "%s list is updated",
-      id == kFetcherIdForLanguageList ? "Language" : "Alpha language");
-  NotifyEvent(__LINE__, message);
+  NotifyEvent(__LINE__, "Language list is updated");
 
-  switch (id) {
-    case kFetcherIdForLanguageList:
-      SetSupportedLanguages(data, &supported_languages_);
-      language_list_fetcher_.reset();
-      break;
-    case kFetcherIdForAlphaLanguageList:
-      SetSupportedLanguages(data, &supported_alpha_languages_);
-      alpha_language_list_fetcher_.reset();
-      break;
-    default:
-      NOTREACHED();
-      break;
-  }
-  UpdateSupportedLanguages();
+  DCHECK_EQ(kFetcherId, id);
 
-  last_updated_ = base::Time::Now();
-}
+  SetSupportedLanguages(data, &all_supported_languages_, &alpha_languages_);
+  language_list_fetcher_.reset();
 
-void TranslateLanguageList::UpdateSupportedLanguages() {
-  all_supported_languages_.clear();
-  std::set<std::string>::const_iterator iter;
-  for (iter = supported_languages_.begin();
-       iter != supported_languages_.end();
-       ++iter) {
-    all_supported_languages_.insert(*iter);
-  }
-  for (iter = supported_alpha_languages_.begin();
-       iter != supported_alpha_languages_.end();
-       ++iter) {
-    all_supported_languages_.insert(*iter);
-  }
+  last_updated_ = base::Time::Now();
 }
diff --git a/chrome/browser/translate/translate_language_list.h b/chrome/browser/translate/translate_language_list.h
index a443817..5e818c0 100644
--- a/chrome/browser/translate/translate_language_list.h
+++ b/chrome/browser/translate/translate_language_list.h
@@ -57,6 +57,7 @@ class TranslateLanguageList : public ResourceRequestAllowedNotifier::Observer {
   // static const values shared with our browser tests.
   static const char kLanguageListCallbackName[];
   static const char kTargetLanguagesKey[];
+  static const char kAlphaLanguagesKey[];
 
  private:
   // Callback function called when TranslateURLFetcher::Request() is finished.
@@ -64,28 +65,16 @@ class TranslateLanguageList : public ResourceRequestAllowedNotifier::Observer {
                                    bool success,
                                    const std::string& data);
 
-  // Updates |all_supported_languages_| to contain the union of
-  // |supported_languages_| and |supported_alpha_languages_|.
-  void UpdateSupportedLanguages();
-
-  // The languages supported by the translation server.
-  std::set<std::string> supported_languages_;
-
-  // The alpha languages supported by the translation server.
-  std::set<std::string> supported_alpha_languages_;
-
-  // All the languages supported by the translation server. It contains the
-  // union of |supported_languages_| and |supported_alpha_languages_|.
+  // All the languages supported by the translation server.
   std::set<std::string> all_supported_languages_;
 
+  // Alpha languages supported by the translation server.
+  std::set<std::string> alpha_languages_;
+
   // A LanguageListFetcher instance to fetch a server providing supported
-  // language list.
+  // language list including alpha languages.
   scoped_ptr<TranslateURLFetcher> language_list_fetcher_;
 
-  // A LanguageListFetcher instance to fetch a server providing supported alpha
-  // language list.
-  scoped_ptr<TranslateURLFetcher> alpha_language_list_fetcher_;
-
   // The last-updated time when the language list is sent.
   base::Time last_updated_;
 
diff --git a/chrome/browser/translate/translate_manager_browsertest.cc b/chrome/browser/translate/translate_manager_browsertest.cc
index 5fd101a..ee3d7ff 100644
--- a/chrome/browser/translate/translate_manager_browsertest.cc
+++ b/chrome/browser/translate/translate_manager_browsertest.cc
@@ -256,6 +256,7 @@ class TranslateManagerBrowserTest : public ChromeRenderViewHostTestHarness,
   }
 
   void SimulateTranslateScriptURLFetch(bool success) {
+    // TODO(hajimehoshi): 0 is a magic number.
     net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0);
     ASSERT_TRUE(fetcher);
     net::URLRequestStatus status;
@@ -268,7 +269,10 @@ class TranslateManagerBrowserTest : public ChromeRenderViewHostTestHarness,
   }
 
   void SimulateSupportedLanguagesURLFetch(
-      bool success, const std::vector<std::string>& languages) {
+      bool success,
+      const std::vector<std::string>& languages,
+      bool use_alpha_languages,
+      const std::vector<std::string>& alpha_languages) {
     net::URLRequestStatus status;
     status.set_status(success ? net::URLRequestStatus::SUCCESS :
                                 net::URLRequestStatus::FAILED);
@@ -286,17 +290,29 @@ class TranslateManagerBrowserTest : public ChromeRenderViewHostTestHarness,
         if (i == 0)
           comma = ",";
       }
+
+      if (use_alpha_languages) {
+        data += base::StringPrintf("},\"%s\": {",
+                                   TranslateLanguageList::kAlphaLanguagesKey);
+        comma = "";
+        for (size_t i = 0; i < alpha_languages.size(); ++i) {
+          data += base::StringPrintf("%s\"%s\": 1", comma,
+                                     alpha_languages[i].c_str());
+          if (i == 0)
+            comma = ",";
+        }
+      }
+
       data += "}})";
     }
-    for (int id = 1; id <= 2; ++id) {
-      net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(id);
-      ASSERT_TRUE(fetcher);
-      fetcher->set_url(fetcher->GetOriginalURL());
-      fetcher->set_status(status);
-      fetcher->set_response_code(success ? 200 : 500);
-      fetcher->SetResponseString(data);
-      fetcher->delegate()->OnURLFetchComplete(fetcher);
-    }
+    // TODO(hajimehoshi): 1 is a magic number.
+    net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(1);
+    ASSERT_TRUE(fetcher != NULL);
+    fetcher->set_url(fetcher->GetOriginalURL());
+    fetcher->set_status(status);
+    fetcher->set_response_code(success ? 200 : 500);
+    fetcher->SetResponseString(data);
+    fetcher->delegate()->OnURLFetchComplete(fetcher);
   }
 
   void SetPrefObserverExpectation(const char* path) {
@@ -627,6 +643,10 @@ TEST_F(TranslateManagerBrowserTest, FetchLanguagesFromTranslateServer) {
   server_languages.push_back("fr-FR");
   server_languages.push_back("xx");
 
+  std::vector<std::string> alpha_languages;
+  alpha_languages.push_back("aa");
+  alpha_languages.push_back("yi");
+
   // First, get the default languages list. Note that calling
   // GetSupportedLanguages() invokes RequestLanguageList() internally.
   std::vector<std::string> default_supported_languages;
@@ -640,47 +660,76 @@ TEST_F(TranslateManagerBrowserTest, FetchLanguagesFromTranslateServer) {
   EXPECT_EQ(default_supported_languages, current_supported_languages);
 
   // Also check that it didn't change if we failed the URL fetch.
-  SimulateSupportedLanguagesURLFetch(false, std::vector<std::string>());
+  SimulateSupportedLanguagesURLFetch(false, std::vector<std::string>(),
+                                     true, std::vector<std::string>());
   current_supported_languages.clear();
   TranslateManager::GetSupportedLanguages(&current_supported_languages);
   EXPECT_EQ(default_supported_languages, current_supported_languages);
 
   // Now check that we got the appropriate set of languages from the server.
-  SimulateSupportedLanguagesURLFetch(true, server_languages);
+  SimulateSupportedLanguagesURLFetch(true, server_languages,
+                                     true, alpha_languages);
   current_supported_languages.clear();
   TranslateManager::GetSupportedLanguages(&current_supported_languages);
   // "xx" can't be displayed in the Translate inforbar, so this is eliminated.
   EXPECT_EQ(server_languages.size() - 1, current_supported_languages.size());
   // Not sure we need to guarantee the order of languages, so we find them.
   for (size_t i = 0; i < server_languages.size(); ++i) {
-    if (server_languages[i] == "xx")
+    const std::string& lang = server_languages[i];
+    if (lang == "xx")
       continue;
     EXPECT_NE(current_supported_languages.end(),
               std::find(current_supported_languages.begin(),
                         current_supported_languages.end(),
-                        server_languages[i]));
+                        lang));
+    bool is_alpha = std::find(alpha_languages.begin(),
+                              alpha_languages.end(),
+                              lang) != alpha_languages.end();
+    EXPECT_EQ(TranslateManager::IsAlphaLanguage(lang), is_alpha);
   }
 }
 
-std::string GetLanguageListString(
-    const std::vector<std::string>& language_list) {
-  // The translate manager is expecting a JSON string like:
-  // sl({'sl': {'XX': 'LanguageName', ...}, 'tl': {'XX': 'LanguageName', ...}})
-  // We only need to set the tl (target languages) dictionary.
-  scoped_ptr<DictionaryValue> target_languages_dict(new DictionaryValue);
-  for (size_t i = 0; i < language_list.size(); ++i) {
-    // The value is ignored, we only use the key.
-    target_languages_dict->Set(language_list[i], Value::CreateNullValue());
-  }
+// Test the fetching of languages from the translate server without 'al'
+// parameter.
+TEST_F(TranslateManagerBrowserTest,
+       FetchLanguagesFromTranslateServerWithoutAlpha) {
+  std::vector<std::string> server_languages;
+  server_languages.push_back("aa");
+  server_languages.push_back("ak");
+  server_languages.push_back("ab");
+  server_languages.push_back("en-CA");
+  server_languages.push_back("zh");
+  server_languages.push_back("yi");
+  server_languages.push_back("fr-FR");
+  server_languages.push_back("xx");
+
+  std::vector<std::string> alpha_languages;
+  alpha_languages.push_back("aa");
+  alpha_languages.push_back("yi");
+
+  // call GetSupportedLanguages to call RequestLanguageList internally.
+  std::vector<std::string> default_supported_languages;
+  TranslateManager::GetSupportedLanguages(&default_supported_languages);
+
+  SimulateSupportedLanguagesURLFetch(true, server_languages,
+                                     false, alpha_languages);
+
+  std::vector<std::string> current_supported_languages;
+  TranslateManager::GetSupportedLanguages(&current_supported_languages);
 
-  DictionaryValue language_list_dict;
-  language_list_dict.Set("tl", target_languages_dict.release());
-  std::string language_list_json_str;
-  base::JSONWriter::Write(&language_list_dict, &language_list_json_str);
-  std::string language_list_str("sl(");
-  language_list_str += language_list_json_str;
-  language_list_str += ")";
-  return language_list_str;
+  // "xx" can't be displayed in the Translate inforbar, so this is eliminated.
+  EXPECT_EQ(server_languages.size() - 1, current_supported_languages.size());
+
+  for (size_t i = 0; i < server_languages.size(); ++i) {
+    const std::string& lang = server_languages[i];
+    if (lang == "xx")
+      continue;
+    EXPECT_NE(current_supported_languages.end(),
+              std::find(current_supported_languages.begin(),
+                        current_supported_languages.end(),
+                        lang));
+    EXPECT_FALSE(TranslateManager::IsAlphaLanguage(lang));
+  }
 }
 
 // Tests auto-translate on page.