Translate: display alpha language name with (alpha) suffix

chrome/browser/translate/translate_language_list.cc

Index: chrome/browser/translate/translate_language_list.cc
diff --git a/chrome/browser/translate/translate_language_list.cc b/chrome/browser/translate/translate_language_list.cc
index 99e3c6e650ae436119538dc2f83a72459232ca08..391204a8125a2c5295bde702fc6091ce763d6c9a 100644
--- a/chrome/browser/translate/translate_language_list.cc
+++ b/chrome/browser/translate/translate_language_list.cc
@@ -111,7 +111,7 @@ const int kMaxRetryLanguageListFetch = 5;
 const char TranslateLanguageList::kLanguageListCallbackName[] = "sl(";
 const char TranslateLanguageList::kTargetLanguagesKey[] = "tl";
 
-TranslateLanguageList::TranslateLanguageList() {
+TranslateLanguageList::TranslateLanguageList() : requesting_alpha_(false) {
   // We default to our hard coded list of languages in
   // |kDefaultSupportedLanguages|. This list will be overriden by a server
   // providing supported langauges list.
@@ -130,8 +130,15 @@ void TranslateLanguageList::OnURLFetchComplete(const net::URLFetcher* source) {
     std::string data;
     source->GetResponseAsString(&data);
     SetSupportedLanguages(data);
+    if (requesting_alpha_)
+      return;
+    scoped_ptr<const net::URLFetcher> delete_ptr(url_fetcher_.release());

[MAD, 2013/05/29 14:09:32]

I don't understand why you need this. url_fetcher_ has already been released
above.

[Takashi Toyoshima, 2013/05/30 06:25:31]

Sorry, this looks mistakenly added on rebasing.

+    FetchLanguageList(true);

[MAD, 2013/05/29 14:09:32]

I don't like this chaining either... I would prefer we use two distinct
URLFetcher objects, in that case, you wouldn't need the subclass I mentioned in
the header file comments. This way, you could simply call Fetch with false, and
then with true right after from within FetchLanguageList (that should be renamed
FetchLanguageLists). 

This would make things more explicit in my opinion.

[Takashi Toyoshima, 2013/05/30 06:25:31]

agreed.

   } else {
     // TODO(toyoshim): Try again. http://crbug.com/244202 .
+    // Also In CrOS, FetchLanguageList is not called at launching Chrome. It
+    // will solve this problem that check if FetchLanguageList is already
+    // called, and call it if needed in InitSupportedLanguage().
     VLOG(9) << "Failed to Fetch languages from: " << kLanguageListFetchURL;
   }
 }
@@ -157,41 +164,16 @@ std::string TranslateLanguageList::GetLanguageCode(
   return chrome_locale.substr(0, hypen_index);
 }
 
-bool TranslateLanguageList::IsSupportedLanguage(
-    const std::string& page_language) {
-  return supported_languages_.count(page_language) != 0;
+bool TranslateLanguageList::IsSupportedLanguage(const std::string& language) {
+  return supported_languages_.count(language) != 0;
 }
 
-void TranslateLanguageList::RequestLanguageList() {
-  if (url_fetcher_.get())
-    return;
-
-  GURL language_list_fetch_url = GURL(kLanguageListFetchURL);
-  language_list_fetch_url =
-      TranslateURLUtil::AddHostLocaleToUrl(language_list_fetch_url);
-  language_list_fetch_url =
-      TranslateURLUtil::AddApiKeyToUrl(language_list_fetch_url);
-
-  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
-  if (command_line.HasSwitch(switches::kEnableTranslateAlphaLanguages)) {
-    language_list_fetch_url = net::AppendQueryParameter(
-        language_list_fetch_url,
-        kAlphaLanguageQueryName,
-        kAlphaLanguageQueryValue);
-  }
-
-  VLOG(9) << "Fetch supporting language list from: "
-          << language_list_fetch_url.spec().c_str();
+bool TranslateLanguageList::IsAlphaLanguage(const std::string& language) {
+  return supported_alpha_languages_.count(language) != 0;
+}
 
-  url_fetcher_.reset(net::URLFetcher::Create(1,
-                                             language_list_fetch_url,
-                                             net::URLFetcher::GET,
-                                             this));
-  url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
-                             net::LOAD_DO_NOT_SAVE_COOKIES);
-  url_fetcher_->SetRequestContext(g_browser_process->system_request_context());
-  url_fetcher_->SetMaxRetriesOn5xx(kMaxRetryLanguageListFetch);
-  url_fetcher_->Start();
+void TranslateLanguageList::RequestLanguageList() {
+  FetchLanguageList(false);
 }
 
 void TranslateLanguageList::SetSupportedLanguages(
@@ -231,12 +213,66 @@ void TranslateLanguageList::SetSupportedLanguages(
     NOTREACHED();
     return;
   }
-  // Now we can clear our current state...
-  supported_languages_.clear();
-  // ... and replace it with the values we just fetched from the server.
-  for (DictionaryValue::Iterator iter(*target_languages);
-       !iter.IsAtEnd();
-       iter.Advance()) {
-    supported_languages_.insert(iter.key());
+
+  if (!requesting_alpha_) {

[MAD, 2013/05/29 14:09:32]

SetSupportedLanguages should receive an argument as opposed to rely on a data
member, it would make things clearer.

[Takashi Toyoshima, 2013/05/30 06:25:31]

Done.

+    // Now we can clear language list.
+    supported_languages_.clear();
+    // ... and replace it with the values we just fetched from the server.
+    for (DictionaryValue::Iterator iter(*target_languages);
+         !iter.IsAtEnd();
+         iter.Advance()) {
+      supported_languages_.insert(iter.key());
+    }
+  } else {
+    // Now we can clear alpha language list.
+    supported_alpha_languages_.clear();
+    // ... and add languages they are not in the supporting list to alpha
+    // language list.
+    for (DictionaryValue::Iterator iter(*target_languages);
+         !iter.IsAtEnd();
+         iter.Advance()) {
+      if (IsSupportedLanguage(iter.key()))
+        continue;
+      // TODO(toyoshim): Check if UI libraries support adding locale.
+      supported_alpha_languages_.insert(iter.key());
+      supported_languages_.insert(iter.key());

[MAD, 2013/05/30 14:17:59]

You used to add alpha languages to both lists, and now you only add it to the
alpha list. I don't get it...

[Takashi Toyoshima, 2013/05/30 14:41:38]

Original logic is expected that stable list is fetched before alpha list. But
now both are fetched in parallel.
So, I decided to adopt different logic to handle them.

+    }
+  }
+}
+
+void TranslateLanguageList::FetchLanguageList(bool alpha) {
+  if (url_fetcher_.get())
+    return;
+
+  GURL language_list_fetch_url = GURL(kLanguageListFetchURL);
+  language_list_fetch_url =
+      TranslateURLUtil::AddHostLocaleToUrl(language_list_fetch_url);
+  language_list_fetch_url =
+      TranslateURLUtil::AddApiKeyToUrl(language_list_fetch_url);
+
+  if (alpha) {
+    const CommandLine& command_line = *CommandLine::ForCurrentProcess();
+    if (!command_line.HasSwitch(switches::kEnableTranslateAlphaLanguages))
+      return;
+
+    language_list_fetch_url = net::AppendQueryParameter(
+        language_list_fetch_url,
+        kAlphaLanguageQueryName,
+        kAlphaLanguageQueryValue);
+
+    requesting_alpha_ = true;
   }
+
+  VLOG(9) << "Fetch supporting language list from: "
+          << language_list_fetch_url.spec().c_str();
+
+  url_fetcher_.reset(net::URLFetcher::Create(1,
+                                             language_list_fetch_url,
+                                             net::URLFetcher::GET,
+                                             this));
+  url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
+                             net::LOAD_DO_NOT_SAVE_COOKIES);
+  url_fetcher_->SetRequestContext(g_browser_process->system_request_context());
+  url_fetcher_->SetMaxRetriesOn5xx(kMaxRetryLanguageListFetch);
+  url_fetcher_->Start();
 }