[Webkit-unassigned] [Bug 80490] [Microdata] Implement cache mechanism for HTMLPropertiesCollection.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=80490





--- Comment #14 from Ryosuke Niwa <rniwa at webkit.org>  2012-03-29 15:23:33 PST ---
(From update of attachment 134572)
View in context: https://bugs.webkit.org/attachment.cgi?id=134572&action=review

Okay, the patch looks much better. Maybe we want one more iteration before we land this.

> Source/WebCore/html/HTMLCollection.h:92
> +#if ENABLE(MICRODATA)
> +        NodeCacheMap propertyCache;
> +        Vector<Element*> itemRefElements;
> +        RefPtr<DOMStringList> propertyNames;
> +        unsigned itemRefElementPosition;
> +        bool hasItemRefElements;
> +#endif

On my second thought, it might be better to store this in HTMLPropertiesCollection or an inner class or HTMLPropertiesCollection
because the change as is will increase the size of all HTMLCollection even though those collections would never use these variables.

> Source/WebCore/html/HTMLPropertiesCollection.cpp:67
> +static Node* nextNodeToProcess(Node* base, Node* node)

Maybe it's better to call this function nextNodeWithProperty? nextNodeToProcess sounds too generic.

> Source/WebCore/html/HTMLPropertiesCollection.cpp:69
> +    // An Microdata item may contain properties which in turn an item itself. Properties can also

"which in turn" what? "which in turn are items themselves"?

> Source/WebCore/html/HTMLPropertiesCollection.cpp:110
> +    std::sort(itemRefElements.begin(), itemRefElements.end(), compareTreeOrder);

This is very inefficient. I think it's probably better to walk the entire document and collect nodes in the document order
since sorting nodes will end up doing the same work anyway.

> Source/WebCore/html/HTMLPropertiesCollection.cpp:184
> +        for (unsigned pos = currentPosition; pos < index; ++pos) {

What's the point of pos local variable here? Can't we just increment currentPosition?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.