DOMTokenList: Update serialization algorithm on add()/remove()

DOMTokenList: Update serialization algorithm on add()/remove()

We should re-serialize the whole token list even if an operation didn't change
the list items.

* blink::ClassList
 This class doesn't use DOMTokenList::tokens_, and usually uses
 ElementData::class_names_. We can't update it before setAttribute().  So this class
 creates SpaceSplitString for mutation on the fly.

* blink::RelList
 This class doesn't use DOMTokenList::tokens_. MutableSet() should refer to
 rel_values_ instead.

* blink::DOMTokenList
 For other DOMTokenLists, we can mutate tokens_ directly.

BUG=600964
Review-Url: https://codereview.chromium.org/2895903002
Cr-Commit-Position: refs/heads/master@{#474229}
Committed: https://chromium.googlesource.com/chromium/src/+/1806913f25daa9da5ca69a0115fe74addaa35d32

[tkent, 2017-05-22 08:55:16]

The CQ bit was checked by tkent@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-22 08:55:37]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tkent, 2017-05-22 08:58:26]

Description was changed from

==========
DOMTokenList: Update serialization algorithm after add()/remove()

We should re-serialize the whole token list even if an operation didn't change
the list items.

BUG=600964
==========

to

==========
DOMTokenList: Update serialization algorithm after add()/remove()

We should re-serialize the whole token list even if an operation didn't change
the list items.

* blink::ClassList
 This class doesn't use DOMTokenList::tokens_, and usually uses
 ElementData::class_names_. We can't update it before setAttribute().  So this
class
 creates SpaceSplitString for mutation on the fly.

* blink::RelList
 This class doesn't use DOMTokenList::tokens_. 

BUG=600964
==========

[tkent, 2017-05-22 09:00:51]

Description was changed from

==========
DOMTokenList: Update serialization algorithm after add()/remove()

We should re-serialize the whole token list even if an operation didn't change
the list items.

* blink::ClassList
 This class doesn't use DOMTokenList::tokens_, and usually uses
 ElementData::class_names_. We can't update it before setAttribute().  So this
class
 creates SpaceSplitString for mutation on the fly.

* blink::RelList
 This class doesn't use DOMTokenList::tokens_. 

BUG=600964
==========

to

==========
DOMTokenList: Update serialization algorithm after add()/remove()

We should re-serialize the whole token list even if an operation didn't change
the list items.

* blink::ClassList
 This class doesn't use DOMTokenList::tokens_, and usually uses
 ElementData::class_names_. We can't update it before setAttribute().  So this
class
 creates SpaceSplitString for mutation on the fly.

* blink::RelList
 This class doesn't use DOMTokenList::tokens_. MutableSet() should refer to
 rel_values_ instead.

* blink::DOMTokenList
 For other DOMTokenLists, we can mutate tokens_ directly.

BUG=600964
==========

[tkent, 2017-05-22 09:02:24]

Description was changed from

==========
DOMTokenList: Update serialization algorithm after add()/remove()

We should re-serialize the whole token list even if an operation didn't change
the list items.

* blink::ClassList
 This class doesn't use DOMTokenList::tokens_, and usually uses
 ElementData::class_names_. We can't update it before setAttribute().  So this
class
 creates SpaceSplitString for mutation on the fly.

* blink::RelList
 This class doesn't use DOMTokenList::tokens_. MutableSet() should refer to
 rel_values_ instead.

* blink::DOMTokenList
 For other DOMTokenLists, we can mutate tokens_ directly.

BUG=600964
==========

to

==========
DOMTokenList: Update serialization algorithm on add()/remove()

We should re-serialize the whole token list even if an operation didn't change
the list items.

* blink::ClassList
 This class doesn't use DOMTokenList::tokens_, and usually uses
 ElementData::class_names_. We can't update it before setAttribute().  So this
class
 creates SpaceSplitString for mutation on the fly.

* blink::RelList
 This class doesn't use DOMTokenList::tokens_. MutableSet() should refer to
 rel_values_ instead.

* blink::DOMTokenList
 For other DOMTokenLists, we can mutate tokens_ directly.

BUG=600964
==========

[tkent, 2017-05-22 09:05:24]

tkent@chromium.org changed reviewers:
+ kochi@chromium.org

[tkent, 2017-05-22 09:05:25]

kochi@, would you review this please?

[commit-bot: I haz the power, 2017-05-22 10:57:24]

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

[commit-bot: I haz the power, 2017-05-22 10:57:25]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[tkent, 2017-05-22 23:36:35]

