Issue 195105: [Mac] Adds animations to the findbar....

Issue 195105: [Mac] Adds animations to the findbar.... (Closed)

Created:
11 years, 3 months ago by rohitrao (ping after 24h)

Modified:
9 years, 7 months ago

Reviewers:
Mark Mentovai, pink (ping after 24hrs)

CC:
chromium-reviews_googlegroups.com, John Grabowski, pam+watch_chromium.org, Ben Goodger (Google), Paweł Hajdan Jr.

Base URL:
svn://svn.chromium.org/chrome/trunk/src/

Visibility:
Public.

Description

[Mac] Adds animations to the findbar. BUG=http://crbug.com/14908 TEST=Findbar should animate open and closed, unless switching tabs. http://src.chromium.org/viewvc/chrome?view=rev&revision=26567

Patch Set 6 : '' #

Total comments: 5

Created: 11 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+407 lines, -230 lines)			Patch
M	chrome/app/nibs/FindBar.xib	View		14 chunks	+274 lines, -191 lines	0 comments	Download
M	chrome/browser/cocoa/find_bar_bridge.mm	View	1	2 chunks	+6 lines, -5 lines	0 comments	Download
M	chrome/browser/cocoa/find_bar_cocoa_controller.h	View	1 2 3	3 chunks	+11 lines, -2 lines	0 comments	Download
M	chrome/browser/cocoa/find_bar_cocoa_controller.mm	View	1 2 3	7 chunks	+103 lines, -23 lines	5 comments	Download
M	chrome/browser/cocoa/find_bar_cocoa_controller_unittest.mm	View	1 2 3 4	2 chunks	+13 lines, -9 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

pink (ping after 24hrs)

LGTM http://codereview.chromium.org/195105/diff/5001/5004 File chrome/browser/cocoa/find_bar_cocoa_controller.mm (right): http://codereview.chromium.org/195105/diff/5001/5004#newcode280 Line 280: [currentAnimation_.release() autorelease]; i assume release() here doesn't ...

11 years, 3 months ago (2009-09-18 13:40:12 UTC) #2

rohitrao (ping after 24h)

http://codereview.chromium.org/195105/diff/5001/5004 File chrome/browser/cocoa/find_bar_cocoa_controller.mm (right): http://codereview.chromium.org/195105/diff/5001/5004#newcode280 Line 280: [currentAnimation_.release() autorelease]; On 2009/09/18 13:40:12, pink wrote: > ...

11 years, 3 months ago (2009-09-18 15:17:57 UTC) #3

Mark Mentovai

11 years, 3 months ago (2009-09-18 15:35:12 UTC) #4

pink (ping after 24hrs)

11 years, 3 months ago (2009-09-18 15:37:41 UTC) #5

The problem is that reset calls release, and he wants it to be
autoreleased (because it's alive on the stack).

On Fri, Sep 18, 2009 at 11:35 AM,  <mark@chromium.org> wrote:
>
> http://codereview.chromium.org/195105/diff/5001/5004
> File chrome/browser/cocoa/find_bar_cocoa_controller.mm (right):
>
> http://codereview.chromium.org/195105/diff/5001/5004#newcode280
> Line 280: [currentAnimation_.release() autorelease];
> This is not what you want. =A0Use current_animation_.reset().
>
> http://codereview.chromium.org/195105
>



--=20
Mike Pinkerton
Mac Weenie
pinkerton@google.com

Expand Messages | Collapse Messages