Add multiline support to RenderTextHarfBuzz.

ui/gfx/render_text_harfbuzz.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef UI_GFX_RENDER_TEXT_HARFBUZZ_H_
 #define UI_GFX_RENDER_TEXT_HARFBUZZ_H_
 
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "third_party/harfbuzz-ng/src/hb.h"
@@ skipping 95 matching lines @@
   std::vector<Rect> GetSubstringBounds(const Range& range) override;
   size_t TextIndexToLayoutIndex(size_t index) const override;
   size_t LayoutIndexToTextIndex(size_t index) const override;
   bool IsValidCursorIndex(size_t index) override;
   void ResetLayout() override;
   void EnsureLayout() override;
   void DrawVisualText(Canvas* canvas) override;
 
  private:
   friend class RenderTextTest;
+  FRIEND_TEST_ALL_PREFIXES(RenderTextTest, Multiline_NormalWidth);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, HarfBuzz_RunDirection);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, HarfBuzz_BreakRunsByUnicodeBlocks);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, HarfBuzz_BreakRunsByEmoji);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, HarfBuzz_SubglyphGraphemeCases);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, HarfBuzz_SubglyphGraphemePartition);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, HarfBuzz_NonExistentFont);
   FRIEND_TEST_ALL_PREFIXES(RenderTextTest, HarfBuzz_UniscribeFallback);
 
+  // Specify the width of a glyph for test. The width of glyphs is very
+  // platform-dependent and environment-dependent. Otherwise multiline test
+  // will become really flaky.
+  void set_glyph_width_for_test(uint8 test_width) {
+    glyph_width_for_test_ = test_width;
+  }
+
   // 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;
   size_t GetRunContainingXCoord(float x, float* offset) const;
 
   // Given a |run|, returns the SelectionModel that contains the logical first
   // or last caret position inside (not at a boundary of) the run.
   // The returned value represents a cursor/caret position without a selection.
   SelectionModel FirstSelectionModelInsideRun(
       const internal::TextRunHarfBuzz* run);
@@ skipping 27 matching lines @@
   // Maps visual run indices to logical run indices and vice versa.
   std::vector<int32_t> visual_to_logical_;
   std::vector<int32_t> logical_to_visual_;
 
   bool needs_layout_;
 
   // ICU grapheme iterator for the layout text. Valid when |!needs_layout_|. Can
   // be NULL in case of an error.
   scoped_ptr<base::i18n::BreakIterator> grapheme_iterator_;
 
+  // The total size of the layouted text.
+  SizeF total_size_;
+
+  // Fixed width of glyphs. This should only be set in test environments.
+  uint8 glyph_width_for_test_;

[oshima, 2015/02/05 21:29:40]

instead of adding this to every instance, can you just use file scoped variable?

[msw, 2015/02/05 21:50:58]

That goes against my recommendation at:
https://codereview.chromium.org/882643005/diff/100001/ui/gfx/render_text_harf...
I don't think multiple RenderText instances should be affected by this test
setting, nor should you have to unset the test value for subsequent tests, and
perhaps sharing a file-static value could cause flaky failures of tests being
run in parallel?

[Jun Mukai, 2015/02/05 22:21:48]

One thing actually I was a bit worried about is the memory consumption -- it
would be great if we can eliminate an extra field just for testing.  I don't
think single uint8 would make big difference though.

[oshima, 2015/02/05 22:42:44]

How about defining virtual function that gets the width of the glyph. you can
define a class for test that return a fixed value, instead of adding test only
member variable. It won't add extra memory except one more vtable.

[Jun Mukai, 2015/02/05 23:28:00]

Rather than that, I would think that exposing the HarfBuzzLineBreaker class is
the right thing to do, so that it doesn't have to care about how harfbuzz
computes the width of glyphs, we can focus on linebreaker logic on the certain
data of text runs.

Actually this is already noted as a TODO(ckocagil) in the cc file. I  am not
thinking to do it here, because that would require rewrites of several test
cases.

[oshima, 2015/02/05 23:48:25]

I'm fine as long as we can eliminate test only member variable.
separate cl is fine.

+
   DISALLOW_COPY_AND_ASSIGN(RenderTextHarfBuzz);
 };
 
 }  // namespace gfx
 
 #endif  // UI_GFX_RENDER_TEXT_HARFBUZZ_H_