Move allow_character_break property to RenderText.

Move allow_character_break property to RenderText.

gfx::ElideRectangleText creates lots of RenderText object which
causes the performance regression. This CL avoids invoking the
method by moving allow_character_break property to RenderText.

BUG=470506
R=msw@chromium.org, sky@chromium.org
TEST=the new test case covers

Committed: https://crrev.com/a19339399b745657a9d9e7f288cfc797f90403f5
Cr-Commit-Position: refs/heads/master@{#322681}

[msw, 2015-03-25 20:54:15]

I'm not entirely sure, but I think this is wrong.

https://codereview.chromium.org/1015533016/diff/1/ui/gfx/render_text.cc
File ui/gfx/render_text.cc (right):

https://codereview.chromium.org/1015533016/diff/1/ui/gfx/render_text.cc#newco...
ui/gfx/render_text.cc:539: cached_bounds_and_offset_valid_ = false;
Should we only run these 3 lines if multi-line is enabled?

https://codereview.chromium.org/1015533016/diff/1/ui/gfx/render_text.h
File ui/gfx/render_text.h (right):

https://codereview.chromium.org/1015533016/diff/1/ui/gfx/render_text.h#newcod...
ui/gfx/render_text.h:767: // Whether the line breaker allows to break lines by
character, not by word.
nit: clarify the actual behavior here (or use gfx::WordWrapBehavior)...

https://codereview.chromium.org/1015533016/diff/1/ui/gfx/render_text.h#newcod...
ui/gfx/render_text.h:768: bool allow_character_break_;
nit: use gfx::WordWrapBehavior?

https://codereview.chromium.org/1015533016/diff/1/ui/gfx/render_text_harfbuzz.cc
File ui/gfx/render_text_harfbuzz.cc (right):

https://codereview.chromium.org/1015533016/diff/1/ui/gfx/render_text_harfbuzz...
ui/gfx/render_text_harfbuzz.cc:340: if (!allow_character_break_ && (line_x_ != 0
|| word_width < *width)) {
I don't think this matches the current Label behavior of toggling between
gfx::WRAP_LONG_WORDS and gfx::TRUNCATE_LONG_WORDS...

Afaict, this is the expected behavior, can you confirm?
1) TRUNCATE_LONG_WORDS (allow_character_break_ == false): Move long words to a
new line (if there is preceding text on the current line), then *truncate any
word content beyond the width of a full line*.
2) WRAP_LONG_WORDS (allow_character_break_ == true): Move long words to a new
line (if there is preceding text on the current line), then *continually wrap
the word content as needed onto subsequent lines.*
3) RenderText ToT: Matches WRAP_LONG_WORDS (allow_character_break_ == true)?
4) With this change: *Never move long words onto new lines* (WRONG), just
continually wrap the word content as needed...

We should clarify the gfx::WordWrapBehavior enums comments and add more testing.

https://codereview.chromium.org/1015533016/diff/1/ui/views/controls/label.cc
File ui/views/controls/label.cc (right):

https://codereview.chromium.org/1015533016/diff/1/ui/views/controls/label.cc#...
ui/views/controls/label.cc:261: // RenderText doesn't support character breaks.
nit: remove this comment and TODO.

[Jun Mukai, 2015-03-26 01:35:08]

https://codereview.chromium.org/1015533016/diff/1/ui/gfx/render_text.cc
File ui/gfx/render_text.cc (right):

https://codereview.chromium.org/1015533016/diff/1/ui/gfx/render_text.cc#newco...
ui/gfx/render_text.cc:539: cached_bounds_and_offset_valid_ = false;
On 2015/03/25 20:54:15, msw wrote:
> Should we only run these 3 lines if multi-line is enabled?

Done.

https://codereview.chromium.org/1015533016/diff/1/ui/gfx/render_text.h
File ui/gfx/render_text.h (right):

https://codereview.chromium.org/1015533016/diff/1/ui/gfx/render_text.h#newcod...
ui/gfx/render_text.h:767: // Whether the line breaker allows to break lines by
character, not by word.
On 2015/03/25 20:54:15, msw wrote:
> nit: clarify the actual behavior here (or use gfx::WordWrapBehavior)...

Using WordWrapBehavior

https://codereview.chromium.org/1015533016/diff/1/ui/gfx/render_text.h#newcod...
ui/gfx/render_text.h:768: bool allow_character_break_;
On 2015/03/25 20:54:15, msw wrote:
> nit: use gfx::WordWrapBehavior?

Done.

https://codereview.chromium.org/1015533016/diff/1/ui/gfx/render_text_harfbuzz.cc
File ui/gfx/render_text_harfbuzz.cc (right):

