Fix Translate bar crashers

Disable the options menu divider so we don't fire an invalid command.

the translate bar was crashing when you would use
the context menu to translate a page with an unknown language.  It was trying to
instantiate an options menu model when it didn't need one, or have a language to
populate it with.  This makes it so the options menu model is not instantiated
unless it needs it, and it only needs it when it actually knows the language.

BUG=47303
TEST=see bug.

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

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

7 files changed, 60 insertions, 43 deletions

diff --git a/chrome/browser/cocoa/translate/translate_infobar_base.h b/chrome/browser/cocoa/translate/translate_infobar_base.h
index 5f8199a..b23250f 100644
--- a/chrome/browser/cocoa/translate/translate_infobar_base.h
+++ b/chrome/browser/cocoa/translate/translate_infobar_base.h
@@ -59,7 +59,10 @@ void AddMenuItem(NSMenu *menu, id target, SEL selector, NSString* title,
   scoped_nsobject<NSPopUpButton> toLanguagePopUp_;
   scoped_nsobject<NSPopUpButton> optionsPopUp_;
   scoped_nsobject<NSButton> showOriginalButton_;
-  scoped_nsobject<NSButton> tryAgainButton_;
+  // This is the button used in the translate message infobar.  It can either be
+  // a "Try Again" button, or a "Show Original" button in the case that the
+  // page was translated from an unknown language.
+  scoped_nsobject<NSButton> translateMessageButton_;
 
   // In the current locale, are the "from" and "to" language popup menu
   // flipped from what they'd appear in English.
@@ -129,6 +132,9 @@ void AddMenuItem(NSMenu *menu, id target, SEL selector, NSString* title,
 // to be empty.
 - (void)rebuildOptionsMenu:(BOOL)hideTitle;
 
+// Whether or not this infobar should show the options popup.
+- (BOOL)shouldShowOptionsPopUp;
+
 @end // TranslateInfoBarControllerBase (ProtectedAPI)
 
 #pragma mark TestingAPI
@@ -147,8 +153,9 @@ void AddMenuItem(NSMenu *menu, id target, SEL selector, NSString* title,
 // Returns the underlying options menu.
 - (NSMenu*)optionsMenu;
 
-// Returns the "try again" button.
-- (NSButton*)tryAgainButton;
+// Returns |translateMessageButton_|, see declaration of member
+// variable for a full description.
+- (NSButton*)translateMessageButton;
 
 @end // TranslateInfoBarControllerBase (TestingAPI)
 
diff --git a/chrome/browser/cocoa/translate/translate_infobar_base.mm b/chrome/browser/cocoa/translate/translate_infobar_base.mm
index e267074..b396de8 100644
--- a/chrome/browser/cocoa/translate/translate_infobar_base.mm
+++ b/chrome/browser/cocoa/translate/translate_infobar_base.mm
@@ -83,17 +83,21 @@ NSTextField* CreateLabel(NSRect bounds) {
 // Adds an item with the specified properties to |menu|.
 void AddMenuItem(NSMenu *menu, id target, SEL selector, NSString* title,
     int tag, bool enabled, bool checked) {
-  NSMenuItem* item = [[[NSMenuItem alloc]
+  if (tag == -1) {
+    [menu addItem:[NSMenuItem separatorItem]];
+  } else {
+    NSMenuItem* item = [[[NSMenuItem alloc]
       initWithTitle:title
              action:selector
       keyEquivalent:@""] autorelease];
-  [item setTag:tag];
-  [menu addItem:item];
-  [item setTarget:target];
-  if (checked)
-    [item setState:NSOnState];
-  if (!enabled)
-    [item setEnabled:NO];
+    [item setTag:tag];
+    [menu addItem:item];
+    [item setTarget:target];
+    if (checked)
+      [item setState:NSOnState];
+    if (!enabled)
+      [item setEnabled:NO];
+  }
 }
 
 }  // namespace TranslateInfoBarUtilities
@@ -178,7 +182,6 @@ InfoBar* TranslateInfoBarDelegate::CreateInfoBar() {
       targetLanguageMenuModel_.reset(
           new LanguagesMenuModel([self delegate],
                                  LanguagesMenuModel::TARGET));
-      optionsMenuModel_.reset(new OptionsMenuModel([self delegate]));
   }
   return self;
 }
@@ -202,8 +205,8 @@ InfoBar* TranslateInfoBarDelegate::CreateInfoBar() {
                                                       pullsDown:NO]);
   toLanguagePopUp_.reset([[NSPopUpButton alloc] initWithFrame:bogusFrame
                                                      pullsDown:NO]);
-  showOriginalButton_.reset([[NSButton alloc] initWithFrame:bogusFrame]);
-  tryAgainButton_.reset([[NSButton alloc] initWithFrame:bogusFrame]);
+  showOriginalButton_.reset([[NSButton alloc] init]);
+  translateMessageButton_.reset([[NSButton alloc] init]);
 }
 
 - (void)sourceLanguageModified:(NSInteger)newLanguageIdx {
@@ -234,6 +237,7 @@ InfoBar* TranslateInfoBarDelegate::CreateInfoBar() {
   [self loadLabelText];
   [self clearAllControls];
   [self showVisibleControls:[self visibleControls]];
+  [optionsPopUp_ setHidden:![self shouldShowOptionsPopUp]];
   [self layout];
   [self adjustOptionsButtonSizeAndVisibilityForView:
       [[self visibleControls] lastObject]];
@@ -304,6 +308,9 @@ InfoBar* TranslateInfoBarDelegate::CreateInfoBar() {
 }
 
 - (void)rebuildOptionsMenu:(BOOL)hideTitle {
+  if (![self shouldShowOptionsPopUp])
+     return;
+
   // The options model doesn't know how to handle state transitions, so rebuild
   // it each time through here.
   optionsMenuModel_.reset(
@@ -334,6 +341,10 @@ InfoBar* TranslateInfoBarDelegate::CreateInfoBar() {
   }
 }
 
+- (BOOL)shouldShowOptionsPopUp {
+  return YES;
+}
+
 - (void)populateLanguageMenus {
   NSMenu* originalLanguageMenu = [fromLanguagePopUp_ menu];
   [originalLanguageMenu setAutoenablesItems:NO];
@@ -415,7 +426,6 @@ InfoBar* TranslateInfoBarDelegate::CreateInfoBar() {
 
   // Set up "Show original" and "Try again" buttons.
   [showOriginalButton_ setFrame:okButtonFrame];
-  [tryAgainButton_ setFrame:okButtonFrame];
 
   // Set each of the buttons and popups to the NSTexturedRoundedBezelStyle
   // (metal-looking) style.
@@ -436,13 +446,11 @@ InfoBar* TranslateInfoBarDelegate::CreateInfoBar() {
 
   [showOriginalButton_ setTarget:self];
   [showOriginalButton_ setAction:@selector(showOriginal:)];
-  [tryAgainButton_ setTarget:self];
-  [tryAgainButton_ setAction:@selector(ok:)];
+  [translateMessageButton_ setTarget:self];
+  [translateMessageButton_ setAction:@selector(messageButtonPressed:)];
 
   [showOriginalButton_
       setTitle:GetNSStringWithFixup(IDS_TRANSLATE_INFOBAR_REVERT)];
-  [tryAgainButton_
-      setTitle:GetNSStringWithFixup(IDS_TRANSLATE_INFOBAR_RETRY)];
 
   // Add and configure controls that are visible in all modes.
   [optionsPopUp_ setAutoresizingMask:NSViewMinXMargin | NSViewMinYMargin |
@@ -505,6 +513,10 @@ InfoBar* TranslateInfoBarDelegate::CreateInfoBar() {
   [super dismiss:nil];
 }
 
+- (void)messageButtonPressed:(id)sender {
+  [self delegate]->MessageInfoBarButtonPressed();
+}
+
 - (IBAction)showOriginal:(id)sender {
   [self delegate]->RevertTranslation();
 }
@@ -559,7 +571,7 @@ InfoBar* TranslateInfoBarDelegate::CreateInfoBar() {
 - (NSArray*)allControls {
   return [NSArray arrayWithObjects:label1_.get(),fromLanguagePopUp_.get(),
       label2_.get(), toLanguagePopUp_.get(), label3_.get(), okButton_,
-      cancelButton_, showOriginalButton_.get(), tryAgainButton_.get(),
+      cancelButton_, showOriginalButton_.get(), translateMessageButton_.get(),
       nil];
 }
 
@@ -567,8 +579,8 @@ InfoBar* TranslateInfoBarDelegate::CreateInfoBar() {
   return [optionsPopUp_ menu];
 }
 
-- (NSButton*)tryAgainButton {
-  return tryAgainButton_.get();
+- (NSButton*)translateMessageButton {
+  return translateMessageButton_.get();
 }
 
 - (bool)verifyLayout {
diff --git a/chrome/browser/cocoa/translate/translate_infobar_unittest.mm b/chrome/browser/cocoa/translate/translate_infobar_unittest.mm
index 8065b40..5ae1c15 100644
--- a/chrome/browser/cocoa/translate/translate_infobar_unittest.mm
+++ b/chrome/browser/cocoa/translate/translate_infobar_unittest.mm
@@ -154,7 +154,8 @@ TEST_F(TranslationInfoBarTest, OptionsMenuItemsHookedUp) {
   // that the target on that is setup correctly.
   for (NSUInteger i = 1; i < [optionsMenuItems count]; ++i) {
     NSMenuItem* item = [optionsMenuItems objectAtIndex:i];
-    EXPECT_EQ([item target], infobar_controller.get());
+    if (![item isSeparatorItem])
+      EXPECT_EQ([item target], infobar_controller.get());
   }
   NSMenuItem* alwaysTranslateLanguateItem = [optionsMenuItems objectAtIndex:1];
   NSMenuItem* neverTranslateLanguateItem = [optionsMenuItems objectAtIndex:2];
diff --git a/chrome/browser/cocoa/translate/translate_message_infobar_controller.h b/chrome/browser/cocoa/translate/translate_message_infobar_controller.h
index ddb7110..985bf2a 100644
--- a/chrome/browser/cocoa/translate/translate_message_infobar_controller.h
+++ b/chrome/browser/cocoa/translate/translate_message_infobar_controller.h
@@ -5,9 +5,6 @@
 #import "chrome/browser/cocoa/translate/translate_infobar_base.h"
 
 @interface TranslateMessageInfobarController : TranslateInfoBarControllerBase {
-  // This keeps track of whether the infobar is displaying a message or an
-  // error. If it is an error it should have a try again button.
-  TranslateInfoBarDelegate::Type state_;
 }
 
 @end
diff --git a/chrome/browser/cocoa/translate/translate_message_infobar_controller.mm b/chrome/browser/cocoa/translate/translate_message_infobar_controller.mm
index 5b4fcaf..1146fdc 100644
--- a/chrome/browser/cocoa/translate/translate_message_infobar_controller.mm
+++ b/chrome/browser/cocoa/translate/translate_message_infobar_controller.mm
@@ -10,24 +10,15 @@ using TranslateInfoBarUtilities::MoveControl;
 
 @implementation TranslateMessageInfobarController
 
-- (id)initWithDelegate:(InfoBarDelegate*)delegate {
-  if ((self = [super initWithDelegate:delegate])) {
-    TranslateInfoBarDelegate* delegate = [self delegate];
-    if (delegate->IsError())
-      state_ = TranslateInfoBarDelegate::TRANSLATION_ERROR;
-    else
-      state_ = TranslateInfoBarDelegate::TRANSLATING;
-  }
-  return self;
-}
-
 - (void)layout {
-  [optionsPopUp_ setHidden:YES];
   [self removeOkCancelButtons];
-  MoveControl(label1_, tryAgainButton_, spaceBetweenControls_ * 2, true);
+  MoveControl(label1_, translateMessageButton_, spaceBetweenControls_ * 2, true);
   TranslateInfoBarDelegate* delegate = [self delegate];
-  if (delegate->IsError())
-    MoveControl(label1_, tryAgainButton_, spaceBetweenControls_ * 2, true);
+  if ([self delegate]->ShouldShowMessageInfoBarButton()) {
+    string16 buttonText = delegate->GetMessageInfoBarButtonText();
+    [translateMessageButton_ setTitle:base::SysUTF16ToNSString(buttonText)];
+    [translateMessageButton_ sizeToFit];
+  }
 }
 
 - (void)adjustOptionsButtonSizeAndVisibilityForView:(NSView*)lastView {
@@ -37,8 +28,8 @@ using TranslateInfoBarUtilities::MoveControl;
 - (NSArray*)visibleControls {
   NSMutableArray* visibleControls =
       [NSMutableArray arrayWithObjects:label1_.get(), nil];
-  if (state_ == TranslateInfoBarDelegate::TRANSLATION_ERROR)
-    [visibleControls addObject:tryAgainButton_];
+  if ([self delegate]->ShouldShowMessageInfoBarButton())
+    [visibleControls addObject:translateMessageButton_];
   return visibleControls;
 }
 
@@ -55,4 +46,8 @@ using TranslateInfoBarUtilities::MoveControl;
   return [super verifyLayout];
 }
 
+- (BOOL)shouldShowOptionsPopUp {
+  return NO;
+}
+
 @end
diff --git a/chrome/browser/translate/translate_infobar_delegate.cc b/chrome/browser/translate/translate_infobar_delegate.cc
index 0a78e8f..08bbddd 100644
--- a/chrome/browser/translate/translate_infobar_delegate.cc
+++ b/chrome/browser/translate/translate_infobar_delegate.cc
@@ -331,6 +331,10 @@ void TranslateInfoBarDelegate::MessageInfoBarButtonPressed() {
       tab_contents_, GetOriginalLanguageCode(), GetTargetLanguageCode());
 }
 
+bool TranslateInfoBarDelegate::ShouldShowMessageInfoBarButton() {
+  return !GetMessageInfoBarButtonText().empty();
+}
+
 bool TranslateInfoBarDelegate::ShouldShowNeverTranslateButton() {
   DCHECK(type_ == BEFORE_TRANSLATE);
   return prefs_.GetTranslationDeniedCount(GetOriginalLanguageCode()) >= 3;
diff --git a/chrome/browser/translate/translate_infobar_delegate.h b/chrome/browser/translate/translate_infobar_delegate.h
index 44bcf5b..0359caf 100644
--- a/chrome/browser/translate/translate_infobar_delegate.h
+++ b/chrome/browser/translate/translate_infobar_delegate.h
@@ -125,6 +125,7 @@ class TranslateInfoBarDelegate : public InfoBarDelegate {
   string16 GetMessageInfoBarText();
   string16 GetMessageInfoBarButtonText();
   void MessageInfoBarButtonPressed();
+  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