RenderTextHarfBuzz: Decide run direction by BiDi embedding level

BUG=382178
NOTRY=true
R=msw

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

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

ui/gfx/render_text_harfbuzz.cc

@@ -863,12 +863,17 @@ void RenderTextHarfBuzz::ItemizeText() {
   const bool is_text_rtl = GetTextDirection() == base::i18n::RIGHT_TO_LEFT;
   DCHECK_NE(0U, text.length());
 
-  // If ICU fails to itemize the text, we set |fake_runs| and create a run that
-  // spans the entire text. This is needed because early returning and leaving
-  // the runs set empty causes some clients to crash/misbehave since they expect
-  // non-zero text metrics from a non-empty text.
+  // If ICU fails to itemize the text, we create a run that spans the entire
+  // text. This is needed because leaving the runs set empty causes some clients
+  // to misbehave since they expect non-zero text metrics from a non-empty text.
   base::i18n::BiDiLineIterator bidi_iterator;
-  bool fake_runs = !bidi_iterator.Open(text, is_text_rtl, false);
+  if (!bidi_iterator.Open(text, is_text_rtl, false)) {
+    internal::TextRunHarfBuzz* run = new internal::TextRunHarfBuzz;
+    run->range = Range(0, text.length());
+    runs_.push_back(run);
+    visual_to_logical_ = logical_to_visual_ = std::vector<int32_t>(1, 0);
+    return;
+  }
 
   // Temporarily apply composition underlines and selection colors.
   ApplyCompositionAndSelectionStyles();
@@ -888,11 +893,10 @@ void RenderTextHarfBuzz::ItemizeText() {
     run->diagonal_strike = style.style(DIAGONAL_STRIKE);
     run->underline = style.style(UNDERLINE);
 
-    if (fake_runs) {
-      run_break = text.length();
-    } else {
     int32 script_item_break = 0;
     bidi_iterator.GetLogicalRun(run_break, &script_item_break, &run->level);
+    // Odd BiDi embedding levels correspond to RTL runs.
+    run->is_rtl = (run->level % 2) == 1;
     // Find the length and script of this script run.
     script_item_break = ScriptInterval(text, run_break,
         script_item_break - run_break, &run->script) + run_break;
@@ -920,17 +924,11 @@ void RenderTextHarfBuzz::ItemizeText() {
         }
       }
     }
-    }
 
     DCHECK(IsValidCodePointIndex(text, run_break));
     style.UpdatePosition(LayoutIndexToTextIndex(run_break));
     run->range.set_end(run_break);
-    UBiDiDirection direction = ubidi_getBaseDirection(
-        text.c_str() + run->range.start(), run->range.length());
-    if (direction == UBIDI_NEUTRAL)
-      run->is_rtl = is_text_rtl;
-    else
-      run->is_rtl = direction == UBIDI_RTL;
+
     runs_.push_back(run);
   }
 

ui/gfx/render_text_harfbuzz.h

@@ -61,7 +61,7 @@ struct GFX_EXPORT TextRunHarfBuzz {
 
 }  // namespace internal
 
-class RenderTextHarfBuzz : public RenderText {
+class GFX_EXPORT RenderTextHarfBuzz : public RenderText {
  public:
   RenderTextHarfBuzz();
   virtual ~RenderTextHarfBuzz();
@@ -90,6 +90,9 @@ class RenderTextHarfBuzz : public RenderText {
   virtual void DrawVisualText(Canvas* canvas) OVERRIDE;
 
  private:
+  friend class RenderTextTest;
+  FRIEND_TEST_ALL_PREFIXES(RenderTextTest, HarfBuzz_RunDirection);
+
   // Return the run index that contains the argument; or the length of the
   // |runs_| vector if argument exceeds the text length or width.
   size_t GetRunContainingCaret(const SelectionModel& caret) const;

ui/gfx/render_text_unittest.cc

@@ -1977,4 +1977,16 @@ TEST_F(RenderTextTest, HarfBuzz_CharToGlyph) {
 
 }
 
+TEST_F(RenderTextTest, HarfBuzz_RunDirection) {
+  RenderTextHarfBuzz render_text;
+  const base::string16 mixed =
+      WideToUTF16(L"\x05D0\x05D1" L"1234" L"\x05D2\x05D3");
+  render_text.SetText(mixed);
+  render_text.EnsureLayout();
+  ASSERT_EQ(3U, render_text.runs_.size());
+  EXPECT_TRUE(render_text.runs_[0]->is_rtl);
+  EXPECT_FALSE(render_text.runs_[1]->is_rtl);
+  EXPECT_TRUE(render_text.runs_[2]->is_rtl);
+}
+
 }  // namespace gfx