Translate: display alpha language name with (alpha) suffix

chrome/browser/translate/translate_language_list.cc

 // Copyright 2013 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 "chrome/browser/translate/translate_language_list.h"
 
 #include <set>
 
 #include "base/command_line.h"
 #include "base/json/json_reader.h"
@@ skipping 93 matching lines @@
 
 // Retry parameter for fetching supporting language list.
 const int kMaxRetryLanguageListFetch = 5;
 
 }  // namespace
 
 // This must be kept in sync with the &cb= value in the kLanguageListFetchURL.
 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.
   for (size_t i = 0; i < arraysize(kDefaultSupportedLanguages); ++i)
     supported_languages_.insert(kDefaultSupportedLanguages[i]);
 }
 
 TranslateLanguageList::~TranslateLanguageList() {}
 
 void TranslateLanguageList::OnURLFetchComplete(const net::URLFetcher* source) {
   DCHECK(url_fetcher_.get() == source);
   scoped_ptr<const net::URLFetcher> delete_ptr(url_fetcher_.release());
 
   if (source->GetStatus().status() == net::URLRequestStatus::SUCCESS &&
       source->GetResponseCode() == net::HTTP_OK) {
     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;
   }
 }
 
 void TranslateLanguageList::GetSupportedLanguages(
     std::vector<std::string>* languages) {
   DCHECK(languages && languages->empty());
   std::set<std::string>::const_iterator iter = supported_languages_.begin();
   for (; iter != supported_languages_.end(); ++iter)
     languages->push_back(*iter);
 }
 
 std::string TranslateLanguageList::GetLanguageCode(
     const std::string& chrome_locale) {
   // Only remove the country code for country specific languages we don't
   // support specifically yet.
   if (IsSupportedLanguage(chrome_locale))
     return chrome_locale;
 
   size_t hypen_index = chrome_locale.find('-');
   if (hypen_index == std::string::npos)
     return chrome_locale;
   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;
+}
+
+bool TranslateLanguageList::IsAlphaLanguage(const std::string& language) {
+  return supported_alpha_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();
-
-  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();
+  FetchLanguageList(false);
 }
 
 void TranslateLanguageList::SetSupportedLanguages(
     const std::string& language_list) {
   // The format is:
   // sl({"sl": {"XX": "LanguageName", ...}, "tl": {"XX": "LanguageName", ...}})
   // Where "sl(" is set in kLanguageListCallbackName
   // and "tl" is kTargetLanguagesKey
   if (!StartsWithASCII(language_list,
                        TranslateLanguageList::kLanguageListCallbackName,
@@ skipping 19 matching lines @@
   // and the other for target languages, we want to use the target languages.
   DictionaryValue* language_dict =
       static_cast<DictionaryValue*>(json_value.get());
   DictionaryValue* target_languages = NULL;
   if (!language_dict->GetDictionary(TranslateLanguageList::kTargetLanguagesKey,
                                     &target_languages) ||
       target_languages == NULL) {
     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();
+}