[Mac][Material Design] Adjust (i) and lock Omnibox icons.

[Mac][Material Design] Adjust (i) and lock Omnibox icons.

This cl	brings the (i) icon into spec, and adjusts the position of the
lock icon. The same lock icon is used in https-valid and ev modes but
appears 1px too low in evcert mode on Retina machines. The issue is the
text label that appears next to the lock is 1px	too high - this cl
fixes the position of the text label as well.

R=tapted@chromium.org
BUG=621277
Committed: https://crrev.com/56a023d3893944b000d4e3fa17fe9b6c5d492c48
Cr-Commit-Position: refs/heads/master@{#401666}

[shrike, 2016-06-21 15:53:40]

PTAL

[tapted, 2016-06-22 05:40:38]

https://codereview.chromium.org/2077743005/diff/40001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/location_bar/bubble_decoration.mm (right):

https://codereview.chromium.org/2077743005/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/location_bar/bubble_decoration.mm:140: if
(baseline_offset_) {
Nothing currently calls SetBaselineOffset -- you could probably rename it to
"SetRetinaBaselineOffset, and check [[[control_view] window] backingScaleFactor]
 (see later comment for context)

https://codereview.chromium.org/2077743005/diff/40001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/location_bar/ev_bubble_decoration.mm (right):

https://codereview.chromium.org/2077743005/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/location_bar/ev_bubble_decoration.mm:63: } else {
nit: now that there's an `else` the `if` condition should be inverted to avoid
double-negatives

https://codereview.chromium.org/2077743005/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/location_bar/ev_bubble_decoration.mm:66: // would align.
This 1/2pt adjustment has no effect on non-Retina.
I don't think the "no effect" comment is true. I see the text shifted down a
whole pixel when I patch this in - http://imgur.com/DyKQrMX

[shrike, 2016-06-22 21:44:50]

PTAL

https://codereview.chromium.org/2077743005/diff/40001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/location_bar/bubble_decoration.mm (right):

https://codereview.chromium.org/2077743005/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/location_bar/bubble_decoration.mm:140: if
(baseline_offset_) {
On 2016/06/22 05:40:38, tapted wrote:
> Nothing currently calls SetBaselineOffset -- you could probably rename it to
> "SetRetinaBaselineOffset, and check [[[control_view] window]
backingScaleFactor]
>  (see later comment for context)

Acknowledged.

https://codereview.chromium.org/2077743005/diff/40001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/location_bar/ev_bubble_decoration.mm (right):

https://codereview.chromium.org/2077743005/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/location_bar/ev_bubble_decoration.mm:63: } else {
On 2016/06/22 05:40:38, tapted wrote:
> nit: now that there's an `else` the `if` condition should be inverted to avoid
> double-negatives

Acknowledged.

https://codereview.chromium.org/2077743005/diff/40001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/location_bar/ev_bubble_decoration.mm:66: // would align.
This 1/2pt adjustment has no effect on non-Retina.
On 2016/06/22 05:40:38, tapted wrote:
> I don't think the "no effect" comment is true. I see the text shifted down a
> whole pixel when I patch this in - http://imgur.com/DyKQrMX

Thank you for double-checking.

[tapted, 2016-06-22 23:50:59]

lgtm - there's an awkward mix of hacker_style and camelCase, but I guess this is
consistent with what's there. Probably a follow-up should do a pass over
bubble_decoration.mm converting some camelCase declarations not between
@interface/@end to hacker_style.

[shrike, 2016-06-23 18:03:40]

On 2016/06/22 23:50:59, tapted wrote:
> lgtm - there's an awkward mix of hacker_style and camelCase, but I guess this
is
> consistent with what's there. Probably a follow-up should do a pass over
> bubble_decoration.mm converting some camelCase declarations not between
> @interface/@end to hacker_style.

Acknowledged. My preference is to fix that right now but since this needs to be
cherry-picked back to M52 I'm going to attend to it in a follow-up cl.
https://crbug.com/622779 .

[shrike, 2016-06-23 18:03:48]

The CQ bit was checked by shrike@chromium.org

[commit-bot: I haz the power, 2016-06-23 18:04:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2077743005/60001

[commit-bot: I haz the power, 2016-06-23 18:50:51]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-23 18:53:18]

Description was changed from

==========
[Mac][Material Design] Adjust (i) and lock Omnibox icons.

This cl	brings the (i) icon into spec, and adjusts the position of the
lock icon. The same lock icon is used in https-valid and ev modes but
appears 1px too low in evcert mode on Retina machines. The issue is the
text label that appears next to the lock is 1px	too high - this cl
fixes the position of the text label as well.

R=tapted@chromium.org
BUG=621277
==========

to

==========
[Mac][Material Design] Adjust (i) and lock Omnibox icons.

This cl	brings the (i) icon into spec, and adjusts the position of the
lock icon. The same lock icon is used in https-valid and ev modes but
appears 1px too low in evcert mode on Retina machines. The issue is the
text label that appears next to the lock is 1px	too high - this cl
fixes the position of the text label as well.

R=tapted@chromium.org
BUG=621277

Committed: https://crrev.com/56a023d3893944b000d4e3fa17fe9b6c5d492c48
Cr-Commit-Position: refs/heads/master@{#401666}
==========

[commit-bot: I haz the power, 2016-06-23 18:53:19]

Patchset 4 (id:??) landed as
https://crrev.com/56a023d3893944b000d4e3fa17fe9b6c5d492c48
Cr-Commit-Position: refs/heads/master@{#401666}