VR: Add URL bar security-related coloring and emphasis.

chrome/browser/android/vr_shell/textures/url_bar_texture.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/android/vr_shell/textures/url_bar_texture.h"
 
 #include "base/strings/utf_string_conversions.h"
 #include "cc/paint/skia_paint_canvas.h"
-#include "components/security_state/core/security_state.h"
+#include "components/url_formatter/url_formatter.h"
 #include "ui/gfx/canvas.h"
+#include "ui/gfx/color_palette.h"
 #include "ui/gfx/font.h"
 #include "ui/gfx/font_list.h"
 #include "ui/gfx/geometry/point_f.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/rect_f.h"
 #include "ui/gfx/paint_vector_icon.h"
 #include "ui/gfx/render_text.h"
 #include "ui/gfx/vector_icon_types.h"
 #include "ui/vector_icons/vector_icons.h"
 
 namespace vr_shell {
 
 namespace {
 
-// TODO(mthiesse): These values are all wrong. The UX spec is unclear.
-static constexpr SkColor kBackground = 0x66D6D6D6;
-static constexpr SkColor kBackgroundHover = 0x6EF0F0F0;
-static constexpr SkColor kBackgroundDown = 0x76F6F6F6;
+// TODO(mthiesse,cjgrant): These values are all wrong. The UX spec is unclear,
+// and some have been adapted to make the URL bar legible.
+static constexpr SkColor kBackground = 0xC0F0F0F0;
+static constexpr SkColor kBackgroundHover = 0xE0F8F8F8;
+static constexpr SkColor kBackgroundDown = 0xFFFFFFFF;
 static constexpr SkColor kForeground = 0xFF333333;
 static constexpr SkColor kSeparatorColor = 0x33000000;
 
-static constexpr SkColor kInfoOutlineIconColor = 0xFF5A5A5A;
-static constexpr SkColor kLockIconColor = 0xFF0B8043;
-static constexpr SkColor kWarningIconColor = 0xFFC73821;
+static constexpr SkColor kUrlTextColor = 0xFF000000;
+static constexpr SkColor kUrlDeemphasizedTextColor = 0xFF5A5A5A;
+static const SkColor kSecureColor = gfx::kGoogleGreen700;
+static const SkColor kWarningColor = gfx::kGoogleRed700;
 
 static constexpr float kWidth = 0.672;
 static constexpr float kHeight = 0.088;
 static constexpr float kFontHeight = 0.027;
 static constexpr float kBackButtonWidth = kHeight;
-static constexpr float kBackIconHeight = 0.05;
+static constexpr float kBackIconHeight = 0.0375;
 static constexpr float kBackIconOffset = 0.005;
 static constexpr float kSecurityFieldWidth = 0.06;
 static constexpr float kSecurityIconHeight = 0.03;
 static constexpr float kUrlRightMargin = 0.02;
 static constexpr float kSeparatorWidth = 0.002;
 
 using security_state::SecurityLevel;
 
-const struct gfx::VectorIcon& getSecurityIcon(int level) {
+const struct gfx::VectorIcon& getSecurityIcon(SecurityLevel level) {
   switch (level) {
     case SecurityLevel::NONE:
     case SecurityLevel::HTTP_SHOW_WARNING:
     case SecurityLevel::SECURITY_WARNING:
       return ui::kInfoOutlineIcon;
     case SecurityLevel::SECURE:
     case SecurityLevel::EV_SECURE:
       return ui::kLockIcon;
-    case SecurityLevel::SECURE_WITH_POLICY_INSTALLED_CERT:
     case SecurityLevel::DANGEROUS:
+      return ui::kWarningIcon;
+    case SecurityLevel::SECURE_WITH_POLICY_INSTALLED_CERT:  // ChromeOS only.
     default:
+      NOTREACHED();
       return ui::kWarningIcon;
   }
 }
 
-SkColor getSecurityIconColor(int level) {
+// See LocationBarView::GetSecureTextColor().
+SkColor getSchemeColor(SecurityLevel level) {
   switch (level) {
     case SecurityLevel::NONE:
     case SecurityLevel::HTTP_SHOW_WARNING:
     case SecurityLevel::SECURITY_WARNING:
-      return kInfoOutlineIconColor;
+      return kUrlDeemphasizedTextColor;
     case SecurityLevel::SECURE:
     case SecurityLevel::EV_SECURE:
-      return kLockIconColor;
-    case SecurityLevel::SECURE_WITH_POLICY_INSTALLED_CERT:
+      return kSecureColor;
     case SecurityLevel::DANGEROUS:
+      return kWarningColor;
+    case SecurityLevel::SECURE_WITH_POLICY_INSTALLED_CERT:  // ChromeOS only.
     default:
-      return kWarningIconColor;
+      NOTREACHED();
+      return kWarningColor;
   }
 }
 
+void SetEmphasis(gfx::RenderText* render_text,
+                 bool emphasis,
+                 const gfx::Range& range) {
+  SkColor color = emphasis ? kUrlTextColor : kUrlDeemphasizedTextColor;
+  if (range.IsValid()) {
+    render_text->ApplyColor(color, range);
+  } else {
+    render_text->SetColor(color);
+  }
+}
+
 gfx::PointF percentToMeters(const gfx::PointF& percent) {
   return gfx::PointF(percent.x() * kWidth, percent.y() * kHeight);
 }
 
 }  // namespace
 
 UrlBarTexture::UrlBarTexture() : security_level_(SecurityLevel::DANGEROUS) {}
 
 UrlBarTexture::~UrlBarTexture() = default;
 
 void UrlBarTexture::SetURL(const GURL& gurl) {
   if (gurl_ != gurl)
     set_dirty();
   gurl_ = gurl;
 }
 
-void UrlBarTexture::SetSecurityLevel(int level) {
-  if (&getSecurityIcon(security_level_) != &getSecurityIcon(level))
+void UrlBarTexture::SetSecurityLevel(SecurityLevel level) {
+  if (security_level_ != level)
     set_dirty();
   security_level_ = level;
 }
 
 float UrlBarTexture::ToPixels(float meters) const {
   return meters * size_.width() / kWidth;
 }
 
 bool UrlBarTexture::HitsBackButton(const gfx::PointF& position) const {
   const gfx::PointF& meters = percentToMeters(position);
@@ skipping 77 matching lines @@
   // Site security state icon.
   if (!gurl_.is_empty()) {
     canvas->save();
     canvas->translate(
         kBackButtonWidth + kSeparatorWidth + kSecurityFieldWidth / 2,
         kHeight / 2);
     canvas->translate(-kSecurityIconHeight / 2, -kSecurityIconHeight / 2);
     const gfx::VectorIcon& icon = getSecurityIcon(security_level_);
     icon_default_height = GetDefaultSizeOfVectorIcon(icon);
     icon_scale = kSecurityIconHeight / icon_default_height;
-    SkColor icon_color = getSecurityIconColor(security_level_);
+    SkColor icon_color = getSchemeColor(security_level_);
     canvas->scale(icon_scale, icon_scale);
     PaintVectorIcon(&gfx_canvas, icon, icon_color);
     canvas->restore();
   }
 
   canvas->restore();
 
   if (!gurl_.is_empty()) {
-    if (last_drawn_gurl_ != gurl_) {
-      // Draw text based on pixel sizes rather than meters, for correct font
-      // sizing.
-      int pixel_font_height = texture_size.height() * kFontHeight / kHeight;
+    if (last_drawn_gurl_ != gurl_ ||
+        last_drawn_security_level_ != security_level_) {
       float url_x = kBackButtonWidth + kSeparatorWidth + kSecurityFieldWidth;
       float url_width = kWidth - url_x - kUrlRightMargin;
       gfx::Rect text_bounds(ToPixels(url_x), 0, ToPixels(url_width),
                             ToPixels(kHeight));
-      gurl_render_texts_ = PrepareDrawStringRect(
-          base::UTF8ToUTF16(gurl_.spec()),
-          GetDefaultFontList(pixel_font_height), SK_ColorBLACK, &text_bounds,
-          kTextAlignmentLeft, kWrappingBehaviorNoWrap);
+      RenderUrl(texture_size, text_bounds);
       last_drawn_gurl_ = gurl_;
+      last_drawn_security_level_ = security_level_;
     }
-    for (auto& render_text : gurl_render_texts_)
-      render_text->Draw(&gfx_canvas);
+    url_render_text_->Draw(&gfx_canvas);
   }
 }
 
+void UrlBarTexture::RenderUrl(const gfx::Size& texture_size,
+                              const gfx::Rect& bounds) {
+  url::Parsed parsed;
+  const base::string16 text = url_formatter::FormatUrl(
+      gurl_, url_formatter::kFormatUrlOmitAll, net::UnescapeRule::NORMAL,
+      &parsed, nullptr, nullptr);
+
+  int pixel_font_height = texture_size.height() * kFontHeight / kHeight;
+  auto font_list = GetFontList(pixel_font_height, text);
+
+  std::unique_ptr<gfx::RenderText> render_text(
+      gfx::RenderText::CreateInstance());
+  render_text->SetText(text);
+  render_text->SetFontList(font_list);
+  render_text->SetColor(SK_ColorBLACK);
+  render_text->SetHorizontalAlignment(gfx::ALIGN_LEFT);
+  render_text->SetDisplayRect(bounds);
+  render_text->SetElideBehavior(gfx::TRUNCATE);

[estark, 2017/05/25 01:28:32]

Does this just truncate at the end? If so you'll need to elide as described in
https://dev.chromium.org/Home/chromium-security/enamel#TOC-Presenting-Origins
instead.

[cjgrant, 2017/05/25 01:55:28]

Certainly.  Eliding (and other things) are called out as To-Do's in Ian's UI
security doc:
https://docs.google.com/document/d/1rACrmg5Lb6xwOnG7So4_2sLeUR02Ra-oYfZvz0ixxHM

+
+  ApplyUrlStyling(text, parsed, render_text.get());
+
+  url_render_text_ = std::move(render_text);
+}
+
+// This method replicates behavior in OmniboxView::UpdateTextStyle(), and

[estark, 2017/05/24 01:11:07]

Would it be possible to share any of this code by exposing it from 
components/omnibox or components/security_state/security_state_ui.h?

Also, can you please add tests for this class, to make sure that URL formatting,
coloring, and emphasis for the VR browser don't regress?

[cjgrant, 2017/05/24 17:40:58]

On reuse, I considered this, but the Omnibox code is heavily tied to its model
and view classes, and needed to be trimmed down to work here.  The most reusable
part is the de-emphasis code, which could be adapted to a helper method taking a
SetEmphasis callback (desktop would supply its SetEmphasis virtual).  This could
hurt readability of the desktop code though, and invites the question "why
aren't you using more omnibox code?", which we hope to in the future.  Would you
like the callback + helper refactor done?

On testing - I had asked some Skia/GFX folks yesterday about mocks or testable
RenderText classes, as the only one I saw was embedded in RenderText unit tests.
 I had hoped to have test inputs as GURLs and security levels, and outputs as
sets of characters with color and style associated, but I don't know if this is
possible with current tools.  Building our own RenderText mock doesn't seem like
a good plan either.  If you have any thoughts on a best approach here, let me
know!  I'll keep digging.

[cjgrant, 2017/05/25 00:50:59]

After some discussion on our team, we'd like to go ahead with a RenderText mock
and use it on this function.  I'm working on these tests now.

+// attempts to maintain similar code structure.
+void UrlBarTexture::ApplyUrlStyling(const base::string16& formatted_url,
+                                    const url::Parsed& parsed,
+                                    gfx::RenderText* render_text) {
+  const url::Component& scheme = parsed.scheme;
+  const url::Component& host = parsed.host;
+
+  enum DeemphasizeComponents {
+    EVERYTHING,
+    ALL_BUT_SCHEME,
+    ALL_BUT_HOST,
+    NOTHING,
+  } deemphasize = NOTHING;
+
+  const base::string16 url_scheme =

[estark, 2017/05/25 01:28:32]

Have you looked at AutocompleteInput::Parse? Are there meaningful differences
between what that produces and what this code will produce? It looks like that
function is called by OmniboxView and does a lot of stuff, so it's not clear to
me whether we need to do that stuff here as well.

[cjgrant, 2017/05/25 01:55:28]

Addressing your top-level comment and this comment here:

This is a read-only, non-interactive URL bar, so anything geared to handle input
(or autocomplete suggestions) isn't hooked up.

I have had a whiteboard session with pkasting, but not in the context of a
read-only bar.  Besides the input-only nature, Clank already has Java-wrapped
instances of ToolbarModel and related pieces to drive its own Omnibox.  I
believe that using OmniboxView for VR means wiring it to other classes that are
owned by Java, and not necessarily meant to drive multiple omniboxes.  This is
absolutely something we have to address for the product, but seems out of scope
for the bare-bones first offering.

+      formatted_url.substr(scheme.begin, scheme.len);
+
+  // Data URLs are rarely human-readable and can be used for spoofing, so draw
+  // attention to the scheme to emphasize "this is just a bunch of data".  For
+  // normal URLs, the host is the best proxy for "identity".
+  // TODO(cjgrant): Handle extensions, if required, for desktop.

[estark, 2017/05/25 01:28:32]

Can you please file a bug for this and include its URL here?

[cjgrant, 2017/05/26 15:58:59]

Done.  crbug/726749

+  if (url_scheme == base::UTF8ToUTF16(url::kDataScheme))
+    deemphasize = ALL_BUT_SCHEME;
+  else if (host.is_nonempty())
+    deemphasize = ALL_BUT_HOST;
+
+  gfx::Range scheme_range = scheme.is_nonempty()
+                                ? gfx::Range(scheme.begin, scheme.end())
+                                : gfx::Range::InvalidRange();
+  switch (deemphasize) {
+    case EVERYTHING:
+      SetEmphasis(render_text, false, gfx::Range::InvalidRange());
+      break;
+    case NOTHING:
+      SetEmphasis(render_text, true, gfx::Range::InvalidRange());
+      break;
+    case ALL_BUT_SCHEME:
+      DCHECK(scheme_range.IsValid());
+      SetEmphasis(render_text, false, gfx::Range::InvalidRange());
+      SetEmphasis(render_text, true, scheme_range);
+      break;
+    case ALL_BUT_HOST:
+      SetEmphasis(render_text, false, gfx::Range::InvalidRange());
+      SetEmphasis(render_text, true, gfx::Range(host.begin, host.end()));
+      break;
+  }
+
+  // Only SECURE and DANGEROUS levels (pages served over HTTPS or flagged by
+  // SafeBrowsing) get a special scheme color treatment. If the security level
+  // is NONE or HTTP_SHOW_WARNING, we do not override the text style previously
+  // applied to the scheme text range by SetEmphasis().
+  if (scheme_range.IsValid() && security_level_ != security_state::NONE &&
+      security_level_ != security_state::HTTP_SHOW_WARNING) {
+    render_text->ApplyColor(getSchemeColor(security_level_), scheme_range);
+    if (security_level_ == SecurityLevel::DANGEROUS) {
+      render_text->ApplyStyle(gfx::TextStyle::DIAGONAL_STRIKE, true,
+                              scheme_range);
+    }
+  }
+}
+
 gfx::Size UrlBarTexture::GetPreferredTextureSize(int maximum_width) const {
   return gfx::Size(maximum_width, maximum_width * kHeight / kWidth);
 }
 
 gfx::SizeF UrlBarTexture::GetDrawnSize() const {
   return size_;
 }
 
 }  // namespace vr_shell