Fix multiline behaviors for RTL text.

ui/gfx/render_text_harfbuzz.cc

Index: ui/gfx/render_text_harfbuzz.cc
diff --git a/ui/gfx/render_text_harfbuzz.cc b/ui/gfx/render_text_harfbuzz.cc
index e4469a6fdfa2500f117317a0b22e84bfac6deb4c..b4a809ad9b5e2bc4fd72aea703a05b7c10c33367 100644
--- a/ui/gfx/render_text_harfbuzz.cc
+++ b/ui/gfx/render_text_harfbuzz.cc
@@ -220,6 +220,7 @@ class HarfBuzzLineBreaker {
                       float min_height,
                       bool multiline,
                       const base::string16& text,
+                      const std::vector<int32_t>& logical_to_visual,
                       const BreakList<size_t>* words,
                       const ScopedVector<internal::TextRunHarfBuzz>& runs)
       : max_width_((max_width == 0) ? SK_ScalarMax : SkIntToScalar(max_width)),
@@ -227,6 +228,7 @@ class HarfBuzzLineBreaker {
         min_height_(min_height),
         multiline_(multiline),
         text_(text),
+        logical_to_visual_(logical_to_visual),
         words_(words),
         runs_(runs),
         text_x_(0),
@@ -265,6 +267,18 @@ class HarfBuzzLineBreaker {
   // A (line index, segment index) pair that specifies a segment in |lines_|.
   typedef std::pair<size_t, size_t> SegmentHandle;
 
+  struct SortByVisualPosition {

[ckocagil, 2015/02/13 19:05:25]

Make this comparator a lambda expression and define it in the caller function.

[Jun Mukai, 2015/02/13 21:57:58]

Done.

+    explicit SortByVisualPosition(const std::vector<int32_t>& logical_to_visual)
+        : logical_to_visual(logical_to_visual) {}
+
+    bool operator()(const internal::LineSegment& segment1,
+                    const internal::LineSegment& segment2) {
+      return logical_to_visual[segment1.run] < logical_to_visual[segment2.run];
+    }
+
+    const std::vector<int32_t>& logical_to_visual;
+  };
+
   internal::LineSegment* SegmentFromHandle(const SegmentHandle& handle) {
     return &lines_[handle.first].segments[handle.second];
   }
@@ -373,6 +387,8 @@ class HarfBuzzLineBreaker {
   void AdvanceLine() {
     if (!lines_.empty()) {
       internal::Line* line = &lines_.back();
+      std::sort(line->segments.begin(), line->segments.end(),
+                SortByVisualPosition(logical_to_visual_));
       line->size.set_height(std::max(min_height_, max_descent_ + max_ascent_));
       line->baseline =
           std::max(min_baseline_, SkScalarRoundToInt(max_ascent_));
@@ -434,6 +450,7 @@ class HarfBuzzLineBreaker {
   const float min_height_;
   const bool multiline_;
   const base::string16& text_;
+  const std::vector<int32_t>& logical_to_visual_;
   const BreakList<size_t>* const words_;
   const ScopedVector<internal::TextRunHarfBuzz>& runs_;
 
@@ -923,7 +940,8 @@ void RenderTextHarfBuzz::EnsureLayout() {
     HarfBuzzLineBreaker line_breaker(
         display_rect().width(), font_list().GetBaseline(),
         std::max(font_list().GetHeight(), min_line_height()), multiline(),
-        GetLayoutText(), multiline() ? &GetLineBreaks() : nullptr, runs_);
+        GetLayoutText(), logical_to_visual_,
+        multiline() ? &GetLineBreaks() : nullptr, runs_);
 
     // TODO(vadimt): Remove ScopedTracker below once crbug.com/431326 is fixed.
     tracked_objects::ScopedTracker tracking_profile3(
@@ -931,7 +949,7 @@ void RenderTextHarfBuzz::EnsureLayout() {
             "431326 RenderTextHarfBuzz::EnsureLayout3"));
 
     for (size_t i = 0; i < runs_.size(); ++i)
-      line_breaker.AddRun(visual_to_logical_[i]);
+      line_breaker.AddRun(i);
     std::vector<internal::Line> lines;
     line_breaker.Finalize(&lines, &total_size_);
     set_lines(&lines);
@@ -939,13 +957,18 @@ void RenderTextHarfBuzz::EnsureLayout() {
 }
 
 void RenderTextHarfBuzz::DrawVisualText(Canvas* canvas) {
+  internal::SkiaTextRenderer renderer(canvas);
+  DrawVisualTextInternal(&renderer);
+}
+
+void RenderTextHarfBuzz::DrawVisualTextInternal(
+    internal::SkiaTextRenderer* renderer) {
   DCHECK(!needs_layout_);
   if (lines().empty())
     return;
 
-  internal::SkiaTextRenderer renderer(canvas);
-  ApplyFadeEffects(&renderer);
-  ApplyTextShadows(&renderer);
+  ApplyFadeEffects(renderer);
+  ApplyTextShadows(renderer);
   ApplyCompositionAndSelectionStyles();
 
   for (size_t i = 0; i < lines().size(); ++i) {
@@ -954,10 +977,10 @@ void RenderTextHarfBuzz::DrawVisualText(Canvas* canvas) {
     SkScalar preceding_segment_widths = 0;
     for (const internal::LineSegment& segment : line.segments) {
       const internal::TextRunHarfBuzz& run = *runs_[segment.run];
-      renderer.SetTypeface(run.skia_face.get());
-      renderer.SetTextSize(SkIntToScalar(run.font_size));
-      renderer.SetFontRenderParams(run.render_params,
-                                   background_is_transparent());
+      renderer->SetTypeface(run.skia_face.get());
+      renderer->SetTextSize(SkIntToScalar(run.font_size));
+      renderer->SetFontRenderParams(run.render_params,
+                                    background_is_transparent());
       Range glyphs_range = run.CharRangeToGlyphRange(segment.char_range);
       scoped_ptr<SkPoint[]> positions(new SkPoint[glyphs_range.length()]);
       SkScalar offset_x =
@@ -984,8 +1007,8 @@ void RenderTextHarfBuzz::DrawVisualText(Canvas* canvas) {
         if (colored_glyphs.is_empty())
           continue;
 
-        renderer.SetForegroundColor(it->second);
-        renderer.DrawPosText(
+        renderer->SetForegroundColor(it->second);
+        renderer->DrawPosText(
             &positions[colored_glyphs.start() - glyphs_range.start()],
             &run.glyphs[colored_glyphs.start()], colored_glyphs.length());
         int start_x = SkScalarRoundToInt(
@@ -995,15 +1018,15 @@ void RenderTextHarfBuzz::DrawVisualText(Canvas* canvas) {
                 ? (SkFloatToScalar(segment.width) + preceding_segment_widths +
                    SkIntToScalar(origin.x()))
                 : positions[colored_glyphs.end() - glyphs_range.start()].x());
-        renderer.DrawDecorations(start_x, origin.y(), end_x - start_x,
-                                 run.underline, run.strike,
-                                 run.diagonal_strike);
+        renderer->DrawDecorations(start_x, origin.y(), end_x - start_x,
+                                  run.underline, run.strike,
+                                  run.diagonal_strike);
       }
       preceding_segment_widths += SkFloatToScalar(segment.width);
     }
   }
 
-  renderer.EndDiagonalStrike();
+  renderer->EndDiagonalStrike();
 
   UndoCompositionAndSelectionStyles();
 }
@@ -1318,8 +1341,8 @@ bool RenderTextHarfBuzz::ShapeRunWithFont(internal::TextRunHarfBuzz* run,
     const SkScalar y_offset = SkFixedToScalar(hb_positions[i].y_offset);
     run->positions[i].set(run->width + x_offset, -y_offset);
     run->width += (glyph_width_for_test_ > 0)
-                      ? SkIntToScalar(glyph_width_for_test_)
-                      : SkFixedToScalar(hb_positions[i].x_advance);
+                      ? glyph_width_for_test_
+                      : SkFixedToFloat(hb_positions[i].x_advance);
     // Round run widths if subpixel positioning is off to match native behavior.
     if (!run->render_params.subpixel_positioning)
       run->width = std::floor(run->width + 0.5f);