Show scheme in black and content in gray for data: protocol urls

components/omnibox/browser/omnibox_view.h

Index: components/omnibox/browser/omnibox_view.h
diff --git a/components/omnibox/browser/omnibox_view.h b/components/omnibox/browser/omnibox_view.h
index ba019537e2f831b66a4ba7a6c8621656e82b2f87..2a81581ddc5ccfca209ae8961258afba4e58cac8 100644
--- a/components/omnibox/browser/omnibox_view.h
+++ b/components/omnibox/browser/omnibox_view.h
@@ -23,6 +23,7 @@
 #include "components/omnibox/browser/autocomplete_match.h"
 #include "ui/base/window_open_disposition.h"
 #include "ui/gfx/native_widget_types.h"
+#include "ui/gfx/range/range.h"
 
 class GURL;
 class OmniboxClient;
@@ -259,6 +260,19 @@ class OmniboxView {
   OmniboxEditController* controller() { return controller_; }
   const OmniboxEditController* controller() const { return controller_; }
 
+  // Subclasses use this to set colors on ranges of the text. Pass an invalid
+  // range to indicate the base level of emphasis for the text.

[Peter Kasting, 2017/03/01 02:39:11]

Nit: Maybe this would be more descriptive while being briefer:

Marks part (or, if |range| is invalid, all) of the current text as emphasized or
de-emphasized, by changing its color.

[elawrence, 2017/03/01 21:49:02]

Done.

+  virtual void SetEmphasis(bool emphasize, gfx::Range);

[Peter Kasting, 2017/03/01 02:39:11]

I'd make these two functions pure virtual.  All subclasses should have to think
about how to implement these, and if there are any today that want to do
nothing, I'd do that in those subclasses directly.

This will also help clarify that these are implemented by subclasses, which you
were trying to indicate in the function comment (and I've removed in my proposed
comment).

Nit: Name the second param

[Robert Sesek, 2017/03/01 18:29:45]

The gfx::Range argument here and on line 269 could be passed as const&.

[elawrence, 2017/03/01 21:49:01]

Done.

[elawrence, 2017/03/01 21:49:02]

Done.

+
+  // Subclasses use this to apply styles to the scheme based on the security
+  // state. An invalid range indicates that the text does not contain a scheme.
+  virtual void UpdateSchemeEmphasis(gfx::Range range);

[Peter Kasting, 2017/03/01 02:39:11]

Nit: I might call this UpdateSchemeStyle() or something, since it's not solely
about "emphasis".  You could comment it:

Sets the color and strikethrough state for |range|, which represents the current
scheme, based on the current security state.  Schemes are displayed in different
ways for different security levels.

[elawrence, 2017/03/01 21:49:01]

Done.

+
+  // Deemphasize parts of the URL to draw attention to whatever best represents

[Peter Kasting, 2017/03/01 02:39:11]

Nit: Descriptive, not imperative

[elawrence, 2017/03/01 21:49:02]

Done.

+  // the "identity" of the URL.
+  void ApplyEmphasis(const base::string16& display_text,

[Peter Kasting, 2017/03/01 02:39:11]

Nit: I wonder if this title is too specific now that this calls
UpdateSchemeEmphasis() (which I propose renaming above).  Maybe it should be
named more like UpdateTextStyle() and commented:

Parses |display_text|, then sets the display emphasis and scheme style
appropriately.  If the text is a query string, there is no scheme, and
everything is emphasized equally, whereas for URLs the scheme is styled based on
the current security state, and parts of the URL are de-emphasized to draw
attention to whatever best represents the "identity" of the current URL.  See
comments in the code for more.

[elawrence, 2017/03/01 21:49:02]

Done.

+                     const AutocompleteSchemeClassifier& classifier);
+
  private:
   friend class OmniboxViewMacTest;