Summary: | Extract named items caches in HTMLCollection as a class | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||
Component: | DOM | Assignee: | Ryosuke Niwa <rniwa> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | benjamin, kling, koivisto | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 129361 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Ryosuke Niwa
2014-02-25 23:13:51 PST
Created attachment 225227 [details]
Cleanup
Created attachment 225234 [details]
Cleanup
Comment on attachment 225234 [details] Cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=225234&action=review r=me though it would be nice to improve naming consistency and factor more stuff to the class. > Source/WebCore/ChangeLog:15 > + Also removed m_isItemRefElementsCacheValid since it was only used by Microdata API removed in r153772. Hah > Source/WebCore/html/HTMLAllCollection.cpp:61 > - if (Vector<Element*>* cache = idCache(name)) { > + if (const Vector<Element*>* cache = caches.idCache(name)) { > if (index < cache->size()) > return cache->at(index); > index -= cache->size(); > } > > - if (Vector<Element*>* cache = nameCache(name)) { > + if (const Vector<Element*>* cache = caches.nameCache(name)) { > if (index < cache->size()) > return cache->at(index); > } It would be nice to factor this kind of stuff to the cache class. > Source/WebCore/html/HTMLCollection.cpp:413 > updateNameCache(); > + ASSERT(m_namedItemCaches); Match the function and variable names (updateNamedElementCache()/m_namedElementCache or similar) > Source/WebCore/html/HTMLCollection.cpp:423 > - if (Vector<Element*>* idResults = idCache(name)) { > + if (const Vector<Element*>* idResults = m_namedItemCaches->idCache(name)) { > if (idResults->size()) > return idResults->at(0); > } > > - if (Vector<Element*>* nameResults = nameCache(name)) { > + if (const Vector<Element*>* nameResults = m_namedItemCaches->nameCache(name)) { > if (nameResults->size()) > return nameResults->at(0); > } and this > Source/WebCore/html/HTMLCollection.h:41 > +class CollectionNamedItemCaches { "Item" adds nothing. How about something like CollectionNamedElementCache? > Source/WebCore/html/HTMLCollection.h:44 > + const Vector<Element*>* idCache(const AtomicString& id) const { return find(m_idCache, id); } > + const Vector<Element*>* nameCache(const AtomicString& name) const { return find(m_nameCache, name); } findElementsWithID/Name? > Source/WebCore/html/HTMLCollection.h:64 > + NodeCacheMap m_idCache; > + NodeCacheMap m_nameCache; Maybe the whole class is the "cache" and these can be something like m_idAttributeMap, m_nameAttributeMap > Source/WebCore/html/HTMLCollection.h:102 > + bool hasIdNameCache() const { return !!m_namedItemCaches; } Please match the variable name. > Source/WebCore/html/HTMLCollection.h:125 > + const CollectionNamedItemCaches& namedItemCachesAssertingExistence() const > + { Drop the AssertingExistence part. It is not important. > Source/WebCore/html/HTMLCollection.h:137 > + void invalidateIdNameCacheMaps() const; Match with other naming. Committed r164772: <http://trac.webkit.org/changeset/164772> (In reply to comment #4) > Committed r164772: <http://trac.webkit.org/changeset/164772> Removed the superfluous change log entry in http://trac.webkit.org/changeset/164773. |