I can't reproduce an indefinite hang. It shows "Not responding" and slows down a lot, but I can still open a new tab and close the offending tab.

In any case, we don't consider this a security vulnerability, so I'll remove it from the security queue: http://www.chromium.org/Home/chromium-security/security-faq?pli=1#TOC-Are-denial-of-service-issues-considered-security-bugs-

mgiuca@, you've worked with Unicode URL issues before, right? Could you triage?

I played around with this a bit. Like Lucas, I can't get it to hang forever, but it does for about 7 seconds. (My guess is that my computer is faster than the reporter's.)

More concerning is that this seems to DOS the Omnibox for all future searches involving that URL.

1. Open Chrome with a fresh user data dir (to avoid corrupting your best Chrome profile).
2. Open the attached HTML file. Wait for it to inject crap into your URL br.
3. For good measure, select the URL bar and hit Enter. This adds the full (10K> character) URL into your Chrome browsing history.
4. Close the tab.
5. In the Omnibox, type "r0ck"... it takes ~7 seconds to show the search results. Any navigation (up/down arrows) within the Omnibox takes another ~7 seconds while that 10K-character URL is in the results.

If you select the result in the suggestions list and hit Shift+Delete to remove it from history (or do the equivalent in chrome://history), everything goes back to normal.

Curiously, this seems to depend on the specific characters being used.

This code uses "\u26C4\u1f47" (note: the "unescape" call is unnecessary). Which is U+26C4 SNOWMAN WITHOUT SNOW and an invalid character. It happens if you mix two high-codepoint Unicode characters (I tried several). It doesn't happen if you repeat just one character. It doesn't happen with ASCII or low Unicode (Latin-1) characters.

I'm profiling now... the bulk of the time is spent in SkGlyphCache_Globals::validate which implies it's something to do with font rendering cache. Possibly the fact that we're alternating two characters thrashes the cache which doesn't happen with a single character repeated. And possibly there is a fast path for Latin-1 characters. I'll investigate more.

OK there are a couple of problems and it's not entirely clear where to start addressing these. 1. Skia, 2. Omnibox, 3. Textfield. (Adding one person who might know about each.)

There are two separate (but similar) cases that trigger this: Omnibox results (suggestions) and Omnibox textfield itself. In both cases, if that giant 10,000 character URL appears, it will take many seconds of UI thread time.

1. Skia.
SkPaint::getTextWidths is extremely slow for very large strings of Unicode characters (that's where profiling shows most of the time is taken up). After executing the repro steps in #2, then typing "r<bksp>r<bksp>r<bksp>" several times into the Omnibox (basically showing this URL in the search results but NOT in the text itself), I see this in perf top:

-  47.80%  libskia.so                        [.] SkGlyphCache_Globals::validate() const
   - SkGlyphCache_Globals::validate() const
      - 34.00% SkGlyphCache::VisitCache(SkTypeface*, SkScalerContextEffects const&, SkDescriptor const*, bool (*)(SkGlyphCache const*, void*), void*)
           99.96% DetachDescProc(SkTypeface*, SkScalerContextEffects const&, SkDescriptor const*, void*)
              SkPaint::descriptorProc(SkSurfaceProps const*, unsigned int, SkMatrix const*, void (*)(SkTypeface*, SkScalerContextEffects const&, SkDescriptor const*, void*), void*) const
              SkPaint::getTextWidths(void const*, unsigned long, float*, SkRect*) const
              gfx::(anonymous namespace)::GetGlyphWidthAndExtents(SkPaint*, unsigned int, int*, hb_glyph_extents_t*)
              gfx::(anonymous namespace)::GetGlyphHorizontalAdvance(hb_font_t*, void*, unsigned int, void*)
              _hb_ot_shape
              hb_shape_plan_execute
              hb_shape
              gfx::RenderTextHarfBuzz::ShapeRunWithFont(std::basic_string<unsigned short, base::string16_char_traits, std::allocator<unsigned short> > const&, gfx::Font const&, gfx::FontRenderParams const&, gfx::
      - 32.99% SkGlyphCache_Globals::internalPurge(unsigned long)
           SkGlyphCache_Globals::attachCacheToHead(SkGlyphCache*)
      - 32.93% SkGlyphCache_Globals::attachCacheToHead(SkGlyphCache*)
           SkPaint::getTextWidths(void const*, unsigned long, float*, SkRect*) const
           gfx::(anonymous namespace)::GetGlyphWidthAndExtents(SkPaint*, unsigned int, int*, hb_glyph_extents_t*)
           gfx::(anonymous namespace)::GetGlyphHorizontalAdvance(hb_font_t*, void*, unsigned int, void*)
           _hb_ot_shape
           hb_shape_plan_execute
           hb_shape
           gfx::RenderTextHarfBuzz::ShapeRunWithFont(std::basic_string<unsigned short, base::string16_char_traits, std::allocator<unsigned short> > const&, gfx::Font const&, gfx::FontRenderParams const&, gfx::int

So I'm not sure why getTextWidths (and by extension VisitCache) is so slow. Note that most of the time is actually spent in validate(), which is debug-only. This even happens on my Release build (albeit I have debugging symbols enabled). So it's possible that either a) on a real build, validate() is not called but the time is spent elsewhere in getTextWidths, or b) on a real build, validate() *is* called and we are incorrectly compiling Skia in debug mode, or c) I'm on the wrong path.

But I think it deserves a look, because 5,000 of "\u26C4\u1f47" is *much* slower than 5,000 of "ab".

2. Omnibox.
OmniboxResultView will send the entire URL to be rendered, regardless of how long it is. This means we will happily send a 10,000 character string to gfx::RenderText, which will have getTextWidths called on it, which is expensive (see number 1). We could truncate this to at most a few hundred characters to eliminate this source of slowdown. That won't solve the omnibox textfield issue, but at least it will mean your PC grinds to a halt just from the nasty URL showing up in suggestions (you would need to actually select it).

As for putting this URL into the text field itself, well that isn't something the Omnibox itself can deal with (we obviously can't truncate the URL we put in the text field, unless we want to actually impose a hard limit on URL length).

