Remove the Allow/Block drop-down from grouped permission requests for cocoa

chrome/browser/ui/cocoa/permission_bubble/permission_bubble_controller.mm

Index: chrome/browser/ui/cocoa/permission_bubble/permission_bubble_controller.mm
diff --git a/chrome/browser/ui/cocoa/permission_bubble/permission_bubble_controller.mm b/chrome/browser/ui/cocoa/permission_bubble/permission_bubble_controller.mm
index 80539cffb81fdb02fc028201b52f5fe53ed24543..6314ab26005b1e34c3bb24bcbe7b520fd19743e7 100644
--- a/chrome/browser/ui/cocoa/permission_bubble/permission_bubble_controller.mm
+++ b/chrome/browser/ui/cocoa/permission_bubble/permission_bubble_controller.mm
@@ -31,7 +31,6 @@
 #include "chrome/browser/ui/exclusive_access/exclusive_access_context.h"
 #include "chrome/browser/ui/exclusive_access/exclusive_access_manager.h"
 #include "chrome/browser/ui/exclusive_access/fullscreen_controller.h"
-#include "chrome/browser/ui/page_info/permission_menu_model.h"
 #include "chrome/browser/ui/permission_bubble/permission_prompt.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/grit/generated_resources.h"
@@ -43,10 +42,8 @@
 #include "ui/base/cocoa/a11y_util.h"
 #include "ui/base/cocoa/cocoa_base_utils.h"
 #import "ui/base/cocoa/controls/hyperlink_text_view.h"
-#import "ui/base/cocoa/menu_controller.h"
 #include "ui/base/cocoa/window_size_constants.h"
 #include "ui/base/l10n/l10n_util_mac.h"
-#include "ui/base/models/simple_menu_model.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/gfx/image/image_skia_util_mac.h"
 #include "ui/gfx/paint_vector_icon.h"
@@ -72,75 +69,6 @@ const NSSize kPermissionIconSize = {18, 18};
 
 }  // namespace
 
-// NSPopUpButton with a menu containing two items: allow and block.
-// One AllowBlockMenuButton is used for each requested permission when there are
-// multiple permissions in the bubble.
-@interface AllowBlockMenuButton : NSPopUpButton {
- @private
-  std::unique_ptr<PermissionMenuModel> menuModel_;
-  base::scoped_nsobject<MenuController> menuController_;
-}
-
-- (id)initForURL:(const GURL&)url
-         allowed:(BOOL)allow
-           index:(int)index
-        delegate:(PermissionPrompt::Delegate*)delegate
-         profile:(Profile*)profile;
-
-// Returns the maximum width of its possible titles.
-- (CGFloat)maximumTitleWidth;
-@end
-
-@implementation AllowBlockMenuButton
-
-- (id)initForURL:(const GURL&)url
-         allowed:(BOOL)allow
-           index:(int)index
-        delegate:(PermissionPrompt::Delegate*)delegate
-         profile:(Profile*)profile {
-  if (self = [super initWithFrame:NSZeroRect pullsDown:NO]) {
-    ContentSetting setting =
-        allow ? CONTENT_SETTING_ALLOW : CONTENT_SETTING_BLOCK;
-    [self setFont:[NSFont systemFontOfSize:[NSFont smallSystemFontSize]]];
-    [self setBordered:NO];
-
-    __block PermissionPrompt::Delegate* blockDelegate = delegate;
-    __block AllowBlockMenuButton* blockSelf = self;
-    PermissionMenuModel::ChangeCallback changeCallback =
-        base::BindBlock(^(const PageInfoUI::PermissionInfo& permission) {
-          blockDelegate->ToggleAccept(
-              index, permission.setting == CONTENT_SETTING_ALLOW);
-          [blockSelf setFrameSize:SizeForPageInfoButtonTitle(
-                                      blockSelf, [blockSelf title])];
-        });
-
-    menuModel_.reset(
-        new PermissionMenuModel(profile, url, setting, changeCallback));
-    menuController_.reset([[MenuController alloc] initWithModel:menuModel_.get()
-                                         useWithPopUpButtonCell:NO]);
-    [self setMenu:[menuController_ menu]];
-    [self selectItemAtIndex:menuModel_->GetIndexOfCommandId(setting)];
-    // Although the frame is reset, below, this sizes the cell properly.
-    [self sizeToFit];
-    // Adjust the size to fit the current title.  Using only -sizeToFit leaves
-    // an ugly amount of whitespace between the title and the arrows because it
-    // will fit to the largest element in the menu, not just the selected item.
-    [self setFrameSize:SizeForPageInfoButtonTitle(self, [self title])];
-  }
-  return self;
-}
-
-- (CGFloat)maximumTitleWidth {
-  CGFloat maxTitleWidth = 0;
-  for (NSMenuItem* item in [self itemArray]) {
-    NSSize size = SizeForPageInfoButtonTitle(self, [item title]);
-    maxTitleWidth = std::max(maxTitleWidth, size.width);
-  }
-  return maxTitleWidth;
-}
-
-@end
-
 @interface PermissionBubbleController ()
 
 // Determines if the bubble has an anchor in a corner or no anchor at all.