The CQ bit was checked by tkent@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-22 23:37:31]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-23 01:44:25]

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

[commit-bot: I haz the power, 2017-05-23 01:44:26]

Dry run: This issue passed the CQ dry run.

[kochi, 2017-05-24 05:46:01]

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

https://codereview.chromium.org/2895903002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/DOMTokenList.cpp:92: //  
InvalidCharacterError.
This comment is identical to line 111-, and the algorithm
seems to correspond to ValidateTokens().
Please move to ValidateTokens().

https://codereview.chromium.org/2895903002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/DOMTokenList.cpp:118: //
https://github.com/whatwg/dom/issues/462
This comment is ambiguous - how about being more specific?
e.g.
// TODO(tkent): This is not conforming to the spec.
// See https://github.com/whatwg/dom/issues/462

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

https://codereview.chromium.org/2895903002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/DOMTokenList.h:76: virtual SpaceSplitString&
MutableSet() { return tokens_; }
How about moving this to protected: section?

[tkent, 2017-05-24 06:05:34]

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

https://codereview.chromium.org/2895903002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/DOMTokenList.cpp:92: //  
InvalidCharacterError.
On 2017/05/24 at 05:46:00, kochi wrote:
> This comment is identical to line 111-, and the algorithm
> seems to correspond to ValidateTokens().
> Please move to ValidateTokens().

Done.

https://codereview.chromium.org/2895903002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/DOMTokenList.cpp:118: //
https://github.com/whatwg/dom/issues/462
On 2017/05/24 at 05:46:00, kochi wrote:
> This comment is ambiguous - how about being more specific?
> e.g.
> // TODO(tkent): This is not conforming to the spec.
> // See https://github.com/whatwg/dom/issues/462

Done.

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

https://codereview.chromium.org/2895903002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/DOMTokenList.h:76: virtual SpaceSplitString&
MutableSet() { return tokens_; }
On 2017/05/24 at 05:46:00, kochi wrote:
> How about moving this to protected: section?

It's reasonable.  Done.

[tkent, 2017-05-24 06:06:49]

The CQ bit was checked by tkent@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-24 06:07:25]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kochi, 2017-05-24 06:23:46]

lgtm

[tkent, 2017-05-24 08:13:03]

The CQ bit was unchecked by tkent@chromium.org

[tkent, 2017-05-24 08:13:06]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2017-05-24 08:13:16]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-24 09:47:44]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495613586205630,
"parent_rev": "dd5069631f086ea22e7c43d9a1f85b0a83fc87df", "commit_rev":
"a3dcc160f3d7f7cbb3f5a594f72321778b7ef62b"}

[commit-bot: I haz the power, 2017-05-24 09:48:03]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495613586205630,
"parent_rev": "cc3ecbd519b29c785382cfa0f3fcfe27a483aa01", "commit_rev":
"1806913f25daa9da5ca69a0115fe74addaa35d32"}

[commit-bot: I haz the power, 2017-05-24 09:48:17]

Description was changed from

==========
DOMTokenList: Update serialization algorithm on add()/remove()

We should re-serialize the whole token list even if an operation didn't change
the list items.

* blink::ClassList
 This class doesn't use DOMTokenList::tokens_, and usually uses
 ElementData::class_names_. We can't update it before setAttribute().  So this
class
 creates SpaceSplitString for mutation on the fly.

* blink::RelList
 This class doesn't use DOMTokenList::tokens_. MutableSet() should refer to
 rel_values_ instead.

* blink::DOMTokenList
 For other DOMTokenLists, we can mutate tokens_ directly.

BUG=600964
==========

to

==========
DOMTokenList: Update serialization algorithm on add()/remove()

We should re-serialize the whole token list even if an operation didn't change
the list items.

* blink::ClassList
 This class doesn't use DOMTokenList::tokens_, and usually uses
 ElementData::class_names_. We can't update it before setAttribute().  So this
class
 creates SpaceSplitString for mutation on the fly.

* blink::RelList
 This class doesn't use DOMTokenList::tokens_. MutableSet() should refer to
 rel_values_ instead.

* blink::DOMTokenList
 For other DOMTokenLists, we can mutate tokens_ directly.

BUG=600964

Review-Url: https://codereview.chromium.org/2895903002
Cr-Commit-Position: refs/heads/master@{#474229}
Committed:
https://chromium.googlesource.com/chromium/src/+/1806913f25daa9da5ca69a0115fe...
==========

[commit-bot: I haz the power, 2017-05-24 09:48:18]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/1806913f25daa9da5ca69a0115fe...