[Mac] Rearrange SSL status icon/label in omnibox.

[Mac] Rearrange SSL status icon/label in omnibox.

Refactor action icons with an eye towards adding additional icons in
the future.  Renames the SSL status icon to be compatible with
pkasting's upcoming changes, and splits it between the icon on the
left and the label floating on the right.

BUG=37865
TEST=SSL icon should be to the left, and clickable.
TEST=SSL label should float to the left of page actions and not be clickable.

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

[Scott Hess - ex-Googler, 2010-03-29 23:29:56]

[pkasting in case this ends up going in before his complementary change.  Pam
because I think she was involved with this code originally.]

A future change will stuff the "starred" button in the page-actions area, which
is pretty simple with this change.

[Scott Hess - ex-Googler, 2010-03-30 19:27:34]

Michael, I don't think my changes will mess up -pageActionFromForIndex:inFrame:,
though there might be merge joy.

[rohitrao (ping after 24h), 2010-03-30 23:50:42]

LGTM

Fix the missing (id).  The rest of my comments are just questions to ponder.

http://codereview.chromium.org/1566004/diff/4001/1009
File chrome/browser/cocoa/autocomplete_text_field_cell.h (right):

http://codereview.chromium.org/1566004/diff/4001/1009#newcode35
chrome/browser/cocoa/autocomplete_text_field_cell.h:35: -
initImageWithView:(LocationBarViewMac::LocationBarImageView*)view;
- (id)... on both these methods.  Also in the .mm file.

http://codereview.chromium.org/1566004/diff/4001/1010
File chrome/browser/cocoa/autocomplete_text_field_cell.mm (right):

http://codereview.chromium.org/1566004/diff/4001/1010#newcode42
chrome/browser/cocoa/autocomplete_text_field_cell.mm:42: const NSInteger
kIconLabelYOffset = 7;
Does this adjust the baseline of the certificate owner text?  Pink and thakis
fought over whether to center this or align with the url text, and thakis won :)

http://codereview.chromium.org/1566004/diff/4001/1010#newcode77
chrome/browser/cocoa/autocomplete_text_field_cell.mm:77: -
initWithView:(LocationBarViewMac::LocationBarImageView*)view
- (id)...

x2 more below.

http://codereview.chromium.org/1566004/diff/4001/1010#newcode98
chrome/browser/cocoa/autocomplete_text_field_cell.mm:98: const NSSize imageSize
= view_->GetImageSize();
Is this used anywhere?

http://codereview.chromium.org/1566004/diff/4001/1010#newcode428
chrome/browser/cocoa/autocomplete_text_field_cell.mm:428: NSMutableArray* result
= [NSMutableArray array];
Is there benefit to creating and caching this array, rather than creating it
every time.

http://codereview.chromium.org/1566004/diff/4001/1010#newcode476
chrome/browser/cocoa/autocomplete_text_field_cell.mm:476: cellFrame.size.width =
NSMinX([icon rect]) - kIconHorizontalPad;
Do we handle page action icon overflow at all?  If not, is there a bug for that?

http://codereview.chromium.org/1566004/diff/4001/1013
File chrome/browser/cocoa/location_bar_view_mac.h (right):

http://codereview.chromium.org/1566004/diff/4001/1013#newcode157
chrome/browser/cocoa/location_bar_view_mac.h:157: virtual NSSize
GetDefaultImageSize() const;
Are you just adding these here in prep for future changes?

[Nico, 2010-03-31 03:04:17]

Dive-by rambling.

http://codereview.chromium.org/1566004/diff/4001/1009
File chrome/browser/cocoa/autocomplete_text_field_cell.h (right):

http://codereview.chromium.org/1566004/diff/4001/1009#newcode19
chrome/browser/cocoa/autocomplete_text_field_cell.h:19: // Omnibox stuff is
settled.
I added this in http://codereview.chromium.org/600133 , it made a lot of code
way easier (previously, all icon walking code explicitly walked security icon,
page actions, etc).

Not that I'm against removing it if you find a good way, but if you don't have a
concrete plan, I'd advise against sprinkling random TODOs into the code base.

[Scott Hess - ex-Googler, 2010-03-31 18:07:53]

