Rewrite Text node attaching to not be N^2

Rewrite Text node attaching to not be N^2

Previously Text node creation used String::append which was N^2.
Now instead we use StringBuilder, batch up all the text from
the network and then split it into Text nodes if necessary
all at once.

This should be a perf win for pages with large text nodes
and slow networks.

BUG=301981
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=161488

[eseidel, 2013-10-10 23:48:58]

PTAL

[esprehn, 2013-10-11 00:02:56]

https://codereview.chromium.org/26129005/diff/11001/Source/core/html/parser/H...
File Source/core/html/parser/HTMLConstructionSite.cpp (right):

https://codereview.chromium.org/26129005/diff/11001/Source/core/html/parser/H...
Source/core/html/parser/HTMLConstructionSite.cpp:342: // document and that we'll
always queue some additional task text to cause it to flush.
text task?

"task text" is weird.

https://codereview.chromium.org/26129005/diff/11001/Source/core/html/parser/H...
Source/core/html/parser/HTMLConstructionSite.cpp:352:
m_pendingText.swap(discardedText);
Adding a discard() method would be prettier.

[abarth-chromium, 2013-10-11 18:10:07]

Are you still looking for a review here, or are you splitting this up into
smaller pieces?

[eseidel, 2013-10-11 18:11:02]

This is the last of the pieces.  I believe it's ready to land.

[eseidel, 2013-10-11 18:11:26]

So yes, please review.

[abarth-chromium, 2013-10-11 18:54:04]

LGTM!

Very nice.  A few minor comments below.

https://codereview.chromium.org/26129005/diff/11001/Source/core/html/parser/H...
File Source/core/html/parser/HTMLConstructionSite.cpp (right):

https://codereview.chromium.org/26129005/diff/11001/Source/core/html/parser/H...
Source/core/html/parser/HTMLConstructionSite.cpp:342: // document and that we'll
always queue some additional task text to cause it to flush.
"task text" -> task

https://codereview.chromium.org/26129005/diff/11001/Source/core/html/parser/H...
Source/core/html/parser/HTMLConstructionSite.cpp:352:
m_pendingText.swap(discardedText);
On 2013/10/11 00:02:56, esprehn wrote:
> Adding a discard() method would be prettier.

Agreed.  It can use swap internally if that's helpful

https://codereview.chromium.org/26129005/diff/11001/Source/core/html/parser/H...
File Source/core/html/parser/HTMLDocumentParser.cpp (right):

https://codereview.chromium.org/26129005/diff/11001/Source/core/html/parser/H...
Source/core/html/parser/HTMLDocumentParser.cpp:561: m_treeBuilder->flush();
Don't you need to check 

if (isStopped())
    return;

here?  It's possible that flushing the tree builder executed JavaScript which
detached the parser.  We don't want to do anything else if that's the case.

https://codereview.chromium.org/26129005/diff/11001/Source/core/html/parser/H...
File Source/core/html/parser/HTMLTreeBuilder.cpp (right):

https://codereview.chromium.org/26129005/diff/11001/Source/core/html/parser/H...
Source/core/html/parser/HTMLTreeBuilder.cpp:405: m_tree.flush();
I wonder if we need a faster reject here in the case where there's nothing to
flush.  If you have perf-bot trouble, I'd keep that in mind.

[eseidel, 2013-10-16 18:30:56]

Turns out I have a bug.

http://pastebin.com/S3rPcgRa

it's attempting to execute document.write( which seems wrong.

[commit-bot: I haz the power, 2013-11-07 01:14:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/eseidel@chromium.org/26129005/27001

[commit-bot: I haz the power, 2013-11-07 05:59:07]

Change committed as 161488