Add multiline support to RenderTextHarfBuzz.

ui/gfx/render_text_harfbuzz.h

Index: ui/gfx/render_text_harfbuzz.h
diff --git a/ui/gfx/render_text_harfbuzz.h b/ui/gfx/render_text_harfbuzz.h
index 7f3a1dcef7396fb5b5f84af79d7d12a7a63a3c15..d0e167ecbed1cc421ec7b9eef17fb1aa0d8e50f1 100644
--- a/ui/gfx/render_text_harfbuzz.h
+++ b/ui/gfx/render_text_harfbuzz.h
@@ -113,6 +113,7 @@ class GFX_EXPORT RenderTextHarfBuzz : public RenderText {
 
  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);
@@ -121,6 +122,13 @@ class GFX_EXPORT RenderTextHarfBuzz : public RenderText {
   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;
@@ -168,6 +176,12 @@ class GFX_EXPORT RenderTextHarfBuzz : public RenderText {
   // 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);
 };