Shows the info bubble when the location bar icon is clicked in the origin chip.

chrome/browser/ui/cocoa/location_bar/origin_chip_decoration.mm

Index: chrome/browser/ui/cocoa/location_bar/origin_chip_decoration.mm
diff --git a/chrome/browser/ui/cocoa/location_bar/origin_chip_decoration.mm b/chrome/browser/ui/cocoa/location_bar/origin_chip_decoration.mm
index 225b2df17f2341f5fcd09f1d07b86494d2c6f1c4..ed00cdfb0025fde5a6756cb44ee9253bc0492ff7 100644
--- a/chrome/browser/ui/cocoa/location_bar/origin_chip_decoration.mm
+++ b/chrome/browser/ui/cocoa/location_bar/origin_chip_decoration.mm
@@ -13,6 +13,7 @@
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/safe_browsing/ui_manager.h"
 #import "chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.h"
+#import "chrome/browser/ui/cocoa/location_bar/location_icon_decoration.h"
 #import "chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.h"
 #import "chrome/browser/ui/cocoa/nsview_additions.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -34,6 +35,10 @@ const CGFloat kOuterRightPadding = 3;
 // be scaled down.
 const CGFloat kIconSize = 19;
 
+// The info-bubble point should look like it points to the bottom of the lock
+// icon. Determined with Pixie.app.
+const CGFloat kPageInfoBubblePointYOffset = 2.0;
+
 const ui::NinePartImageIds kNormalImages[3] = {
   IMAGE_GRID(IDR_ORIGIN_CHIP_NORMAL),
   IMAGE_GRID(IDR_ORIGIN_CHIP_HOVER),
@@ -60,12 +65,16 @@ const ui::NinePartImageIds kEVImages[3] = {
 
 }  // namespace
 
-OriginChipDecoration::OriginChipDecoration(LocationBarViewMac* owner)
+OriginChipDecoration::OriginChipDecoration(
+    LocationBarViewMac* owner,
+    LocationIconDecoration* location_icon)

[groby-ooo-7-16, 2014/04/10 21:26:09]

I'm really queasy about the idea of decorations depending on each other. Should
we instead hoist the code for showing the settings into LocationBarViewMac?

(Ultimately I think they can be pulled out even further, given that there's
similar code in page_info_helper.cc,  location_icon_decoration.mm, and 2
instances in render_view_context_menu.cc. But that's probably beyond scope for
this CL)

[macourteau, 2014/04/28 19:19:24]

That's probably a good idea (though as you say, it might be better to do that in
a separate CL). The EVBubbleDecoration also depends on the
LocationIconDecoration (that's what it uses to display the page info bubble as
well).

I'll add a TODO in the LocationIconDecoration class.

     : ButtonDecoration(kNormalImages[0], IDR_LOCATION_BAR_HTTP,
                        kNormalImages[1], IDR_LOCATION_BAR_HTTP,
                        kNormalImages[2], IDR_LOCATION_BAR_HTTP, 0),
       attributes_([[NSMutableDictionary alloc] init]),
+      icon_rect_(NSZeroRect),
       info_(this, owner->browser()->profile()),
