Some minor change to the translate classes to address the remarks

from Jungshik review:
http://codereview.chromium.org/552216/show

TEST=None
BUG=None

Review URL: http://codereview.chromium.org/566024

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

4 files changed, 39 insertions, 36 deletions

diff --git a/chrome/browser/renderer_host/translation_service.cc b/chrome/browser/renderer_host/translation_service.cc
index f3512e7..38f11c3 100644
--- a/chrome/browser/renderer_host/translation_service.cc
+++ b/chrome/browser/renderer_host/translation_service.cc
@@ -43,8 +43,8 @@ LocaleToCLDLanguage kLocaleToCLDLanguages[] = {
     { "en-GB", "en" },
     { "en-US", "en" },
     { "es-419", "es" },
-    { "pt-PT", "pt" },
     { "pt-BR", "pt" },
+    { "pt-PT", "pt" },
 };
 
 // The list of languages the Google translation server supports.
@@ -331,14 +331,14 @@ void TranslationService::OnURLFetchComplete(const URLFetcher* source,
 
   // If the response is a simple string, put it in an array.  (The JSONReader
   // requires an array or map at the root.)
-  std::string str;
+  std::string wrapped_data;
   if (data.size() > 1U && data[0] == '"') {
-    str.append("[");
-    str.append(data);
-    str.append("]");
+    wrapped_data.append("[");
+    wrapped_data.append(data);
+    wrapped_data.append("]");
   }
-  scoped_ptr<Value> value(base::JSONReader::Read(str.empty() ? data : str,
-                                                 true));
+  scoped_ptr<Value> value(base::JSONReader::Read(
+      wrapped_data.empty() ? data : wrapped_data, true));
   if (!value.get()) {
     NOTREACHED() << "Translation server returned invalid JSON response.";
     TranslationFailed(source);
@@ -349,15 +349,15 @@ void TranslationService::OnURLFetchComplete(const URLFetcher* source,
   // string.
   TextChunksList translated_chunks_list;
   if (value->IsType(Value::TYPE_STRING)) {
-    string16 str16;
-    if (!value->GetAsUTF16(&str16)) {
+    string16 translated_text;
+    if (!value->GetAsUTF16(&translated_text)) {
       NOTREACHED();
       TranslationFailed(source);
       return;
     }
-    TextChunks text_chunks;
-    text_chunks.push_back(str16);
-    translated_chunks_list.push_back(text_chunks);
+    TextChunks translated_text_chunks;
+    translated_text_chunks.push_back(translated_text);
+    translated_chunks_list.push_back(translated_text_chunks);
   } else {
     if (!value->IsType(Value::TYPE_LIST)) {
       NOTREACHED() << "Translation server returned unexpected JSON response "
@@ -365,19 +365,20 @@ void TranslationService::OnURLFetchComplete(const URLFetcher* source,
       TranslationFailed(source);
       return;
     }
-    ListValue* list = static_cast<ListValue*>(value.get());
-    for (size_t i = 0; i < list->GetSize(); ++i) {
+    ListValue* translated_text_list = static_cast<ListValue*>(value.get());
+    for (size_t i = 0; i < translated_text_list->GetSize(); ++i) {
       string16 translated_text;
-      if (!list->GetStringAsUTF16(i, &translated_text)) {
+      if (!translated_text_list->GetStringAsUTF16(i, &translated_text)) {
         NOTREACHED() << "Translation server returned unexpected JSON response "
             " (unexpected type in list).";
         TranslationFailed(source);
         return;
       }
       translated_text = UnescapeForHTML(translated_text);
-      TranslationService::TextChunks text_chunks;
-      TranslationService::SplitTextChunks(translated_text, &text_chunks);
-      translated_chunks_list.push_back(text_chunks);
+      TranslationService::TextChunks translated_text_chunks;
+      TranslationService::SplitIntoTextChunks(translated_text,
+                                              &translated_text_chunks);
+      translated_chunks_list.push_back(translated_text_chunks);
     }
   }
 
@@ -491,8 +492,8 @@ string16 TranslationService::MergeTextChunks(const TextChunks& text_chunks) {
 }
 
 // static
-void TranslationService::SplitTextChunks(const string16& translated_text,
-                                         TextChunks* text_chunks) {
+void TranslationService::SplitIntoTextChunks(const string16& translated_text,
+                                             TextChunks* text_chunks) {
   const string16 kOpenTag = ASCIIToUTF16("<a _CR_TR_ ");
   const string16 kCloseTag = ASCIIToUTF16("</a>");
   const size_t open_tag_len = kOpenTag.size();
diff --git a/chrome/browser/renderer_host/translation_service.h b/chrome/browser/renderer_host/translation_service.h
index 8250eb5..e77c7cb 100644
--- a/chrome/browser/renderer_host/translation_service.h
+++ b/chrome/browser/renderer_host/translation_service.h
@@ -21,7 +21,7 @@ class TranslateURLFetcherDelegate;
 class URLFetcher;
 
 // The TranslationService class is used to translate text.
-// There is one TranslationService is per renderer process.
+// There is one TranslationService per renderer process.
 // It receives requests to translate text from the different render views of the
 // render process, provided in lists (text chunks), where the words should be
 // translated without changing the chunks order.
@@ -80,7 +80,7 @@ class TranslationService : public URLFetcher::Delegate {
   friend class TranslationServiceTest;
   friend class TranslateURLFetcherDelegate;
   FRIEND_TEST(TranslationServiceTest, MergeTestChunks);
-  FRIEND_TEST(TranslationServiceTest, SplitTestChunks);
+  FRIEND_TEST(TranslationServiceTest, SplitIntoTextChunks);
   FRIEND_TEST(TranslationServiceTest, RemoveTag);
 
   struct TranslationRequest;
@@ -108,7 +108,7 @@ class TranslationService : public URLFetcher::Delegate {
       RendererRequestInfoMap;
 
   // Sends the passed request to the translations server.
-  // Warning the request is deleted when this call returns.
+  // Warning: the request is deleted when this call returns.
   void SendRequestToTranslationServer(TranslationRequest* request);
 
   // Called by the URLFetcherDelegate when the translation associated with
@@ -129,8 +129,8 @@ class TranslationService : public URLFetcher::Delegate {
 
   // Splits the translated text into its original text chunks, removing the
   // anchor tags wrapper that were added to preserve order.
-  static void SplitTextChunks(const string16& translated_text,
-                              TextChunks* text_chunks);
+  static void SplitIntoTextChunks(const string16& translated_text,
+                                  TextChunks* text_chunks);
 
   // Removes the HTML anchor tag surrounding |text| and returns the resulting
   // string.
diff --git a/chrome/browser/renderer_host/translation_service_unittest.cc b/chrome/browser/renderer_host/translation_service_unittest.cc
index 70a438f..5d18fe9 100644
--- a/chrome/browser/renderer_host/translation_service_unittest.cc
+++ b/chrome/browser/renderer_host/translation_service_unittest.cc
@@ -190,17 +190,17 @@ TEST_F(TranslationServiceTest, RemoveTag) {
 
 // Tests that we deal correctly with the various results the translation server
 // can return, including the buggy ones.
-TEST_F(TranslationServiceTest, SplitTestChunks) {
+TEST_F(TranslationServiceTest, SplitIntoTextChunks) {
   // Simple case.
   std::vector<string16> text_chunks;
-  TranslationService::SplitTextChunks(ASCIIToUTF16("Hello"), &text_chunks);
+  TranslationService::SplitIntoTextChunks(ASCIIToUTF16("Hello"), &text_chunks);
   ASSERT_EQ(1U, text_chunks.size());
   EXPECT_EQ(ASCIIToUTF16("Hello"), text_chunks[0]);
 
   text_chunks.clear();
 
   // Multiple chunks case, correct syntax.
-  TranslationService::SplitTextChunks(
+  TranslationService::SplitIntoTextChunks(
       ASCIIToUTF16("<a _CR_TR_ id='0'>Bonjour</a>"
                    "<a _CR_TR_ id='1'> mon nom</a>"
                    "<a _CR_TR_ id='2'> est</a>"
@@ -216,7 +216,7 @@ TEST_F(TranslationServiceTest, SplitTestChunks) {
   // For info, original input:
   // <a _CR_TRANSLATE_ id='0'> Experience </a><a _CR_TRANSLATE_ id='1'>Nexus One
   // </a><a _CR_TRANSLATE_ id='2'>, the new Android phone from Google</a>
-  TranslationService::SplitTextChunks(
+  TranslationService::SplitIntoTextChunks(
       ASCIIToUTF16("<a _CR_TR_ id='0'>Experience</a> <a _CR_TR_ id='1'>Nexus"
                    "<a _CR_TR_ id='2'> One,</a></a> <a _CR_TR_ id='2'>the new "
                    "Android Phone</a>"), &text_chunks);
@@ -402,7 +402,7 @@ TEST_F(TranslationServiceTest, MoreThanMaxSizeRequests) {
   std::string one_kb_string(1024U, 'A');
   TextChunks text_chunks;
   text_chunks.push_back(ASCIIToUTF16(one_kb_string));
-  // Send 2 small requests, than a big one.
+  // Send 2 small requests, then a big one.
   translation_service_.Translate(1, 0, 0, text_chunks, "en", "fr", false);
   translation_service_.Translate(1, 0, 1, text_chunks, "en", "fr", false);
   // We need a string big enough to be more than 30KB on top of the other 2
diff --git a/chrome/browser/translate/translate_manager.cc b/chrome/browser/translate/translate_manager.cc
index 85d1260..6de7bf2 100644
--- a/chrome/browser/translate/translate_manager.cc
+++ b/chrome/browser/translate/translate_manager.cc
@@ -79,20 +79,22 @@ void TranslateManager::InitiateTranslation(TabContents* tab,
     return;
   }
 
-  std::string chrome_lang = g_browser_process->GetApplicationLocale();
-  chrome_lang = TranslationService::GetLanguageCode(chrome_lang);
+  std::string ui_lang = TranslationService::GetLanguageCode(
+      g_browser_process->GetApplicationLocale());
 
   // We don't want to translate:
   // - any Chrome specific page (New Tab Page, Download, History... pages).
   // - similar languages (ex: en-US to en).
   // - any user black-listed URLs or user selected language combination.
-  if (entry->url().SchemeIs("chrome") || page_lang == chrome_lang ||
+  // TODO(jcampan): we might also want to look at the accepeted languages and
+  //                not translate those.
+  if (entry->url().SchemeIs("chrome") || page_lang == ui_lang ||
       !TranslatePrefs::CanTranslate(prefs, page_lang, entry->url())) {
     return;
   }
-  if (TranslatePrefs::ShouldAutoTranslate(prefs, page_lang, chrome_lang)) {
+  if (TranslatePrefs::ShouldAutoTranslate(prefs, page_lang, ui_lang)) {
     // The user has previously select "always translate" for this language.
-    tab->TranslatePage(page_lang, chrome_lang);
+    tab->TranslatePage(page_lang, ui_lang);
     return;
   }
 
@@ -110,7 +112,7 @@ void TranslateManager::InitiateTranslation(TabContents* tab,
   // Prompts the user if he/she wants the page translated.
   tab->AddInfoBar(new BeforeTranslateInfoBarDelegate(tab, prefs,
                                                      entry->url(),
-                                                     page_lang, chrome_lang));
+                                                     page_lang, ui_lang));
 }
 
 PrefService* TranslateManager::GetPrefService(TabContents* tab) {