https://codereview.chromium.org/1015533016/diff/1/ui/gfx/render_text_harfbuzz...
ui/gfx/render_text_harfbuzz.cc:340: if (!allow_character_break_ && (line_x_ != 0
|| word_width < *width)) {
On 2015/03/25 20:54:15, msw wrote:
> I don't think this matches the current Label behavior of toggling between
> gfx::WRAP_LONG_WORDS and gfx::TRUNCATE_LONG_WORDS...
> 
> Afaict, this is the expected behavior, can you confirm?
> 1) TRUNCATE_LONG_WORDS (allow_character_break_ == false): Move long words to a
> new line (if there is preceding text on the current line), then *truncate any
> word content beyond the width of a full line*.
> 2) WRAP_LONG_WORDS (allow_character_break_ == true): Move long words to a new
> line (if there is preceding text on the current line), then *continually wrap
> the word content as needed onto subsequent lines.*
> 3) RenderText ToT: Matches WRAP_LONG_WORDS (allow_character_break_ == true)?
> 4) With this change: *Never move long words onto new lines* (WRONG), just
> continually wrap the word content as needed...
> 
> We should clarify the gfx::WordWrapBehavior enums comments and add more
testing.

Rewritten the logic, and enriched the tests. PTAL.

https://codereview.chromium.org/1015533016/diff/1/ui/views/controls/label.cc
File ui/views/controls/label.cc (right):

https://codereview.chromium.org/1015533016/diff/1/ui/views/controls/label.cc#...
ui/views/controls/label.cc:261: // RenderText doesn't support character breaks.
On 2015/03/25 20:54:15, msw wrote:
> nit: remove this comment and TODO.

Done.

[msw, 2015-03-26 19:36:30]

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text.h
File ui/gfx/render_text.h (right):

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text.h#n...
ui/gfx/render_text.h:770: // |multiline_| is set.
nit: mention the default behavior here.

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_har...
File ui/gfx/render_text_harfbuzz.cc (right):

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_har...
ui/gfx/render_text_harfbuzz.cc:305: // smaller than |*next_char| if
word_wrap_behavior is TRUNCATE_LONG_WORDS.
nit: "smaller than |*next_char| for certain word wrapping behaviors."
(that should eventually be the case for elide too, right?)

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_har...
ui/gfx/render_text_harfbuzz.cc:332: DCHECK(word_wrap_behavior_ ==
IGNORE_LONG_WORDS ||
nit: maybe just DCHECK_NE WRAP_LONG_WORDS?

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_har...
ui/gfx/render_text_harfbuzz.cc:364: *end_char = i;
Shouldn't end_char be set as soon as the width exceeds the available width, and
not afterward? Otherwise, doesn't this extend end_char for each overflowing
char, so that it'll match next_char? ie. shouldn't the condition above be:
        } else if (word_wrap_behavior_ == TRUNCATE_LONG_WORDS && !truncated) {

Actually, I wonder if you can simplify the code a bit:
1) Remove this "else if" block and |truncated|.
2) After "if (*width > available_width)" add "else { *end_char = i; }".
3) Change 335 and 382 to check "if (word_wrap_behavior_ == IGNORE_LONG_WORDS)".

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_uni...
File ui/gfx/render_text_unittest.cc (right):

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_uni...
ui/gfx/render_text_unittest.cc:2310: render_text.Draw(&canvas);
nit: either render_text.SetWordWrapBehavior(IGNORE_LONG_WORDS); or
DCHECK_EQ(IGNORE_LONG_WORDS, render_text.word_wrap_behavior()); before this.

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_uni...
ui/gfx/render_text_unittest.cc:2312: EXPECT_EQ(Range(0, 4),
render_text.lines()[0].segments[0].char_range);
I think you'll need to do render_text.set_glyph_width_for_test(glyph_width) to
ensure that you get the same char ranges on each line across platforms (with
different fonts that yield different glyph widths). Then you can just do
render_text.SetDisplayRect(Rect(0, 0, 4 * glyph_width, 0)); above and remove
lines 2303 and 2304.

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_uni...
ui/gfx/render_text_unittest.cc:2320: EXPECT_GT(11u,
render_text.lines()[1].segments[0].char_range.end());
With a fixed width, you can expect this to equal (4, 8).

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_uni...
ui/gfx/render_text_unittest.cc:2327: EXPECT_LE(4u, render_text.lines().size());
With a fixed width, you can expect this to equal 4 and the following lines can
expect exact ranges; I think they'll be: (0, 4), (4, 8), (8, 11), (11, 14).

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/text_constants.h
File ui/gfx/text_constants.h (right):

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/text_constants....
ui/gfx/text_constants.h:18: // Specifies the word wrapping behavior of
|ElideRectangleText()| when a word
nit: Consider revising the comments like this:
// Specifies the wrapping behavior when a word exceeds available display width.
// All words that are too wide will be put on a new line, and then:
enum WordWrapBehavior {
  IGNORE_LONG_WORDS,   // Overflowing word text is left on that line.
  TRUNCATE_LONG_WORDS, // Overflowing word text is truncated.
  ELIDE_LONG_WORDS,    // Overflowing word text is elided with an ellipsis.
  WRAP_LONG_WORDS,     // Overflowing word text is wrapped over multiple lines.
};

