TranslateInfoBarDelegate2 were leaked. (Yikes!)

This revealed that the NormalTranslate unit-test was incorrect (it was not testing that changing the source or target language from the after translate infobar causes a translation).
This also revealed that returning reference string in the delegate is not a brilliant idea as when we switch infobars, the new infobar might be using a string from the old infobar that might have been deleted.

BUG=None
TEST=Linux heap check bot should be green.

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

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

3 files changed, 42 insertions, 18 deletions

diff --git a/chrome/browser/translate/translate_infobar_delegate2.cc b/chrome/browser/translate/translate_infobar_delegate2.cc
index 3fd6648..f27601a 100644
--- a/chrome/browser/translate/translate_infobar_delegate2.cc
+++ b/chrome/browser/translate/translate_infobar_delegate2.cc
@@ -2,10 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <algorithm>
-
 #include "chrome/browser/translate/translate_infobar_delegate2.h"
 
+#include <algorithm>
+
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
 #include "chrome/browser/browser_process.h"
@@ -83,23 +83,23 @@ int TranslateInfoBarDelegate2::GetLanguageCount() const {
   return static_cast<int>(languages_.size());
 }
 
-const std::string& TranslateInfoBarDelegate2::GetLanguageCodeAt(
+std::string TranslateInfoBarDelegate2::GetLanguageCodeAt(
     int index) const {
   DCHECK(index >=0 && index < GetLanguageCount());
   return languages_[index].first;
 }
 
-const string16& TranslateInfoBarDelegate2::GetLanguageDisplayableNameAt(
+string16 TranslateInfoBarDelegate2::GetLanguageDisplayableNameAt(
     int index) const {
   DCHECK(index >=0 && index < GetLanguageCount());
   return languages_[index].second;
 }
 
-const std::string& TranslateInfoBarDelegate2::GetOriginalLanguageCode() const {
+std::string TranslateInfoBarDelegate2::GetOriginalLanguageCode() const {
   return GetLanguageCodeAt(original_language_index());
 }
 
-const std::string& TranslateInfoBarDelegate2::GetTargetLanguageCode() const {
+std::string TranslateInfoBarDelegate2::GetTargetLanguageCode() const {
   return GetLanguageCodeAt(target_language_index());
 }
 
@@ -155,6 +155,10 @@ void TranslateInfoBarDelegate2::InfoBarDismissed() {
   UMA_HISTOGRAM_COUNTS("Translate.DeclineTranslateCloseInfobar", 1);
 }
 
+void TranslateInfoBarDelegate2::InfoBarClosed() {
+  delete this;
+}
+
 SkBitmap* TranslateInfoBarDelegate2::GetIcon() const {
   return ResourceBundle::GetSharedInstance().GetBitmapNamed(
       IDR_INFOBAR_TRANSLATE);
diff --git a/chrome/browser/translate/translate_infobar_delegate2.h b/chrome/browser/translate/translate_infobar_delegate2.h
index bb535ad..130c3d5 100644
--- a/chrome/browser/translate/translate_infobar_delegate2.h
+++ b/chrome/browser/translate/translate_infobar_delegate2.h
@@ -49,10 +49,10 @@ class TranslateInfoBarDelegate2 : public InfoBarDelegate {
   int GetLanguageCount() const;
 
   // Returns the ISO code for the language at |index|.
-  const std::string& GetLanguageCodeAt(int index) const;
+  std::string GetLanguageCodeAt(int index) const;
 
   // Returns the displayable name for the language at |index|.
-  const string16& GetLanguageDisplayableNameAt(int index) const;
+  string16 GetLanguageDisplayableNameAt(int index) const;
 
   TabContents* tab_contents() const { return tab_contents_; }
 
@@ -62,8 +62,8 @@ class TranslateInfoBarDelegate2 : public InfoBarDelegate {
   int target_language_index() const { return target_language_index_; }
 
   // Convenience methods.
-  const std::string& GetOriginalLanguageCode() const;
-  const std::string& GetTargetLanguageCode() const;
+  std::string GetOriginalLanguageCode() const;
+  std::string GetTargetLanguageCode() const;
 
   // Called by the InfoBar to notify that the original/target language has
   // changed and is now the language at |language_index|.
@@ -89,7 +89,8 @@ class TranslateInfoBarDelegate2 : public InfoBarDelegate {
 
   // InfoBarDelegate implementation:
   virtual InfoBar* CreateInfoBar();
-  void InfoBarDismissed();
+  virtual void InfoBarDismissed();
+  virtual void InfoBarClosed();
   virtual SkBitmap* GetIcon() const;
   virtual InfoBarDelegate::Type GetInfoBarType();
   virtual TranslateInfoBarDelegate2* AsTranslateInfoBarDelegate2() {
diff --git a/chrome/browser/translate/translate_manager2_unittest.cc b/chrome/browser/translate/translate_manager2_unittest.cc
index 5f171e2..1f55723 100644
--- a/chrome/browser/translate/translate_manager2_unittest.cc
+++ b/chrome/browser/translate/translate_manager2_unittest.cc
@@ -303,19 +303,38 @@ TEST_F(TranslateManager2Test, NormalTranslate) {
   ASSERT_TRUE(infobar != NULL);
   EXPECT_EQ(TranslateInfoBarDelegate2::AFTER_TRANSLATE, infobar->type());
 
-  // Simulate translating again from there but with 2 different languages.
+  // Simulate changing the original language, this should trigger a translation.
+  process()->sink().ClearMessages();
+  std::string new_original_lang = infobar->GetLanguageCodeAt(0);
   infobar->SetOriginalLanguage(0);
-  infobar->SetTargetLanguage(1);
-  std::string new_original_lang = infobar->GetOriginalLanguageCode();
-  std::string new_target_lang = infobar->GetTargetLanguageCode();
+  EXPECT_TRUE(GetTranslateMessage(&page_id, &original_lang, &target_lang));
+  EXPECT_EQ(0, page_id);
+  EXPECT_EQ(new_original_lang, original_lang);
+  EXPECT_EQ("en", target_lang);
+  // Simulate the render notifying the translation has been done.
+  rvh()->TestOnMessageReceived(ViewHostMsg_PageTranslated(0, 0,
+      new_original_lang, "en", TranslateErrors::NONE));
+  // infobar is now invalid.
+  TranslateInfoBarDelegate2* new_infobar = GetTranslateInfoBar();
+  ASSERT_TRUE(new_infobar != NULL);
+  EXPECT_NE(infobar, new_infobar);
+  infobar = new_infobar;
+
+  // Simulate changing the target language, this should trigger a translation.
   process()->sink().ClearMessages();
-  infobar->Translate();
-
-  // Test that we sent the right message to the renderer.
+  std::string new_target_lang = infobar->GetLanguageCodeAt(1);
+  infobar->SetTargetLanguage(1);
   EXPECT_TRUE(GetTranslateMessage(&page_id, &original_lang, &target_lang));
   EXPECT_EQ(0, page_id);
   EXPECT_EQ(new_original_lang, original_lang);
   EXPECT_EQ(new_target_lang, target_lang);
+  // Simulate the render notifying the translation has been done.
+  rvh()->TestOnMessageReceived(ViewHostMsg_PageTranslated(0, 0,
+      new_original_lang, new_target_lang, TranslateErrors::NONE));
+  // infobar is now invalid.
+  new_infobar = GetTranslateInfoBar();
+  ASSERT_TRUE(new_infobar != NULL);
+  EXPECT_NE(infobar, new_infobar);
 }
 
 TEST_F(TranslateManager2Test, TranslateScriptNotAvailable) {