Add theme support for the yellow star on the Mac....

Add theme support for the yellow star on the Mac.

BUG=none
TEST=go to a bookmarked page; the outline of the yellow star should be themed


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

[pink (ping after 24hrs), 2009-07-21 16:32:35]

http://codereview.chromium.org/155851/diff/1001/1005
File chrome/browser/cocoa/gradient_button_cell.h (right):

http://codereview.chromium.org/155851/diff/1001/1005#newcode34
Line 34: NSImage* overlayImage_;
why not use scoped_nsobject here?

http://codereview.chromium.org/155851/diff/1001/1005#newcode40
Line 40: // An image to overlay on top of the existing image; not themed. May be
nil.
worth doing as an @property(retain)?

http://codereview.chromium.org/155851/diff/1001/1004
File chrome/browser/cocoa/gradient_button_cell.mm (right):

http://codereview.chromium.org/155851/diff/1001/1004#newcode242
Line 242: if (self.overlayImage) {
dot notation disallowed by style guide, here and below

[Avi (use Gerrit), 2009-07-21 16:44:03]

Had to do some rewriting anyway to fix drawing issues, which obviated some
comments...

http://codereview.chromium.org/155851/diff/1001/1005
File chrome/browser/cocoa/gradient_button_cell.h (right):

http://codereview.chromium.org/155851/diff/1001/1005#newcode34
Line 34: NSImage* overlayImage_;
On 2009/07/21 16:32:35, pink wrote:
> why not use scoped_nsobject here?

hmmm ok

http://codereview.chromium.org/155851/diff/1001/1005#newcode40
Line 40: // An image to overlay on top of the existing image; not themed. May be
nil.
On 2009/07/21 16:32:35, pink wrote:
> worth doing as an @property(retain)?

Can't; the setter needs to do an invalidation.

http://codereview.chromium.org/155851/diff/1001/1004
File chrome/browser/cocoa/gradient_button_cell.mm (right):

http://codereview.chromium.org/155851/diff/1001/1004#newcode242
Line 242: if (self.overlayImage) {
On 2009/07/21 16:32:35, pink wrote:
> dot notation disallowed by style guide, here and below

dealt with anyway with the revision.

[pink (ping after 24hrs), 2009-07-21 17:02:46]

http://codereview.chromium.org/155851/diff/1001/1005
File chrome/browser/cocoa/gradient_button_cell.h (right):

http://codereview.chromium.org/155851/diff/1001/1005#newcode34
Line 34: NSImage* overlayImage_;
Why not? Saves you from having to do the refcounting yourself.

http://codereview.chromium.org/155851/diff/1001/1005#newcode40
Line 40: // An image to overlay on top of the existing image; not themed. May be
nil.
On 2009/07/21 16:44:03, Avi wrote:
> On 2009/07/21 16:32:35, pink wrote:
> > worth doing as an @property(retain)?
> 
> Can't; the setter needs to do an invalidation.

You can provide your own getter and setter. Making it a property doesn't require
them to be auto-generated.

[pink (ping after 24hrs), 2009-07-21 17:39:29]

LGTM with one question

http://codereview.chromium.org/155851/diff/1011/18
File chrome/browser/cocoa/gradient_button_cell.mm (right):

http://codereview.chromium.org/155851/diff/1011/18#newcode248
Line 248: if (underlayImage_) {
can this be factored out? it's exactly the same as above snippet.

[Avi (use Gerrit), 2009-07-21 18:58:25]

Addressed issues.

http://codereview.chromium.org/155851/diff/1001/1005
File chrome/browser/cocoa/gradient_button_cell.h (right):

http://codereview.chromium.org/155851/diff/1001/1005#newcode40
Line 40: // An image to overlay on top of the existing image; not themed. May be
nil.
Can't do it if the variable underlying it is a scoped_nsobject :)

[pink (ping after 24hrs), 2009-07-21 19:05:49]

LGTM