[rAC, OSX] Use bubble windows for error messages.

[rAC, OSX] Use bubble windows for error messages.

In the short term, this addresses the issue of messages that are too long
to fit in the dialog - the window will extend outside the dialog. In the
long run, this also prepares for further changes around the error bubble
that do require the BaseBubble machinery.

R=rsesek@chromium.org
BUG=308200
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=233458

[groby-ooo-7-16, 2013-11-04 23:17:34]

PTAL - screenshot of the "too large" message at http://imgur.com/yKhs7Us

[Robert Sesek, 2013-11-04 23:49:18]

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/autof...
File chrome/browser/ui/cocoa/autofill/autofill_details_container.mm (right):

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/autof...
chrome/browser/ui/cocoa/autofill/autofill_details_container.mm:134:
NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
Is it possible for the bubble to close itself ever, with the change you're
making to BaseBubbleController? If no, then maybe you don't need to do this
since this code is in exclusive control of the bubble's lifetime?

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/autof...
File chrome/browser/ui/cocoa/autofill/autofill_error_bubble_controller.h
(right):

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/autof...
chrome/browser/ui/cocoa/autofill/autofill_error_bubble_controller.h:28: 
nit: remove blank line

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/autof...
File chrome/browser/ui/cocoa/autofill/autofill_error_bubble_controller.mm
(right):

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/autof...
chrome/browser/ui/cocoa/autofill/autofill_error_bubble_controller.mm:37:
gfx::SkColorToCalibratedNSColor(kWarningColor)];
nit: indent 2 more spaces

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/autof...
chrome/browser/ui/cocoa/autofill/autofill_error_bubble_controller.mm:63: 
nit: remove blank line

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/base_...
File chrome/browser/ui/cocoa/base_bubble_controller.mm (right):

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/base_...
chrome/browser/ui/cocoa/base_bubble_controller.mm:208: // window. This only
matters if the window is key.
I'd put this comment closer to where this is implemented.

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/base_...
chrome/browser/ui/cocoa/base_bubble_controller.mm:213: if (![window
isKeyWindow])
Comment please.

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/base_...
chrome/browser/ui/cocoa/base_bubble_controller.mm:214: return event;
nit: blank line after.

[groby-ooo-7-16, 2013-11-05 00:51:47]

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/autof...
File chrome/browser/ui/cocoa/autofill/autofill_details_container.mm (right):

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/autof...
chrome/browser/ui/cocoa/autofill/autofill_details_container.mm:134:
NSNotificationCenter* center = [NSNotificationCenter defaultCenter];
Yes, it can, when the parent window resigns key status, and when the user
switches tabs.

(I'm not sure this ultimately should be in BaseBubble, but that's for another
CL)

On 2013/11/04 23:49:18, rsesek wrote:
> Is it possible for the bubble to close itself ever, with the change you're
> making to BaseBubbleController? If no, then maybe you don't need to do this
> since this code is in exclusive control of the bubble's lifetime?

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/autof...
File chrome/browser/ui/cocoa/autofill/autofill_error_bubble_controller.h
(right):

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/autof...
chrome/browser/ui/cocoa/autofill/autofill_error_bubble_controller.h:28: 
On 2013/11/04 23:49:18, rsesek wrote:
> nit: remove blank line

Done.

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/autof...
File chrome/browser/ui/cocoa/autofill/autofill_error_bubble_controller.mm
(right):

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/autof...
chrome/browser/ui/cocoa/autofill/autofill_error_bubble_controller.mm:37:
gfx::SkColorToCalibratedNSColor(kWarningColor)];
On 2013/11/04 23:49:18, rsesek wrote:
> nit: indent 2 more spaces

Done.

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/autof...
chrome/browser/ui/cocoa/autofill/autofill_error_bubble_controller.mm:63: 
On 2013/11/04 23:49:18, rsesek wrote:
> nit: remove blank line

Done.

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/base_...
File chrome/browser/ui/cocoa/base_bubble_controller.mm (right):

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/base_...
chrome/browser/ui/cocoa/base_bubble_controller.mm:208: // window. This only
matters if the window is key.
On 2013/11/04 23:49:18, rsesek wrote:
> I'd put this comment closer to where this is implemented.

Done.

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/base_...
chrome/browser/ui/cocoa/base_bubble_controller.mm:213: if (![window
isKeyWindow])
On 2013/11/04 23:49:18, rsesek wrote:
> Comment please.

Done.

https://codereview.chromium.org/49063012/diff/1/chrome/browser/ui/cocoa/base_...
chrome/browser/ui/cocoa/base_bubble_controller.mm:214: return event;
On 2013/11/04 23:49:18, rsesek wrote:
> nit: blank line after.

Done.

[Robert Sesek, 2013-11-05 01:43:26]

LGTM

https://codereview.chromium.org/49063012/diff/120002/chrome/browser/ui/cocoa/...
File
chrome/browser/ui/cocoa/autofill/autofill_error_bubble_controller_unittest.mm
(right):

https://codereview.chromium.org/49063012/diff/120002/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/autofill/autofill_error_bubble_controller_unittest.mm:27:

nit: extra blank

[groby-ooo-7-16, 2013-11-05 01:51:52]

https://codereview.chromium.org/49063012/diff/120002/chrome/browser/ui/cocoa/...
File
chrome/browser/ui/cocoa/autofill/autofill_error_bubble_controller_unittest.mm
(right):

https://codereview.chromium.org/49063012/diff/120002/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/autofill/autofill_error_bubble_controller_unittest.mm:27:

On 2013/11/05 01:43:26, rsesek wrote:
> nit: extra blank

Done.

[commit-bot: I haz the power, 2013-11-05 02:02:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/groby@chromium.org/49063012/200001

[commit-bot: I haz the power, 2013-11-05 03:05:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/groby@chromium.org/49063012/200001

[commit-bot: I haz the power, 2013-11-05 04:24:45]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-05 04:39:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/groby@chromium.org/49063012/200001

[commit-bot: I haz the power, 2013-11-05 05:25:18]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_clang.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-11-05 05:47:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/groby@chromium.org/49063012/200001

[Ilya Sherman, 2013-11-05 05:54:52]

Ah, I'd been re-ticking the CQ box on the assumption that the CQ was just being
flaky, but it looks like there is a legitimate failure in
BaseBubbleControllerTest.LionClickOutsideCloses.  I'll stop abusing the poor bot
infrastructure now :P

[groby-ooo-7-16, 2013-11-06 01:52:48]

rsesek: Heads up - changed the BaseBubbleController according to our discussion

[commit-bot: I haz the power, 2013-11-06 21:47:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/groby@chromium.org/49063012/620001

[Robert Sesek, 2013-11-06 23:06:02]

LGTM. Sorry for the delay - in and out of meetings all day.

https://codereview.chromium.org/49063012/diff/620001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/autofill/autofill_error_bubble_controller.h
(right):

https://codereview.chromium.org/49063012/diff/620001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/autofill/autofill_error_bubble_controller.h:14:
@interface AutofillErrorBubbleController : BaseBubbleController {
@private

https://codereview.chromium.org/49063012/diff/620001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/autofill/autofill_error_bubble_controller.h:25: 
nit: extra blank line

https://codereview.chromium.org/49063012/diff/620001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/base_bubble_controller.h (left):

https://codereview.chromium.org/49063012/diff/620001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/base_bubble_controller.h:40: bool
shouldOpenAsKeyWindow_;
Thanks for this fix.

[commit-bot: I haz the power, 2013-11-07 01:18:09]

Change committed as 233458