Moves the animation for the download_started into an overlay window to get...

Moves the animation for the download_started into an overlay window to get
rid of jankyness on 10.6. It really shouldn't have worked right on 10.5 either.
The layer needed to be hosted in a view. Views should not overlap.
The only ways to do this correctly are:
a) Turn on setWantsLayer for the browser view and make a subview to host our 
   animation.
b) An overlay window
Went with 'b', because 'a' may be a big memory hit, and I'm not sure how
webkit works in a layered environment.
BUG=23500
TEST=see bug


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

[dmac, 2009-11-12 00:44:11]

Tested on 10.5 and 10.6 and looks pretty buttery smooth to me.

[Nico, 2009-11-12 01:53:55]

On 2009/11/12 00:44:11, dmac wrote:
> Tested on 10.5 and 10.6 and looks pretty buttery smooth to me.

Avi said that overlapping views are ok since 10.5…but using an overlay window
looks cleaner anyway I guess.

[rohitrao (ping after 24h), 2009-11-12 02:38:28]

I was going to ask about what happens when the window is resized, but it looks
like you've handled that already.  A similar question is what will happen when
the download shelf starts animating in.  Will it be possible to animate towards
a moving target?

[Nico, 2009-11-12 04:35:07]

http://codereview.chromium.org/385053/diff/1/2
File chrome/browser/cocoa/download_started_animation_mac.mm (right):

http://codereview.chromium.org/385053/diff/1/2#newcode109
Line 109: NSWindow *parentWindow = [tabContentsView window];
could this be null? If you click a download in a tab and then switch tabs before
the download actually starts (due to network delays), the tab contents view has
no window.

(Yes, the existing code had the same problem if this really is a problem.)

http://codereview.chromium.org/385053/diff/1/2#newcode181
Line 181: [layer addAnimation:animationGroup
forKey:@"downloadOpacityAndPosition"];
For my education: How can a "position" animation on |layer| move the window the
layer lives in?

[dmac, 2009-11-12 04:51:13]

On 2009/11/12 01:53:55, Nico wrote:
> On 2009/11/12 00:44:11, dmac wrote:
> > Tested on 10.5 and 10.6 and looks pretty buttery smooth to me.
> 
> Avi said that overlapping views are ok since 10.5…but using an overlay window
> looks cleaner anyway I guess.

overlapping views are only ok on 10.5+ if layers are on.

http://stackoverflow.com/questions/466297/is-there-a-proper-way-to-handle-ove...

[dmac, 2009-11-12 04:53:18]

On 2009/11/12 02:38:28, rohitrao wrote:
> I was going to ask about what happens when the window is resized, but it looks
> like you've handled that already.  A similar question is what will happen when
> the download shelf starts animating in.  Will it be possible to animate
towards
> a moving target?

We can animate to a moving target.... just have to change the end point for the
animation. It will take some refactoring though.

[dmac, 2009-11-12 17:54:21]

On 2009/11/12 04:35:07, Nico wrote:
> http://codereview.chromium.org/385053/diff/1/2
> File chrome/browser/cocoa/download_started_animation_mac.mm (right):
> 
> http://codereview.chromium.org/385053/diff/1/2#newcode109
> Line 109: NSWindow *parentWindow = [tabContentsView window];
> could this be null? If you click a download in a tab and then switch tabs
before
> the download actually starts (due to network delays), the tab contents view
has
> no window.
> 
> (Yes, the existing code had the same problem if this really is a problem.)

Fixed.

> 
> http://codereview.chromium.org/385053/diff/1/2#newcode181
> Line 181: [layer addAnimation:animationGroup
> forKey:@"downloadOpacityAndPosition"];
> For my education: How can a "position" animation on |layer| move the window
the
> layer lives in?

It doesn't... I move a layer that I add to the root layer of the window.See line
153.

[dmac, 2009-11-12 17:54:59]

Thanks for the comments guys... anything else?

[Nico, 2009-11-12 18:06:58]

LG

http://codereview.chromium.org/385053/diff/6001/6002
File chrome/browser/cocoa/download_started_animation_mac.mm (right):

http://codereview.chromium.org/385053/diff/6001/6002#newcode121
Line 121: if ((self = [super initWithContentRect:frame
An overview along the lines of "The window is as high as the tab contents and as
wide as the arrow image and hosts a layer that animates the sliding and fading"
would've saved me some time :-)

[Nico, 2009-11-12 18:09:36]

http://codereview.chromium.org/385053/diff/6001/6002
File chrome/browser/cocoa/download_started_animation_mac.mm (right):

http://codereview.chromium.org/385053/diff/6001/6002#newcode111
Line 111: // The tab has been moved out of the window.
"The tab is no longer the frontmost tab."?

[dmac, 2009-11-12 18:38:54]

Added your two comments, and shrunk the size of the window to cut down on memory
usage. Could you take another quick glance?

[Nico, 2009-11-12 18:46:09]

SLG.

On Thu, Nov 12, 2009 at 10:38 AM,  <dmaclach@chromium.org> wrote:
> Added your two comments, and shrunk the size of the window to cut down on
> memory
> usage. Could you take another quick glance?
>
>
> http://codereview.chromium.org/385053
>