+      location_icon_(location_icon),
       owner_(owner) {
   DCHECK(owner_);
   [attributes_ setObject:GetFont() forKey:NSFontAttributeName];
@@ -169,19 +178,19 @@ void OriginChipDecoration::DrawInFrame(NSRect frame, NSView* control_view) {
     icon_x_trailing_offset = std::floor(icon_x_trailing_offset);
   }
 
-  NSRect icon_rect = NSMakeRect(kInnerLeftPadding + icon_x_leading_offset,
-                                icon_y_inset, icon_width, icon_height);
-  [icon drawInRect:icon_rect
+  icon_rect_ = NSMakeRect(kInnerLeftPadding + icon_x_leading_offset,
+                          icon_y_inset, icon_width, icon_height);
+
+  [icon drawInRect:icon_rect_
           fromRect:NSZeroRect
          operation:NSCompositeSourceOver
           fraction:1.0
     respectFlipped:YES
              hints:nil];
 
-  NSRect label_rect =
-      NSMakeRect(NSMaxX(icon_rect) + icon_x_trailing_offset + kIconLabelPadding,
-                 0, [label_ sizeWithAttributes:attributes_].width,
-                 frame.size.height);
+  NSRect label_rect = NSMakeRect(
+      NSMaxX(icon_rect_) + icon_x_trailing_offset + kIconLabelPadding,
+      0, [label_ sizeWithAttributes:attributes_].width, frame.size.height);
   DrawLabel(label_, attributes_, label_rect);
 }
 
@@ -189,15 +198,26 @@ NSString* OriginChipDecoration::GetToolTip() {
   return label_.get();
 }
 
-bool OriginChipDecoration::OnMousePressed(NSRect frame) {
-  // TODO(macourteau): reveal the permissions bubble if the click was inside the
-  // icon's bounds - otherwise, show the URL.
+bool OriginChipDecoration::OnMousePressed(NSRect frame, NSPoint location) {
+  // Reveal the permissions bubble if the click was inside the icon's bounds;
+  // otherwise, show the URL.
+  if (!NSEqualRects(icon_rect_, NSZeroRect) &&

[groby-ooo-7-16, 2014/04/10 21:26:09]

Is NSPointInRect not enough? (It's not if the decoration could feasibly include
(0,0) as a point - otherwise, it should be)

[macourteau, 2014/04/28 19:19:24]

Done.

+      NSPointInRect(location, icon_rect_)) {
+    owner_->GetOmniboxView()->model()->set_focused_via_location_icon();

[groby-ooo-7-16, 2014/04/10 21:26:09]

Technically, the delayed focus should still set focus?

[macourteau, 2014/04/28 19:19:24]

I'm not sure I get the question? The omnibox does get focus when the origin chip
is clicked.

[groby-ooo-7-16, 2014/04/30 21:55:39]

Yes, but that should be handled via the cell's focus machinery. We shouldn't
need to manually set focus.

[macourteau, 2014/05/01 19:04:48]

This wasn't actually setting focus - the set_focused_via_location_icon was just
setting state meaning that focus had occurred through the location icon (instead
of the omnibox). I've removed the set_focused_... method anyway :)

+    return location_icon_->OnMousePressed(frame, location);
+  }
+
   UMA_HISTOGRAM_COUNTS("OriginChip.Pressed", 1);
   content::RecordAction(base::UserMetricsAction("OriginChipPress"));
   owner_->GetOmniboxView()->ShowURL();
   return true;
 }
 
+NSPoint OriginChipDecoration::GetBubblePointInFrame(NSRect frame) {
+  return NSMakePoint(NSMidX(icon_rect_),
+                     NSMaxY(icon_rect_) - kPageInfoBubblePointYOffset);
+}
+
 void OriginChipDecoration::OnExtensionIconImageChanged(
       extensions::IconImage* image) {
   if (image) {
@@ -219,9 +239,9 @@ void OriginChipDecoration::OnSafeBrowsingMatch(
 }
 
 bool OriginChipDecoration::ShouldShow() const {

[groby-ooo-7-16, 2014/04/10 21:26:09]

That code is identical to views. Does it make sense to have ShouldShowOriginChip
on the ToolbarModel?

[macourteau, 2014/04/28 19:19:24]

Done.

-  return chrome::ShouldDisplayOriginChip() ||
-      (owner_->GetToolbarModel()->WouldOmitURLDueToOriginChip() &&
-       owner_->GetToolbarModel()->origin_chip_enabled());
+  return chrome::ShouldDisplayOriginChipV2() &&
+      owner_->GetToolbarModel()->WouldOmitURLDueToOriginChip() &&
+      owner_->GetToolbarModel()->origin_chip_enabled();
 }
 
 // TODO(macourteau): Implement eliding of the host.