Fix DCHECK conditions for GetSubstringBounds(...)

This CL is fixing the DCHECK for GetSubstringBounds(...) which
was detected to fail sometime on albatros build.

1) Use IsBoundedBy(...) instead of Contains(...)
   The first problem was cause by the range [end,end) which is
   not considered to be part of the range (a.k.a Contains(....) will
   return false). IsBoundedBy accepts the empty ending range to be
   part of the range.

2) Some eliding behaviors may break the mapping between text indexes
   and display text indexes. The new check implementation is ensuring
   that eliding is not causing any invalid offsets.

We are expecting clusterfuzz to hit these DCHECKs until it's fixed.

R=gab@chromium.org, msw@chromium.org

Bug: 1146674, 1085014
Change-Id: I219ef476009fc85edb7b836d5143022f51ecb8e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2521252Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826022}

ui/gfx/render_text_harfbuzz.cc

@@ -1419,11 +1419,11 @@ SizeF RenderTextHarfBuzz::GetLineSizeF(const SelectionModel& caret) {
 std::vector<Rect> RenderTextHarfBuzz::GetSubstringBounds(const Range& range) {
   EnsureLayout();
   DCHECK(!update_display_run_list_);
-  DCHECK(Range(0, text().length()).Contains(range));
+  DCHECK(range.IsBoundedBy(Range(0, text().length())));
   const Range grapheme_range = ExpandRangeToGraphemeBoundary(range);
   const Range display_range(TextIndexToDisplayIndex(grapheme_range.start()),
                             TextIndexToDisplayIndex(grapheme_range.end()));
-  DCHECK(Range(0, GetDisplayText().length()).Contains(display_range));
+  DCHECK(IsValidDisplayRange(display_range));
 
   std::vector<Rect> rects;
   if (display_range.is_empty())
@@ -1497,6 +1497,7 @@ RangeF RenderTextHarfBuzz::GetCursorSpan(const Range& text_range) {
 
   Range display_range(TextIndexToDisplayIndex(valid_range.start()),
                       TextIndexToDisplayIndex(next_grapheme_start));
+  DCHECK(IsValidDisplayRange(display_range));
 
   // Although highly likely, there's no guarantee that a single text run is used
   // for the entire cursor span. For example, Unicode Variation Selectors are
@@ -2236,6 +2237,28 @@ const internal::TextRunList* RenderTextHarfBuzz::GetRunList() const {
   return const_cast<RenderTextHarfBuzz*>(this)->GetRunList();
 }
 
+bool RenderTextHarfBuzz::IsValidDisplayRange(Range display_range) {
+  // The |display_text_| is an elided version of |layout_text_|. Removing
+  // codepoints from the text may break the conversion for codepoint offsets
+  // between text to display_text offset. For elding behaviors that truncate
+  // codepoint at the end, the conversion will work just fine. But for eliding
+  // behavior that truncate at the beginning of middle of the text, the offsets
+  // are completely wrong and should not be used.
+  // TODO(http://crbug.com/1085014): Fix eliding for the broken cases.
+  switch (elide_behavior()) {
+    case NO_ELIDE:
+    case FADE_TAIL:
+      return display_range.IsBoundedBy(Range(0, GetDisplayText().length()));
+    case TRUNCATE:
+    case ELIDE_TAIL:
+      return display_range.IsBoundedBy(Range(0, GetLayoutText().length()));
+    case ELIDE_HEAD:
+    case ELIDE_MIDDLE:
+    case ELIDE_EMAIL:
+      return !text_elided();
+  }
+}
+
 bool RenderTextHarfBuzz::GetDecoratedTextForRange(
     const Range& range,
     DecoratedText* decorated_text) {

ui/gfx/render_text_harfbuzz.h

@@ -286,6 +286,10 @@ class GFX_EXPORT RenderTextHarfBuzz : public RenderText {
   // Makes sure that text runs for layout text are shaped.
   void EnsureLayoutRunList();
 
+  // Returns whether the display range is still a valid range after the eliding
+  // pass.
+  bool IsValidDisplayRange(Range display_range);
+
   // RenderText:
   internal::TextRunList* GetRunList() override;
   const internal::TextRunList* GetRunList() const override;

ui/gfx/render_text_unittest.cc

@@ -7629,6 +7629,36 @@ TEST_F(RenderTextTest, LineEndSelections) {
   }
 }
 
+TEST_F(RenderTextTest, GetSubstringBounds) {
+  const float kGlyphWidth = 5;
+  SetGlyphWidth(kGlyphWidth);
+  RenderText* render_text = GetRenderText();
+  render_text->SetText(UTF8ToUTF16("abc"));
+  render_text->SetCursorEnabled(false);
+  render_text->SetElideBehavior(NO_ELIDE);
+
+  EXPECT_EQ(GetSubstringBoundsUnion(Range(0, 1)).width(), kGlyphWidth);
+  EXPECT_EQ(GetSubstringBoundsUnion(Range(1, 2)).width(), kGlyphWidth);
+  EXPECT_EQ(GetSubstringBoundsUnion(Range(1, 3)).width(), 2 * kGlyphWidth);
+
+  EXPECT_EQ(GetSubstringBoundsUnion(Range(0, 0)).width(), 0);
+  EXPECT_EQ(GetSubstringBoundsUnion(Range(3, 3)).width(), 0);
+
+  // Apply eliding so display text has 2 visible character.
+  render_text->SetDisplayRect(Rect(0, 0, 2 * kGlyphWidth, 100));
+  render_text->SetElideBehavior(TRUNCATE);
+
+  EXPECT_EQ(GetSubstringBoundsUnion(Range(0, 1)).width(), kGlyphWidth);
+  EXPECT_EQ(GetSubstringBoundsUnion(Range(1, 2)).width(), kGlyphWidth);
+  EXPECT_EQ(GetSubstringBoundsUnion(Range(1, 3)).width(), kGlyphWidth);
+  // Check a fully elided range.
+  EXPECT_EQ(GetSubstringBoundsUnion(Range(2, 3)).width(), 0);
+
+  // Empty ranges result in empty rect.
+  EXPECT_EQ(GetSubstringBoundsUnion(Range(0, 0)).width(), 0);
+  EXPECT_EQ(GetSubstringBoundsUnion(Range(3, 3)).width(), 0);
+}
+
 // Tests that GetSubstringBounds rounds outward when glyphs have floating-point
 // widths.
 TEST_F(RenderTextTest, GetSubstringBoundsFloatingPoint) {