Clean ClassList and DOMSettableTokenList
Created attachment 209723 [details] Patch
Comment on attachment 209723 [details] Patch Attachment 209723 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1587206
Comment on attachment 209723 [details] Patch Attachment 209723 [details] did not pass gtk-ews (gtk): Output: http://webkit-queues.appspot.com/results/1582232
Created attachment 209727 [details] Patch
Comment on attachment 209727 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209727&action=review > Source/WebCore/ChangeLog:13 > + -Move the implementation of virtual functions to the cpp file. > + -Clean the #includes. > + -Make the implemented pure virtual methods final. > + -Make the element() accessor const. Maybe put a space after each "-"? > Source/WebCore/html/ClassList.cpp:85 > if (m_element->document()->inQuirksMode()) { I don't get this. It's not like m_element's document transition between quirks mode and strict mode... We should be able to determine this at the creation time (probably not in this patch though). > Source/WebCore/html/DOMSettableTokenList.h:55 > AtomicString m_value; We should get rid of this member variable.
Comment on attachment 209727 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209727&action=review >> Source/WebCore/html/ClassList.cpp:85 >> if (m_element->document()->inQuirksMode()) { > > I don't get this. It's not like m_element's document transition between quirks mode and strict mode... > We should be able to determine this at the creation time (probably not in this patch though). Hm... I suppose you can move the element from a strict mode document to a quirks mode document.
Committed r154707: <http://trac.webkit.org/changeset/154707>
Comment on attachment 209727 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=209727&action=review > Source/WebCore/html/ClassList.h:40 > + virtual void ref() OVERRIDE FINAL; Is the class ClassList not something we can mark FINAL? If we did we could save marking all those functions final. > Source/WebCore/html/DOMSettableTokenList.h:38 > class DOMSettableTokenList : public DOMTokenList, public RefCounted<DOMSettableTokenList> { Can this class be marked FINAL?
(In reply to comment #8) > (From update of attachment 209727 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=209727&action=review > > > Source/WebCore/html/ClassList.h:40 > > + virtual void ref() OVERRIDE FINAL; > > Is the class ClassList not something we can mark FINAL? If we did we could save marking all those functions final. > > > Source/WebCore/html/DOMSettableTokenList.h:38 > > class DOMSettableTokenList : public DOMTokenList, public RefCounted<DOMSettableTokenList> { > > Can this class be marked FINAL? That's what I tried with the previous patch that did not build. The bindings subclasses those. I have more refactoring in progress. The end result should be nicer.