Handle extra whitespaces in the DOM token list

We don't need to preserve extra whitespaces on removing tokens from DOMTokenList.

BUG=526282
Committed: https://crrev.com/7598887af4bd5ad4f6cb513a943d4e780ff9799a
Cr-Commit-Position: refs/heads/master@{#356481}

[tanay.c, 2015-10-20 06:11:57]

tanay.c@samsung.com changed reviewers:
+ jianli@chromium.org, tkent@chromium.org

[tanay.c, 2015-10-20 06:11:57]

Please have a look.

[tanay.c, 2015-10-25 12:31:45]

PTAL

[tkent, 2015-10-26 03:21:02]

We should re-write DOMTokenList::removeTokens() entirely so that it just
serializes a SpaceSplitString.  But the current approach would be acceptable.

https://codereview.chromium.org/1411123004/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/dom/Element/classlist-remove-token.html
(right):

https://codereview.chromium.org/1411123004/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/dom/Element/classlist-remove-token.html:5:
<script>
Please add |description(what-this-tests)|.

https://codereview.chromium.org/1411123004/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/dom/Element/classlist-remove-token.html:9:
shouldBe("div1.className", "'b c'");
Use shouldBeEqualToString().

https://codereview.chromium.org/1411123004/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/dom/Element/classlist-remove-token.html:14:
shouldBe("div2.className", "'a c'");
Use shouldBeEqualToString()

https://codereview.chromium.org/1411123004/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/dom/Element/classlist-remove-token.html:19:
shouldBe("div3.className", "'a b'");
Use shouldBeEqualToString()

https://codereview.chromium.org/1411123004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/DOMTokenList.cpp (right):

https://codereview.chromium.org/1411123004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/DOMTokenList.cpp:275: while (j > 0 &&
isHTMLSpace<UChar>(output[j - 1]))
I think the |while| loop is unnecessary.  We need to remove at most one space at
the last, right?

[tanay.c, 2015-10-26 09:38:51]

Thanks for the comments, included the changes. PTAL

https://codereview.chromium.org/1411123004/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/dom/Element/classlist-remove-token.html
(right):

https://codereview.chromium.org/1411123004/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/dom/Element/classlist-remove-token.html:5:
<script>
On 2015/10/26 03:21:02, tkent wrote:
> Please add |description(what-this-tests)|.

Done.

https://codereview.chromium.org/1411123004/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/dom/Element/classlist-remove-token.html:9:
shouldBe("div1.className", "'b c'");
On 2015/10/26 03:21:02, tkent wrote:
> Use shouldBeEqualToString().

Done.

https://codereview.chromium.org/1411123004/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/dom/Element/classlist-remove-token.html:14:
shouldBe("div2.className", "'a c'");
On 2015/10/26 03:21:02, tkent wrote:
> Use shouldBeEqualToString()

Done.

https://codereview.chromium.org/1411123004/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/dom/Element/classlist-remove-token.html:19:
shouldBe("div3.className", "'a b'");
On 2015/10/26 03:21:02, tkent wrote:
> Use shouldBeEqualToString()

Done.

https://codereview.chromium.org/1411123004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/DOMTokenList.cpp (right):

https://codereview.chromium.org/1411123004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/DOMTokenList.cpp:275: while (j > 0 &&
isHTMLSpace<UChar>(output[j - 1]))
On 2015/10/26 03:21:02, tkent wrote:
> I think the |while| loop is unnecessary.  We need to remove at most one space
at
> the last, right?

Yes, using an if condition instead

[tkent, 2015-10-26 23:30:39]

https://codereview.chromium.org/1411123004/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/dom/Element/classlist-remove-token.html
(right):

https://codereview.chromium.org/1411123004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/dom/Element/classlist-remove-token.html:1:
<!DOCTYPE html>
I realized fast/dom/HTMLElement/script-tests/class-list.js covered
classList.remove().  So, you don't need to add new test, and add new test cases
to class-list.js if you think its coverage is not enough.

https://codereview.chromium.org/1411123004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/DOMTokenList.cpp (right):

https://codereview.chromium.org/1411123004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/DOMTokenList.cpp:275: if
(isHTMLSpace<UChar>(output[j - 1]))
Buffer underflow.  |j| can be 0.

[tanay.c, 2015-10-27 10:44:39]

Added the changes for comments. PTAL.

https://codereview.chromium.org/1411123004/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/dom/Element/classlist-remove-token.html
(right):

https://codereview.chromium.org/1411123004/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/dom/Element/classlist-remove-token.html:1:
<!DOCTYPE html>
On 2015/10/26 23:30:38, tkent wrote:
> I realized fast/dom/HTMLElement/script-tests/class-list.js covered
> classList.remove().  So, you don't need to add new test, and add new test
cases
> to class-list.js if you think its coverage is not enough.

Done. These tests are removed. Added them to class-list.js

https://codereview.chromium.org/1411123004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/DOMTokenList.cpp (right):

https://codereview.chromium.org/1411123004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/DOMTokenList.cpp:275: if
(isHTMLSpace<UChar>(output[j - 1]))
On 2015/10/26 23:30:38, tkent wrote:
> Buffer underflow.  |j| can be 0.

Done.

[tkent, 2015-10-27 22:31:26]

Description was changed from

==========
Handle extra whitespaces in the DOM token list

BUG=526282
==========

to

==========
We don't need to preserve extra whitespaces on removing tokens from
DOMTokenList.

BUG=526282
==========

[tkent, 2015-10-27 22:31:46]

The CQ bit was checked by tkent@chromium.org

[tkent, 2015-10-27 22:31:46]

lgtm

[commit-bot: I haz the power, 2015-10-27 22:35:33]

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

[commit-bot: I haz the power, 2015-10-28 01:49:12]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-28 01:50:02]

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