Adds close button to cocoa permissions bubble.

Adds close button to cocoa permissions bubble.

BUG=None

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255120

[groby-ooo-7-16, 2014-03-04 21:04:00]

https://codereview.chromium.org/186643005/diff/1/chrome/browser/ui/cocoa/webs...
File chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
(right):

https://codereview.chromium.org/186643005/diff/1/chrome/browser/ui/cocoa/webs...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:148:
base::scoped_nsobject<NSView> closeButton([[self closeButton] retain]);
I'm surprised this doesn't generate a warning - you're forward-calling an
undefined method.

https://codereview.chromium.org/186643005/diff/1/chrome/browser/ui/cocoa/webs...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:152:
NSWidth([closeButton frame]) + chrome_style::kCloseButtonPadding;
Wait - why is this adding twice the width? Border padding? You should probably
call it out in a separate constant, for clarity.

https://codereview.chromium.org/186643005/diff/1/chrome/browser/ui/cocoa/webs...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:292: -
(NSView*)closeButton {
Please add to private api.

https://codereview.chromium.org/186643005/diff/1/chrome/browser/ui/cocoa/webs...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:297:
[button setAction:@selector(onBlock:)];
Is "close" really the same as "block"? Shouldn't it be "leave as is"/"ask again
later"?

[leng, 2014-03-04 23:35:52]

https://codereview.chromium.org/186643005/diff/1/chrome/browser/ui/cocoa/webs...
File chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
(right):

https://codereview.chromium.org/186643005/diff/1/chrome/browser/ui/cocoa/webs...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:148:
base::scoped_nsobject<NSView> closeButton([[self closeButton] retain]);
On 2014/03/04 21:04:01, groby wrote:
> I'm surprised this doesn't generate a warning - you're forward-calling an
> undefined method.

There are a bunch of these.

https://codereview.chromium.org/186643005/diff/1/chrome/browser/ui/cocoa/webs...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:152:
NSWidth([closeButton frame]) + chrome_style::kCloseButtonPadding;
On 2014/03/04 21:04:01, groby wrote:
> Wait - why is this adding twice the width? Border padding? You should probably
> call it out in a separate constant, for clarity.

I don't think I'm adding the width twice?  I've put some comments - do they
help?

https://codereview.chromium.org/186643005/diff/1/chrome/browser/ui/cocoa/webs...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:292: -
(NSView*)closeButton {
On 2014/03/04 21:04:01, groby wrote:
> Please add to private api.

Done.

https://codereview.chromium.org/186643005/diff/1/chrome/browser/ui/cocoa/webs...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:297:
[button setAction:@selector(onBlock:)];
On 2014/03/04 21:04:01, groby wrote:
> Is "close" really the same as "block"? Shouldn't it be "leave as is"/"ask
again
> later"?

Done.

[groby-ooo-7-16, 2014-03-05 00:19:05]

LGTM

https://codereview.chromium.org/186643005/diff/1/chrome/browser/ui/cocoa/webs...
File chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm
(right):

https://codereview.chromium.org/186643005/diff/1/chrome/browser/ui/cocoa/webs...
chrome/browser/ui/cocoa/website_settings/permission_bubble_controller.mm:152:
NSWidth([closeButton frame]) + chrome_style::kCloseButtonPadding;
On 2014/03/04 23:35:52, leng wrote:
> On 2014/03/04 21:04:01, groby wrote:
> > Wait - why is this adding twice the width? Border padding? You should
probably
> > call it out in a separate constant, for clarity.
> 
> I don't think I'm adding the width twice?  I've put some comments - do they
> help?

Never mind. kCloseButtonPadding somehow looked like kCloseButtonSize to me.
Obviously, I am not sufficiently caffeinated.

[leng, 2014-03-05 00:21:34]

The CQ bit was checked by leng@chromium.org

[commit-bot: I haz the power, 2014-03-05 00:23:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/leng@chromium.org/186643005/20001

[commit-bot: I haz the power, 2014-03-05 01:26:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/leng@chromium.org/186643005/20001

[commit-bot: I haz the power, 2014-03-05 06:17:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-05 06:17:01]

Retried try job too often on win for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[leng, 2014-03-05 16:47:27]

The CQ bit was checked by leng@chromium.org

[commit-bot: I haz the power, 2014-03-05 16:47:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/leng@chromium.org/186643005/20001

[commit-bot: I haz the power, 2014-03-05 16:54:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-05 16:54:15]

Try jobs failed on following builders: linux_chromium_rel

[leng, 2014-03-05 17:03:41]

The CQ bit was checked by leng@chromium.org

[commit-bot: I haz the power, 2014-03-05 17:03:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/leng@chromium.org/186643005/20001

[commit-bot: I haz the power, 2014-03-05 19:51:39]

Change committed as 255120