Bug 123823 – Factor index cache for NodeLists and HTMLCollections to a class

• Component: DOM
• Status: RESOLVED FIXED

[Antti Koivisto]

share code

[Antti Koivisto]

Created attachment 216071 [details]
patch

[EFL EWS Bot]

Comment on attachment 216071 [details]
patch

Attachment 216071 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/21278006

[EFL EWS Bot]

Comment on attachment 216071 [details]
patch

Attachment 216071 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/20998227

[Ryosuke Niwa]

Comment on attachment 216071 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=216071&action=review

> Source/WebCore/dom/CollectionIndexCache.h:36
> +    unsigned size(const Collection&) const;

It's probably better call this length to match the terminology in HTMLCollection & NodeList.

> Source/WebCore/dom/CollectionIndexCache.h:46
> +    mutable unsigned m_cachedSize : 31;
> +    mutable unsigned m_cachedSizeValid : 1;

Ditto.

> Source/WebCore/dom/CollectionIndexCache.h:47
> +    mutable unsigned m_cachedCurrentPosition;

We should probably call this m_cachedCurrentIndex for consistency.

> Source/WebCore/dom/CollectionIndexCache.h:82
> +    if (index < m_cachedCurrentPosition - index) {
> +        // Start is closer to the target.

Instead of adding a comment, we can just define a bool as in:
const bool startIsCloserToTarget = index < m_cachedCurrentPosition - index;

> Source/WebCore/dom/CollectionIndexCache.h:104
> +    if (m_cachedSizeValid && m_cachedSize - index < index - m_cachedCurrentPosition) {
> +        // End is closer to the target.

Ditto.

> Source/WebCore/dom/CollectionIndexCache.h:129
> +    if (m_cachedSizeValid && index >= m_cachedSize)

We should assert that !m_cachedCurrentNode || index == m_cachedCurrentPosition.

> Source/WebCore/dom/CollectionIndexCache.h:138
> +    if (m_cachedSizeValid && m_cachedSize - index < index) {

Maybe we can define a boolean like:
const bool endIsCloserToTarget = m_cachedSizeValid && m_cachedSize - index < index;
?

[Ryosuke Niwa]

Also, you probably need to add the header to CMakeList.txt

[Build Bot]

Comment on attachment 216071 [details]
patch

Attachment 216071 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/21288014

New failing tests:
dom/html/level2/html/HTMLIsIndexElement02.html

[Build Bot]

Created attachment 216081 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Antti Koivisto]

https://trac.webkit.org/r158698 (with a fix for failing test and stylistic changes)