@@ -156,12 +84,6 @@ const NSSize kPermissionIconSize = {18, 18};
 // requesting settings for |host|.
 - (NSView*)titleWithOrigin:(const GURL&)origin;
 
-// Returns an autoreleased NSView displaying a menu for |request|.  The
-// menu will be initialized as 'allow' if |allow| is YES.
-- (NSView*)menuForRequest:(PermissionRequest*)request
-                  atIndex:(int)index
-                    allow:(BOOL)allow;
-
 // Returns an autoreleased NSView of a button with |title| and |action|.
 - (NSView*)buttonWithTitle:(NSString*)title
                     action:(SEL)action;
@@ -291,24 +213,11 @@ const NSSize kPermissionIconSize = {18, 18};
   NSView* contentView = [[self window] contentView];
   [contentView setSubviews:@[]];
 
-  BOOL singlePermission = requests.size() == 1;
-
   // Create one button to use as a guide for the permissions' y-offsets.
-  base::scoped_nsobject<NSView> allowOrOkButton;
-  if (singlePermission) {
-    NSString* allowTitle = l10n_util::GetNSString(IDS_PERMISSION_ALLOW);
-    allowOrOkButton.reset([[self buttonWithTitle:allowTitle
-                                          action:@selector(onAllow:)] retain]);
-  } else {
-    NSString* okTitle = l10n_util::GetNSString(IDS_OK);
-    allowOrOkButton.reset([[self buttonWithTitle:okTitle
-                                          action:@selector(ok:)] retain]);
-  }
-  CGFloat yOffset = 2 * kVerticalPadding + NSMaxY([allowOrOkButton frame]);
-
-  base::scoped_nsobject<NSMutableArray> permissionMenus;
-  if (!singlePermission)
-    permissionMenus.reset([[NSMutableArray alloc] init]);
+  NSString* allowTitle = l10n_util::GetNSString(IDS_PERMISSION_ALLOW);
+  base::scoped_nsobject<NSView> allowButton(
+      [[self buttonWithTitle:allowTitle action:@selector(onAllow:)] retain]);
+  CGFloat yOffset = 2 * kVerticalPadding + NSMaxY([allowButton frame]);
 
   CGFloat maxPermissionLineWidth = 0;
   CGFloat verticalPadding = 0.0f;
@@ -321,18 +230,6 @@ const NSSize kPermissionIconSize = {18, 18};
     [permissionView setFrameOrigin:origin];
     [contentView addSubview:permissionView];
 
-    if (!singlePermission) {
-      int index = it - requests.begin();
-      base::scoped_nsobject<NSView> menu([[self
-          menuForRequest:(*it)atIndex:index
-                   allow:delegate->AcceptStates()[index] ? YES : NO] retain]);
-      // Align vertically.  Horizontal alignment will be adjusted once the
-      // widest permission is know.
-      [PermissionBubbleController alignCenterOf:menu
-                           verticallyToCenterOf:permissionView];
-      [permissionMenus addObject:menu];
-      [contentView addSubview:menu];
-    }
     maxPermissionLineWidth = std::max(
         maxPermissionLineWidth, NSMaxX([permissionView frame]));
     yOffset += NSHeight([permissionView frame]);
@@ -351,23 +248,13 @@ const NSSize kPermissionIconSize = {18, 18};
   base::scoped_nsobject<NSView> closeButton([[self closeButton] retain]);
 
   // Determine the dimensions of the bubble.
-  // Once the height and width are set, the buttons and permission menus can
-  // be laid out correctly.
+  // Once the height and width are set, the buttons can be laid out correctly.
   NSRect bubbleFrame = NSMakeRect(0, 0, kBubbleMinWidth, 0);
 
   // Fix the height of the bubble relative to the title.
   bubbleFrame.size.height = NSMaxY([titleView frame]) + kVerticalPadding +
                             info_bubble::kBubbleArrowHeight;
 
-  if (!singlePermission) {
-    // Add the maximum menu width to the bubble width.
-    CGFloat maxMenuWidth = 0;
-    for (AllowBlockMenuButton* button in permissionMenus.get()) {
-      maxMenuWidth = std::max(maxMenuWidth, [button maximumTitleWidth]);
-    }
-    maxPermissionLineWidth += maxMenuWidth;
-  }
-
   // The title and 'x' button row must fit within the bubble.
   CGFloat titleRowWidth = NSMaxX([titleView frame]) +
                           NSWidth([closeButton frame]) +
@@ -378,8 +265,7 @@ const NSSize kPermissionIconSize = {18, 18};
                std::max(titleRowWidth, maxPermissionLineWidth)) +
       kHorizontalPadding;
 
