Fix the Mac status bubble.

Fix the Mac status bubble.

It would DCHECK if a fade-in were ever interrupted by a fade-out, or vice versa.

BUG=338110
TEST=as in bug

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

[Avi (use Gerrit), 2014-02-14 20:22:13]

For your enjoyment.

[Mark Mentovai, 2014-02-14 20:34:04]

Cool. LGTM.

https://codereview.chromium.org/164333006/diff/40001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/status_bubble_mac.mm (right):

https://codereview.chromium.org/164333006/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/status_bubble_mac.mm:66: @interface NSAnimationContext
(LionSDK)
I think we like to unify these in base/mac/sdk_forward_declarations.h now.

https://codereview.chromium.org/164333006/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/status_bubble_mac.mm:67: + (void)runAnimationGroup:(void
(^)(NSAnimationContext *context))changes
This file, like most Chrome code, prefers “Type* variable” to “Type *variable”.
You’ve mixed styles in your own code.

https://codereview.chromium.org/164333006/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/status_bubble_mac.mm:117: if (completionHandler_) {
Can you DCHECK in this block that you’re on 10.6? Or DCHECK that
NSAnimationContext has -runAnimationGroup:completionHandler:? I want a slightly
stronger guarantee that we don’t enter the funky (but neat) fallback path by
accident on systems where we shouldn’t need to.

https://codereview.chromium.org/164333006/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/status_bubble_mac.mm:122: completionHandler_ = nil;
Crafty.

[Avi (use Gerrit), 2014-02-14 20:44:18]

The CQ bit was checked by avi@chromium.org

[Avi (use Gerrit), 2014-02-14 20:44:33]

https://codereview.chromium.org/164333006/diff/40001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/status_bubble_mac.mm (right):

https://codereview.chromium.org/164333006/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/status_bubble_mac.mm:66: @interface NSAnimationContext
(LionSDK)
On 2014/02/14 20:34:04, Mark Mentovai wrote:
> I think we like to unify these in base/mac/sdk_forward_declarations.h now.

Done.

https://codereview.chromium.org/164333006/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/status_bubble_mac.mm:67: + (void)runAnimationGroup:(void
(^)(NSAnimationContext *context))changes
On 2014/02/14 20:34:04, Mark Mentovai wrote:
> This file, like most Chrome code, prefers “Type* variable” to “Type
*variable”.
> You’ve mixed styles in your own code.

Done.

https://codereview.chromium.org/164333006/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/status_bubble_mac.mm:117: if (completionHandler_) {
On 2014/02/14 20:34:04, Mark Mentovai wrote:
> Can you DCHECK in this block that you’re on 10.6? Or DCHECK that
> NSAnimationContext has -runAnimationGroup:completionHandler:? I want a
slightly
> stronger guarantee that we don’t enter the funky (but neat) fallback path by
> accident on systems where we shouldn’t need to.

Sure.

(You do mean that NSAnimationContext DOESN'T have
-runAnimationGroup:completionHandler:, right?)

https://codereview.chromium.org/164333006/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/status_bubble_mac.mm:122: completionHandler_ = nil;
On 2014/02/14 20:34:04, Mark Mentovai wrote:
> Crafty.

Thank you.

[commit-bot: I haz the power, 2014-02-14 20:45:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/164333006/90002

[Mark Mentovai, 2014-02-14 20:49:21]

LGTM

https://codereview.chromium.org/164333006/diff/40001/chrome/browser/ui/cocoa/...
File chrome/browser/ui/cocoa/status_bubble_mac.mm (right):

https://codereview.chromium.org/164333006/diff/40001/chrome/browser/ui/cocoa/...
chrome/browser/ui/cocoa/status_bubble_mac.mm:117: if (completionHandler_) {
Avi wrote:
> On 2014/02/14 20:34:04, Mark Mentovai wrote:
> > Can you DCHECK in this block that you’re on 10.6? Or DCHECK that
> > NSAnimationContext has -runAnimationGroup:completionHandler:? I want a
> slightly
> > stronger guarantee that we don’t enter the funky (but neat) fallback path by
> > accident on systems where we shouldn’t need to.
> 
> Sure.
> 
> (You do mean that NSAnimationContext DOESN'T have
> -runAnimationGroup:completionHandler:, right?)

Yeah, of course.

Remember when we were talking about that exercise yesterday and I said you
weren’t allowed to use “not?” I must have applied that same constraint to what I
wrote.

[commit-bot: I haz the power, 2014-02-14 22:59:59]

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

[commit-bot: I haz the power, 2014-02-14 23:00:01]

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

[Avi (use Gerrit), 2014-02-14 23:00:17]

The CQ bit was checked by avi@chromium.org

[commit-bot: I haz the power, 2014-02-14 23:07:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/avi@chromium.org/164333006/90002

[commit-bot: I haz the power, 2014-02-15 03:47:45]

Change committed as 251499