https://codereview.chromium.org/1015533016/diff/100001/ui/views/controls/labe...
File ui/views/controls/label.cc (right):

https://codereview.chromium.org/1015533016/diff/100001/ui/views/controls/labe...
ui/views/controls/label.cc:159: ResetLayout();
nit: perhaps the layout shouldn't be reset (nor should we toggle
|is_first_paint_text_|) if the label isn't multiline.

[Jun Mukai, 2015-03-26 22:47:39]

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text.h
File ui/gfx/render_text.h (right):

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text.h#n...
ui/gfx/render_text.h:770: // |multiline_| is set.
On 2015/03/26 19:36:29, msw wrote:
> nit: mention the default behavior here.

Done.

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_har...
File ui/gfx/render_text_harfbuzz.cc (right):

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_har...
ui/gfx/render_text_harfbuzz.cc:305: // smaller than |*next_char| if
word_wrap_behavior is TRUNCATE_LONG_WORDS.
On 2015/03/26 19:36:29, msw wrote:
> nit: "smaller than |*next_char| for certain word wrapping behaviors."
> (that should eventually be the case for elide too, right?)

Done.

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_har...
ui/gfx/render_text_harfbuzz.cc:332: DCHECK(word_wrap_behavior_ ==
IGNORE_LONG_WORDS ||
On 2015/03/26 19:36:30, msw wrote:
> nit: maybe just DCHECK_NE WRAP_LONG_WORDS?

Done.

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_har...
ui/gfx/render_text_harfbuzz.cc:364: *end_char = i;
On 2015/03/26 19:36:30, msw wrote:
> Shouldn't end_char be set as soon as the width exceeds the available width,
and
> not afterward? Otherwise, doesn't this extend end_char for each overflowing
> char, so that it'll match next_char? ie. shouldn't the condition above be:
>         } else if (word_wrap_behavior_ == TRUNCATE_LONG_WORDS && !truncated) {
> 
> Actually, I wonder if you can simplify the code a bit:
> 1) Remove this "else if" block and |truncated|.
> 2) After "if (*width > available_width)" add "else { *end_char = i; }".
> 3) Change 335 and 382 to check "if (word_wrap_behavior_ ==
IGNORE_LONG_WORDS)".

You are right, "!truncated" is necessary in the condition. My test case didn't
catch that. Thanks!

however I'm afraid the latter approach doesn't work, anyways truncated flag is
necessary to truncate in middle of a word, we want to record the first character
which exceeds the available width (when the behavior is TRUNCATE).

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_uni...
File ui/gfx/render_text_unittest.cc (right):

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_uni...
ui/gfx/render_text_unittest.cc:2310: render_text.Draw(&canvas);
On 2015/03/26 19:36:30, msw wrote:
> nit: either render_text.SetWordWrapBehavior(IGNORE_LONG_WORDS); or
> DCHECK_EQ(IGNORE_LONG_WORDS, render_text.word_wrap_behavior()); before this.

Done.

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_uni...
ui/gfx/render_text_unittest.cc:2312: EXPECT_EQ(Range(0, 4),
render_text.lines()[0].segments[0].char_range);
On 2015/03/26 19:36:30, msw wrote:
> I think you'll need to do render_text.set_glyph_width_for_test(glyph_width) to
> ensure that you get the same char ranges on each line across platforms (with
> different fonts that yield different glyph widths). Then you can just do
> render_text.SetDisplayRect(Rect(0, 0, 4 * glyph_width, 0)); above and remove
> lines 2303 and 2304.

Done, great.

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_uni...
ui/gfx/render_text_unittest.cc:2320: EXPECT_GT(11u,
render_text.lines()[1].segments[0].char_range.end());
On 2015/03/26 19:36:30, msw wrote:
> With a fixed width, you can expect this to equal (4, 8).