As you noted, some of the refactoring in here is partial, with obvious future
directions.  My main goal right now is to not make things worse, but there
should be additional stuff in a week or two, depending on schedule.

http://codereview.chromium.org/1566004/diff/4001/1009
File chrome/browser/cocoa/autocomplete_text_field_cell.h (right):

http://codereview.chromium.org/1566004/diff/4001/1009#newcode35
chrome/browser/cocoa/autocomplete_text_field_cell.h:35: -
initImageWithView:(LocationBarViewMac::LocationBarImageView*)view;
On 2010/03/30 23:50:42, rohitrao wrote:
> - (id)... on both these methods.  Also in the .mm file.

Done.

http://codereview.chromium.org/1566004/diff/4001/1010
File chrome/browser/cocoa/autocomplete_text_field_cell.mm (right):

http://codereview.chromium.org/1566004/diff/4001/1010#newcode42
chrome/browser/cocoa/autocomplete_text_field_cell.mm:42: const NSInteger
kIconLabelYOffset = 7;
On 2010/03/30 23:50:42, rohitrao wrote:
> Does this adjust the baseline of the certificate owner text?  Pink and thakis
> fought over whether to center this or align with the url text, and thakis won
:)

Yes.  IMHO it looks poor to have adjacent text runs which use completely
different baselines.

http://codereview.chromium.org/1566004/diff/4001/1010#newcode77
chrome/browser/cocoa/autocomplete_text_field_cell.mm:77: -
initWithView:(LocationBarViewMac::LocationBarImageView*)view
On 2010/03/30 23:50:42, rohitrao wrote:
> - (id)...
> 
> x2 more below.

Done.

http://codereview.chromium.org/1566004/diff/4001/1010#newcode98
chrome/browser/cocoa/autocomplete_text_field_cell.mm:98: const NSSize imageSize
= view_->GetImageSize();
On 2010/03/30 23:50:42, rohitrao wrote:
> Is this used anywhere?

Oops.  Removed.  Uncertain why the compiler didn't bitch about an unused
variable.

http://codereview.chromium.org/1566004/diff/4001/1010#newcode428
chrome/browser/cocoa/autocomplete_text_field_cell.mm:428: NSMutableArray* result
= [NSMutableArray array];
On 2010/03/30 23:50:42, rohitrao wrote:
> Is there benefit to creating and caching this array, rather than creating it
> every time.

Possibly.  I'm not immediately worried about performance issues, it's pretty
light-weight, just an array of objects which process cached NSSize information,
pretty much.  So a load of Objective-C messaging is certainly involved.  It
could potentially be regenerated in -resetFieldEditorFrameIfNeeded.  I think
that would be a reasonable idea for refactoring because right now various
independent callers make decisions which have to match (click-testing really
needs to match what was drawn).

http://codereview.chromium.org/1566004/diff/4001/1010#newcode476
chrome/browser/cocoa/autocomplete_text_field_cell.mm:476: cellFrame.size.width =
NSMinX([icon rect]) - kIconHorizontalPad;
On 2010/03/30 23:50:42, rohitrao wrote:
> Do we handle page action icon overflow at all?  If not, is there a bug for
that?

I don't know if there's a specific bug.  I've added a note to the uber bug, I
plan to just work within that bug for another couple days and then burst out
other stuff depending on whether it seems appropriate to defer to later.

http://codereview.chromium.org/1566004/diff/4001/1013
File chrome/browser/cocoa/location_bar_view_mac.h (right):

http://codereview.chromium.org/1566004/diff/4001/1013#newcode157
chrome/browser/cocoa/location_bar_view_mac.h:157: virtual NSSize
GetDefaultImageSize() const;
On 2010/03/30 23:50:42, rohitrao wrote:
> Are you just adding these here in prep for future changes?

The previous field code used GetPreferredSize() as a special-case for
page-actions, while for other items it used GetImage() and retrieved the size
from that.  GetPreferredSize() returned a default size in case of no image. 
This wraps it all up so that GetImageSize() tells the caller what the size of
the item will be, so it doesn't care about the specific action being called.

That said, AFAICT it might be reasonable to just make them all the same size and
be done with it, in order to provide visually-consistent layout and a
decently-sized target for events.  In which case a fair by of dynamic
calculation can be chucked.