Changeset 31144 in webkit
- Timestamp:
- Mar 18, 2008 6:42:00 PM (16 years ago)
- Location:
- trunk
- Files:
-
- 6 added
- 14 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/LayoutTests/ChangeLog
r31142 r31144 1 2008-03-18 Sam Weinig <sam@webkit.org> 2 3 Reviewed by Anders Carlsson. 4 5 Tests for http://bugs.webkit.org/show_bug.cgi?id=17057 6 REGRESSION: Frequent random crashes in WebCore::JSNodeList::indexGetter 7 <rdar://problem/5725058> 8 9 * fast/dom/NodeList/5725058-crash-scenario-1-expected.txt: Added. 10 * fast/dom/NodeList/5725058-crash-scenario-1.html: Added. 11 * fast/dom/NodeList/5725058-crash-scenario-2-expected.txt: Added. 12 * fast/dom/NodeList/5725058-crash-scenario-2.html: Added. 13 * fast/dom/NodeList/5725058-crash-scenario-3-expected.txt: Added. 14 * fast/dom/NodeList/5725058-crash-scenario-3.html: Added. 15 1 16 2008-03-18 Sam Weinig <sam@webkit.org> 2 17 -
trunk/WebCore/ChangeLog
r31143 r31144 1 2008-03-18 Sam Weinig <sam@webkit.org> 2 3 Reviewed by Anders Carlsson. 4 5 Fix for http://bugs.webkit.org/show_bug.cgi?id=17057 6 REGRESSION: Frequent random crashes in WebCore::JSNodeList::indexGetter 7 <rdar://problem/5725058> 8 9 Tests: fast/dom/NodeList/5725058-crash-scenario-1.html 10 fast/dom/NodeList/5725058-crash-scenario-2.html 11 fast/dom/NodeList/5725058-crash-scenario-3.html 12 13 * dom/ChildNodeList.cpp: 14 (WebCore::ChildNodeList::ChildNodeList): 15 * dom/ChildNodeList.h: 16 Remove rootNodeChildrenChanged() method and fix the constructor to not 17 pass in a needsNotifications argument to DynamicNodeList, as it no longer 18 takes one. 19 20 * dom/ClassNodeList.cpp: 21 (WebCore::ClassNodeList::ClassNodeList): 22 Don't pass the needsNotifications argument to DynamicNodeList. 23 24 * dom/ContainerNode.cpp: 25 (WebCore::ContainerNode::childrenChanged): 26 Rename call to hasNodeLists() to hasNodeListCaches(). 27 28 * dom/Document.cpp: 29 (WebCore::Document::Document): 30 (WebCore::Document::~Document): Zero out the m_document variable to signify 31 to destructors down the destruction chain that this is a Document type node 32 being destructed, and thus, accessing document() is prohibited. 33 * dom/Document.h: 34 (WebCore::Document::addNodeListCache): Renamed from addNodeList. 35 (WebCore::Document::removeNodeListCache): Renamed from removeNodeList, adds assertion. 36 (WebCore::Document::hasNodeListCaches): Renamed from hasNodeListCaches. 37 Rename m_numNodeLists to m_numNodeListCaches. 38 39 * dom/DynamicNodeList.cpp: 40 (WebCore::DynamicNodeList::DynamicNodeList): 41 (WebCore::DynamicNodeList::~DynamicNodeList): 42 (WebCore::DynamicNodeList::invalidateCache): 43 (WebCore::DynamicNodeList::Caches::Caches): 44 * dom/DynamicNodeList.h: 45 (WebCore::DynamicNodeList::hasOwnCaches): 46 Remove the needsNotifications concept from DynamicNodeList, instead, manually 47 invalidate the cache for lists that own their own cache. 48 49 * dom/NameNodeList.cpp: 50 (WebCore::NameNodeList::NameNodeList): 51 * dom/NameNodeList.h: 52 Remove rootNodeAttributeChanged() method and fix the constructor to not 53 pass in a needsNotifications argument to DynamicNodeList, as it no longer 54 takes one. 55 56 * dom/Node.cpp: 57 (WebCore::Node::~Node): Decrement the document's nodeListCache count 58 if we had a NodeListsNodeData cache and this is not the Document being 59 destructor, as tagged by a null m_document. 60 (WebCore::Node::childNodes): Increment the document's nodeListCache count 61 if we need create the NodeListsNodeData. 62 (WebCore::Node::registerDynamicNodeList): Increment the document's nodeListCache count 63 if we need create the NodeListsNodeData. Change to invalidate all the caches, instead 64 of just the ChildNodeList, if document has had no NodeListCaches. 65 (WebCore::Node::unregisterDynamicNodeList): Change to remove the cache from the m_listsWithCaches 66 set if it is owned by the NodeList and clear the m_nodeLists if it is empty. 67 (WebCore::Node::notifyLocalNodeListsAttributeChanged): Move logic to 68 NodeListsNodeData::invalidateAttributeCaches and clear the cache pointer if it is empty. 69 (WebCore::Node::notifyLocalNodeListsChildrenChanged): Move logic to 70 NodeListsNodeData::invalidateCaches and clear the cache pointer if it is empty. 71 (WebCore::Node::notifyNodeListsChildrenChanged): Cleanup. 72 (WebCore::Node::getElementsByName): Increment the document's nodeListCache count 73 if we need create the NodeListsNodeData. 74 (WebCore::Node::getElementsByClassName): Increment the document's nodeListCache count 75 if we need create the NodeListsNodeData. 76 77 (WebCore::NodeListsNodeData::invalidateCaches): Added. 78 (WebCore::NodeListsNodeData::invalidateAttributeCaches): Added. 79 (WebCore::NodeListsNodeData::isEmpty): Added. 80 81 * dom/TagNodeList.cpp: 82 (WebCore::TagNodeList::TagNodeList): 83 Don't pass the needsNotifications argument to DynamicNodeList. 84 1 85 2008-03-18 Matt Lilek <webkit@mattlilek.com> 2 86 -
trunk/WebCore/dom/ChildNodeList.cpp
r28990 r31144 29 29 30 30 ChildNodeList::ChildNodeList(PassRefPtr<Node> rootNode, DynamicNodeList::Caches* info) 31 : DynamicNodeList(rootNode, info , false)31 : DynamicNodeList(rootNode, info) 32 32 { 33 33 } … … 104 104 } 105 105 106 void ChildNodeList::rootNodeChildrenChanged()107 {108 // For child node lists, the common cache is reset in Node::notifyLocalNodeListsChildrenChanged()109 ASSERT(!m_ownsCaches);110 }111 112 106 } // namespace WebCore -
trunk/WebCore/dom/ChildNodeList.h
r28990 r31144 36 36 virtual Node* item(unsigned index) const; 37 37 38 virtual void rootNodeChildrenChanged();39 40 38 protected: 41 39 virtual bool nodeMatches(Node*) const; -
trunk/WebCore/dom/ClassNodeList.cpp
r29663 r31144 38 38 39 39 ClassNodeList::ClassNodeList(PassRefPtr<Node> rootNode, const String& classNames, DynamicNodeList::Caches* caches) 40 : DynamicNodeList(rootNode, caches , true)40 : DynamicNodeList(rootNode, caches) 41 41 { 42 42 m_classNames.parseClassAttribute(classNames, m_rootNode->document()->inCompatMode()); -
trunk/WebCore/dom/ContainerNode.cpp
r31075 r31144 701 701 if (!changedByParser && childCountDelta) 702 702 document()->nodeChildrenChanged(this, beforeChange, afterChange, childCountDelta); 703 if (document()->hasNodeList s())703 if (document()->hasNodeListCaches()) 704 704 notifyNodeListsChildrenChanged(); 705 705 } -
trunk/WebCore/dom/Document.cpp
r31122 r31144 297 297 , m_useSecureKeyboardEntryWhenActive(false) 298 298 , m_isXHTML(isXHTML) 299 , m_numNodeList s(0)299 , m_numNodeListCaches(0) 300 300 #if ENABLE(DATABASE) 301 301 , m_hasOpenDatabases(false) … … 456 456 if (m_styleSheets) 457 457 m_styleSheets->documentDestroyed(); 458 459 m_document = 0; 458 460 } 459 461 -
trunk/WebCore/dom/Document.h
r31122 r31144 85 85 class IntPoint; 86 86 class MouseEventWithHitTestResults; 87 class NameNodeList;88 87 class NodeFilter; 89 88 class NodeIterator; 90 class NodeList;91 89 class Page; 92 90 class PlatformMouseEvent; … … 701 699 #endif 702 700 703 void addNodeList () { m_numNodeLists++; }704 void removeNodeList () { m_numNodeLists--; }705 bool hasNodeList s() const { return m_numNodeLists != 0; }701 void addNodeListCache() { ++m_numNodeListCaches; } 702 void removeNodeListCache() { ASSERT(m_numNodeListCaches > 0); --m_numNodeListCaches; } 703 bool hasNodeListCaches() const { return m_numNodeListCaches; } 706 704 707 705 void updateFocusAppearanceSoon(); … … 960 958 bool m_isXHTML; 961 959 962 unsigned m_numNodeList s;960 unsigned m_numNodeListCaches; 963 961 964 962 #if ENABLE(DATABASE) -
trunk/WebCore/dom/DynamicNodeList.cpp
r28990 r31144 29 29 namespace WebCore { 30 30 31 DynamicNodeList::DynamicNodeList(PassRefPtr<Node> rootNode , bool needsNotifications)31 DynamicNodeList::DynamicNodeList(PassRefPtr<Node> rootNode) 32 32 : m_rootNode(rootNode) 33 33 , m_caches(new Caches) 34 34 , m_ownsCaches(true) 35 , m_needsNotifications(needsNotifications)36 35 { 37 36 m_rootNode->registerDynamicNodeList(this); 38 37 } 39 38 40 DynamicNodeList::DynamicNodeList(PassRefPtr<Node> rootNode, DynamicNodeList::Caches* info, bool needsNotifications)39 DynamicNodeList::DynamicNodeList(PassRefPtr<Node> rootNode, DynamicNodeList::Caches* caches) 41 40 : m_rootNode(rootNode) 42 , m_caches( info)41 , m_caches(caches) 43 42 , m_ownsCaches(false) 44 , m_needsNotifications(needsNotifications)45 43 { 46 44 m_rootNode->registerDynamicNodeList(this); 45 ++caches->refCount; 47 46 } 48 47 … … 52 51 if (m_ownsCaches) 53 52 delete m_caches; 53 else 54 --m_caches->refCount; 54 55 } 55 56 … … 158 159 } 159 160 160 void DynamicNodeList:: rootNodeChildrenChanged()161 void DynamicNodeList::invalidateCache() 161 162 { 163 // This should only be called for node lists that own their own caches. 164 ASSERT(m_ownsCaches); 162 165 m_caches->reset(); 163 }164 165 void DynamicNodeList::rootNodeAttributeChanged()166 {167 166 } 168 167 … … 171 170 , isLengthCacheValid(false) 172 171 , isItemCacheValid(false) 172 , refCount(0) 173 173 { 174 174 } -
trunk/WebCore/dom/DynamicNodeList.h
r28990 r31144 46 46 bool isLengthCacheValid : 1; 47 47 bool isItemCacheValid : 1; 48 unsigned refCount; 48 49 }; 49 50 50 DynamicNodeList(PassRefPtr<Node> rootNode , bool needsNotifications);51 DynamicNodeList(PassRefPtr<Node> rootNode, Caches* , bool needsNotifications);51 DynamicNodeList(PassRefPtr<Node> rootNode); 52 DynamicNodeList(PassRefPtr<Node> rootNode, Caches*); 52 53 virtual ~DynamicNodeList(); 53 54 54 bool needsNotifications() const { return m_needsNotifications; }55 bool hasOwnCaches() const { return m_ownsCaches; } 55 56 56 57 // DOM methods & attributes for NodeList … … 60 61 61 62 // Other methods (not part of DOM) 62 virtual void rootNodeChildrenChanged(); 63 virtual void rootNodeAttributeChanged(); 63 void invalidateCache(); 64 64 65 65 protected: … … 69 69 mutable Caches* m_caches; 70 70 bool m_ownsCaches; 71 bool m_needsNotifications;72 71 73 72 private: -
trunk/WebCore/dom/NameNodeList.cpp
r28990 r31144 33 33 34 34 NameNodeList::NameNodeList(PassRefPtr<Node> rootNode, const String& name, DynamicNodeList::Caches* caches) 35 : DynamicNodeList(rootNode, caches , true)35 : DynamicNodeList(rootNode, caches) 36 36 , m_nodeName(name) 37 37 { 38 }39 40 void NameNodeList::rootNodeAttributeChanged()41 {42 DynamicNodeList::rootNodeChildrenChanged();43 38 } 44 39 -
trunk/WebCore/dom/NameNodeList.h
r28990 r31144 37 37 NameNodeList(PassRefPtr<Node> rootNode, const String& name, DynamicNodeList::Caches*); 38 38 39 virtual void rootNodeAttributeChanged();40 41 39 private: 42 40 virtual bool nodeMatches(Node*) const; -
trunk/WebCore/dom/Node.cpp
r31075 r31144 58 58 using namespace HTMLNames; 59 59 60 typedef HashSet<DynamicNodeList*> NodeListSet;61 60 struct NodeListsNodeData { 62 NodeListSet m_listsToNotify; 61 typedef HashSet<DynamicNodeList*> NodeListSet; 62 NodeListSet m_listsWithCaches; 63 63 64 DynamicNodeList::Caches m_childNodeListCaches; 64 65 65 66 typedef HashMap<String, DynamicNodeList::Caches*> CacheMap; 66 67 CacheMap m_classNodeListCaches; 67 68 CacheMap m_nameNodeListCaches; 68 69 69 70 ~NodeListsNodeData() 70 71 { … … 72 73 deleteAllValues(m_nameNodeListCaches); 73 74 } 75 76 void invalidateCaches(); 77 void invalidateCachesThatDependOnAttributes(); 78 bool isEmpty() const; 74 79 }; 80 81 // -------- 75 82 76 83 bool Node::isSupported(const String& feature, const String& version) … … 167 174 --NodeCounter::count; 168 175 #endif 176 177 if (m_nodeLists && m_document) 178 document()->removeNodeListCache(); 179 169 180 if (renderer()) 170 181 detach(); … … 194 205 PassRefPtr<NodeList> Node::childNodes() 195 206 { 196 if (!m_nodeLists) 207 if (!m_nodeLists) { 197 208 m_nodeLists.set(new NodeListsNodeData); 209 document()->addNodeListCache(); 210 } 198 211 199 212 return new ChildNodeList(this, &m_nodeLists->m_childNodeListCaches); … … 407 420 void Node::registerDynamicNodeList(DynamicNodeList* list) 408 421 { 409 if (!m_nodeLists) 422 if (!m_nodeLists) { 410 423 m_nodeLists.set(new NodeListsNodeData); 411 else if (!m_document->hasNodeLists()) 424 document()->addNodeListCache(); 425 } else if (!m_document->hasNodeListCaches()) { 412 426 // We haven't been receiving notifications while there were no registered lists, so the cache is invalid now. 413 m_nodeLists-> m_childNodeListCaches.reset();414 415 if (list->needsNotifications()) 416 m_nodeLists->m_listsToNotify.add(list);417 m_document->addNodeList();427 m_nodeLists->invalidateCaches(); 428 } 429 430 if (list->hasOwnCaches()) 431 m_nodeLists->m_listsWithCaches.add(list); 418 432 } 419 433 … … 421 435 { 422 436 ASSERT(m_nodeLists); 423 m_document->removeNodeList(); 424 if (list->needsNotifications()) 425 m_nodeLists->m_listsToNotify.remove(list); 437 if (list->hasOwnCaches()) { 438 m_nodeLists->m_listsWithCaches.remove(list); 439 if (m_nodeLists->isEmpty()) { 440 m_nodeLists.clear(); 441 document()->removeNodeListCache(); 442 } 443 } 426 444 } 427 445 … … 431 449 return; 432 450 433 NodeListSet::iterator end = m_nodeLists->m_listsToNotify.end(); 434 for (NodeListSet::iterator i = m_nodeLists->m_listsToNotify.begin(); i != end; ++i) 435 (*i)->rootNodeAttributeChanged(); 451 m_nodeLists->invalidateCachesThatDependOnAttributes(); 452 453 if (m_nodeLists->isEmpty()) { 454 m_nodeLists.clear(); 455 document()->removeNodeListCache(); 456 } 436 457 } 437 458 … … 447 468 return; 448 469 449 m_nodeLists->m_childNodeListCaches.reset(); 450 451 NodeListSet::iterator end = m_nodeLists->m_listsToNotify.end(); 452 for (NodeListSet::iterator i = m_nodeLists->m_listsToNotify.begin(); i != end; ++i) 453 (*i)->rootNodeChildrenChanged(); 470 m_nodeLists->invalidateCaches(); 471 472 NodeListsNodeData::NodeListSet::iterator end = m_nodeLists->m_listsWithCaches.end(); 473 for (NodeListsNodeData::NodeListSet::iterator i = m_nodeLists->m_listsWithCaches.begin(); i != end; ++i) 474 (*i)->invalidateCache(); 475 476 if (m_nodeLists->isEmpty()) { 477 m_nodeLists.clear(); 478 document()->removeNodeListCache(); 479 } 454 480 } 455 481 456 482 void Node::notifyNodeListsChildrenChanged() 457 483 { 458 for (Node *n = this; n; n = n->parentNode())484 for (Node* n = this; n; n = n->parentNode()) 459 485 n->notifyLocalNodeListsChildrenChanged(); 460 486 } … … 1176 1202 PassRefPtr<NodeList> Node::getElementsByName(const String& elementName) 1177 1203 { 1178 if (!m_nodeLists) 1204 if (!m_nodeLists) { 1179 1205 m_nodeLists.set(new NodeListsNodeData); 1206 document()->addNodeListCache(); 1207 } 1180 1208 1181 1209 pair<NodeListsNodeData::CacheMap::iterator, bool> result = m_nodeLists->m_nameNodeListCaches.add(elementName, 0); … … 1188 1216 PassRefPtr<NodeList> Node::getElementsByClassName(const String& classNames) 1189 1217 { 1190 if (!m_nodeLists) 1218 if (!m_nodeLists) { 1191 1219 m_nodeLists.set(new NodeListsNodeData); 1220 document()->addNodeListCache(); 1221 } 1192 1222 1193 1223 pair<NodeListsNodeData::CacheMap::iterator, bool> result = m_nodeLists->m_classNodeListCaches.add(classNames, 0); … … 1642 1672 #endif 1643 1673 1644 } 1674 // -------- 1675 1676 void NodeListsNodeData::invalidateCaches() 1677 { 1678 m_childNodeListCaches.reset(); 1679 invalidateCachesThatDependOnAttributes(); 1680 } 1681 1682 void NodeListsNodeData::invalidateCachesThatDependOnAttributes() 1683 { 1684 CacheMap::iterator classCachesEnd = m_classNodeListCaches.end(); 1685 for (CacheMap::iterator it = m_classNodeListCaches.begin(); it != classCachesEnd; ++it) 1686 it->second->reset(); 1687 1688 CacheMap::iterator nameCachesEnd = m_nameNodeListCaches.end(); 1689 for (CacheMap::iterator it = m_nameNodeListCaches.begin(); it != nameCachesEnd; ++it) 1690 it->second->reset(); 1691 } 1692 1693 bool NodeListsNodeData::isEmpty() const 1694 { 1695 if (!m_listsWithCaches.isEmpty()) 1696 return false; 1697 1698 if (m_childNodeListCaches.refCount) 1699 return false; 1700 1701 CacheMap::const_iterator classCachesEnd = m_classNodeListCaches.end(); 1702 for (CacheMap::const_iterator it = m_classNodeListCaches.begin(); it != classCachesEnd; ++it) { 1703 if (it->second->refCount) 1704 return false; 1705 } 1706 1707 CacheMap::const_iterator nameCachesEnd = m_nameNodeListCaches.end(); 1708 for (CacheMap::const_iterator it = m_nameNodeListCaches.begin(); it != nameCachesEnd; ++it) { 1709 if (it->second->refCount) 1710 return false; 1711 } 1712 1713 return true; 1714 } 1715 1716 // -------- 1717 1718 } // namespace WebCore 1645 1719 1646 1720 #ifndef NDEBUG -
trunk/WebCore/dom/TagNodeList.cpp
r28988 r31144 31 31 32 32 TagNodeList::TagNodeList(PassRefPtr<Node> rootNode, const AtomicString& namespaceURI, const AtomicString& localName) 33 : DynamicNodeList(rootNode , true)33 : DynamicNodeList(rootNode) 34 34 , m_namespaceURI(namespaceURI) 35 35 , m_localName(localName)
Note: See TracChangeset
for help on using the changeset viewer.