HTMLContainer and NodeList are unrelated types in DOM yet our HTMLContainer inherits NodeList for code sharing reasons. While some code does get shared the types are sufficiently different that this results in lots of unnecessary branches, complexity and general awkwardness. Code sharing can be better achieved by other means than inheritance.
Created attachment 216016 [details] patch
and by HTMLContainer I mean HTMLCollection
Comment on attachment 216016 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=216016&action=review > Source/WebCore/dom/Document.cpp:502 > + for (unsigned i = 0; i < WTF_ARRAY_LENGTH(m_nodeListAndCollectionCounts); i++) ++i LOL > Source/WebCore/dom/Document.cpp:592 > + for (unsigned i = 0; i < WTF_ARRAY_LENGTH(m_nodeListAndCollectionCounts); i++) Ditto. > Source/WebCore/dom/Document.cpp:3361 > +void Document::registerNodeList(LiveNodeList* list) LiveNodeList& > Source/WebCore/dom/Document.cpp:3368 > +void Document::unregisterNodeList(LiveNodeList* list) LiveNodeList& > Source/WebCore/dom/Document.cpp:3377 > +void Document::registerCollection(HTMLCollection* list) HTMLCollection& Also 'list'? > Source/WebCore/dom/Document.cpp:3386 > +{ HTMLCollection& Also 'list'? > Source/WebCore/dom/Document.h:704 > + bool shouldInvalidateNodeListAndCollectionCaches(const QualifiedName* attrName = 0) const; nullptr > Source/WebCore/dom/LiveNodeList.cpp:43 > +inline bool isMatchingElement(const NodeListType*, Element*); So > Source/WebCore/dom/LiveNodeList.cpp:45 > +template <> inline bool isMatchingElement(const LiveNodeList* nodeList, Element* element) many > Source/WebCore/dom/LiveNodeList.cpp:50 > +template <> inline bool isMatchingElement(const HTMLTagNodeList* nodeList, Element* element) pointers > Source/WebCore/dom/LiveNodeList.cpp:55 > +template <> inline bool isMatchingElement(const ClassNodeList* nodeList, Element* element) everywhere :| > Source/WebCore/dom/LiveNodeList.cpp:67 > + return 0; nullptr
Comment on attachment 216016 [details] patch Attachment 216016 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/19638999
Comment on attachment 216016 [details] patch We shouldn't do this at least until we move getElementsByTagName, etc… off of NodeList. That was the whole point in creating this mess in the first place. Sorry for leaving the mess but let me finish the transition first.
Namely, I'm referring to https://bugs.webkit.org/show_bug.cgi?id=110611.
Antti and I had some discussions on IRC. Antti intends to land this patch and then re-share code for the caches and I'll implement the transition once he finished that work.
Comment on attachment 216016 [details] patch Sure go ahead.
http://trac.webkit.org/changeset/158663 broke the build everywhere ... :-/ Are you working on the fix or do you need any help?
was fixed in https://trac.webkit.org/r158665
(In reply to comment #10) > was fixed in https://trac.webkit.org/r158665 No, it wasn't. Windows, EFL and GTK build is still broken.
maybe http://trac.webkit.org/changeset/158667 ... let's see