3. views::Textfield
Again, Textfield will happily send a 10,000 character string to gfx::RenderText even if it only intends to display a small part of it. We might be able to snip up the string to avoid calculating the full length of the text.

We could potentially dig deeper on any of these, but I would guess the best value would be found in:
a) Investigating why Unicode characters take so long to process in getTextWidths.
b) Imposing a maximum length for URLs in OmniboxResultView.

Here's a diff I used to quick-fix some of these issues and also log some data.

diff --git a/chrome/browser/ui/views/omnibox/omnibox_result_view.cc b/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
index 8dc6d54..f970e95 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
@@ -456,8 +456,10 @@ std::unique_ptr<gfx::RenderText> OmniboxResultView::CreateClassifiedRenderText(
     const base::string16& text,
     const ACMatchClassifications& classifications,
     bool force_dim) const {
-  std::unique_ptr<gfx::RenderText> render_text(CreateRenderText(text));
+  std::unique_ptr<gfx::RenderText> render_text(
+      CreateRenderText(text.substr(0, 100)));
   const size_t text_length = render_text->text().length();
+  LOG(ERROR) << "Omnibox: text_length = " << text_length;
   for (size_t i = 0; i < classifications.size(); ++i) {
     const size_t text_start = classifications[i].offset;
     if (text_start >= text_length)
diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
index 0753d84..e7be8f9 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
@@ -391,8 +391,8 @@ void OmniboxViewViews::ExecuteCommand(int command_id, int event_flags) {
 
 void OmniboxViewViews::SetTextAndSelectedRange(const base::string16& text,
                                                const gfx::Range& range) {
-  SetText(text);
-  SelectRange(range);
+  SetText(text.substr(0, 100));
+  SelectRange(range.Intersect(gfx::Range(0, 100)));
 }
 
 base::string16 OmniboxViewViews::GetSelectedText() const {
diff --git a/ui/views/controls/textfield/textfield.cc b/ui/views/controls/textfield/textfield.cc
index dc0a9f2..237c1e2 100644
--- a/ui/views/controls/textfield/textfield.cc
+++ b/ui/views/controls/textfield/textfield.cc
@@ -7,6 +7,7 @@
 #include <string>
 #include <utility>
 
+#include "base/debug/stack_trace.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
 #include "ui/accessibility/ax_view_state.h"
@@ -340,6 +341,9 @@ void Textfield::SetTextInputFlags(int flags) {
 }
 
 void Textfield::SetText(const base::string16& new_text) {
+  LOG(ERROR) << "Textfield: SetText len = " << new_text.size();
+  if (new_text.size() > 5000)
+    base::debug::StackTrace().Print();
   model_->SetText(new_text);
   OnCaretBoundsChanged();
   SchedulePaint();

SkGlyphCache_Globals::validate() is an empty non-virtual stub function except in debug builds. +herb@ who did some work last fall to attempt to parallelize the glyph cache.

This could mean that:
 (1) we've got a partially-debug build? ("Release_Developer" configuration would do this, as would a DCHECK_ALWAYS_ON build)
 (2) that empty function call isn't being optimized out, and we can't afford it?

Do we have call counts? Are we just adding millions of things to the linked list of caches? If validate() is running it's walking the full list on more-or-less every cache access.

There might be a semantic mismatch between DCHECK_ALWAYS_ON and Skia's policy that if debug mode is no we're totally non-performant in order to be totally paranoid.

Having the result view truncate before rendering is likely OK as long as we can conservatively estimate the number of character we need to send to ensure we don't truncate the string early.  Not sure how to estimate that.  Perhaps sending as many characters as we have pixels wide would work.

*quietly wishes the ui code used the same text constructs that blink does (textblobs etc), maybe that would help*

#5: I am compiling with DCHECK_ALWAYS_ON in Release build (which usually means DCHECKs but no other debug flags are set). So it's likely the case that a real Chrome doesn't have validate() calls, and therefore my profiling is incorrectly blaming validate() for the time taken.

However, I think it could still be related to the Skia glyph cache, just would be spending the time in some other function than validate(). I'll recompile without validate() and see what the profiling shows.

Still repros.  Clearly not P-1 given the lack of attention to this issue.

mgiuca@, clearly you're not working on this anymore.  Care to mark it as Available and, while you're at it, guess what you think the most impactful change would be?  I.e., which of the component teams should take the first stab at fixing it?

Sorry for late reply.

Yes, I don't really have time to dig into this. Marking as available. +jdonnelly from Omnibox.

I think #3 has a fairly good summary. I'm not sure whether this is directly a bug in Skia, Omnibox or Textfield (since it's kind of an overlap of all three). But I think Omnibox might be the best place to look at it since it's the highest level component that's affected by this issue.

The performance problem with SkPaint::getTextWidths is that gfx gets the advances one character at a time. The gfx code should be changed to get the metrics of many characters at the same time similar to the strategy that blink uses.

See GetGlyphHorizontalAdvance for an example of getting one glyph's worth of metrics at a time.

https://cs.chromium.org/chromium/src/ui/gfx/harfbuzz_font_skia.cc?rcl=681f559189cb660a8c503c4cf24d653df911bcd6&l=103

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

I think this is still an issue and would be good to fix. Not a priority though and nobody is active on it to my knowledge.