implement improvements for translate infobar

- add "Show original" button to revert translated page (backend is currently a TODO for jay)
- modify state machine of translate infobar to show "Translating..." when subsequent translations are triggered for a page i.e. when the user modifies the source or target languages
- remove obsolete strings (introduced when privacy dialog was thought to be needed)

BUG=35553,36682
TEST=verify per bug reports.
Review URL: http://codereview.chromium.org/845006

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

6 files changed, 101 insertions, 52 deletions

diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index abeb941..4645315 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -806,6 +806,11 @@ void TabContents::TranslatePage(const std::string& source_lang,
   render_view_host()->TranslatePage(entry->page_id(), source_lang, target_lang);
 }
 
+void TabContents::RevertTranslatedPage() {
+  // TODO(jcampan): revert translated page to original and remove translate
+  // infobar.
+}
+
 ConstrainedWindow* TabContents::CreateConstrainedDialog(
       ConstrainedWindowDelegate* delegate) {
   ConstrainedWindow* window =
diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h
index d6d688d..4e42e8d 100644
--- a/chrome/browser/tab_contents/tab_contents.h
+++ b/chrome/browser/tab_contents/tab_contents.h
@@ -352,6 +352,9 @@ class TabContents : public PageNavigator,
   void TranslatePage(const std::string& source_lang,
                      const std::string& target_lang);
 
+  // Reverts a translated page to original page.
+  void RevertTranslatedPage();
+
   // Window management ---------------------------------------------------------
 
   // Create a new window constrained to this TabContents' clip and visibility.
diff --git a/chrome/browser/translate/translate_infobars_delegates.cc b/chrome/browser/translate/translate_infobars_delegates.cc
index 948e337..174ca08 100644
--- a/chrome/browser/translate/translate_infobars_delegates.cc
+++ b/chrome/browser/translate/translate_infobars_delegates.cc
@@ -42,6 +42,7 @@ void TranslateInfoBarDelegate::InfoBarClosed() {
 // TranslateInfoBarDelegate: public: -------------------------------------------
 
 void TranslateInfoBarDelegate::UpdateState(TranslateState new_state) {
+  translation_pending_ = false;
   if (state_ != new_state)
     state_ = new_state;
 }
@@ -67,11 +68,17 @@ void TranslateInfoBarDelegate::GetAvailableTargetLanguages(
 }
 
 void TranslateInfoBarDelegate::Translate() {
-  if (state_ == kBeforeTranslate)
-    UpdateState(kTranslating);
+  // We only really send page for translation if original and target languages
+  // are different, so only in this case is translation really pending.
+  if (original_lang_index_ != target_lang_index_)
+    translation_pending_ = true;
   tab_contents_->TranslatePage(original_lang_code(), target_lang_code());
 }
 
+void TranslateInfoBarDelegate::RevertTranslation() {
+  tab_contents_->RevertTranslatedPage();
+}
+
 void TranslateInfoBarDelegate::TranslationDeclined() {
   // Remember that the user declined the translation so as to prevent showing a
   // translate infobar for that page again.  (TranslateManager initiates
@@ -224,6 +231,7 @@ TranslateInfoBarDelegate::TranslateInfoBarDelegate(TabContents* tab_contents,
       tab_contents_(tab_contents),
       prefs_(user_prefs),
       state_(state),
+      translation_pending_(false),
       site_(url.HostNoBrackets()),
       original_lang_index_(original_lang_index),
       target_lang_index_(target_lang_index),
diff --git a/chrome/browser/translate/translate_infobars_delegates.h b/chrome/browser/translate/translate_infobars_delegates.h
index ce8cede..30e4346 100644
--- a/chrome/browser/translate/translate_infobars_delegates.h
+++ b/chrome/browser/translate/translate_infobars_delegates.h
@@ -16,6 +16,8 @@ class TranslateInfoBarDelegate : public InfoBarDelegate {
  public:
   enum TranslateState {
     kBeforeTranslate = 1,
+    // TODO(playmobil or erg): remove kTranslating state when mac and linux code
+    // have been updated to use transaction_pending() instead.
     kTranslating,
     kAfterTranslate,
     kTranslationFailed,
@@ -36,6 +38,7 @@ class TranslateInfoBarDelegate : public InfoBarDelegate {
   void ModifyOriginalLanguage(int lang_index);
   void ModifyTargetLanguage(int lang_index);
   void Translate();
+  void RevertTranslation();
   void TranslationDeclined();
   bool IsLanguageBlacklisted();
   void ToggleLanguageBlacklist();
@@ -65,6 +68,9 @@ class TranslateInfoBarDelegate : public InfoBarDelegate {
   TranslateState state() const {
     return state_;
   }
+  bool translation_pending() const {
+    return translation_pending_;
+  }
 
   // Retrieve the text for the toolbar label.  The toolbar label is a bit
   // strange since we need to place popup menus inside the string in question.
@@ -110,6 +116,7 @@ class TranslateInfoBarDelegate : public InfoBarDelegate {
   TabContents* tab_contents_;  // Weak.
   TranslatePrefs prefs_;
   TranslateState state_;
+  bool translation_pending_;
   std::string site_;
   int original_lang_index_;
   int target_lang_index_;
diff --git a/chrome/browser/views/infobars/translate_infobars.cc b/chrome/browser/views/infobars/translate_infobars.cc
index 1caedec..649b971 100644
--- a/chrome/browser/views/infobars/translate_infobars.cc
+++ b/chrome/browser/views/infobars/translate_infobars.cc
@@ -230,6 +230,7 @@ TranslateInfoBar::TranslateInfoBar(TranslateInfoBarDelegate* delegate)
       accept_button_(NULL),
       deny_button_(NULL),
       target_language_menu_button_(NULL),
+      revert_button_(NULL),
       swapped_language_placeholders_(false) {
   // Initialize icon.
   icon_ = new views::ImageView;
@@ -264,10 +265,11 @@ TranslateInfoBar::~TranslateInfoBar() {
 void TranslateInfoBar::UpdateState(
     TranslateInfoBarDelegate::TranslateState new_state) {
   // Create and initialize state-dependent controls if necessary.
+  bool translation_pending = GetDelegate()->translation_pending();
   switch (new_state) {
     case TranslateInfoBarDelegate::kAfterTranslate:
-      CreateLabels();
       if (!target_language_menu_button_) {
+        CreateLabels();
         string16 language_name =
             TranslateInfoBarDelegate::GetDisplayNameForLocale(
                 GetDelegate()->target_lang_code());
@@ -275,6 +277,11 @@ void TranslateInfoBar::UpdateState(
             UTF16ToWideHack(language_name));
         AddChildView(target_language_menu_button_);
       }
+      if (!revert_button_) {
+        revert_button_ = new TranslateTextButton(this,
+            IDS_TRANSLATE_INFOBAR_REVERT);
+        AddChildView(revert_button_);
+      }
       break;
 
     case TranslateInfoBarDelegate::kBeforeTranslate:
@@ -292,36 +299,35 @@ void TranslateInfoBar::UpdateState(
       }
       break;
 
-    case TranslateInfoBarDelegate::kTranslating:
-      if (!label_1_)
-        CreateLabels();
-      if (!translating_label_) {
-        translating_label_ = new views::Label(
-            l10n_util::GetString(IDS_TRANSLATE_INFOBAR_TRANSLATING));
-        translating_label_->SetColor(SK_ColorBLACK);
-        translating_label_->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
-        AddChildView(translating_label_);
-      }
-      break;
-
     default:
       NOTREACHED() << "Invalid translate state change";
       break;
   }
 
+  // If translation is pending, create "Translating..." label.
+  if (translation_pending && !translating_label_) {
+    translating_label_ = new views::Label(
+        l10n_util::GetString(IDS_TRANSLATE_INFOBAR_TRANSLATING));
+    translating_label_->SetColor(SK_ColorBLACK);
+    translating_label_->SetHorizontalAlignment(views::Label::ALIGN_LEFT);
+    AddChildView(translating_label_);
+  }
+
   // Determine visibility of state-dependent controls.
   if (accept_button_)
-    accept_button_->SetVisible(
+    accept_button_->SetVisible(!translation_pending &&
         new_state == TranslateInfoBarDelegate::kBeforeTranslate);
   if (deny_button_)
-    deny_button_->SetVisible(
+    deny_button_->SetVisible(!translation_pending &&
         new_state == TranslateInfoBarDelegate::kBeforeTranslate);
   if (target_language_menu_button_)
     target_language_menu_button_->SetVisible(
         new_state == TranslateInfoBarDelegate::kAfterTranslate);
+  if (revert_button_)
+    revert_button_->SetVisible(!translation_pending &&
+        new_state == TranslateInfoBarDelegate::kAfterTranslate);
   if (translating_label_)
-    translating_label_->SetVisible(
-        new_state == TranslateInfoBarDelegate::kTranslating);
+    translating_label_->SetVisible(translation_pending);
 
   // Trigger layout and repaint.
   Layout();
@@ -368,7 +374,8 @@ void TranslateInfoBar::Layout() {
   gfx::Size translating_ps;
   if (label_3_)
     label3_ps = label_3_->GetPreferredSize();
-  if (state == TranslateInfoBarDelegate::kTranslating)
+  bool translation_pending = GetDelegate()->translation_pending();
+  if (translation_pending)
     translating_ps = translating_label_->GetPreferredSize();
   int text1_width = label1_ps.width();
   int text2_width = label2_ps.width();
@@ -397,37 +404,48 @@ void TranslateInfoBar::Layout() {
       GetSpacingAfterFirstLanguageButton(), InfoBar::OffsetY(this, label2_ps),
       text2_width, label2_ps.height());
 
+  int prev_right = label_2_->bounds().right();
+
   // If second language menu button is available, place it after label_2.
   if (button2) {
     gfx::Size button2_ps = button2->GetPreferredSize();
-    button2->SetBounds(label_2_->bounds().right() +
-        InfoBar::kButtonInLabelSpacing, OffsetY(this, button2_ps),
-        button2_ps.width(), button2_ps.height());
+    button2->SetBounds(prev_right + InfoBar::kButtonInLabelSpacing,
+        OffsetY(this, button2_ps), button2_ps.width(), button2_ps.height());
+    prev_right = button2->bounds().right();
 
     // If label_3 is available, place it after second language menu button.
     if (label_3_) {
       gfx::Size label3_ps = label_3_->GetPreferredSize();
-      label_3_->SetBounds(button2->bounds().right() +
-          InfoBar::kButtonInLabelSpacing, InfoBar::OffsetY(this, label3_ps),
-          text3_width, label3_ps.height());
+      label_3_->SetBounds(prev_right + InfoBar::kButtonInLabelSpacing,
+          InfoBar::OffsetY(this, label3_ps), text3_width, label3_ps.height());
+      prev_right = label_3_->bounds().right();
+    }
+
+    // If no translation is pending, layout revert button.
+    if (!translation_pending && revert_button_) {
+      gfx::Size revert_ps = revert_button_->GetPreferredSize();
+      revert_button_->SetBounds(prev_right + InfoBar::kEndOfLabelSpacing,
+          OffsetY(this, revert_ps), revert_ps.width(), revert_ps.height());
     }
   }
 
-  // If translate state is kBeforeTranslate, layout accept and deny butons.
-  if (state == TranslateInfoBarDelegate::kBeforeTranslate) {
+  // If translate state is kBeforeTranslate with no pending translation,
+  // layout accept and deny butons.
+  if (state == TranslateInfoBarDelegate::kBeforeTranslate &&
+      !translation_pending) {
     gfx::Size accept_ps = accept_button_->GetPreferredSize();
-    accept_button_->SetBounds(
-        (label_3_ ? label_3_->bounds().right() : label_2_->bounds().right()) +
-        InfoBar::kEndOfLabelSpacing,
+    accept_button_->SetBounds(prev_right + InfoBar::kEndOfLabelSpacing,
         OffsetY(this, accept_ps), accept_ps.width(), accept_ps.height());
     gfx::Size deny_ps = deny_button_->GetPreferredSize();
     deny_button_->SetBounds(
         accept_button_->bounds().right() + InfoBar::kButtonButtonSpacing,
         OffsetY(this, deny_ps), deny_ps.width(), deny_ps.height());
-  } else if (state == TranslateInfoBarDelegate::kTranslating) {
-    // If translate state is kTranslating, layout "Translating..." label.
+  }
+
+  // If translation is pending, layout "Translating..." label.
+  if (translation_pending) {
     translating_label_->SetBounds(
-        label_2_->bounds().right() + InfoBar::kEndOfLabelSpacing,
+        prev_right + InfoBar::kEndOfLabelSpacing,
         InfoBar::OffsetY(this, translating_ps),
         translating_width, translating_ps.height());
   }
@@ -484,7 +502,7 @@ void TranslateInfoBar::RunMenu(views::View* source, const gfx::Point& pt) {
     }
 
     case kMenuIDOriginalLanguage: {
-      if (GetDelegate()->state() != TranslateInfoBarDelegate::kTranslating) {
+      if (!translating_label_ || !translating_label_->IsVisible()) {
         if (!original_language_menu_model_.get()) {
           original_language_menu_model_.reset(
               new LanguagesMenuModel(this, GetDelegate(), true));
@@ -500,16 +518,18 @@ void TranslateInfoBar::RunMenu(views::View* source, const gfx::Point& pt) {
     }
 
     case kMenuIDTargetLanguage: {
-      if (!target_language_menu_model_.get()) {
-        target_language_menu_model_.reset(
-            new LanguagesMenuModel(this, GetDelegate(), false));
-        target_language_menu_menu_.reset(
-            new views::Menu2(target_language_menu_model_.get()));
+      if (!translating_label_ || !translating_label_->IsVisible()) {
+        if (!target_language_menu_model_.get()) {
+          target_language_menu_model_.reset(
+              new LanguagesMenuModel(this, GetDelegate(), false));
+          target_language_menu_menu_.reset(
+              new views::Menu2(target_language_menu_model_.get()));
+        }
+        views::Menu2::Alignment alignment;
+        gfx::Point menu_position = DetermineMenuPositionAndAlignment(
+            target_language_menu_button_, &alignment);
+        target_language_menu_menu_->RunMenuAt(menu_position, alignment);
       }
-      views::Menu2::Alignment alignment;
-      gfx::Point menu_position = DetermineMenuPositionAndAlignment(
-          target_language_menu_button_, &alignment);
-      target_language_menu_menu_->RunMenuAt(menu_position, alignment);
       break;
     }
 
@@ -598,6 +618,8 @@ void TranslateInfoBar::ButtonPressed(
   } else if (sender == deny_button_) {
     GetDelegate()->TranslationDeclined();
     RemoveInfoBar();
+  } else if (sender == revert_button_) {
+    GetDelegate()->RevertTranslation();
   } else {  // Let base InfoBar handle close button.
     InfoBar::ButtonPressed(sender, event);
   }
@@ -612,9 +634,7 @@ void TranslateInfoBar::Observe(NotificationType type,
   TabContents* tab = Source<TabContents>(source).ptr();
   if (tab != GetDelegate()->tab_contents())
     return;
-  if (!target_language_menu_button_ ||
-      !target_language_menu_button_->IsVisible())
-    UpdateState(TranslateInfoBarDelegate::kAfterTranslate);
+  UpdateState(TranslateInfoBarDelegate::kAfterTranslate);
 }
 
 // TranslateInfoBar, private: --------------------------------------------------
@@ -713,14 +733,19 @@ void TranslateInfoBar::OnLanguageModified(views::MenuButton* menu_button,
   menu_button->SetText(UTF16ToWideHack(new_language));
   menu_button->ClearMaxTextSize();
   menu_button->SizeToPreferredSize();
-  Layout();
-  SchedulePaint();
+
   // Clear options menu model so that it'll be created with new language.
   options_menu_model_.reset();
+
   // Selecting an item from the "from language" menu in the before translate
   // phase shouldn't trigger translation - http://crbug.com/36666
-  if (GetDelegate()->state() == TranslateInfoBarDelegate::kAfterTranslate)
+  if (GetDelegate()->state() == TranslateInfoBarDelegate::kAfterTranslate) {
     GetDelegate()->Translate();
+    UpdateState(GetDelegate()->state());
+  }
+
+  Layout();
+  SchedulePaint();
 }
 
 inline TranslateInfoBarDelegate* TranslateInfoBar::GetDelegate() const {
@@ -728,8 +753,8 @@ inline TranslateInfoBarDelegate* TranslateInfoBar::GetDelegate() const {
 }
 
 inline int TranslateInfoBar::GetSpacingAfterFirstLanguageButton() const {
-  return (GetDelegate()->state() == TranslateInfoBarDelegate::kBeforeTranslate ?
-      10 : kButtonInLabelSpacing);
+  return (GetDelegate()->state() == TranslateInfoBarDelegate::kAfterTranslate ?
+      kButtonInLabelSpacing : 10);
 }
 
 // TranslateInfoBarDelegate, InfoBarDelegate overrides: ------------------
diff --git a/chrome/browser/views/infobars/translate_infobars.h b/chrome/browser/views/infobars/translate_infobars.h
index c343eb0..3ea8534 100644
--- a/chrome/browser/views/infobars/translate_infobars.h
+++ b/chrome/browser/views/infobars/translate_infobars.h
@@ -77,6 +77,7 @@ class TranslateInfoBar : public InfoBar,
   TranslateTextButton* deny_button_;
   views::MenuButton* original_language_menu_button_;
   views::MenuButton* target_language_menu_button_;
+  TranslateTextButton* revert_button_;
   views::MenuButton* options_menu_button_;
 
   scoped_ptr<LanguagesMenuModel> original_language_menu_model_;