Bug 102886 - HTMLCollection should use the same storage as DynamicNodeList
Summary: HTMLCollection should use the same storage as DynamicNodeList
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks: 78909
  Show dependency treegraph
 
Reported: 2012-11-20 23:39 PST by Ryosuke Niwa
Modified: 2012-11-22 01:15 PST (History)
10 users (show)

See Also:


Attachments
cleanup (18.28 KB, patch)
2012-11-20 23:42 PST, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Fixed GTK+ & FEL builds (18.98 KB, patch)
2012-11-21 12:59 PST, Ryosuke Niwa
koivisto: review+
eflews.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2012-11-20 23:39:06 PST
HTMLCollection should use the same storage as DynamicNodeList
Comment 1 Ryosuke Niwa 2012-11-20 23:42:41 PST
Created attachment 175348 [details]
cleanup
Comment 2 EFL EWS Bot 2012-11-20 23:50:31 PST
Comment on attachment 175348 [details]
cleanup

Attachment 175348 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14901855
Comment 3 Ryosuke Niwa 2012-11-21 12:59:22 PST
Created attachment 175502 [details]
Fixed GTK+ & FEL builds
Comment 4 Antti Koivisto 2012-11-21 13:05:48 PST
Comment on attachment 175502 [details]
Fixed GTK+ & FEL builds

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

r=me

> Source/WebCore/dom/NodeRareData.h:82
> +        NodeListAtomicNameCacheMap::AddResult result = m_atomicNameCaches.add(
> +        namedNodeListKey(DynamicNodeList::PastLastNodeListType + collectionType - FirstNodeCollectionType, starAtom), 0);

This spills awkwardly to the next line. You should just construct the key on a separate line.

> Source/WebCore/dom/NodeRareData.h:94
> +        return static_cast<T*>(m_atomicNameCaches.get(namedNodeListKey(DynamicNodeList::PastLastNodeListType + type - FirstNodeCollectionType, starAtom)));

This is pretty long too.
Comment 5 EFL EWS Bot 2012-11-21 13:22:49 PST
Comment on attachment 175502 [details]
Fixed GTK+ & FEL builds

Attachment 175502 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14950119
Comment 6 Ryosuke Niwa 2012-11-21 13:38:57 PST
Committed r135429: <http://trac.webkit.org/changeset/135429>
Comment 7 Ryosuke Niwa 2012-11-21 14:03:53 PST
One more build fix for EFL & GTK+ landed in http://trac.webkit.org/changeset/135431.
Comment 8 Eric Seidel (no email) 2012-11-22 00:14:24 PST
Curious what (if any) memory/perf effects this is expected to have.
Comment 9 Ryosuke Niwa 2012-11-22 01:15:10 PST
It shouldn't have any.