Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(535)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1629403003: Merge DOMSettableTokensList into DOMTokensList (Closed)

Created:
4 years, 11 months ago by Yoav Weiss
Modified:
4 years, 10 months ago
CC:
blink-reviews, blink-reviews-dom_chromium.org, blink-reviews-html_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, gavinp+prerender_chromium.org, rwlbuis, sof
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Merge DOMSettableTokensList into DOMTokensList This CL tackles the merging of the two token lists, in order to simplify both implementation and spec. I2InS: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/sXT-rWAd0kk BUG=584612 Committed: https://crrev.com/2ca6e94455f692f3cdf09ecf15a306ab6159773a Cr-Commit-Position: refs/heads/master@{#375574}

Patch Set 1 #

Patch Set 2 : Removed ref and deref #

Total comments: 1

Patch Set 3 : Fixed oilpan #

Patch Set 4 : rebase #

Patch Set 5 : Fixed layout tests #

Patch Set 6 : moar fixed tests #

Total comments: 6

Patch Set 7 : Removed PutForwards and RefCounted base class #

Patch Set 8 : Fixed test expectations #

Total comments: 2

Patch Set 9 : removed non-oilpan inheritance #

Unified diffs Side-by-side diffs Delta from patch set Stats (+127 lines, -655 lines) Patch
D third_party/WebKit/LayoutTests/fast/dom/HTMLOutputElement/dom-settable-token-list.html View 1 2 3 4 1 chunk +0 lines, -9 lines 0 comments Download
D third_party/WebKit/LayoutTests/fast/dom/HTMLOutputElement/dom-settable-token-list-expected.txt View 1 2 3 4 1 chunk +0 lines, -78 lines 0 comments Download
A + third_party/WebKit/LayoutTests/fast/dom/HTMLOutputElement/dom-token-list.html View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
A + third_party/WebKit/LayoutTests/fast/dom/HTMLOutputElement/dom-token-list-expected.txt View 1 2 3 4 1 chunk +4 lines, -4 lines 0 comments Download
D third_party/WebKit/LayoutTests/fast/dom/HTMLOutputElement/script-tests/dom-settable-token-list.js View 1 2 3 4 1 chunk +0 lines, -255 lines 0 comments Download
A + third_party/WebKit/LayoutTests/fast/dom/HTMLOutputElement/script-tests/dom-token-list.js View 1 2 3 4 2 chunks +6 lines, -6 lines 0 comments Download
M third_party/WebKit/LayoutTests/imported/web-platform-tests/dom/historical-expected.txt View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/imported/web-platform-tests/dom/interfaces-expected.txt View 1 2 3 4 5 6 7 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/inspector/console/console-format-collections-expected.txt View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt View 1 2 3 4 6 7 3 chunks +2 lines, -4 lines 0 comments Download
M third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt View 1 2 3 4 6 7 3 chunks +2 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/core.gypi View 1 2 3 4 2 chunks +0 lines, -3 lines 0 comments Download
D third_party/WebKit/Source/core/dom/DOMSettableTokenList.h View 1 1 chunk +0 lines, -91 lines 0 comments Download
D third_party/WebKit/Source/core/dom/DOMSettableTokenList.cpp View 1 1 chunk +0 lines, -68 lines 0 comments Download
D third_party/WebKit/Source/core/dom/DOMSettableTokenList.idl View 1 1 chunk +0 lines, -32 lines 0 comments Download
M third_party/WebKit/Source/core/dom/DOMTokenList.h View 1 2 3 4 5 6 7 8 4 chunks +36 lines, -14 lines 0 comments Download
M third_party/WebKit/Source/core/dom/DOMTokenList.cpp View 1 chunk +20 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/DOMTokenList.idl View 1 chunk +5 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/ElementRareData.h View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/dom/Node.h View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/html/ClassList.h View 1 2 chunks +2 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/core/html/ClassList.cpp View 1 1 chunk +1 line, -13 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLAnchorElement.idl View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/html/HTMLAreaElement.idl View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/html/HTMLIFrameElement.h View 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLIFrameElement.cpp View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLIFrameElement.idl View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/html/HTMLIFrameElementSandbox.h View 1 2 3 2 chunks +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLIFrameElementSandbox.cpp View 1 2 3 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLLinkElement.h View 5 chunks +6 lines, -6 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLLinkElement.cpp View 1 2 3 4 4 chunks +4 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLLinkElement.idl View 5 6 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/html/HTMLLinkElementSizesAttributeTest.cpp View 3 chunks +3 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLOutputElement.h View 3 chunks +4 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLOutputElement.cpp View 1 2 3 3 chunks +3 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLOutputElement.idl View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/html/HTMLOutputElementTest.cpp View 1 2 3 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/html/HTMLTableCellElement.idl View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/html/RelList.h View 1 1 chunk +2 lines, -8 lines 0 comments Download
M third_party/WebKit/Source/core/html/RelList.cpp View 1 2 3 1 chunk +1 line, -13 lines 0 comments Download

Messages

