[Translate] Translate delegate shortcut configuration

Allow platform specific configurations to decide when to trigger the extra buttons (now shortcuts) 

BUG=234416
TBR=rohitrao,sky,erg

Review URL: https://chromiumcodereview.appspot.com/14188047

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

9 files changed, 79 insertions, 34 deletions

diff --git a/chrome/browser/translate/translate_infobar_delegate.cc b/chrome/browser/translate/translate_infobar_delegate.cc
index 4bdf77e..b09a7cb 100644
--- a/chrome/browser/translate/translate_infobar_delegate.cc
+++ b/chrome/browser/translate/translate_infobar_delegate.cc
@@ -31,13 +31,11 @@ TranslateInfoBarDelegate::~TranslateInfoBarDelegate() {
 }
 
 // static
-void TranslateInfoBarDelegate::Create(InfoBarService* infobar_service,
-                                      bool replace_existing_infobar,
-                                      Type infobar_type,
-                                      TranslateErrors::Type error_type,
-                                      PrefService* prefs,
-                                      const std::string& original_language,
-                                      const std::string& target_language) {
+void TranslateInfoBarDelegate::Create(
+    InfoBarService* infobar_service, bool replace_existing_infobar,
+    Type infobar_type, TranslateErrors::Type error_type, PrefService* prefs,
+    const ShortcutConfiguration& shortcut_config,
+    const std::string& original_language, const std::string& target_language) {
   // Check preconditions.
   if (infobar_type != TRANSLATION_ERROR) {
     DCHECK(TranslateManager::IsSupportedLanguage(target_language));
@@ -61,7 +59,8 @@ void TranslateInfoBarDelegate::Create(InfoBarService* infobar_service,
   // Create the new delegate.
   scoped_ptr<TranslateInfoBarDelegate> infobar(
       new TranslateInfoBarDelegate(infobar_type, error_type, infobar_service,
-                                   prefs, original_language, target_language));
+                                   prefs, shortcut_config,
+                                   original_language, target_language));
   infobar->UpdateBackgroundAnimation(old_delegate);
 
   // Add the new delegate if necessary.
@@ -234,16 +233,18 @@ bool TranslateInfoBarDelegate::ShouldShowMessageInfoBarButton() {
   return !GetMessageInfoBarButtonText().empty();
 }
 
-bool TranslateInfoBarDelegate::ShouldShowNeverTranslateButton() {
+bool TranslateInfoBarDelegate::ShouldShowNeverTranslateShortcut() {
   DCHECK_EQ(BEFORE_TRANSLATE, infobar_type_);
   return !web_contents()->GetBrowserContext()->IsOffTheRecord() &&
-      (prefs_.GetTranslationDeniedCount(original_language_code()) >= 3);
+      (prefs_.GetTranslationDeniedCount(original_language_code()) >=
+          shortcut_config_.never_translate_min_count);
 }
 
-bool TranslateInfoBarDelegate::ShouldShowAlwaysTranslateButton() {
+bool TranslateInfoBarDelegate::ShouldShowAlwaysTranslateShortcut() {
   DCHECK_EQ(BEFORE_TRANSLATE, infobar_type_);
   return !web_contents()->GetBrowserContext()->IsOffTheRecord() &&
-      (prefs_.GetTranslationAcceptedCount(original_language_code()) >= 3);
+      (prefs_.GetTranslationAcceptedCount(original_language_code()) >=
+          shortcut_config_.always_translate_min_count);
 }
 
 void TranslateInfoBarDelegate::UpdateBackgroundAnimation(
@@ -287,6 +288,7 @@ TranslateInfoBarDelegate::TranslateInfoBarDelegate(
     TranslateErrors::Type error_type,
     InfoBarService* infobar_service,
     PrefService* prefs,
+    ShortcutConfiguration shortcut_config,
     const std::string& original_language,
     const std::string& target_language)
     : InfoBarDelegate(infobar_service),
@@ -296,7 +298,8 @@ TranslateInfoBarDelegate::TranslateInfoBarDelegate(
       initial_original_language_index_(kNoIndex),
       target_language_index_(kNoIndex),
       error_type_(error_type),
-      prefs_(prefs) {
+      prefs_(prefs),
+      shortcut_config_(shortcut_config) {
   DCHECK_NE((infobar_type == TRANSLATION_ERROR),
             (error_type_ == TranslateErrors::NONE));
 
diff --git a/chrome/browser/translate/translate_infobar_delegate.h b/chrome/browser/translate/translate_infobar_delegate.h
index 3518506..5ac5986 100644
--- a/chrome/browser/translate/translate_infobar_delegate.h
+++ b/chrome/browser/translate/translate_infobar_delegate.h
@@ -18,6 +18,13 @@
 
 class PrefService;
 
+// The defaults after which extra shortcuts for options
+// can be shown.
+struct ShortcutConfiguration {
+  int always_translate_min_count;
+  int never_translate_min_count;
+};
+
 class TranslateInfoBarDelegate : public InfoBarDelegate {
  public:
   // The different types of infobars that can be shown for translation.
@@ -57,6 +64,7 @@ class TranslateInfoBarDelegate : public InfoBarDelegate {
                      Type infobar_type,
                      TranslateErrors::Type error_type,
                      PrefService* prefs,
+                     const ShortcutConfiguration& shortcut_config,
                      const std::string& original_language,
                      const std::string& target_language);
 
@@ -140,10 +148,12 @@ class TranslateInfoBarDelegate : public InfoBarDelegate {
   bool ShouldShowMessageInfoBarButton();
 
   // Called by the before translate infobar to figure-out if it should show
-  // an extra button to let the user black-list/white-list that language (based
-  // on how many times the user accepted/declined translation).
-  bool ShouldShowNeverTranslateButton();
-  bool ShouldShowAlwaysTranslateButton();
+  // an extra shortcut to let the user black-list/white-list that language
+  // (based on how many times the user accepted/declined translation).
+  // The shortcut itself is platform specific, it can be a button or a new bar
+  // for example.
+  bool ShouldShowNeverTranslateShortcut();
+  bool ShouldShowAlwaysTranslateShortcut();
 
   // Sets this infobar background animation based on the previous infobar shown.
   // A fading background effect is used when transitioning from a normal state
@@ -170,6 +180,7 @@ class TranslateInfoBarDelegate : public InfoBarDelegate {
                            TranslateErrors::Type error_type,
                            InfoBarService* infobar_service,
                            PrefService* prefs,
+                           ShortcutConfiguration shortcut_config,
                            const std::string& original_language,
                            const std::string& target_language);
   Type infobar_type_;
@@ -219,6 +230,8 @@ class TranslateInfoBarDelegate : public InfoBarDelegate {
   // The translation related preferences.
   TranslatePrefs prefs_;
 
+  // Translation shortcut configuration
+  ShortcutConfiguration shortcut_config_;
   DISALLOW_COPY_AND_ASSIGN(TranslateInfoBarDelegate);
 };
 
diff --git a/chrome/browser/translate/translate_manager.cc b/chrome/browser/translate/translate_manager.cc
index 5f7248b..9bc976a 100644
--- a/chrome/browser/translate/translate_manager.cc
+++ b/chrome/browser/translate/translate_manager.cc
@@ -474,6 +474,7 @@ void TranslateManager::OnURLFetchComplete(const net::URLFetcher* source) {
             TranslateInfoBarDelegate::TRANSLATION_ERROR,
             TranslateErrors::NETWORK,
             profile->GetPrefs(),
+            ShortcutConfig(),
             request.source_lang,
             request.target_lang);
       } else {
@@ -577,7 +578,8 @@ void TranslateManager::InitiateTranslation(WebContents* web_contents,
   TranslateInfoBarDelegate::Create(
       InfoBarService::FromWebContents(web_contents), false,
       TranslateInfoBarDelegate::BEFORE_TRANSLATE, TranslateErrors::NONE,
-      profile->GetPrefs(), language_code, target_lang);
+      profile->GetPrefs(), ShortcutConfig(),
+      language_code, target_lang);
 }
 
 void TranslateManager::InitiateTranslationPosted(
@@ -607,10 +609,11 @@ void TranslateManager::TranslatePage(WebContents* web_contents,
 
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
+
   TranslateInfoBarDelegate::Create(
       InfoBarService::FromWebContents(web_contents), true,
       TranslateInfoBarDelegate::TRANSLATING, TranslateErrors::NONE,
-      profile->GetPrefs(), source_lang, target_lang);
+      profile->GetPrefs(), ShortcutConfig(), source_lang, target_lang);
 
   if (!translate_script_.empty()) {
     DoTranslatePage(web_contents, translate_script_, source_lang, target_lang);
@@ -715,7 +718,7 @@ void TranslateManager::PageTranslated(WebContents* web_contents,
       (details->error_type == TranslateErrors::NONE) ?
           TranslateInfoBarDelegate::AFTER_TRANSLATE :
           TranslateInfoBarDelegate::TRANSLATION_ERROR,
-      details->error_type, prefs, details->source_language,
+      details->error_type, prefs, ShortcutConfig(), details->source_language,
       details->target_language);
 }
 
@@ -891,3 +894,19 @@ std::string TranslateManager::GetTargetLanguage(PrefService* prefs) {
   }
   return std::string();
 }
+
+// static
+ShortcutConfiguration TranslateManager::ShortcutConfig() {
+  ShortcutConfiguration config;
+
+  // The android implementation does not offer a drop down for space
+  // reason so we are more aggressive showing the shortcuts for never translate.
+  #if defined(OS_ANDROID)
+  config.never_translate_min_count = 1;
+  #else
+  config.never_translate_min_count = 3;
+  #endif  // defined(OS_ANDROID)
+
+  config.always_translate_min_count = 3;
+  return config;
+}
diff --git a/chrome/browser/translate/translate_manager.h b/chrome/browser/translate/translate_manager.h
index 61c5e77..78b3a3f 100644
--- a/chrome/browser/translate/translate_manager.h
+++ b/chrome/browser/translate/translate_manager.h
@@ -25,6 +25,7 @@ template <typename T> struct DefaultSingletonTraits;
 class GURL;
 struct PageTranslatedDetails;
 class PrefService;
+struct ShortcutConfiguration;
 class TranslateInfoBarDelegate;
 
 namespace content {
@@ -176,6 +177,10 @@ class TranslateManager : public content::NotificationObserver,
   // If no language is found then an empty string is returned.
   static std::string GetTargetLanguage(PrefService* prefs);
 
+  // Returns the different parameters used to decide whether extra shortcuts
+  // are needed.
+  static ShortcutConfiguration ShortcutConfig();
+
   content::NotificationRegistrar notification_registrar_;
 
   // Each PrefChangeRegistrar only tracks a single PrefService, so a map from
diff --git a/chrome/browser/translate/translate_manager_browsertest.cc b/chrome/browser/translate/translate_manager_browsertest.cc
index 6c87f0f..1d4e4fc 100644
--- a/chrome/browser/translate/translate_manager_browsertest.cc
+++ b/chrome/browser/translate/translate_manager_browsertest.cc
@@ -1358,11 +1358,11 @@ TEST_F(TranslateManagerBrowserTest, BeforeTranslateExtraButtons) {
     EXPECT_EQ(TranslateInfoBarDelegate::BEFORE_TRANSLATE,
               infobar->infobar_type());
     if (i < 7) {
-      EXPECT_FALSE(infobar->ShouldShowAlwaysTranslateButton());
+      EXPECT_FALSE(infobar->ShouldShowAlwaysTranslateShortcut());
       infobar->Translate();
       process()->sink().ClearMessages();
     } else {
-      EXPECT_TRUE(infobar->ShouldShowAlwaysTranslateButton());
+      EXPECT_TRUE(infobar->ShouldShowAlwaysTranslateShortcut());
     }
     if (i == 3)
       test_profile->set_incognito(false);
@@ -1391,10 +1391,10 @@ TEST_F(TranslateManagerBrowserTest, BeforeTranslateExtraButtons) {
     EXPECT_EQ(TranslateInfoBarDelegate::BEFORE_TRANSLATE,
               infobar->infobar_type());
     if (i < 7) {
-      EXPECT_FALSE(infobar->ShouldShowNeverTranslateButton());
+      EXPECT_FALSE(infobar->ShouldShowNeverTranslateShortcut());
       infobar->TranslationDeclined();
     } else {
-      EXPECT_TRUE(infobar->ShouldShowNeverTranslateButton());
+      EXPECT_TRUE(infobar->ShouldShowNeverTranslateShortcut());
     }
     if (i == 3)
       test_profile->set_incognito(false);
diff --git a/chrome/browser/ui/cocoa/infobars/before_translate_infobar_controller.mm b/chrome/browser/ui/cocoa/infobars/before_translate_infobar_controller.mm
index 369d0f5..145b41e 100644
--- a/chrome/browser/ui/cocoa/infobars/before_translate_infobar_controller.mm
+++ b/chrome/browser/ui/cocoa/infobars/before_translate_infobar_controller.mm
@@ -96,10 +96,10 @@ NSButton* CreateNSButtonWithResourceIDAndParameter(
       label1_.get(), fromLanguagePopUp_.get(), label2_.get(),
       cancelButton_, okButton_, nil];
 
-  if ([self delegate]->ShouldShowNeverTranslateButton())
+  if ([self delegate]->ShouldShowNeverTranslateShortcut())
     [visibleControls addObject:neverTranslateButton_.get()];
 
-  if ([self delegate]->ShouldShowAlwaysTranslateButton())
+  if ([self delegate]->ShouldShowAlwaysTranslateShortcut())
     [visibleControls addObject:alwaysTranslateButton_.get()];
 
   return visibleControls;
diff --git a/chrome/browser/ui/cocoa/infobars/translate_infobar_unittest.mm b/chrome/browser/ui/cocoa/infobars/translate_infobar_unittest.mm
index b5493cc..91327c3 100644
--- a/chrome/browser/ui/cocoa/infobars/translate_infobar_unittest.mm
+++ b/chrome/browser/ui/cocoa/infobars/translate_infobar_unittest.mm
@@ -37,9 +37,10 @@ class MockTranslateInfoBarDelegate : public TranslateInfoBarDelegate {
   MockTranslateInfoBarDelegate(TranslateInfoBarDelegate::Type type,
                                TranslateErrors::Type error,
                                InfoBarService* infobar_service,
-                               PrefService* prefs)
+                               PrefService* prefs,
+                               ShortcutConfiguration config)
       : TranslateInfoBarDelegate(type, error, infobar_service, prefs,
-                                 "en", "es") {
+                                 config, "en", "es") {
   }
 
   MOCK_METHOD0(Translate, void());
@@ -79,11 +80,15 @@ class TranslationInfoBarTest : public CocoaProfileTest {
         InfoBarService::FromWebContents(web_contents_.get());
     Profile* profile =
         Profile::FromBrowserContext(web_contents_->GetBrowserContext());
+    ShortcutConfiguration config;
+    config.never_translate_min_count = 3;
+    config.always_translate_min_count = 3;
     infobar_delegate_.reset(new MockTranslateInfoBarDelegate(
         type,
         error,
         infobar_service,
-        profile->GetPrefs()));
+        profile->GetPrefs(),
+        config));
     [[infobar_controller_ view] removeFromSuperview];
     scoped_ptr<InfoBar> infobar(
         static_cast<InfoBarDelegate*>(infobar_delegate_.get())->
diff --git a/chrome/browser/ui/gtk/infobars/before_translate_infobar_gtk.cc b/chrome/browser/ui/gtk/infobars/before_translate_infobar_gtk.cc
index 3cf2b45..cd2bef3 100644
--- a/chrome/browser/ui/gtk/infobars/before_translate_infobar_gtk.cc
+++ b/chrome/browser/ui/gtk/infobars/before_translate_infobar_gtk.cc
@@ -61,7 +61,7 @@ void BeforeTranslateInfoBar::InitWidgets() {
   gtk_box_pack_start(GTK_BOX(hbox), button, FALSE, FALSE, 0);
 
   TranslateInfoBarDelegate* delegate = GetDelegate();
-  if (delegate->ShouldShowNeverTranslateButton()) {
+  if (delegate->ShouldShowNeverTranslateShortcut()) {
     std::string label = l10n_util::GetStringFUTF8(
         IDS_TRANSLATE_INFOBAR_NEVER_TRANSLATE,
         delegate->language_name_at(delegate->original_language_index()));
@@ -71,7 +71,7 @@ void BeforeTranslateInfoBar::InitWidgets() {
     gtk_box_pack_start(GTK_BOX(hbox), button, FALSE, FALSE, 0);
   }
 
-  if (delegate->ShouldShowAlwaysTranslateButton()) {
+  if (delegate->ShouldShowAlwaysTranslateShortcut()) {
     std::string label = l10n_util::GetStringFUTF8(
         IDS_TRANSLATE_INFOBAR_ALWAYS_TRANSLATE,
         delegate->language_name_at(delegate->original_language_index()));
diff --git a/chrome/browser/ui/views/infobars/before_translate_infobar.cc b/chrome/browser/ui/views/infobars/before_translate_infobar.cc
index bcd258c2..955c4f79 100644
--- a/chrome/browser/ui/views/infobars/before_translate_infobar.cc
+++ b/chrome/browser/ui/views/infobars/before_translate_infobar.cc
@@ -119,13 +119,13 @@ void BeforeTranslateInfoBar::ViewHierarchyChanged(bool is_add,
 
   const string16& language(
       delegate->language_name_at(delegate->original_language_index()));
-  if (delegate->ShouldShowNeverTranslateButton()) {
-    DCHECK(!delegate->ShouldShowAlwaysTranslateButton());
+  if (delegate->ShouldShowNeverTranslateShortcut()) {
+    DCHECK(!delegate->ShouldShowAlwaysTranslateShortcut());
     never_translate_button_ = CreateLabelButton(this,
         l10n_util::GetStringFUTF16(IDS_TRANSLATE_INFOBAR_NEVER_TRANSLATE,
                                    language), false);
     AddChildView(never_translate_button_);
-  } else if (delegate->ShouldShowAlwaysTranslateButton()) {
+  } else if (delegate->ShouldShowAlwaysTranslateShortcut()) {
     always_translate_button_ = CreateLabelButton(this,
         l10n_util::GetStringFUTF16(IDS_TRANSLATE_INFOBAR_ALWAYS_TRANSLATE,
                                    language), false);