fixed the distorted behavior on the mediastreaminfobar on Mac

chrome/browser/ui/cocoa/infobars/media_stream_infobar_controller.mm

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "chrome/browser/ui/cocoa/infobars/media_stream_infobar_controller.h"
 
 #import <Cocoa/Cocoa.h>
 #include <string>
 
 #import "base/sys_string_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/media/media_stream_devices_menu_model.h"
 #include "chrome/browser/ui/cocoa/hover_close_button.h"
 #include "chrome/browser/ui/cocoa/infobars/infobar.h"
+#import "chrome/browser/ui/cocoa/infobars/infobar_utilities.h"
 #include "chrome/browser/ui/media_stream_infobar_delegate.h"
 #include "grit/generated_resources.h"
 #import "third_party/GTM/AppKit/GTMUILocalizerAndLayoutTweaker.h"
 #include "ui/base/l10n/l10n_util.h"
 
+using InfoBarUtilities::CreateLabel;
+using InfoBarUtilities::MoveControl;
+using InfoBarUtilities::VerifyControlOrderAndSpacing;
 using l10n_util::GetNSStringWithFixup;
 
-static const CGFloat kSpaceBetweenControls = 8;
-
 InfoBar* MediaStreamInfoBarDelegate::CreateInfoBar(InfoBarTabHelper* owner) {
   MediaStreamInfoBarController* infobar_controller =
       [[MediaStreamInfoBarController alloc] initWithDelegate:this
                                                        owner:owner];
   return new InfoBar(infobar_controller, this);
 }
 
-namespace {
-
-// Puts |toMove| to the right or left of |anchor|.
-void SizeAndPlaceControl(NSView* toModify, NSView* anchor, bool after) {
-  [GTMUILocalizerAndLayoutTweaker sizeToFitView:toModify];
-  NSRect toModifyFrame = [toModify frame];
-  NSRect anchorFrame = [anchor frame];
-  if (after) {
-    toModifyFrame.origin.x = NSMaxX(anchorFrame) + kSpaceBetweenControls;
-  } else {
-    toModifyFrame.origin.x = NSMinX(anchorFrame) - kSpaceBetweenControls -
-        NSWidth(toModifyFrame);
-  }
-  [toModify setFrame:toModifyFrame];
-}
-
-}  // namespace
-
 
 @interface MediaStreamInfoBarController(Private)
 
 // Adds text for all buttons and text fields.
 - (void)setLabelTexts;
 
 // Populates the device menu with device id:s.
 - (void)rebuildDeviceMenu;
 
 // Arranges all buttons and pup-up menu relative each other.
 - (void)arrangeInfobarLayout;
 
+// Create all the components for the infobar.
+- (void)constructView;
+
+- (void)showDeviceMenuTitle:(BOOL)showTitle;
 @end
 
 
 @implementation MediaStreamInfoBarController
 
 - (id)initWithDelegate:(MediaStreamInfoBarDelegate*)delegate
                  owner:(InfoBarTabHelper*)owner {
   if (self = [super initWithDelegate:delegate owner:owner]) {
     deviceMenuModel_.reset(new MediaStreamDevicesMenuModel(delegate));
-
-    label1_.reset([[NSTextField alloc] init]);
-    [label1_ setEditable:NO];
-    [label1_ setDrawsBackground:NO];
-    [label1_ setBordered:NO];
-
-    [okButton_ setBezelStyle:NSTexturedRoundedBezelStyle];
-    [cancelButton_ setBezelStyle:NSTexturedRoundedBezelStyle];
-
-    deviceMenu_.reset([[NSPopUpButton alloc] init]);
-    [deviceMenu_ setBezelStyle:NSTexturedRoundedBezelStyle];
-    [deviceMenu_ setAutoresizingMask:NSViewMaxXMargin];
-    [[deviceMenu_ cell] setArrowPosition:NSPopUpArrowAtBottom];
-    NSMenu* menu = [deviceMenu_ menu];
-    [menu setDelegate:nil];
-    [menu setAutoenablesItems:NO];
   }
   return self;
 }
 
 - (void)dealloc {
   [[NSNotificationCenter defaultCenter] removeObserver:self];
   [deviceMenu_ removeAllItems];
   [super dealloc];
 }
 