Done.

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_uni...
ui/gfx/render_text_unittest.cc:2327: EXPECT_LE(4u, render_text.lines().size());
On 2015/03/26 19:36:30, msw wrote:
> With a fixed width, you can expect this to equal 4 and the following lines can
> expect exact ranges; I think they'll be: (0, 4), (4, 8), (8, 11), (11, 14).

Done.

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/text_constants.h
File ui/gfx/text_constants.h (right):

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/text_constants....
ui/gfx/text_constants.h:18: // Specifies the word wrapping behavior of
|ElideRectangleText()| when a word
On 2015/03/26 19:36:30, msw wrote:
> nit: Consider revising the comments like this:
> // Specifies the wrapping behavior when a word exceeds available display
width.
> // All words that are too wide will be put on a new line, and then:
> enum WordWrapBehavior {
>   IGNORE_LONG_WORDS,   // Overflowing word text is left on that line.
>   TRUNCATE_LONG_WORDS, // Overflowing word text is truncated.
>   ELIDE_LONG_WORDS,    // Overflowing word text is elided with an ellipsis.
>   WRAP_LONG_WORDS,     // Overflowing word text is wrapped over multiple
lines.
> };

Done.

https://codereview.chromium.org/1015533016/diff/100001/ui/views/controls/labe...
File ui/views/controls/label.cc (right):

https://codereview.chromium.org/1015533016/diff/100001/ui/views/controls/labe...
ui/views/controls/label.cc:159: ResetLayout();
On 2015/03/26 19:36:30, msw wrote:
> nit: perhaps the layout shouldn't be reset (nor should we toggle
> |is_first_paint_text_|) if the label isn't multiline.

Done.

[msw, 2015-03-26 23:05:55]

I think you can simplify the end_char code, please take another look, ping me if
you want more concrete advice.

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_har...
File ui/gfx/render_text_harfbuzz.cc (right):

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_har...
ui/gfx/render_text_harfbuzz.cc:364: *end_char = i;
On 2015/03/26 22:47:38, Jun Mukai wrote:
> On 2015/03/26 19:36:30, msw wrote:
> > Shouldn't end_char be set as soon as the width exceeds the available width,
> and
> > not afterward? Otherwise, doesn't this extend end_char for each overflowing
> > char, so that it'll match next_char? ie. shouldn't the condition above be:
> >         } else if (word_wrap_behavior_ == TRUNCATE_LONG_WORDS && !truncated)
{
> > 
> > Actually, I wonder if you can simplify the code a bit:
> > 1) Remove this "else if" block and |truncated|.
> > 2) After "if (*width > available_width)" add "else { *end_char = i; }".
> > 3) Change 335 and 382 to check "if (word_wrap_behavior_ ==
> IGNORE_LONG_WORDS)".
> 
> You are right, "!truncated" is necessary in the condition. My test case didn't
> catch that. Thanks!
> 
> however I'm afraid the latter approach doesn't work, anyways truncated flag is
> necessary to truncate in middle of a word, we want to record the first
character
> which exceeds the available width (when the behavior is TRUNCATE).

Hmm, if we only set "*end_char = i;" if (*width <= available_width), as I
suggested in #2 above, and when (word_wrap_behavior_ != TRUNCATE_LONG_WORDS) for
lines 335 and 383, as I suggested in #3, wouldn't that correctly capture the
first character that exceeds the available width?

https://codereview.chromium.org/1015533016/diff/120001/ui/gfx/render_text_uni...
File ui/gfx/render_text_unittest.cc (right):

https://codereview.chromium.org/1015533016/diff/120001/ui/gfx/render_text_uni...
ui/gfx/render_text_unittest.cc:2325: "kTestStrings[%" PRIuS "] %d", i,
kTestScenarios[i].behavior));
nit: kTestScenarios

[Jun Mukai, 2015-03-27 00:31:58]

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_har...
File ui/gfx/render_text_harfbuzz.cc (right):