Total messages: 27 (10 generated)
Yoav Weiss
4 years, 11 months ago (2016-01-26 10:23:39 UTC) #4
Yoav Weiss
Failing tests are related to the API changes caused by this CL. Leaving them as ...
4 years, 11 months ago (2016-01-26 13:15:45 UTC) #5
Yoav Weiss
On 2016/01/26 13:15:45, Yoav Weiss wrote: > Failing tests are related to the API changes ...
4 years, 10 months ago (2016-02-01 14:35:49 UTC) #6
philipj_slow
Since it'll remove the DOMSettableTokenList interface, an "Intent to Implement and Ship" sounds good, with ...
4 years, 10 months ago (2016-02-02 06:20:59 UTC) #7
Yoav Weiss
On 2016/02/02 06:20:59, philipj_OOO_til_Feb15 wrote: > Since it'll remove the DOMSettableTokenList interface, an "Intent to ...
4 years, 10 months ago (2016-02-12 07:56:54 UTC) #9
Yoav Weiss
On 2016/02/12 07:56:54, Yoav Weiss wrote: > On 2016/02/02 06:20:59, philipj_OOO_til_Feb15 wrote: > > Since ...
4 years, 10 months ago (2016-02-15 09:55:25 UTC) #10
philipj_slow
Won't have time to review this today, may get around to it tomorrow if it's ...
4 years, 10 months ago (2016-02-15 10:23:20 UTC) #11
Yoav Weiss
tkent and/or dominicc, could you take a look?
4 years, 10 months ago (2016-02-15 10:25:44 UTC) #13
tkent
https://codereview.chromium.org/1629403003/diff/100001/third_party/WebKit/Source/core/dom/DOMTokenList.h File third_party/WebKit/Source/core/dom/DOMTokenList.h (right): https://codereview.chromium.org/1629403003/diff/100001/third_party/WebKit/Source/core/dom/DOMTokenList.h#newcode52 third_party/WebKit/Source/core/dom/DOMTokenList.h:52: , public RefCounted<DOMTokenList> The base class should be RefCountedWillBeGarbageCollectedFinalized<DOMTokenList>? ...
4 years, 10 months ago (2016-02-16 03:45:11 UTC) #14
Yoav Weiss
Thanks for reviewing! :) Changed according to comments. PTAL? https://codereview.chromium.org/1629403003/diff/100001/third_party/WebKit/Source/core/dom/DOMTokenList.h File third_party/WebKit/Source/core/dom/DOMTokenList.h (right): https://codereview.chromium.org/1629403003/diff/100001/third_party/WebKit/Source/core/dom/DOMTokenList.h#newcode52 third_party/WebKit/Source/core/dom/DOMTokenList.h:52: ...
4 years, 10 months ago (2016-02-16 09:12:01 UTC) #15
Yoav Weiss
All green now :)
4 years, 10 months ago (2016-02-16 12:36:20 UTC) #16
tkent
lgtm https://codereview.chromium.org/1629403003/diff/140001/third_party/WebKit/Source/core/dom/DOMTokenList.h File third_party/WebKit/Source/core/dom/DOMTokenList.h (right): https://codereview.chromium.org/1629403003/diff/140001/third_party/WebKit/Source/core/dom/DOMTokenList.h#newcode52 third_party/WebKit/Source/core/dom/DOMTokenList.h:52: , public RefCounted<DOMTokenList> This should be removed.
4 years, 10 months ago (2016-02-16 13:21:57 UTC) #17
Yoav Weiss
https://codereview.chromium.org/1629403003/diff/140001/third_party/WebKit/Source/core/dom/DOMTokenList.h File third_party/WebKit/Source/core/dom/DOMTokenList.h (right): https://codereview.chromium.org/1629403003/diff/140001/third_party/WebKit/Source/core/dom/DOMTokenList.h#newcode52 third_party/WebKit/Source/core/dom/DOMTokenList.h:52: , public RefCounted<DOMTokenList> On 2016/02/16 13:21:57, tkent wrote: > ...
4 years, 10 months ago (2016-02-16 13:39:44 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1629403003/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1629403003/160001
4 years, 10 months ago (2016-02-16 13:47:04 UTC) #21
commit-bot: I haz the power
Committed patchset #9 (id:160001)
4 years, 10 months ago (2016-02-16 15:00:21 UTC) #23
commit-bot: I haz the power
Patchset 9 (id:??) landed as https://crrev.com/2ca6e94455f692f3cdf09ecf15a306ab6159773a Cr-Commit-Position: refs/heads/master@{#375574}
4 years, 10 months ago (2016-02-16 22:52:47 UTC) #25
sof
4 years, 10 months ago (2016-02-18 11:49:31 UTC) #27
Message was sent while issue was closed. 
https://codereview.chromium.org/1629403003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/ClassList.h (left):

https://codereview.chromium.org/1629403003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/ClassList.h:48: #if !ENABLE(OILPAN)
This looks odd - why can you let go of the forwarding refcounting here?

Powered by Google App Engine
This is Rietveld 408576698