So much code duplication!
Created attachment 118139 [details] cleanup
Comment on attachment 118139 [details] cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=118139&action=review > Source/WebCore/dom/Node.cpp:1003 > + if (data && data->nodeLists() && (!treeScope() || !treeScope()->hasNodeListCaches())) Oops, I obviously don't need to check for data here. Will fix before landing it.
Created attachment 118143 [details] Fixed some bug
Comment on attachment 118143 [details] Fixed some bug View in context: https://bugs.webkit.org/attachment.cgi?id=118143&action=review > Source/WebCore/dom/NodeRareData.h:135 > + setNodeLists(NodeListsNodeData::create()); > + if (TreeScope* treeScope = node->treeScope()) > + treeScope->addNodeListCache(); Do we really want this inlined? I would have put this part into a non-inline helper function. Also, this function is kind of long. Maybe we could put it outside the class definition.
Comment on attachment 118143 [details] Fixed some bug View in context: https://bugs.webkit.org/attachment.cgi?id=118143&action=review >> Source/WebCore/dom/NodeRareData.h:135 >> + treeScope->addNodeListCache(); > > Do we really want this inlined? I would have put this part into a non-inline helper function. > > Also, this function is kind of long. Maybe we could put it outside the class definition. Sure, I can put this in Node.cpp. Thanks for the review.
Comment on attachment 118143 [details] Fixed some bug View in context: https://bugs.webkit.org/attachment.cgi?id=118143&action=review >>> Source/WebCore/dom/NodeRareData.h:135 >>> + setNodeLists(NodeListsNodeData::create()); >>> + if (TreeScope* treeScope = node->treeScope()) >>> + treeScope->addNodeListCache(); >> >> Do we really want this inlined? I would have put this part into a non-inline helper function. >> >> Also, this function is kind of long. Maybe we could put it outside the class definition. > > Sure, I can put this in Node.cpp. Thanks for the review. I meant to apply that comment to all the code inside if (!m_nodeLists)
Comment on attachment 118143 [details] Fixed some bug View in context: https://bugs.webkit.org/attachment.cgi?id=118143&action=review >>>> Source/WebCore/dom/NodeRareData.h:135 >>>> + treeScope->addNodeListCache(); >>> >>> Do we really want this inlined? I would have put this part into a non-inline helper function. >>> >>> Also, this function is kind of long. Maybe we could put it outside the class definition. >> >> Sure, I can put this in Node.cpp. Thanks for the review. > > I meant to apply that comment to all the code inside if (!m_nodeLists) Ok. Will add createNodeLists() and put that in Node.cpp.
Committed r102208: <http://trac.webkit.org/changeset/102208>