[Mac] Convert content settings to LocationBarDecoration, cleanup.

[Mac] Convert content settings to LocationBarDecoration, cleanup.

Cleanup removes all of the AutocompleteTextFieldIcon-based stuff from
the cell implementation.

BUG=none
TEST=content settings still appear at the right times and places.
TEST=content settings can be clicked on and the popup is positioned right.

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

[Scott Hess - ex-Googler, 2010-07-16 17:49:59]

Getting to the final stretch, here!  Feels nice to delete code from
AutocompleteTextFieldCell :-).

[rohitrao (ping after 24h), 2010-07-16 18:17:24]

LGTM, with a few nits.

There's probably some more cleaning up we can do in the
AutocompleteTextFieldCell and LocationBarViewMac code, but that can wait until
next week :)  I'll read through it at some point and see what I find.

http://codereview.chromium.org/2888014/diff/1/5
File chrome/browser/cocoa/location_bar/autocomplete_text_field_cell.mm (left):

http://codereview.chromium.org/2888014/diff/1/5#oldcode512
chrome/browser/cocoa/location_bar/autocomplete_text_field_cell.mm:512: -
(NSArray*)layedOutIcons:(NSRect)cellFrame {
I was hoping this would go away.

http://codereview.chromium.org/2888014/diff/1/7
File chrome/browser/cocoa/location_bar/content_setting_decoration.mm (right):

http://codereview.chromium.org/2888014/diff/1/7#newcode66
chrome/browser/cocoa/location_bar/content_setting_decoration.mm:66: frame =
GetDrawRectInFrame(frame);
You didn't change the name of this method?

http://codereview.chromium.org/2888014/diff/1/9
File chrome/browser/cocoa/location_bar/location_bar_view_mac.mm (right):

http://codereview.chromium.org/2888014/diff/1/9#newcode505
chrome/browser/cocoa/location_bar/location_bar_view_mac.mm:505: for (size_t i =
content_setting_decorations_.size(); i--; ) {
Is this a standard way of writing for loops?  It was a bit too clever for me, so
if it's not widely-used, I'd prefer something a little simpler :)

[Scott Hess - ex-Googler, 2010-07-16 18:44:32]

thanks for the fast turnaround.

http://codereview.chromium.org/2888014/diff/1/7
File chrome/browser/cocoa/location_bar/content_setting_decoration.mm (right):

http://codereview.chromium.org/2888014/diff/1/7#newcode66
chrome/browser/cocoa/location_bar/content_setting_decoration.mm:66: frame =
GetDrawRectInFrame(frame);
On 2010/07/16 18:17:24, rohitrao wrote:
> You didn't change the name of this method?

I think I blathered about "Get DrawRect In Frame" meaning DrawRect was a noun. 
And I didn't have a better idea.  ImageRect?

http://codereview.chromium.org/2888014/diff/1/9
File chrome/browser/cocoa/location_bar/location_bar_view_mac.mm (right):

http://codereview.chromium.org/2888014/diff/1/9#newcode505
chrome/browser/cocoa/location_bar/location_bar_view_mac.mm:505: for (size_t i =
content_setting_decorations_.size(); i--; ) {
On 2010/07/16 18:17:24, rohitrao wrote:
> Is this a standard way of writing for loops?  It was a bit too clever for me,
so
> if it's not widely-used, I'd prefer something a little simpler :)

I don't know, it's iterating from top to bottom ... was going to suggest
reversing the list at creation, but ScopedVector doesn't have that directly, so
would be annoying.  Instead I observe that the drawing-direction here is
extremely cargo-cult, and I can see no evidence in the views code that this is
the desired thing.  So I'll just send them over in the order they came in and
hope for the best.

[rohitrao (ping after 24h), 2010-07-16 18:46:12]

LGTM

>> You didn't change the name of this method?
>
> I think I blathered about "Get DrawRect In Frame" meaning DrawRect was a
> noun.  And I didn't have a better idea.  ImageRect?

Meh, was mostly just making sure you had tried to compile this :)
Speaking of, another mac try run might not be bad, since the patch
failed the last time.

[Scott Hess - ex-Googler, 2010-07-16 18:52:02]

Yeah, I've queued up the try run...

On Fri, Jul 16, 2010 at 11:46 AM, Rohit Rao <rohitrao@chromium.org> wrote:
> LGTM
>
>>> You didn't change the name of this method?
>>
>> I think I blathered about "Get DrawRect In Frame" meaning DrawRect was a
>> noun.  And I didn't have a better idea.  ImageRect?
>
> Meh, was mostly just making sure you had tried to compile this :)
> Speaking of, another mac try run might not be bad, since the patch
> failed the last time.
>