Introduce ForwardsTextBuffer and BackwardsTextBuffer

Introduce ForwardsTextBuffer and BackwardsTextBuffer

The two new classes wrap |WTF::Vector| such that, |ForwardsTextBuffer|
allows efficients appending, while |BackwardsTextBuffer| allows efficient
prepending, respectively.

This is part of a bigger CL [1], which is in turn part of another even
bigger CL [2].

[1] https://codereview.chromium.org/1647483003
[2] https://codereview.chromium.org/1604783002

BUG=n/a
TEST=webkit_unit_tests --gtest_filter=*TextBufferTest.*

Committed: https://crrev.com/4fcb037b7549a5e1ae7a75885978696e2ead225d
Cr-Commit-Position: refs/heads/master@{#372918}

[Xiaocheng, 2016-01-29 10:19:36]

xiaochengh@chromium.org changed reviewers:
+ tkent@chromium.org, yosin@chromium.org

[Xiaocheng, 2016-01-29 10:19:36]

PTAL.

[yosin_UTC9, 2016-02-01 03:40:55]

Class implementations are good.
Please make tests simpler and intuitive.

https://codereview.chromium.org/1650543002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/iterators/ForwardsTextBufferTest.cpp
(right):

https://codereview.chromium.org/1650543002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/iterators/ForwardsTextBufferTest.cpp:16:
const unsigned numPush = 16;
This test seems to be complex.
Can we make simpler test?

For example:

ForwardsTextBuffer buffer;
buffer.pushCahracters("abc", 3);
buffer.pushCharacters("d", 1);
buffer.pushCharacters(nullptr, 0);
buffer.pushCharacters("\0", 1);

EXPECT_FALSE(buffer.isEmpty());
EXPECT_EQ(5, buffer.size());
EXPECT_EQ("abcd", buffer.data());
EXPECT_EQ('a', buffer[0]);
EXPECT_EQ('d', buffer[3]);
EXPECT_EQ('\0', buffer[4]);

[Xiaocheng, 2016-02-01 04:36:24]

Thanks for the review.

https://codereview.chromium.org/1650543002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/iterators/ForwardsTextBufferTest.cpp
(right):

https://codereview.chromium.org/1650543002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/iterators/ForwardsTextBufferTest.cpp:16:
const unsigned numPush = 16;
On 2016/02/01 at 03:40:55, Yosi_UTC9 wrote:
> This test seems to be complex.
> Can we make simpler test?
> 
> For example:
> 
> ForwardsTextBuffer buffer;
> buffer.pushCahracters("abc", 3);
> buffer.pushCharacters("d", 1);
> buffer.pushCharacters(nullptr, 0);
> buffer.pushCharacters("\0", 1);
> 
> EXPECT_FALSE(buffer.isEmpty());
> EXPECT_EQ(5, buffer.size());
> EXPECT_EQ("abcd", buffer.data());
> EXPECT_EQ('a', buffer[0]);
> EXPECT_EQ('d', buffer[3]);
> EXPECT_EQ('\0', buffer[4]);

I'll see if the tests can be simplified. Maybe I should also add some test for
edge cases as you implied.

However, I may still need some test cases with larger amount of data, because we
need to ensure that the correctness under buffer reallocation is tested.

[yosin_UTC9, 2016-02-01 04:46:15]

On 2016/02/01 at 04:36:24, xiaochengh wrote:
> Thanks for the review.
> 
>
https://codereview.chromium.org/1650543002/diff/1/third_party/WebKit/Source/c...
> File
third_party/WebKit/Source/core/editing/iterators/ForwardsTextBufferTest.cpp
(right):
> 
>
https://codereview.chromium.org/1650543002/diff/1/third_party/WebKit/Source/c...
>
third_party/WebKit/Source/core/editing/iterators/ForwardsTextBufferTest.cpp:16:
const unsigned numPush = 16;
> On 2016/02/01 at 03:40:55, Yosi_UTC9 wrote:
> > This test seems to be complex.
> > Can we make simpler test?
> > 
> > For example:
> > 
> > ForwardsTextBuffer buffer;
> > buffer.pushCahracters("abc", 3);
> > buffer.pushCharacters("d", 1);
> > buffer.pushCharacters(nullptr, 0);
> > buffer.pushCharacters("\0", 1);
> > 
> > EXPECT_FALSE(buffer.isEmpty());
> > EXPECT_EQ(5, buffer.size());
> > EXPECT_EQ("abcd", buffer.data());
> > EXPECT_EQ('a', buffer[0]);
> > EXPECT_EQ('d', buffer[3]);
> > EXPECT_EQ('\0', buffer[4]);
> 
> I'll see if the tests can be simplified. Maybe I should also add some test for
edge cases as you implied.
> 
> However, I may still need some test cases with larger amount of data, because
we need to ensure that the correctness under buffer reallocation is tested.

I think you want to test |grow()| and |shiftData()|.

Vector<UChar, 1024 * 8> largeChunk;
for (size_t index = 0; index < largeChunk; ++index)
  largeChunk[index] = index % 256;
buffer.pushCharacters("a", 1);
buffer.pushCharacters(largeChunk, largeChunk.size());
EXPECT_EQ(1023 % 256, buffer[1024]);
...

[Xiaocheng, 2016-02-02 06:26:33]

The test cases are simplified. PTAL.

[yosin_UTC9, 2016-02-02 07:04:51]

lgtm

Thanks for simplification

+tkent@ for core/core.gypi changes.

[tkent, 2016-02-02 07:11:06]

lgtm

[Xiaocheng, 2016-02-02 07:13:34]

The CQ bit was checked by xiaochengh@chromium.org

[commit-bot: I haz the power, 2016-02-02 07:13:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1650543002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1650543002/20001

[commit-bot: I haz the power, 2016-02-02 08:12:25]

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

[commit-bot: I haz the power, 2016-02-02 08:12:26]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Xiaocheng, 2016-02-02 09:42:34]

The CQ bit was checked by xiaochengh@chromium.org

[Xiaocheng, 2016-02-02 09:42:34]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org,
yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/1650543002/#ps40001
(title: "Stop the win compiler's complaint")

[commit-bot: I haz the power, 2016-02-02 09:42:44]

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

[commit-bot: I haz the power, 2016-02-02 09:49:26]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-02 09:57:00]