@@ skipping 21 matching lines @@
   [super removeSelf];
 }
 
 - (IBAction)deviceMenuChanged:(id)item {
   // Notify the menu model about the change and rebuild the menu.
   deviceMenuModel_->ExecuteCommand([item tag]);
   [self rebuildDeviceMenu];
 }
 
 - (void)addAdditionalControls {
-  // Get a frame to use as inital frame for all buttons.
-  NSRect initFrame = [okButton_ frame];
+  // Create all the components for the infobar.
+  [self constructView];
 
-  // Setup up the text label and add the device menu to the infobar.
-  // TODO(mflodman) Use |label_| instead.
-  [label1_ setFrame:[label_ frame]];
-  [[label_ superview] replaceSubview:label_ with:label1_.get()];
-  label_.reset();
+  // Get layout information from the NIB.
+  NSRect cancelButtonFrame = [cancelButton_ frame];
+  NSRect okButtonFrame = [okButton_ frame];
 
-  [deviceMenu_ setFrame:initFrame];
-  [infoBarView_ addSubview:deviceMenu_];
-
-  // Add text to all buttons and the text field.
-  [self setLabelTexts];
+  // Verify Cancel button is on the left of OK button.
+  DCHECK(NSMaxX(cancelButtonFrame) < NSMinX(okButtonFrame));

[no longer working on chromium, 2012/07/27 10:09:56]

We don't allow overlap between the buttons. So it is definitely a good idea to
add a DCHECK.
Do you know if it is all right to add a DCHECK like this here? I am not sure how
the OS knows cancel button is on the left of the ok button, I suppose
arrangeInfobarLayout has not been called yet.

[Scott Hess - ex-Googler, 2012/07/27 16:35:20]

The nib file controls this, the OS doesn't care.  So the DCHECK makes sure that
someone doesn't edit the nib file to put the controls in the other order, or
overlap them.  I don't think it should ever just re-arrange things willy-nilly,
so I think this is quite sufficient.

[no longer working on chromium, 2012/07/30 11:36:31]

Done.

+  spaceBetweenControls_ =  NSMinX(okButtonFrame) - NSMaxX(cancelButtonFrame);
 
   // Arrange the initial layout.
   [self arrangeInfobarLayout];
 
-  // Build the device popup menu.
-  [self rebuildDeviceMenu];
-
   // Make sure we get notified of infobar view size changes.
   // TODO(mflodman) Find if there is a way to use 'setAutorezingMask' instead.
   [infoBarView_ setPostsFrameChangedNotifications:YES];
   [[NSNotificationCenter defaultCenter]
       addObserver:self
          selector:@selector(didChangeFrame:)
              name:NSViewFrameDidChangeNotification
            object:infoBarView_];
 }
 
