[Mac] Star as a rhs-decoration in AutocompleteTextFieldCell.

[Mac] Star as a rhs-decoration in AutocompleteTextFieldCell.

Refactor to convert star decoration to LocationBarDecoration.

BUG=none
TEST=Star icon continues to work.
TEST=Bookmark bubble pops in the right place.

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

[rohitrao (ping after 24h), 2010-07-14 17:02:57]

LGTM

What's the plan for multiple RHS decorations, with crazy ordering rules?  Page
action icons go on the left, in the order they were added, but the star is
always on the right, unless the moon is aligned incorrectly.  Do we just put all
that crazy logic into Layout()?

http://codereview.chromium.org/2971004/diff/1/3
File chrome/browser/cocoa/location_bar/autocomplete_text_field_cell.mm (right):

http://codereview.chromium.org/2971004/diff/1/3#newcode112
chrome/browser/cocoa/location_bar/autocomplete_text_field_cell.mm:112: const
NSRect cell_frame,
Is Trung going to yell about constness here?

http://codereview.chromium.org/2971004/diff/1/3#newcode188
chrome/browser/cocoa/location_bar/autocomplete_text_field_cell.mm:188:
DCHECK_EQ(decorations->size(), decoration_frames->size());
Does it make sense to move this check into the helper function?  (Also below.)

http://codereview.chromium.org/2971004/diff/1/4
File chrome/browser/cocoa/location_bar/image_decoration.h (right):

http://codereview.chromium.org/2971004/diff/1/4#newcode23
chrome/browser/cocoa/location_bar/image_decoration.h:23: NSRect
GetDrawRectInFrame(NSRect frame);
I got confused by a method name that started with two verbs :)

http://codereview.chromium.org/2971004/diff/1/11
File chrome/browser/cocoa/location_bar/star_decoration.mm (right):

http://codereview.chromium.org/2971004/diff/1/11#newcode35
chrome/browser/cocoa/location_bar/star_decoration.mm:35: return
NSMakePoint(NSMidX(draw_frame), NSMaxY(draw_frame) - 4.0);
Where did 4.0 come from?  Can it be a constant?

[Scott Hess - ex-Googler, 2010-07-14 20:19:20]

On 2010/07/14 17:02:57, rohitrao wrote:
> LGTM

Applied your suggestions, and merged in the globe fix (I had fixed it alrady as
part of breaking out ImageDecoration, the shame).  That one will go in first.

> What's the plan for multiple RHS decorations, with crazy ordering rules?  Page
> action icons go on the left, in the order they were added, but the star is
> always on the right, unless the moon is aligned incorrectly.  Do we just put
all
> that crazy logic into Layout()?

Layout() will have all that crazy logic.  I believe that everything is ordered
the same way at all times, but visibility changes depending on mode.  So
everything will be present in the list, but sometimes visible, sometimes not. 
There's also a level of size-to-fit involved which I suspect will lead to some
tweaks.  I believe all of this can (reasonably) be labelled "bugs" rather than
"features", though, so long as all of of the infrastructure is in there baking.

http://codereview.chromium.org/2971004/diff/1/3
File chrome/browser/cocoa/location_bar/autocomplete_text_field_cell.mm (right):

http://codereview.chromium.org/2971004/diff/1/3#newcode112
chrome/browser/cocoa/location_bar/autocomplete_text_field_cell.mm:112: const
NSRect cell_frame,
On 2010/07/14 17:02:57, rohitrao wrote:
> Is Trung going to yell about constness here?

I'll un-const them.

http://codereview.chromium.org/2971004/diff/1/3#newcode188
chrome/browser/cocoa/location_bar/autocomplete_text_field_cell.mm:188:
DCHECK_EQ(decorations->size(), decoration_frames->size());
On 2010/07/14 17:02:57, rohitrao wrote:
> Does it make sense to move this check into the helper function?  (Also below.)

I will add another check to the helper function.  Hard to see a downside to
excessive checks on this.

Could replace the parallel vectors with a vector of pairs.  WDYT?  I'm kind of
mixed on the notion, since everything will get a bit more verbose (the overall
statement being made will be the same, but using fewer longer sentences instead
of more shorter sentences).

http://codereview.chromium.org/2971004/diff/1/4
File chrome/browser/cocoa/location_bar/image_decoration.h (right):

http://codereview.chromium.org/2971004/diff/1/4#newcode23
chrome/browser/cocoa/location_bar/image_decoration.h:23: NSRect
GetDrawRectInFrame(NSRect frame);
On 2010/07/14 17:02:57, rohitrao wrote:
> I got confused by a method name that started with two verbs :)

Hmm.  DrawRect being a noun, here, but I can see what you mean.  I didn't like
the Rect and Frame both in there, but DrawFrame seemed even worse.

GetImageRectInFrame(), maybe?

http://codereview.chromium.org/2971004/diff/1/11
File chrome/browser/cocoa/location_bar/star_decoration.mm (right):

http://codereview.chromium.org/2971004/diff/1/11#newcode35
chrome/browser/cocoa/location_bar/star_decoration.mm:35: return
NSMakePoint(NSMidX(draw_frame), NSMaxY(draw_frame) - 4.0);
On 2010/07/14 17:02:57, rohitrao wrote:
> Where did 4.0 come from?  Can it be a constant?

Came from the earlier code, which came from inspection.  Will move to a
constant.