Description was changed from

==========
Introduce ForwardsTextBuffer and BackwardsTextBuffer

The two new classes wrap |WTF::Vector| such that, |ForwardsTextBuffer|
allows efficients appending, while |BackwardsTextBuffer| allows efficient
prepending, respectively.

This is part of a bigger CL [1], which is in turn part of another even
bigger CL [2].

[1] https://codereview.chromium.org/1647483003
[2] https://codereview.chromium.org/1604783002

BUG=n/a
TEST=webkit_unit_tests --gtest_filter=*TextBufferTest.*
==========

to

==========
Introduce ForwardsTextBuffer and BackwardsTextBuffer

The two new classes wrap |WTF::Vector| such that, |ForwardsTextBuffer|
allows efficients appending, while |BackwardsTextBuffer| allows efficient
prepending, respectively.

This is part of a bigger CL [1], which is in turn part of another even
bigger CL [2].

[1] https://codereview.chromium.org/1647483003
[2] https://codereview.chromium.org/1604783002

BUG=n/a
TEST=webkit_unit_tests --gtest_filter=*TextBufferTest.*

Committed: https://crrev.com/4fcb037b7549a5e1ae7a75885978696e2ead225d
Cr-Commit-Position: refs/heads/master@{#372918}
==========

[commit-bot: I haz the power, 2016-02-02 09:57:01]

Patchset 3 (id:??) landed as
https://crrev.com/4fcb037b7549a5e1ae7a75885978696e2ead225d
Cr-Commit-Position: refs/heads/master@{#372918}