-  // Now that the bubble's dimensions have been set, lay out the buttons and
-  // menus.
+  // Now that the bubble's dimensions have been set, lay out the buttons.
 
   // Place the close button at the upper-right-hand corner of the bubble.
   NSPoint closeButtonOrigin =
@@ -394,32 +280,22 @@ const NSSize kPermissionIconSize = {18, 18};
   [contentView addSubview:closeButton];
 
   // Position the allow/ok button.
-  CGFloat xOrigin = NSWidth(bubbleFrame) - NSWidth([allowOrOkButton frame]) -
+  CGFloat xOrigin = NSWidth(bubbleFrame) - NSWidth([allowButton frame]) -
                     kButtonRightEdgePadding;
-  [allowOrOkButton setFrameOrigin:NSMakePoint(xOrigin, kVerticalPadding)];
-  [contentView addSubview:allowOrOkButton];
-
-  if (singlePermission) {
-    base::scoped_nsobject<NSView> blockButton;
-    blockButton.reset([[self blockButton] retain]);
-    CGFloat width = [PermissionBubbleController matchWidthsOf:blockButton
-                                                        andOf:allowOrOkButton];
-    // Ensure the allow/ok button is still in the correct position.
-    xOrigin = NSWidth(bubbleFrame) - width - kHorizontalPadding;
-    [allowOrOkButton setFrameOrigin:NSMakePoint(xOrigin, kVerticalPadding)];
-    // Line up the block button.
-    xOrigin = NSMinX([allowOrOkButton frame]) - width - kBetweenButtonsPadding;
-    [blockButton setFrameOrigin:NSMakePoint(xOrigin, kVerticalPadding)];
-    [contentView addSubview:blockButton];
-  } else {
-    // Adjust the horizontal origin for each menu so that its right edge
-    // lines up with the right edge of the ok button.
-    CGFloat rightEdge = NSMaxX([allowOrOkButton frame]);
-    for (NSView* view in permissionMenus.get()) {
-      [view setFrameOrigin:NSMakePoint(rightEdge - NSWidth([view frame]),
-                                       NSMinY([view frame]))];
-    }
-  }
+  [allowButton setFrameOrigin:NSMakePoint(xOrigin, kVerticalPadding)];
+  [contentView addSubview:allowButton];
+
+  base::scoped_nsobject<NSView> blockButton;
+  blockButton.reset([[self blockButton] retain]);

[tapted, 2017/06/06 08:06:03]

Hrm. It's weird to make this scoped_nsobject - there's no need for a retain
count to live on the stack

`NSView* blockButton = [self makeBlockButton]`

would be clearer. (addSubview will retain it internally).

I guess this is fine too though. I wouldn't worry about changing it (we'd wanna
change buttonWithTitle into something more "makeFoo" to be consistent..)

+  CGFloat width =
+      [PermissionBubbleController matchWidthsOf:blockButton andOf:allowButton];
+  // Ensure the allow/ok button is still in the correct position.
+  xOrigin = NSWidth(bubbleFrame) - width - kHorizontalPadding;
+  [allowButton setFrameOrigin:NSMakePoint(xOrigin, kVerticalPadding)];
+  // Line up the block button.
+  xOrigin = NSMinX([allowButton frame]) - width - kBetweenButtonsPadding;
+  [blockButton setFrameOrigin:NSMakePoint(xOrigin, kVerticalPadding)];
+  [contentView addSubview:blockButton];
 
   bubbleFrame = [[self window] frameRectForContentRect:bubbleFrame];
   if ([[self window] isVisible]) {
@@ -437,7 +313,7 @@ const NSSize kPermissionIconSize = {18, 18};
     [self setAnchorPoint:[self getExpectedAnchorPoint]];
     [self showWindow:nil];
     [[self window] makeFirstResponder:nil];
-    [[self window] setInitialFirstResponder:allowOrOkButton.get()];
+    [[self window] setInitialFirstResponder:allowButton.get()];
   }
 }
 
@@ -532,20 +408,6 @@ const NSSize kPermissionIconSize = {18, 18};
   return titleView.autorelease();
 }
 
-- (NSView*)menuForRequest:(PermissionRequest*)request
-                  atIndex:(int)index
-                    allow:(BOOL)allow {
-  DCHECK(request);
-  DCHECK(delegate_);
-  base::scoped_nsobject<AllowBlockMenuButton> button(
-      [[AllowBlockMenuButton alloc] initForURL:request->GetOrigin()
-                                       allowed:allow
-                                         index:index
-                                      delegate:delegate_
-                                       profile:browser_->profile()]);
-  return button.autorelease();
-}
-
 - (NSView*)buttonWithTitle:(NSString*)title
                     action:(SEL)action {
   base::scoped_nsobject<NSButton> button(