Improve settings override bubble to indicate policy installed extensions

chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac.mm

Index: chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac.mm
diff --git a/chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac.mm b/chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac.mm
index 12e764ff5fe88632829e183c0dfded575b19f53e..2ef1efc28cf50b5b81fe17fa59c3db9c5f1395a1 100644
--- a/chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac.mm
+++ b/chrome/browser/ui/cocoa/extensions/toolbar_actions_bar_bubble_mac.mm
@@ -16,7 +16,9 @@
 #include "third_party/skia/include/core/SkColor.h"
 #import "ui/base/cocoa/controls/hyperlink_button_cell.h"
 #import "ui/base/cocoa/hover_button.h"
+#include "ui/base/resource/resource_bundle.h"
 #import "ui/base/cocoa/window_size_constants.h"
+#include "ui/gfx/image/image_skia_util_mac.h"
 #include "ui/native_theme/native_theme.h"
 #include "ui/native_theme/native_theme_mac.h"
 
@@ -59,7 +61,9 @@ CGFloat kMinWidth = 320.0;
 @synthesize actionButton = actionButton_;
 @synthesize itemList = itemList_;
 @synthesize dismissButton = dismissButton_;
-@synthesize learnMoreButton = learnMoreButton_;
+@synthesize link = link_;
+@synthesize label = label_;
+@synthesize iconView = iconView_;
 
 - (id)initWithParentWindow:(NSWindow*)parentWindow
                anchorPoint:(NSPoint)anchorPoint
@@ -177,29 +181,56 @@ CGFloat kMinWidth = 320.0;
 
 - (void)layout {
   // First, construct the pieces of the bubble that have a fixed width: the
-  // heading, and the button strip (the learn more link, the action button, and
-  // the dismiss button).
+  // heading, and the button strip (the extra view (icon and/or (linked) text),
+  // the action button, and the dismiss button).
   NSTextField* heading =
       [self addTextFieldWithString:delegate_->GetHeadingText()
                           fontSize:13.0
                          alignment:NSLeftTextAlignment];
   NSSize headingSize = [heading frame].size;
 
-  base::string16 learnMore = delegate_->GetLearnMoreButtonText();
-  NSSize learnMoreSize = NSZeroSize;
-  if (!learnMore.empty()) {  // The "learn more" link is optional.
-    NSAttributedString* learnMoreString =
-        [self attributedStringWithString:learnMore
-                                fontSize:13.0
-                               alignment:NSLeftTextAlignment];
-    learnMoreButton_ =
-        [[HyperlinkButtonCell buttonWithString:learnMoreString.string] retain];
-    [learnMoreButton_ setTarget:self];
-    [learnMoreButton_ setAction:@selector(onButtonClicked:)];
-    [[[self window] contentView] addSubview:learnMoreButton_];
-    [learnMoreButton_ sizeToFit];
-    learnMoreSize = NSMakeSize(NSWidth([learnMoreButton_ frame]),
-                               NSHeight([learnMoreButton_ frame]));
+  std::unique_ptr<ToolbarActionsBarBubbleDelegate::ExtraViewInfo>
+      extra_view_info = delegate_->GetExtraViewInfo();
+
+  int resource_id = extra_view_info->resource_id;
+
+  NSSize extraViewIconSize = NSZeroSize;
+  if (resource_id != -1) {  // The extra view icon is optional.
+    NSImage* i = gfx::NSImageFromImageSkia(

[Devlin, 2016/10/12 20:16:54]

I think this would be more efficient as
ResourceBundle::GetSharedInstance().GetNativeImageNamed(id).ToNSImage().

[catmullings, 2016/10/14 23:16:46]

Done.

+        *(ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
+            resource_id)));
+    NSRect frame = NSMakeRect(0, 0, i.size.width, i.size.height);
+    iconView_ = [[NSImageView alloc] initWithFrame:frame];

[Devlin, 2016/10/12 20:16:53]

maybe init it with the image instead? 
https://developer.apple.com/reference/appkit/nsimageview/1644708-imageviewwit...

That might also default the size to be correct.

[catmullings, 2016/10/14 23:16:46]

As discussed, disregarding this suggestion because imageViewWithImage only works
for MacOS 10.12+, but Chrome can only compile an older version.

+    [iconView_
+        setImage:gfx::NSImageFromImageSkia(*(

[Devlin, 2016/10/12 20:16:53]

We should be able to avoid re-fetching the image.

[catmullings, 2016/10/14 23:16:46]

Done.

+                     ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
+                         resource_id)))];
+    extraViewIconSize = i.size;
+
+    [[[self window] contentView] addSubview:iconView_];
+  }
+
+  NSSize extraViewTextSize = NSZeroSize;
+  const base::string16& text = extra_view_info->text;
+  if (!text.empty()) {  // The extra view text is optional.
+    if (extra_view_info->is_text_linked) {
+      NSAttributedString* linkString =
+          [self attributedStringWithString:text
+                                  fontSize:13.0
+                                 alignment:NSLeftTextAlignment];
+      link_ = [[HyperlinkButtonCell buttonWithString:linkString.string] retain];
+      [link_ setTarget:self];
+      [link_ setAction:@selector(onButtonClicked:)];
+      [[[self window] contentView] addSubview:link_];
+      [link_ sizeToFit];
+    } else {
+      label_ = [self addTextFieldWithString:text
+                                   fontSize:13.0
+                                  alignment:NSLeftTextAlignment];
+    }
+    extraViewTextSize =
+        (label_) ? [label_ frame].size

[Devlin, 2016/10/12 20:16:53]

no need for parens

[catmullings, 2016/10/14 23:16:46]

Done.

+                 : NSMakeSize(NSWidth([link_ frame]), NSHeight([link_ frame]));

[Devlin, 2016/10/12 20:16:53]

Why not use the size property on frame?

[catmullings, 2016/10/14 23:16:46]

I can. I was following the patterns below for setting the button sizes. See line
241 below. 

[catmullings, 2016/10/14 23:16:46]

Done.

   }
 
   base::string16 cancelStr = delegate_->GetDismissButtonText();