https://codereview.chromium.org/1015533016/diff/100001/ui/gfx/render_text_har...
ui/gfx/render_text_harfbuzz.cc:364: *end_char = i;
On 2015/03/26 23:05:54, msw wrote:
> On 2015/03/26 22:47:38, Jun Mukai wrote:
> > On 2015/03/26 19:36:30, msw wrote:
> > > Shouldn't end_char be set as soon as the width exceeds the available
width,
> > and
> > > not afterward? Otherwise, doesn't this extend end_char for each
overflowing
> > > char, so that it'll match next_char? ie. shouldn't the condition above be:
> > >         } else if (word_wrap_behavior_ == TRUNCATE_LONG_WORDS &&
!truncated)
> {
> > > 
> > > Actually, I wonder if you can simplify the code a bit:
> > > 1) Remove this "else if" block and |truncated|.
> > > 2) After "if (*width > available_width)" add "else { *end_char = i; }".
> > > 3) Change 335 and 382 to check "if (word_wrap_behavior_ ==
> > IGNORE_LONG_WORDS)".
> > 
> > You are right, "!truncated" is necessary in the condition. My test case
didn't
> > catch that. Thanks!
> > 
> > however I'm afraid the latter approach doesn't work, anyways truncated flag
is
> > necessary to truncate in middle of a word, we want to record the first
> character
> > which exceeds the available width (when the behavior is TRUNCATE).
> 
> Hmm, if we only set "*end_char = i;" if (*width <= available_width), as I
> suggested in #2 above, and when (word_wrap_behavior_ != TRUNCATE_LONG_WORDS)
for
> lines 335 and 383, as I suggested in #3, wouldn't that correctly capture the
> first character that exceeds the available width?

Aagh, I see. Sorry for my poor understanding, now I got it.
Fixed (to be more precise, *end_char is not i, it's char_range.end(), otherwise
it's truncated a bit more aggressively).

https://codereview.chromium.org/1015533016/diff/120001/ui/gfx/render_text_uni...
File ui/gfx/render_text_unittest.cc (right):

https://codereview.chromium.org/1015533016/diff/120001/ui/gfx/render_text_uni...
ui/gfx/render_text_unittest.cc:2325: "kTestStrings[%" PRIuS "] %d", i,
kTestScenarios[i].behavior));
On 2015/03/26 23:05:55, msw wrote:
> nit: kTestScenarios

Done.

[msw, 2015-03-27 01:45:27]

lgtm with an optional nit, nice work.

https://codereview.chromium.org/1015533016/diff/140001/ui/gfx/render_text_har...
File ui/gfx/render_text_harfbuzz.cc (right):

https://codereview.chromium.org/1015533016/diff/140001/ui/gfx/render_text_har...
ui/gfx/render_text_harfbuzz.cc:379: *end_char = *next_char;
optional nit: "*end_char = *next_char = run.range.end();" or "*end_char =
run.range.end();"

[Jun Mukai, 2015-03-27 16:36:12]

https://codereview.chromium.org/1015533016/diff/140001/ui/gfx/render_text_har...
File ui/gfx/render_text_harfbuzz.cc (right):

https://codereview.chromium.org/1015533016/diff/140001/ui/gfx/render_text_har...
ui/gfx/render_text_harfbuzz.cc:379: *end_char = *next_char;
On 2015/03/27 01:45:27, msw wrote:
> optional nit: "*end_char = *next_char = run.range.end();" or "*end_char =
> run.range.end();"

Done.

[Jun Mukai, 2015-03-27 16:36:27]

mukai@chromium.org changed reviewers:
+ sky@chromium.org

[Jun Mukai, 2015-03-27 16:36:40]

sky, please review ui/views

[sky, 2015-03-27 20:35:30]

LGTM

[Jun Mukai, 2015-03-27 20:44:28]

The CQ bit was checked by mukai@chromium.org

[Jun Mukai, 2015-03-27 20:44:29]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org
Link to the patchset: https://codereview.chromium.org/1015533016/#ps160001
(title: "comments addressed")

[commit-bot: I haz the power, 2015-03-27 20:45:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1015533016/160001

[commit-bot: I haz the power, 2015-03-27 22:20:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-27 22:20:47]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Jun Mukai, 2015-03-27 22:55:21]

the test failed because the width for truncated lines is longer, it reports
untruncated width. Fixed RTHB a bit.

[Jun Mukai, 2015-03-27 22:55:27]

The CQ bit was checked by mukai@chromium.org

[Jun Mukai, 2015-03-27 22:55:27]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org,
sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1015533016/#ps180001
(title: "line width for truncated words")

[commit-bot: I haz the power, 2015-03-27 22:56:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1015533016/180001

[commit-bot: I haz the power, 2015-03-28 00:07:18]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-03-28 00:08:08]

Patchset 10 (id:??) landed as
https://crrev.com/a19339399b745657a9d9e7f288cfc797f90403f5
Cr-Commit-Position: refs/heads/master@{#322681}

[sullivan, 2015-06-24 21:24:42]

A revert of this CL (patchset #10 id:180001) has been created in
https://codereview.chromium.org/1205153002/ by sullivan@chromium.org.

The reason for reverting is: Regresses memory on Linux by 6MB.

BUG=471608.