@@ skipping 23 matching lines @@
   [okButton_ setTitle:GetNSStringWithFixup(IDS_MEDIA_CAPTURE_ALLOW)];
   [cancelButton_ setTitle:GetNSStringWithFixup(IDS_MEDIA_CAPTURE_DENY)];
 
   // Populating |deviceMenu_| is handled separately.
 }
 
 - (void)arrangeInfobarLayout {
   // Set correct size for text frame.
   [GTMUILocalizerAndLayoutTweaker sizeToFitView:label1_];
 
-  // Place |okButton_| to the right of the text field.
-  SizeAndPlaceControl(okButton_, label1_, true);
+  // From left to right: label_, cancelButton_ and okButton_.
+  MoveControl(label1_, cancelButton_, spaceBetweenControls_, true);
+  MoveControl(cancelButton_, okButton_, spaceBetweenControls_, true);
 
-  // Place |cancelButton| to the right of [okButton_|.
-  SizeAndPlaceControl(cancelButton_, okButton_, true);
+  // Build the device option popup menu.
+  [deviceMenu_ setHidden:NO];
+  [self showDeviceMenuTitle:YES];
+  [GTMUILocalizerAndLayoutTweaker sizeToFitView:deviceMenu_];
+  MoveControl(closeButton_, deviceMenu_, spaceBetweenControls_, false);
 
-  // |deviceMenu_| is floating to the right, besides |closeButton_|.
-  SizeAndPlaceControl(deviceMenu_, closeButton_, false);
+  // Hide the title of deviceMenu_ or even the whole menu when it overlaps
+  // with the okButton_.
+  if (!VerifyControlOrderAndSpacing(okButton_, deviceMenu_))  {
+    [self showDeviceMenuTitle:NO];
+    [GTMUILocalizerAndLayoutTweaker sizeToFitView:deviceMenu_];

[Scott Hess - ex-Googler, 2012/07/27 16:35:20]

Maybe move this line (and the occurance above) right into -showDeviceMenuTitle:?

[no longer working on chromium, 2012/07/30 11:36:31]

Done.

+    MoveControl(closeButton_, deviceMenu_, spaceBetweenControls_, false);
+    if (!VerifyControlOrderAndSpacing(okButton_, deviceMenu_)) {
+      [deviceMenu_ setHidden:YES];
+    }
+  }
+}
+
+- (void)constructView {
+  // Use the ok button frame as inital frame for all components.
+  NSRect initFrame = [okButton_ frame];
+
+  // Setup the text label and add the device menu to the infobar.
+  // TODO(mflodman) Use |label_| instead.
+  label1_.reset([[NSTextField alloc] init]);
+  [label1_ setEditable:NO];
+  [label1_ setDrawsBackground:NO];
+  [label1_ setBordered:NO];
+  [label1_ setFrame:[label_ frame]];
+  [[label_ superview] replaceSubview:label_ with:label1_];
+  label_.reset();
+
+  [okButton_ setBezelStyle:NSTexturedRoundedBezelStyle];
+  [cancelButton_ setBezelStyle:NSTexturedRoundedBezelStyle];
+
+  // Add text to all buttons and the text field.
+  [self setLabelTexts];
+
+  // Setup the device options menu.
+  deviceMenu_.reset([[NSPopUpButton alloc] initWithFrame:initFrame
+                                               pullsDown:YES]);
+  [self rebuildDeviceMenu];
+  [GTMUILocalizerAndLayoutTweaker sizeToFitView:deviceMenu_];

[Scott Hess - ex-Googler, 2012/07/27 16:35:20]

I would put the setBezelStyle: and setAutoresizingMask: lines above the
-rebuildDeviceMenu line, and definitely above the -sizeToFitView: line.  I would
consider them in the might-or-might-not-modify sizing class of methods.

[no longer working on chromium, 2012/07/30 11:36:31]

Done.

+  [deviceMenu_ setBezelStyle:NSTexturedRoundedBezelStyle];
+  [deviceMenu_ setAutoresizingMask:NSViewMinXMargin];
+  NSMenu* menu = [deviceMenu_ menu];
+  [menu setDelegate:nil];
+  [menu setAutoenablesItems:NO];
+
+  // Add "options" popup z-ordered below all other controls so when we
+  // resize the toolbar it doesn't hide them.
+  [infoBarView_ addSubview:deviceMenu_
+                positioned:NSWindowBelow
+                relativeTo:nil];
 }
 
 - (void)rebuildDeviceMenu {
   // Remove all old menu items and rebuild from scratch.
   [deviceMenu_ removeAllItems];
   NSMenu* menu = [deviceMenu_ menu];
 
   // Add title item.
   NSString* menuTitle = GetNSStringWithFixup(
       IDS_MEDIA_CAPTURE_DEVICES_MENU_TITLE);
@@ skipping 16 matching lines @@
                                          action:@selector(deviceMenuChanged:)
                                   keyEquivalent:@""];
       [item setTarget:self];
       [item setTag:deviceMenuModel_->GetCommandIdAt(i)];
       if (deviceMenuModel_->IsItemCheckedAt(i)) {
         [item setState:NSOnState];
       }
       [item setEnabled:deviceMenuModel_->IsEnabledAt(i)];
     }
   }
-  [GTMUILocalizerAndLayoutTweaker sizeToFitView:deviceMenu_];
+}
+
+- (void)showDeviceMenuTitle:(BOOL)showTitle {
+  if (showTitle) {
+    NSString* menuTitle = GetNSStringWithFixup(
+        IDS_MEDIA_CAPTURE_DEVICES_MENU_TITLE);
+    [[[deviceMenu_ menu] itemAtIndex:0] setTitle:menuTitle];
+    [[deviceMenu_ cell] setArrowPosition:NSPopUpArrowAtBottom];
+  } else {
+    [[[deviceMenu_ menu] itemAtIndex:0] setTitle:@""];
+    [[deviceMenu_ cell] setArrowPosition:NSPopUpArrowAtCenter];
+  }
 }
 
 - (void)didChangeFrame:(NSNotification*)notification {
   [self arrangeInfobarLayout];
 }
 
 @end  // implementation MediaStreamInfoBarController