@@ -230,8 +261,10 @@ CGFloat kMinWidth = 320.0;
     buttonStripWidth += actionButtonSize.width + kButtonPadding;
   if (dismissButton_)
     buttonStripWidth += dismissButtonSize.width + kButtonPadding;
-  if (learnMoreButton_)
-    buttonStripWidth += learnMoreSize.width + kButtonPadding;
+  if (iconView_)
+    buttonStripWidth += extraViewIconSize.width + kButtonPadding;
+  if (link_ || label_)
+    buttonStripWidth += extraViewTextSize.width + kButtonPadding;
 
   CGFloat headingWidth = headingSize.width;
   CGFloat windowWidth =
@@ -287,13 +320,28 @@ CGFloat kMinWidth = 320.0;
         dismissButtonSize.height)];
     currentMaxWidth -= (dismissButtonSize.width + kButtonPadding);
   }
-  if (learnMoreButton_) {
-    CGFloat learnMoreHeight =
-        currentHeight + (buttonStripHeight - learnMoreSize.height) / 2.0;
-    [learnMoreButton_ setFrame:NSMakeRect(kHorizontalPadding,
-                                          learnMoreHeight,
-                                          learnMoreSize.width,
-                                          learnMoreSize.height)];
+  if (label_ || link_) {
+    CGFloat extraViewTextHeight =
+        currentHeight + (buttonStripHeight - extraViewTextSize.height) / 2.0;
+    if (link_) {
+      [link_ setFrame:NSMakeRect(currentMaxWidth - extraViewTextSize.width,

[Devlin, 2016/10/12 20:16:53]

this would be cleaner if we define the frame above (since it's the same) and
then just do
if (link_)
  [link_ setFrame:frame]
else
  [label_ setFrame:frame]

[catmullings, 2016/10/14 23:16:46]

Yup, much better. I'll do that.

[catmullings, 2016/10/14 23:16:46]

Done.

+                                 extraViewTextHeight, extraViewTextSize.width,
+                                 extraViewTextSize.height)];
+    } else {
+      [label_ setFrame:NSMakeRect(currentMaxWidth - extraViewTextSize.width,
+                                  extraViewTextHeight, extraViewTextSize.width,
+                                  extraViewTextSize.height)];
+    }
+    currentMaxWidth -= extraViewTextSize.width + kButtonPadding;
+  }
+  if (iconView_) {
+    CGFloat extraViewIconHeight =
+        currentHeight + (buttonStripHeight - extraViewIconSize.height) / 2.0;
+
+    [iconView_
+        setFrame:NSMakeRect(kHorizontalPadding, extraViewIconHeight,
+                            extraViewIconSize.width, extraViewIconSize.height)];
+    currentMaxWidth -= extraViewIconSize.width + kButtonPadding;
   }
   // Buttons have some inherit padding of their own, so we don't need quite as
   // much space here.
@@ -336,8 +384,8 @@ CGFloat kMinWidth = 320.0;
     return;
   ToolbarActionsBarBubbleDelegate::CloseAction action =
       ToolbarActionsBarBubbleDelegate::CLOSE_EXECUTE;
-  if (learnMoreButton_ && sender == learnMoreButton_) {
-    action = ToolbarActionsBarBubbleDelegate::CLOSE_LEARN_MORE;
+  if (link_ && sender == link_) {
+    action = ToolbarActionsBarBubbleDelegate::CLOSE_LINK;
   } else if (dismissButton_ && sender == dismissButton_) {
     action = ToolbarActionsBarBubbleDelegate::CLOSE_DISMISS_USER_ACTION;
   } else {