Revert of https://codereview.chromium.org/107513011/

Reason for revert: Test fails on main waterfall.

TBR=jshin@chromium.org,msw@chromium.org,pkasting@chromium.org,skanuj@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=327833

Review URL: https://codereview.chromium.org/103053014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@241050 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 299 insertions, 439 deletions

diff --git a/base/i18n/rtl.cc b/base/i18n/rtl.cc
index 851b036..d9818e8 100644
--- a/base/i18n/rtl.cc
+++ b/base/i18n/rtl.cc
@@ -163,21 +163,6 @@ TextDirection GetFirstStrongCharacterDirection(const string16& text) {
   return LEFT_TO_RIGHT;
 }
 
-TextDirection GetLastStrongCharacterDirection(const string16& text) {
-  const UChar* string = text.c_str();
-  size_t position = text.length();
-  while (position > 0) {
-    UChar32 character;
-    size_t prev_position = position;
-    U16_PREV(string, 0, prev_position, character);
-    TextDirection direction = GetCharacterDirection(character);
-    if (direction != UNKNOWN_DIRECTION)
-      return direction;
-    position = prev_position;
-  }
-  return LEFT_TO_RIGHT;
-}
-
 TextDirection GetStringDirection(const string16& text) {
   const UChar* string = text.c_str();
   size_t length = text.length();
diff --git a/base/i18n/rtl.h b/base/i18n/rtl.h
index aa5f681..c80d2f8 100644
--- a/base/i18n/rtl.h
+++ b/base/i18n/rtl.h
@@ -64,7 +64,7 @@ BASE_I18N_EXPORT bool ICUIsRTL();
 BASE_I18N_EXPORT TextDirection GetTextDirectionForLocale(
     const char* locale_name);
 
-// Given the string in |text|, returns the directionality of the first or last
+// Given the string in |text|, returns the directionality of the first
 // character with strong directionality in the string. If no character in the
 // text has strong directionality, LEFT_TO_RIGHT is returned. The Bidi
 // character types L, LRE, LRO, R, AL, RLE, and RLO are considered as strong
@@ -72,8 +72,6 @@ BASE_I18N_EXPORT TextDirection GetTextDirectionForLocale(
 // for more information.
 BASE_I18N_EXPORT TextDirection GetFirstStrongCharacterDirection(
     const string16& text);
-BASE_I18N_EXPORT TextDirection GetLastStrongCharacterDirection(
-    const string16& text);
 
 // Given the string in |text|, returns LEFT_TO_RIGHT or RIGHT_TO_LEFT if all the
 // strong directionality characters in the string are of the same
diff --git a/base/i18n/rtl_unittest.cc b/base/i18n/rtl_unittest.cc
index 8faaccf..58772b0 100644
--- a/base/i18n/rtl_unittest.cc
+++ b/base/i18n/rtl_unittest.cc
@@ -46,8 +46,6 @@ TEST_F(RTLTest, GetFirstStrongCharacterDirection) {
   } cases[] = {
     // Test pure LTR string.
     { L"foo bar", LEFT_TO_RIGHT },
-    // Test pure RTL string.
-    { L"\x05d0\x05d1\x05d2 \x05d3\x0d4\x05d5", RIGHT_TO_LEFT},
     // Test bidi string in which the first character with strong directionality
     // is a character with type L.
     { L"foo \x05d0 bar", LEFT_TO_RIGHT },
@@ -109,68 +107,6 @@ TEST_F(RTLTest, GetFirstStrongCharacterDirection) {
               GetFirstStrongCharacterDirection(WideToUTF16(cases[i].text)));
 }
 
-
-// Note that the cases with LRE, LRO, RLE and RLO are invalid for
-// GetLastStrongCharacterDirection because they should be followed by PDF
-// character.
-TEST_F(RTLTest, GetLastStrongCharacterDirection) {
-  struct {
-    const wchar_t* text;
-    TextDirection direction;
-  } cases[] = {
-    // Test pure LTR string.
-    { L"foo bar", LEFT_TO_RIGHT },
-    // Test pure RTL string.
-    { L"\x05d0\x05d1\x05d2 \x05d3\x0d4\x05d5", RIGHT_TO_LEFT},
-    // Test bidi string in which the last character with strong directionality
-    // is a character with type L.
-    { L"foo \x05d0 bar", LEFT_TO_RIGHT },
-    // Test bidi string in which the last character with strong directionality
-    // is a character with type R.
-    { L"\x05d0 foo bar \x05d3", RIGHT_TO_LEFT },
-    // Test bidi string which ends with a character with weak directionality
-    // and in which the last character with strong directionality is a
-    // character with type L.
-    { L"!foo \x05d0 bar!", LEFT_TO_RIGHT },
-    // Test bidi string which ends with a character with weak directionality
-    // and in which the last character with strong directionality is a
-    // character with type R.
-    { L",\x05d0 foo bar \x05d1,", RIGHT_TO_LEFT },
-    // Test bidi string in which the last character with strong directionality
-    // is a character with type AL.
-    { L"\x0622 foo \x05d0 bar \x0622", RIGHT_TO_LEFT },
-    // Test a string without strong directionality characters.
-    { L",!.{}", LEFT_TO_RIGHT },
-    // Test empty string.
-    { L"", LEFT_TO_RIGHT },
-    // Test characters in non-BMP (e.g. Phoenician letters. Please refer to
-    // http://demo.icu-project.org/icu-bin/ubrowse?scr=151&b=10910 for more
-    // information).
-    {
-#if defined(WCHAR_T_IS_UTF32)
-       L"abc 123" L" ! \x10910 !",
-#elif defined(WCHAR_T_IS_UTF16)
-       L"abc 123" L" ! \xd802\xdd10 !",
-#else
-#error wchar_t should be either UTF-16 or UTF-32
-#endif
-      RIGHT_TO_LEFT },
-    {
-#if defined(WCHAR_T_IS_UTF32)
-       L"abc 123" L" ! \x10401 !",
-#elif defined(WCHAR_T_IS_UTF16)
-       L"abc 123" L" ! \xd801\xdc01 !",
-#else
-#error wchar_t should be either UTF-16 or UTF-32
-#endif
-      LEFT_TO_RIGHT },
-   };
-
-  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); ++i)
-    EXPECT_EQ(cases[i].direction,
-              GetLastStrongCharacterDirection(WideToUTF16(cases[i].text)));
-}
-
 TEST_F(RTLTest, GetStringDirection) {
   struct {
     const wchar_t* text;
diff --git a/chrome/browser/ui/views/omnibox/omnibox_result_view.cc b/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
index 8903375..9938b547 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_result_view.cc
@@ -357,58 +357,213 @@ int OmniboxResultView::DrawString(
     }
   }
 
-  scoped_ptr<gfx::RenderText> render_text(gfx::RenderText::CreateInstance());
-  const size_t text_length = text.length();
-  render_text->SetText(text);
-  render_text->SetFontList(font_list_);
-  render_text->SetMultiline(false);
-  render_text->SetCursorEnabled(false);
-  if (is_url)
-    render_text->SetDirectionalityMode(gfx::DIRECTIONALITY_FORCE_LTR);
-
-  // Apply classifications.
-  for (size_t i = 0; i < classifications.size(); ++i) {
-    const size_t text_start = classifications[i].offset;
-    if (text_start >= text_length)
-      break;
-
-    const size_t text_end = (i < (classifications.size() - 1)) ?
-      std::min(classifications[i + 1].offset, text_length) : text_length;
-    const gfx::Range current_range(text_start, text_end);
-
-    // Calculate style-related data.
-    if (classifications[i].style & ACMatchClassification::MATCH)
-      render_text->ApplyStyle(gfx::BOLD, true, current_range);
-
-    ColorKind colorKind = TEXT;
-    if (classifications[i].style & ACMatchClassification::URL)
-      colorKind = URL;
-    else if (force_dim ||
-        (classifications[i].style & ACMatchClassification::DIM))
-      colorKind = DIMMED_TEXT;
-    render_text->ApplyColor(GetColor(GetState(), colorKind), current_range);
+  // Split the text into visual runs.  We do this first so that we don't need to
+  // worry about whether our eliding might change the visual display in
+  // unintended ways, e.g. by removing directional markings or by adding an
+  // ellipsis that's not enclosed in appropriate markings.
+  base::i18n::BiDiLineIterator bidi_line;
+  if (!bidi_line.Open(text, base::i18n::IsRTL(), is_url))
+    return x;
+  const int num_runs = bidi_line.CountRuns();
+  ScopedVector<gfx::RenderText> render_texts;
+  Runs runs;
+  for (int run = 0; run < num_runs; ++run) {
+    int run_start_int = 0, run_length_int = 0;
+    // The index we pass to GetVisualRun corresponds to the position of the run
+    // in the displayed text. For example, the string "Google in HEBREW" (where
+    // HEBREW is text in the Hebrew language) has two runs: "Google in " which
+    // is an LTR run, and "HEBREW" which is an RTL run. In an LTR context, the
+    // run "Google in " has the index 0 (since it is the leftmost run
+    // displayed). In an RTL context, the same run has the index 1 because it
+    // is the rightmost run. This is why the order in which we traverse the
+    // runs is different depending on the locale direction.
+    const UBiDiDirection run_direction = bidi_line.GetVisualRun(
+        (base::i18n::IsRTL() && !is_url) ? (num_runs - run - 1) : run,
+        &run_start_int, &run_length_int);
+    DCHECK_GT(run_length_int, 0);
+    runs.push_back(RunData());
+    RunData* current_run = &runs.back();
+    current_run->run_start = run_start_int;
+    const size_t run_end = current_run->run_start + run_length_int;
+    current_run->visual_order = run;
+    current_run->is_rtl = !is_url && (run_direction == UBIDI_RTL);
+
+    // Compute classifications for this run.
+    for (size_t i = 0; i < classifications.size(); ++i) {
+      const size_t text_start =
+          std::max(classifications[i].offset, current_run->run_start);
+      if (text_start >= run_end)
+        break;  // We're past the last classification in the run.
+
+      const size_t text_end = (i < (classifications.size() - 1)) ?
+          std::min(classifications[i + 1].offset, run_end) : run_end;
+      if (text_end <= current_run->run_start)
+        continue;  // We haven't reached the first classification in the run.
+
+      render_texts.push_back(gfx::RenderText::CreateInstance());
+      gfx::RenderText* render_text = render_texts.back();
+      current_run->classifications.push_back(render_text);
+      render_text->SetText(text.substr(text_start, text_end - text_start));
+      render_text->SetFontList(font_list_);
+
+      // Calculate style-related data.
+      if (classifications[i].style & ACMatchClassification::MATCH)
+        render_text->SetStyle(gfx::BOLD, true);
+      const ResultViewState state = GetState();
+      if (classifications[i].style & ACMatchClassification::URL)
+        render_text->SetColor(GetColor(state, URL));
+      else if (classifications[i].style & ACMatchClassification::DIM)
+        render_text->SetColor(GetColor(state, DIMMED_TEXT));
+      else
+        render_text->SetColor(GetColor(state, force_dim ? DIMMED_TEXT : TEXT));
+
+      current_run->pixel_width += render_text->GetStringSize().width();
+    }
+    DCHECK(!current_run->classifications.empty());
   }
-
+  DCHECK(!runs.empty());
+
+  // Sort into logical order so we can elide logically.
+  std::sort(runs.begin(), runs.end(), &SortRunsLogically);
+
+  // Now determine what to elide, if anything.  Several subtle points:
+  //   * Because we have the run data, we can get edge cases correct, like
+  //     whether to place an ellipsis before or after the end of a run when the
+  //     text needs to be elided at the run boundary.
+  //   * The "or one before it" comments below refer to cases where an earlier
+  //     classification fits completely, but leaves too little space for an
+  //     ellipsis that turns out to be needed later.  These cases are commented
+  //     more completely in Elide().
   int remaining_width = mirroring_context_->remaining_width(x);
+  for (Runs::iterator i(runs.begin()); i != runs.end(); ++i) {
+    if (i->pixel_width > remaining_width) {
+      // This run or one before it needs to be elided.
+      for (Classifications::iterator j(i->classifications.begin());
+           j != i->classifications.end(); ++j) {
+        const int width = (*j)->GetStringSize().width();
+        if (width > remaining_width) {
+          // This classification or one before it needs to be elided.  Erase all
+          // further classifications and runs so Elide() can simply reverse-
+          // iterate over everything to find the specific classification to
+          // elide.
+          i->classifications.erase(++j, i->classifications.end());
+          runs.erase(++i, runs.end());
+          Elide(&runs, remaining_width);
+          break;
+        }
+        remaining_width -= width;
+      }
+      break;
+    }
+    remaining_width -= i->pixel_width;
+  }
 
-  // No need to try anything if we can't even show a solitary character.
-  if ((text_length == 1) &&
-      (remaining_width < render_text->GetContentWidth())) {
-    return x;
+  // Sort back into visual order so we can display the runs correctly.
+  std::sort(runs.begin(), runs.end(), &SortRunsVisually);
+
+  // Draw the runs.
+  for (Runs::iterator i(runs.begin()); i != runs.end(); ++i) {
+    const bool reverse_visible_order = (i->is_rtl != base::i18n::IsRTL());
+    if (reverse_visible_order)
+      std::reverse(i->classifications.begin(), i->classifications.end());
+    for (Classifications::const_iterator j(i->classifications.begin());
+         j != i->classifications.end(); ++j) {
+      const gfx::Size size = (*j)->GetStringSize();
+      // Align the text runs to a common baseline.
+      const gfx::Rect rect(
+          mirroring_context_->mirrored_left_coord(x, x + size.width()), y,
+          size.width(), height());
+      (*j)->SetDisplayRect(rect);
+      (*j)->Draw(canvas);
+      x += size.width();
+    }
   }
 
-  if (render_text->GetContentWidth() > remaining_width)
-    render_text->SetElideBehavior(gfx::ELIDE_AT_END);
+  return x;
+}
 
-  // Set the display rect to trigger eliding.
-  render_text->SetDisplayRect(gfx::Rect(
-      mirroring_context_->mirrored_left_coord(x, x + remaining_width), y,
-      remaining_width, height()));
-  render_text->set_clip_to_display_rect(true);
-  render_text->Draw(canvas);
+void OmniboxResultView::Elide(Runs* runs, int remaining_width) const {
+  // The complexity of this function is due to edge cases like the following:
+  // We have 100 px of available space, an initial classification that takes 86
+  // px, and a font that has a 15 px wide ellipsis character.  Now if the first
+  // classification is followed by several very narrow classifications (e.g. 3
+  // px wide each), we don't know whether we need to elide or not at the time we
+  // see the first classification -- it depends on how many subsequent
+  // classifications follow, and some of those may be in the next run (or
+  // several runs!).  This is why instead we let our caller move forward until
+  // we know we definitely need to elide, and then in this function we move
+  // backward again until we find a string that we can successfully do the
+  // eliding on.
+  bool on_trailing_classification = true;
+  for (Runs::reverse_iterator i(runs->rbegin()); i != runs->rend(); ++i) {
+    for (Classifications::reverse_iterator j(i->classifications.rbegin());
+         j != i->classifications.rend(); ++j) {
+      if (!on_trailing_classification) {
+        // We also add this classification's width (sans ellipsis) back to the
+        // available width since we want to consider the available space we'll
+        // have when we draw this classification.
+        remaining_width += (*j)->GetStringSize().width();
+
+        // If we reached here, we couldn't fit an ellipsis in the space taken by
+        // the previous classifications we looped over (see comments at bottom
+        // of loop).  Append one here to represent those elided portions.
+        (*j)->SetText((*j)->text() + kEllipsis);
+      }
+      on_trailing_classification = false;
+
+      // Can we fit at least an ellipsis?
+      gfx::Font font((*j)->GetStyle(gfx::BOLD) ?
+          (*j)->GetPrimaryFont().DeriveFont(0, gfx::Font::BOLD) :
+          (*j)->GetPrimaryFont());
+      base::string16 elided_text(
+          gfx::ElideText((*j)->text(), font, remaining_width,
+          gfx::ELIDE_AT_END));
+      Classifications::reverse_iterator prior(j + 1);
+      const bool on_leading_classification =
+          (prior == i->classifications.rend());
+      if (elided_text.empty() && (remaining_width >= ellipsis_width_) &&
+          on_leading_classification) {
+        // Edge case: This classification is bold, we can't fit a bold ellipsis
+        // but we can fit a normal one, and this is the first classification in
+        // the run.  We should display a lone normal ellipsis, because appending
+        // one to the end of the previous run might put it in the wrong visual
+        // location (if the previous run is reversed from the normal visual
+        // order).
+        // NOTE: If this isn't the first classification in the run, we don't
+        // need to bother with this; see note below.
+        elided_text = kEllipsis;
+      }
+      if (!elided_text.empty()) {
+        // Success.  Elide this classification and stop.
+        (*j)->SetText(elided_text);
+
+        // If we could only fit an ellipsis, then only make it bold if there was
+        // an immediate prior classification in this run that was also bold, or
+        // it will look orphaned.
+        if ((*j)->GetStyle(gfx::BOLD) && (elided_text.length() == 1) &&
+            (on_leading_classification || !(*prior)->GetStyle(gfx::BOLD)))
+          (*j)->SetStyle(gfx::BOLD, false);
+
+        // Erase any other classifications that come after the elided one.
+        i->classifications.erase(j.base(), i->classifications.end());
+        runs->erase(i.base(), runs->end());
+        return;
+      }
+
+      // We couldn't fit an ellipsis.  Move back one classification,
+      // append an ellipsis, and try again.
+      // NOTE: In the edge case that a bold ellipsis doesn't fit but a
+      // normal one would, and we reach here, then there is a previous
+      // classification in this run, and so either:
+      //   * It's normal, and will be able to draw successfully with the
+      //     ellipsis we'll append to it, or
+      //   * It is also bold, in which case we don't want to fall back
+      //     to a normal ellipsis anyway (see comment above).
+    }
+  }
 
-  // Need to call GetContentWidth again as the SetDisplayRect may modify it.
-  return x + render_text->GetContentWidth();
+  // We couldn't draw anything.
+  runs->clear();
 }
 
 void OmniboxResultView::Layout() {
diff --git a/chrome/browser/ui/views/omnibox/omnibox_result_view.h b/chrome/browser/ui/views/omnibox/omnibox_result_view.h
index 886e1d5..07167bf 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_result_view.h
+++ b/chrome/browser/ui/views/omnibox/omnibox_result_view.h
@@ -111,6 +111,22 @@ class OmniboxResultView : public views::View,
   gfx::ImageSkia GetIcon() const;
   const gfx::ImageSkia* GetKeywordIcon() const;
 
+  // Elides |runs| to fit in |remaining_width|.  The runs in |runs| should be in
+  // logical order.
+  //
+  // When we need to elide a run, the ellipsis will be placed at the end of that
+  // run.  This means that if we elide a run whose visual direction is opposite
+  // that of the drawing context, the ellipsis will not be at the "end" of the
+  // drawn string.  For example, if in an LTR context we have the LTR run
+  // "LTR_STRING" and the RTL run "RTL_STRING", the unelided text would be drawn
+  // like:
+  //     LTR_STRING GNIRTS_LTR
+  // If we need to elide the RTL run, then it will be drawn like:
+  //     LTR_STRING ...RTS_LTR
+  // Instead of:
+  //     LTR_STRING RTS_LTR...
+  void Elide(Runs* runs, int remaining_width) const;
+
   // views::View:
   virtual void Layout() OVERRIDE;
   virtual void OnBoundsChanged(const gfx::Rect& previous_bounds) OVERRIDE;
diff --git a/ui/gfx/render_text.cc b/ui/gfx/render_text.cc
index 23f6740..e8e3436 100644
--- a/ui/gfx/render_text.cc
+++ b/ui/gfx/render_text.cc
@@ -19,7 +19,6 @@
 #include "ui/gfx/skia_util.h"
 #include "ui/gfx/text_constants.h"
 #include "ui/gfx/text_elider.h"
-#include "ui/gfx/text_utils.h"
 #include "ui/gfx/utf16_indexing.h"
 
 namespace gfx {
@@ -366,6 +365,7 @@ void RenderText::SetText(const base::string16& text) {
 
   obscured_reveal_index_ = -1;
   UpdateLayoutText();
+  ResetLayout();
 }
 
 void RenderText::SetHorizontalAlignment(HorizontalAlignment alignment) {
@@ -411,6 +411,7 @@ void RenderText::SetObscured(bool obscured) {
     obscured_reveal_index_ = -1;
     cached_bounds_and_offset_valid_ = false;
     UpdateLayoutText();
+    ResetLayout();
   }
 }
 
@@ -421,6 +422,7 @@ void RenderText::SetObscuredRevealIndex(int index) {
   obscured_reveal_index_ = index;
   cached_bounds_and_offset_valid_ = false;
   UpdateLayoutText();
+  ResetLayout();
 }
 
 void RenderText::SetMultiline(bool multiline) {
@@ -431,23 +433,11 @@ void RenderText::SetMultiline(bool multiline) {
   }
 }
 
-void RenderText::SetElideBehavior(ElideBehavior elide_behavior) {
-  // TODO(skanuj) : Add a test for triggering layout change.
-  if (elide_behavior_ != elide_behavior) {
-    elide_behavior_ = elide_behavior;
-    UpdateLayoutText();
-  }
-}
-
 void RenderText::SetDisplayRect(const Rect& r) {
-  if (r != display_rect_) {
-    display_rect_ = r;
-    baseline_ = kInvalidBaseline;
-    cached_bounds_and_offset_valid_ = false;
-    lines_.clear();
-    if (elide_behavior_ != gfx::NO_ELIDE)
-      UpdateLayoutText();
-  }
+  display_rect_ = r;
+  baseline_ = kInvalidBaseline;
+  cached_bounds_and_offset_valid_ = false;
+  lines_.clear();
 }
 
 void RenderText::SetCursorPosition(size_t position) {
@@ -840,7 +830,6 @@ RenderText::RenderText()
       obscured_(false),
       obscured_reveal_index_(-1),
       truncate_length_(0),
-      elide_behavior_(NO_ELIDE),
       multiline_(false),
       fade_head_(false),
       fade_tail_(false),
@@ -1120,114 +1109,13 @@ void RenderText::UpdateLayoutText() {
     }
   }
 
-  const base::string16& text = GetLayoutText();
+  const base::string16& text = obscured_ ? layout_text_ : text_;
   if (truncate_length_ > 0 && truncate_length_ < text.length()) {
     // Truncate the text at a valid character break and append an ellipsis.
     icu::StringCharacterIterator iter(text.c_str());
     iter.setIndex32(truncate_length_ - 1);
     layout_text_.assign(text.substr(0, iter.getIndex()) + gfx::kEllipsisUTF16);
   }
-
-  if (elide_behavior_ != NO_ELIDE && display_rect_.width() > 0 &&
-      GetContentWidth() > display_rect_.width()) {
-    base::string16 elided_text = ElideText(GetLayoutText());
-
-    // This doesn't trim styles so ellipsis may get rendered as a different
-    // style than the preceding text. See crbug.com/327850.
-    layout_text_.assign(elided_text);
-  }
-  ResetLayout();
-}
-
-// TODO(skanuj): Fix code duplication with ElideText in ui/gfx/text_elider.cc
-// See crbug.com/327846
-base::string16 RenderText::ElideText(const base::string16& text) {
-  if (text.empty())
-    return text;
-
-  const bool insert_ellipsis = (elide_behavior_ != TRUNCATE_AT_END);
-  // Create a RenderText copy with attributes that affect the rendering width.
-  scoped_ptr<RenderText> render_text(CreateInstance());
-  render_text->SetFontList(font_list_);
-  render_text->SetDirectionalityMode(directionality_mode_);
-  render_text->SetCursorEnabled(cursor_enabled_);
-  render_text->styles_ = styles_;
-  render_text->colors_ = colors_;
-  render_text->SetText(text);
-  const float current_text_pixel_width = render_text->GetContentWidth();
-
-  const base::string16 ellipsis = base::string16(kEllipsisUTF16);
-  const bool elide_in_middle = false;
-  StringSlicer slicer(text, ellipsis, elide_in_middle);
-
-  // Pango will return 0 width for absurdly long strings. Cut the string in
-  // half and try again.
-  // This is caused by an int overflow in Pango (specifically, in
-  // pango_glyph_string_extents_range). It's actually more subtle than just
-  // returning 0, since on super absurdly long strings, the int can wrap and
-  // return positive numbers again. Detecting that is probably not worth it
-  // (eliding way too much from a ridiculous string is probably still
-  // ridiculous), but we should check other widths for bogus values as well.
-  if (current_text_pixel_width <= 0 && !text.empty())
-    return ElideText(slicer.CutString(text.length() / 2, insert_ellipsis));
-
-  if (current_text_pixel_width <= display_rect_.width())
-    return text;
-
-  if (insert_ellipsis &&
-      GetStringWidthF(ellipsis, font_list_) > display_rect_.width())
-    return base::string16();
-
-  // Use binary search to compute the elided text.
-  size_t lo = 0;
-  size_t hi = text.length() - 1;
-
-  for (size_t guess = (lo + hi) / 2; lo <= hi; guess = (lo + hi) / 2) {
-    // Restore styles and colors. They will be truncated to size by SetText.
-    render_text->styles_ = styles_;
-    render_text->colors_ = colors_;
-    base::string16 new_text = slicer.CutString(guess, false);
-    render_text->SetText(new_text);
-
-    // This has to be an additional step so that the ellipsis is rendered with
-    // same style as trailing part of the text.
-    if (insert_ellipsis) {
-      // When ellipsis follows text whose directionality is not the same as that
-      // of the whole text, it will be rendered with the directionality of the
-      // whole text. Since we want ellipsis to indicate continuation of the
-      // preceding text, we force the directionality of ellipsis to be same as
-      // the preceding text using LTR or RTL markers.
-      base::i18n::TextDirection leading_text_direction =
-          base::i18n::GetFirstStrongCharacterDirection(new_text);
-      base::i18n::TextDirection trailing_text_direction =
-          base::i18n::GetLastStrongCharacterDirection(new_text);
-      new_text += ellipsis;
-      if (trailing_text_direction != leading_text_direction) {
-        if (trailing_text_direction == base::i18n::LEFT_TO_RIGHT)
-          new_text += base::i18n::kLeftToRightMark;
-        else
-          new_text += base::i18n::kRightToLeftMark;
-      }
-      render_text->SetText(new_text);
-    }
-
-    // We check the width of the whole desired string at once to ensure we
-    // handle kerning/ligatures/etc. correctly.
-    const int guess_width = render_text->GetContentWidth();
-    if (guess_width == display_rect_.width())
-      break;
-    if (guess_width > display_rect_.width()) {
-      hi = guess - 1;
-      // Move back if we are on loop terminating condition, and guess is wider
-      // than available.
-      if (hi < lo)
-        lo = hi;
-    } else {
-      lo = guess + 1;
-    }
-  }
-
-  return render_text->text();
 }
 
 void RenderText::UpdateCachedBoundsAndOffset() {
diff --git a/ui/gfx/render_text.h b/ui/gfx/render_text.h
index 308b673..8696d69 100644
--- a/ui/gfx/render_text.h
+++ b/ui/gfx/render_text.h
@@ -27,7 +27,6 @@
 #include "ui/gfx/shadow_value.h"
 #include "ui/gfx/size_f.h"
 #include "ui/gfx/text_constants.h"
-#include "ui/gfx/text_elider.h"
 #include "ui/gfx/vector2d.h"
 
 class SkCanvas;
@@ -228,12 +227,6 @@ class GFX_EXPORT RenderText {
   // WARNING: Only use this for system limits, it lacks complex text support.
   void set_truncate_length(size_t length) { truncate_length_ = length; }
 
-  // Elides the text to fit in |display_rect| according to the specified
-  // |elide_behavior|. |ELIDE_IN_MIDDLE| is not supported.
-  // If both truncate and elide are specified, the shorter of the two will be
-  // applicable.
-  void SetElideBehavior(ElideBehavior elide_behavior);
-
   const Rect& display_rect() const { return display_rect_; }
   void SetDisplayRect(const Rect& r);
 
@@ -542,8 +535,6 @@ class GFX_EXPORT RenderText {
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, ApplyColorAndStyle);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, ObscuredText);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, RevealObscuredText);
-  FRIEND_TEST_ALL_PREFIXES(RenderTextTest, ElidedText);
-  FRIEND_TEST_ALL_PREFIXES(RenderTextTest, ElidedObscuredText);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, TruncatedText);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, TruncatedObscuredText);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, GraphemePositions);
@@ -565,10 +556,6 @@ class GFX_EXPORT RenderText {
   // Updates |layout_text_| if the text is obscured or truncated.
   void UpdateLayoutText();
 
-  // Elides |text| to fit in the |display_rect_| with given |elide_behavior_|.
-  // See ElideText in ui/gfx/text_elider.cc for reference.
-  base::string16 ElideText(const base::string16& text);
-
   // Update the cached bounds and display offset to ensure that the current
   // cursor is within the visible display area.
   void UpdateCachedBoundsAndOffset();
@@ -641,9 +628,6 @@ class GFX_EXPORT RenderText {
   // The maximum length of text to display, 0 forgoes a hard limit.
   size_t truncate_length_;
 
-  // The behavior for eliding or truncating.
-  ElideBehavior elide_behavior_;
-
   // The obscured and/or truncated text that will be displayed.
   base::string16 layout_text_;
 
diff --git a/ui/gfx/render_text_unittest.cc b/ui/gfx/render_text_unittest.cc
index aa28b24..c8c50d4 100644
--- a/ui/gfx/render_text_unittest.cc
+++ b/ui/gfx/render_text_unittest.cc
@@ -401,92 +401,6 @@ TEST_F(RenderTextTest, RevealObscuredText) {
   EXPECT_EQ(valid_expect_5_and_6, render_text->GetLayoutText());
 }
 
-TEST_F(RenderTextTest, ElidedText) {
-  // TODO(skanuj) : Add more test cases for following
-  // - RenderText styles.
-  // - Cross interaction of truncate, elide and obscure.
-  // - ElideText tests from text_elider.cc.
-  struct {
-    const wchar_t* text;
-    const wchar_t* layout_text;
-  } cases[] = {
-    // Strings shorter than the elision width should be laid out in full.
-    { L""        , L""        },
-    { kWeak      , kWeak      },
-    { kLtr       , kLtr       },
-    { kLtrRtl    , kLtrRtl    },
-    { kLtrRtlLtr , kLtrRtlLtr },
-    { kRtl       , kRtl       },
-    { kRtlLtr    , kRtlLtr    },
-    { kRtlLtrRtl , kRtlLtrRtl },
-    // Strings as long as the elision width should be laid out in full.
-    { L"012ab",   L"012ab"   },
-    // Long strings should be elided with an ellipsis appended at the end.
-    { L"012" L"abc"                  , L"012a\x2026"               },
-    { L"012" L"ab" L"\x5d0\x5d1"     , L"012a\x2026"               },
-    { L"012" L"a" L"\x5d1" L"b"      , L"012a\x2026"               },
-    // No RLM marker added as digits (012) have weak directionality.
-    { L"01" L"\x5d0\x5d1\x5d2" L"ab" , L"01\x5d0\x5d1\x2026"       },
-    // RLM marker added as "ab" have strong LTR directionality.
-    { L"ab" L"\x5d0\x5d1\x5d2" L"ab" , L"ab\x5d0\x5d1\x2026\x200f" },
-    // Complex script is not handled. In this example, the "\x0915\x093f" is a
-    // compound glyph, but only half of it is elided.
-    { L"0123\x0915\x093f" L"abcd"          , L"0123\x0915\x2026"          },
-    // Surrogate pairs should be elided reasonably enough.
-    { L"0\x05e9\x05bc\x05c1\x05b8"         , L"0\x05e9\x05bc\x05c1\x05b8" },
-    { L"0\x05e9\x05bc\x05c1\x05b8" L"ab"   , L"0\x05e9\x05bc\x2026"       },
-    { L"01\x05e9\x05bc\x05c1\x05b8" L"ab"  , L"01\x05e9\x2026"            },
-    { L"012\x05e9\x05bc\x05c1\x05b8" L"ab" , L"012\x2026"                 },
-    { L"012\xF0\x9D\x84\x9E"               , L"012\xF0\x2026"             },
-  };
-
-  scoped_ptr<RenderText> expected_render_text(RenderText::CreateInstance());
-  expected_render_text->SetFontList(FontList("serif, Sans serif, 12px"));
-  expected_render_text->SetDisplayRect(gfx::Rect(0, 0, 9999, 100));
-
-  scoped_ptr<RenderText> render_text(RenderText::CreateInstance());
-  render_text->SetFontList(FontList("serif, Sans serif, 12px"));
-  render_text->SetElideBehavior(gfx::ELIDE_AT_END);
-  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(cases); i++) {
-    expected_render_text->SetText(WideToUTF16(cases[i].layout_text));
-    int expected_width = expected_render_text->GetContentWidth();
-    render_text->SetText(WideToUTF16(cases[i].text));
-#if defined(OS_WIN)
-    // TODO(skanuj): It seems in some cases (on Windows XP), the original text
-    // is shorter than the elided text.  Those cases should be modified by
-    // adding extra chars in the end.  Currently this problem happens for the
-    // following test case:
-    // { L"012\xF0\x9D\x84\x9E"               , L"012\xF0\x2026"             },
-    int original_width = render_text->GetContentWidth();
-    if ((base::win::GetVersion() < base::win::VERSION_VISTA) &&
-        (expected_width >= original_width))
-      continue;
-#endif
-    render_text->SetDisplayRect(gfx::Rect(0, 0, expected_width, 100));
-    EXPECT_EQ(WideToUTF16(cases[i].text), render_text->text());
-    EXPECT_EQ(WideToUTF16(cases[i].layout_text), render_text->GetLayoutText())
-        << "->For case " << i << ": " << cases[i].text << "\n";
-    expected_render_text->SetText(base::string16());
-  }
-}
-
-TEST_F(RenderTextTest, ElidedObscuredText) {
-  scoped_ptr<RenderText> expected_render_text(RenderText::CreateInstance());
-  expected_render_text->SetFontList(FontList("serif, Sans serif, 12px"));
-  expected_render_text->SetDisplayRect(gfx::Rect(0, 0, 9999, 100));
-  expected_render_text->SetText(WideToUTF16(L"**\x2026"));
-
-  scoped_ptr<RenderText> render_text(RenderText::CreateInstance());
-  render_text->SetFontList(FontList("serif, Sans serif, 12px"));
-  render_text->SetElideBehavior(gfx::ELIDE_AT_END);
-  render_text->SetDisplayRect(
-      gfx::Rect(0, 0, expected_render_text->GetContentWidth(), 100));
-  render_text->SetObscured(true);
-  render_text->SetText(WideToUTF16(L"abcdef"));
-  EXPECT_EQ(WideToUTF16(L"abcdef"), render_text->text());
-  EXPECT_EQ(WideToUTF16(L"**\x2026"), render_text->GetLayoutText());
-}
-
 TEST_F(RenderTextTest, TruncatedText) {
   struct {
     const wchar_t* text;
diff --git a/ui/gfx/text_elider.cc b/ui/gfx/text_elider.cc
index c58a96d..5b94696 100644
--- a/ui/gfx/text_elider.cc
+++ b/ui/gfx/text_elider.cc
@@ -39,6 +39,68 @@ const base::char16 kForwardSlash = '/';
 
 namespace {
 
+// Helper class to split + elide text, while respecting UTF16 surrogate pairs.
+class StringSlicer {
+ public:
+  StringSlicer(const base::string16& text,
+               const base::string16& ellipsis,
+               bool elide_in_middle)
+      : text_(text),
+        ellipsis_(ellipsis),
+        elide_in_middle_(elide_in_middle) {
+  }
+
+  // Cuts |text_| to be |length| characters long. If |elide_in_middle_| is true,
+  // the middle of the string is removed to leave equal-length pieces from the
+  // beginning and end of the string; otherwise, the end of the string is
+  // removed and only the beginning remains. If |insert_ellipsis| is true,
+  // then an ellipsis character will be inserted at the cut point.
+  base::string16 CutString(size_t length, bool insert_ellipsis) {
+    const base::string16 ellipsis_text = insert_ellipsis ? ellipsis_
+                                                         : base::string16();
+
+    if (!elide_in_middle_)
+      return text_.substr(0, FindValidBoundaryBefore(length)) + ellipsis_text;
+
+    // We put the extra character, if any, before the cut.
+    const size_t half_length = length / 2;
+    const size_t prefix_length = FindValidBoundaryBefore(length - half_length);
+    const size_t suffix_start_guess = text_.length() - half_length;
+    const size_t suffix_start = FindValidBoundaryAfter(suffix_start_guess);
+    const size_t suffix_length =
+        half_length - (suffix_start_guess - suffix_start);
+    return text_.substr(0, prefix_length) + ellipsis_text +
+           text_.substr(suffix_start, suffix_length);
+  }
+
+ private:
+  // Returns a valid cut boundary at or before |index|.
+  size_t FindValidBoundaryBefore(size_t index) const {
+    DCHECK_LE(index, text_.length());
+    if (index != text_.length())
+      U16_SET_CP_START(text_.data(), 0, index);
+    return index;
+  }
+
+  // Returns a valid cut boundary at or after |index|.
+  size_t FindValidBoundaryAfter(size_t index) const {
+    DCHECK_LE(index, text_.length());
+    if (index != text_.length())
+      U16_SET_CP_LIMIT(text_.data(), 0, index, text_.length());
+    return index;
+  }
+
+  // The text to be sliced.
+  const base::string16& text_;
+
+  // Ellipsis string to use.
+  const base::string16& ellipsis_;
+
+  // If true, the middle of the string will be elided.
+  bool elide_in_middle_;
+
+  DISALLOW_COPY_AND_ASSIGN(StringSlicer);
+};
 
 // Build a path from the first |num_components| elements in |path_elements|.
 // Prepends |path_prefix|, appends |filename|, inserts ellipsis if appropriate.
@@ -88,46 +150,6 @@ base::string16 ElideComponentizedPath(
 
 }  // namespace
 
-StringSlicer::StringSlicer(const base::string16& text,
-             const base::string16& ellipsis,
-             bool elide_in_middle)
-    : text_(text),
-      ellipsis_(ellipsis),
-      elide_in_middle_(elide_in_middle) {
-}
-
-base::string16 StringSlicer::CutString(size_t length, bool insert_ellipsis) {
-  const base::string16 ellipsis_text = insert_ellipsis ? ellipsis_
-                                                       : base::string16();
-
-  if (!elide_in_middle_)
-    return text_.substr(0, FindValidBoundaryBefore(length)) + ellipsis_text;
-
-  // We put the extra character, if any, before the cut.
-  const size_t half_length = length / 2;
-  const size_t prefix_length = FindValidBoundaryBefore(length - half_length);
-  const size_t suffix_start_guess = text_.length() - half_length;
-  const size_t suffix_start = FindValidBoundaryAfter(suffix_start_guess);
-  const size_t suffix_length =
-      half_length - (suffix_start_guess - suffix_start);
-  return text_.substr(0, prefix_length) + ellipsis_text +
-         text_.substr(suffix_start, suffix_length);
-}
-
-size_t StringSlicer::FindValidBoundaryBefore(size_t index) const {
-  DCHECK_LE(index, text_.length());
-  if (index != text_.length())
-    U16_SET_CP_START(text_.data(), 0, index);
-  return index;
-}
-
-size_t StringSlicer::FindValidBoundaryAfter(size_t index) const {
-  DCHECK_LE(index, text_.length());
-  if (index != text_.length())
-    U16_SET_CP_LIMIT(text_.data(), 0, index, text_.length());
-  return index;
-}
-
 base::string16 ElideEmail(const base::string16& email,
                           const FontList& font_list,
                           float available_pixel_width) {
@@ -455,8 +477,7 @@ base::string16 ElideText(const base::string16& text,
   // (eliding way too much from a ridiculous string is probably still
   // ridiculous), but we should check other widths for bogus values as well.
   if (current_text_pixel_width <= 0 && !text.empty()) {
-    const base::string16 cut =
-      slicer.CutString(text.length() / 2, insert_ellipsis);
+    const base::string16 cut = slicer.CutString(text.length() / 2, false);
     return ElideText(cut, font_list, available_pixel_width, elide_behavior);
   }
 
@@ -472,23 +493,20 @@ base::string16 ElideText(const base::string16& text,
   size_t hi = text.length() - 1;
   size_t guess;
   for (guess = (lo + hi) / 2; lo <= hi; guess = (lo + hi) / 2) {
-    // We check the width of the whole desired string at once to ensure we
+    // We check the length of the whole desired string at once to ensure we
     // handle kerning/ligatures/etc. correctly.
-    // TODO(skanuj) : Handle directionality of ellipsis based on adjacent
-    // characters.  See crbug.com/327963.
     const base::string16 cut = slicer.CutString(guess, insert_ellipsis);
-    const float guess_width = GetStringWidthF(cut, font_list);
-    if (guess_width == available_pixel_width)
-      break;
-    if (guess_width > available_pixel_width) {
+    const float guess_length = GetStringWidthF(cut, font_list);
+    // Check again that we didn't hit a Pango width overflow. If so, cut the
+    // current string in half and start over.
+    if (guess_length <= 0) {
+      return ElideText(slicer.CutString(guess / 2, false),
+                       font_list, available_pixel_width, elide_behavior);
+    }
+    if (guess_length > available_pixel_width)
       hi = guess - 1;
-      // Move back if we are on loop terminating condition, and guess is wider
-      // than available.
-      if (hi < lo)
-        lo = hi;
-    } else {
+    else
       lo = guess + 1;
-    }
   }
 
   return slicer.CutString(guess, insert_ellipsis);
diff --git a/ui/gfx/text_elider.h b/ui/gfx/text_elider.h
index eef1f8e..4eaca82 100644
--- a/ui/gfx/text_elider.h
+++ b/ui/gfx/text_elider.h
@@ -29,38 +29,6 @@ class FontList;
 GFX_EXPORT extern const char kEllipsis[];
 GFX_EXPORT extern const base::char16 kEllipsisUTF16[];
 
-
-// Helper class to split + elide text, while respecting UTF16 surrogate pairs.
-class StringSlicer {
- public:
-  StringSlicer(const base::string16& text,
-               const base::string16& ellipsis,
-               bool elide_in_middle);
-
-  // Cuts |text_| to be |length| characters long. If |elide_in_middle_| is true,
-  // the middle of the string is removed to leave equal-length pieces from the
-  // beginning and end of the string; otherwise, the end of the string is
-  // removed and only the beginning remains. If |insert_ellipsis| is true,
-  // then an ellipsis character will be inserted at the cut point.
-  base::string16 CutString(size_t length, bool insert_ellipsis);
-
- private:
-  // Returns a valid cut boundary at or before/after |index|.
-  size_t FindValidBoundaryBefore(size_t index) const;
-  size_t FindValidBoundaryAfter(size_t index) const;
-
-  // The text to be sliced.
-  const base::string16& text_;
-
-  // Ellipsis string to use.
-  const base::string16& ellipsis_;
-
-  // If true, the middle of the string will be elided.
-  bool elide_in_middle_;
-
-  DISALLOW_COPY_AND_ASSIGN(StringSlicer);
-};
-
 // Elides a well-formed email address (e.g. username@domain.com) to fit into
 // |available_pixel_width| using the specified |font_list|.
 // This function guarantees that the string returned will contain at least one
@@ -100,9 +68,7 @@ enum ElideBehavior {
   // Add ellipsis in the middle of the string.
   ELIDE_IN_MIDDLE,
   // Truncate the end of the string.
-  TRUNCATE_AT_END,
-  // No eliding of the string.
-  NO_ELIDE
+  TRUNCATE_AT_END
 };
 
 // Elides |text| to fit in |available_pixel_width| according to the specified