VR: Support security chip text on URL bar.

VR: Support security chip text on URL bar.

- Add rendering of "Secure", "Not secure" or "Dangerous" next to the
  security icon in the URL bar.
- If present, include the text in the PageInfo clickable region.
- Plumb malware status through to the URL bar to properly differentiate
  "Not secure" from "Dangerous", as is done by ToolbarModel.
- Fix the broken HTTPS strike through from diagonal to horizontal, as
  a UX study determined is ideal.

BUG=734206
Review-Url: https://codereview.chromium.org/2946523002
Cr-Commit-Position: refs/heads/master@{#480635}
Committed: https://chromium.googlesource.com/chromium/src/+/b5ae18a01a48b511ca116d9edfb0edf1b037aa8f

[cjgrant, 2017-06-16 21:28:29]

cjgrant@chromium.org changed reviewers:
+ ymalik@chromium.org

[ymalik, 2017-06-19 14:33:05]

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/textures/url_bar_texture.cc (right):

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture.cc:285: if
(!gurl_.is_empty() && chip_string_id != 0) {
I'm not sure if we want to show this in the transient case for now. Could you
just add a && !web_vr for now and we can add it back later if security thinks we
need it (I only remember them mentioning the security status and domain)?

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture.cc:286: float
chip_max_width = kWidth - left_edge - kUrlRightMargin;
Are we allowing the chip text portion to take precedence over the url? Is this
the behavior on other platforms? 

It should probably never happen in EN because we know exactly what we're
showing, but I don't know if this is true for other languages.

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture.cc:296: // TODO: Can we
call this more than once?
Please clarify what this means and add owner or bug.

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture.cc:333: float url_width
= kWidth - url_x - kUrlRightMargin;
Correct me if I'm wrong, but this looks like we're giving more real estate to
the URL than we actually have. If string_size.width() == chip_max_width, we're
still adding kFieldSpacing and kSeparatorWidth to left_edge. We should probably
remove that from chip_max_width above?

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/textures/url_bar_texture.h (right):

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture.h:68: gfx::PointF
UrlBarPositionMeters() const;
This is no longer used.

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture.h:86: gfx::RectF
security_hit_region_ = gfx::RectF(0, 0, 0, 0);
Should we do a similar thing for the back button for consistency? It would be
less prettier though because we still have to have a check for
HitsTransparentRegion.

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/textures/url_bar_texture_unittest.cc
(right):

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture_unittest.cc:239:
TEST(UrlBarTexture, Visualize) {
Did you mean to commit this?

[cjgrant, 2017-06-19 15:39:39]

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/textures/url_bar_texture.cc (right):

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture.cc:285: if
(!gurl_.is_empty() && chip_string_id != 0) {
On 2017/06/19 14:33:05, ymalik wrote:
> I'm not sure if we want to show this in the transient case for now. Could you
> just add a && !web_vr for now and we can add it back later if security thinks
we
> need it (I only remember them mentioning the security status and domain)? 

I've disabled it entirely in this CL.  A follow-on CL will enable (for browser,
and possibly WebVR).

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture.cc:286: float
chip_max_width = kWidth - left_edge - kUrlRightMargin;
On 2017/06/19 14:33:05, ymalik wrote:
> Are we allowing the chip text portion to take precedence over the url? Is this
> the behavior on other platforms? 
> 
> It should probably never happen in EN because we know exactly what we're
> showing, but I don't know if this is true for other languages. 

The behavior on desktop is to elide both, allocating about equal space to
either.  We don't elide at all yet, so I think we want to have the chip take all
the space it needs.  We'd then bail as we currently do if the origin doesn't fit
in the remaining space.

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture.cc:296: // TODO: Can we
call this more than once?
On 2017/06/19 14:33:05, ymalik wrote:
> Please clarify what this means and add owner or bug.

Lets talk offline about this.

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture.cc:333: float url_width
= kWidth - url_x - kUrlRightMargin;
On 2017/06/19 14:33:05, ymalik wrote:
> Correct me if I'm wrong, but this looks like we're giving more real estate to
> the URL than we actually have. If string_size.width() == chip_max_width, we're
> still adding kFieldSpacing and kSeparatorWidth to left_edge. We should
probably
> remove that from chip_max_width above?

I don't think there's a problem. Over-the-shoulder chat?

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/textures/url_bar_texture.h (right):

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture.h:68: gfx::PointF
UrlBarPositionMeters() const;
On 2017/06/19 14:33:05, ymalik wrote:
> This is no longer used.

Done.

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture.h:86: gfx::RectF
security_hit_region_ = gfx::RectF(0, 0, 0, 0);
On 2017/06/19 14:33:05, ymalik wrote:
> Should we do a similar thing for the back button for consistency? It would be
> less prettier though because we still have to have a check for
> HitsTransparentRegion. 

Done.  No problem with HitsTransparentRegion.  I think using this approach is
good from a consistency perspective.

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/textures/url_bar_texture_unittest.cc
(right):

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture_unittest.cc:239:
TEST(UrlBarTexture, Visualize) {
On 2017/06/19 14:33:05, ymalik wrote:
> Did you mean to commit this?

I meant to discuss whether we want example code checked in.  I think it's better
if we leave it out, as each texture needs custom test code.  Removed.

https://codereview.chromium.org/2946523002/diff/20001/chrome/browser/android/...
File chrome/browser/android/vr_shell/textures/url_bar_texture.cc (right):

https://codereview.chromium.org/2946523002/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/textures/url_bar_texture.cc:126:
has_security_chip_(false),
I'm leaving this feature disabled until enamel/UX/us decide we want it.

[ymalik, 2017-06-19 15:54:20]

lgtm

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
File chrome/browser/android/vr_shell/textures/url_bar_texture.cc (right):

https://codereview.chromium.org/2946523002/diff/1/chrome/browser/android/vr_s...
chrome/browser/android/vr_shell/textures/url_bar_texture.cc:333: float url_width
= kWidth - url_x - kUrlRightMargin;
On 2017/06/19 15:39:39, cjgrant wrote:
> On 2017/06/19 14:33:05, ymalik wrote:
> > Correct me if I'm wrong, but this looks like we're giving more real estate
to
> > the URL than we actually have. If string_size.width() == chip_max_width,
we're
> > still adding kFieldSpacing and kSeparatorWidth to left_edge. We should
> probably
> > remove that from chip_max_width above?
> 
> I don't think there's a problem. Over-the-shoulder chat?

Done.

https://codereview.chromium.org/2946523002/diff/20001/chrome/browser/android/...
File chrome/browser/android/vr_shell/textures/url_bar_texture.cc (right):

https://codereview.chromium.org/2946523002/diff/20001/chrome/browser/android/...
chrome/browser/android/vr_shell/textures/url_bar_texture.cc:126:
has_security_chip_(false),
On 2017/06/19 15:39:39, cjgrant wrote:
> I'm leaving this feature disabled until enamel/UX/us decide we want it.

Acknowledged.

[cjgrant, 2017-06-19 21:31:29]

The CQ bit was checked by cjgrant@chromium.org

[commit-bot: I haz the power, 2017-06-19 21:32:04]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-19 23:33:43]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1497907889286070,
"parent_rev": "b4c8c8d2cfb4bc8e91907a6e34d67aef6f813220", "commit_rev":
"b5ae18a01a48b511ca116d9edfb0edf1b037aa8f"}

[commit-bot: I haz the power, 2017-06-19 23:33:56]

Description was changed from

==========
VR: Support security chip text on URL bar.

- Add rendering of "Secure", "Not secure" or "Dangerous" next to the
  security icon in the URL bar.
- If present, include the text in the PageInfo clickable region.
- Plumb malware status through to the URL bar to properly differentiate
  "Not secure" from "Dangerous", as is done by ToolbarModel.
- Fix the broken HTTPS strike through from diagonal to horizontal, as
  a UX study determined is ideal.

BUG=734206
==========

to

==========
VR: Support security chip text on URL bar.

- Add rendering of "Secure", "Not secure" or "Dangerous" next to the
  security icon in the URL bar.
- If present, include the text in the PageInfo clickable region.
- Plumb malware status through to the URL bar to properly differentiate
  "Not secure" from "Dangerous", as is done by ToolbarModel.
- Fix the broken HTTPS strike through from diagonal to horizontal, as
  a UX study determined is ideal.

BUG=734206

Review-Url: https://codereview.chromium.org/2946523002
Cr-Commit-Position: refs/heads/master@{#480635}
Committed:
https://chromium.googlesource.com/chromium/src/+/b5ae18a01a48b511ca116d9edfb0...
==========

[commit-bot: I haz the power, 2017-06-19 23:33:57]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/b5ae18a01a48b511ca116d9edfb0...