Mac status bubble delays and fades

Status bubbles should wait before showing and hiding, and should fade in and
out on the Mac.

This fixes the fades, which were actually written but unfortunately not
working due to a silly bug.  It also adds the delays, cleaning up a TODO.
It fixes some questionable logic ("hide by calling Hide() and then FadeIn()").
Finally, it allows better reuse of the status bubble NSWindow, and fixes a bug
that prevented the status bubble window from properly attaching to its parent
window if the parent was offscreen at attachment time.  Unit tests for the new
behavior are included, and as a bonus, I've added better testing for some
existing behavior.

See http://dev.chromium.org/user-experience/status-bubble for the design of
the status bubble.  Also, consult chrome/browser/views/status_bubble_views.cc
for the implementation used on Windows.

BUG=24495
TEST=Mouse over some links

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=28749

[Mark Mentovai, 2009-10-10 03:03:34]

You're going to love this.

[Mark Mentovai, 2009-10-11 23:23:32]

I added the delays too and fixed a few other things in there.

[pink (ping after 24hrs), 2009-10-12 14:52:10]

drive-by

http://codereview.chromium.org/269045/diff/2002/6001
File chrome/browser/cocoa/status_bubble_mac.mm (right):

http://codereview.chromium.org/269045/diff/2002/6001#newcode144
Line 144: // Use kMinimumTimeInterval to set the opacity as rapidly as possible.
As we've observed in other bugs, there's an appkit bug where this trick doesn't
work for alpha fades. The trick is to copy the animations array, remove it for
the alpha key, perform the animation (with delay), then restore the animations
array. Contact me out of band and i'll send the code.

http://codereview.chromium.org/269045/diff/2002/6001#newcode264
Line 264: // There's no callback that fires when a fade transition completes, so
you can set a delegate on the CAAnimation object you get back from
animationForKey: that will call you when the animation is complete.

http://codereview.chromium.org/269045/diff/2002/6001#newcode306
Line 306: NSTimeInterval duration =
see other comment about this trick not working.

[Avi (use Gerrit), 2009-10-12 14:56:39]

LG

http://codereview.chromium.org/269045/diff/2002/6001
File chrome/browser/cocoa/status_bubble_mac.mm (right):

http://codereview.chromium.org/269045/diff/2002/6001#newcode269
Line 269: if (state_ == kBubbleShowingFadeIn && [window_ alphaValue] ==
kBubbleOpacity)
I think that asking the window what its alphaValue is works if Cocoa is doing
the animation but not if CA is doing it. For future-safety, does it work to ask
the animator what the alphaValue is?

[Mark Mentovai, 2009-10-12 16:00:41]

Avi wrote:
http://codereview.chromium.org/269045/diff/2002/6001#newcode269
> Line 269: if (state_ == kBubbleShowingFadeIn && [window_ alphaValue] ==
> kBubbleOpacity)
> I think that asking the window what its alphaValue is works if Cocoa is doing
> the animation but not if CA is doing it. For future-safety, does it work to
ask
> the animator what the alphaValue is?

This works, patch updated.

I'm still trying to figure out whether pink's comment about canceling an
in-progress animation applies here.  It seems like it's only a problem to cancel
an animation by replacing it with a new zero-duration animation, but we
shouldn't ever be doing that.  Just to be safe, I did add a check that enforces
a minimum duration of kMinimumTimeInterval on the fades.

[Avi (use Gerrit), 2009-10-12 16:12:38]

SLGTM but check with pink.

WRT the original int variables, I'll be committing hara kiri later today for the
embarrassment. Let me know if that'll affect your scheduling. :)

[pink (ping after 24hrs), 2009-10-12 16:17:43]

I believe as the original reviewer, I'm on the hook to fall on my sword as well.

On Mon, Oct 12, 2009 at 12:12 PM,  <avi@chromium.org> wrote:
> SLGTM but check with pink.
>
> WRT the original int variables, I'll be committing hara kiri later today for
> the
> embarrassment. Let me know if that'll affect your scheduling. :)
>
> http://codereview.chromium.org/269045
>



-- 
Mike Pinkerton
Mac Weenie
pinkerton@google.com

[John Grabowski, 2009-10-12 18:25:16]

Pink and Avi, please save suicide until after the next milestone.
jrg


On Mon, Oct 12, 2009 at 9:17 AM, Mike Pinkerton <pinkerton@chromium.org>wrote:

> I believe as the original reviewer, I'm on the hook to fall on my sword as
> well.
>
> On Mon, Oct 12, 2009 at 12:12 PM,  <avi@chromium.org> wrote:
> > SLGTM but check with pink.
> >
> > WRT the original int variables, I'll be committing hara kiri later today
> for
> > the
> > embarrassment. Let me know if that'll affect your scheduling. :)
> >
> > http://codereview.chromium.org/269045
> >
>
>
>
> --
> Mike Pinkerton
> Mac Weenie
> pinkerton@google.com
>

[Mark Mentovai, 2009-10-12 21:28:33]

Switched to the delegate.  Also, FadeIn and FadeOut started getting out of hand
with the duplication, so I merged them.

One final look?

[Avi (use Gerrit), 2009-10-12 21:58:51]

SLG