Render string sequences of more than 64k characters.

Render string sequences of more than 64k characters.

InlineBox can only handle sequences of text that fits in an
unsigned short and while that is normally not a problem,
it can be a problem for someone trying to upload a long
chunk of data in an <input> field. <input> uses normal
text layout to render the text so it gets the same limitations
as document text.

The symptoms of this overflow is typically missing text.

This fixes the problem by splitting BidiRun objects that would
have referred to too-long sequences of characters into multiple
BidiRun objects.

BUG=155241, 448424, 448425
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=188548

[Daniel Bratell, 2015-01-15 16:48:26]

bratell@opera.com changed reviewers:
+ eae@chromium.org

[Daniel Bratell, 2015-01-15 16:48:26]

eae, can you take a look at this? The main motivation is to fix the forms bug,
and that the other bugs were fixed as well were more of a lucky coincidence.
I've tried with some Hebrew and bidi and it seems to work.

[eae, 2015-01-15 20:07:07]

https://codereview.chromium.org/853903002/diff/20001/Source/core/rendering/In...
File Source/core/rendering/InlineIterator.h (right):

https://codereview.chromium.org/853903002/diff/20001/Source/core/rendering/In...
Source/core/rendering/InlineIterator.h:627: bool addEmptyRun = (end == start);
The general approach seems fine, I have a hard time wrapping my head around the
concept of addEmptyRun though. Could you either try to reorganize the code to
not need it or add a comment explaining what it is used for.

https://codereview.chromium.org/853903002/diff/20001/Source/platform/text/Bid...
File Source/platform/text/BidiResolver.h (right):

https://codereview.chromium.org/853903002/diff/20001/Source/platform/text/Bid...
Source/platform/text/BidiResolver.h:339: endOffset += 1;
I know you just moved this but a comment explaining why we need to increase the
offset would be useful.

[Daniel Bratell, 2015-01-16 13:47:32]

Added some documentation. Please take a new look!

https://codereview.chromium.org/853903002/diff/20001/Source/core/rendering/In...
File Source/core/rendering/InlineIterator.h (right):

https://codereview.chromium.org/853903002/diff/20001/Source/core/rendering/In...
Source/core/rendering/InlineIterator.h:627: bool addEmptyRun = (end == start);
On 2015/01/15 20:07:07, eae wrote:
> The general approach seems fine, I have a hard time wrapping my head around
the
> concept of addEmptyRun though. Could you either try to reorganize the code to
> not need it or add a comment explaining what it is used for.

There is code that calls this with start == end and expects a BidiRun to be
created. If I just had the loop there would be no BidiRun anymore for that case.
Those that do that create an empty BidiRun and then modify the positions. I
think it was used for trailing whitespace.

I will add some kind of comment.

https://codereview.chromium.org/853903002/diff/20001/Source/platform/text/Bid...
File Source/platform/text/BidiResolver.h (right):

https://codereview.chromium.org/853903002/diff/20001/Source/platform/text/Bid...
Source/platform/text/BidiResolver.h:339: endOffset += 1;
On 2015/01/15 20:07:07, eae wrote:
> I know you just moved this but a comment explaining why we need to increase
the
> offset would be useful.

Done.

[eae, 2015-01-16 16:08:32]

LGTM

[eae, 2015-01-16 16:12:24]

The CQ bit was checked by eae@chromium.org

[commit-bot: I haz the power, 2015-01-16 16:13:31]

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

[commit-bot: I haz the power, 2015-01-16 21:59:57]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=188548