Bug 90118

Summary: Share the same cache in HTMLCollection and DynamicNodeLists
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: DOMAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, kling, koivisto, mifenton, tkent, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 90111, 90240    
Bug Blocks: 89919    
Attachments:
Description Flags
work in progress
none
Patch
none
Updated for ToT
none
Fixed a merge error andersca: review+, andersca: commit-queue+

Ryosuke Niwa
Reported 2012-06-27 17:49:14 PDT
In order to resolve the bug 89919, we need to first make the interface of the cache in HTMLCollection and DynamicNodeList similar.
Attachments
work in progress (27.99 KB, patch)
2012-06-27 17:50 PDT, Ryosuke Niwa
no flags
Patch (35.77 KB, patch)
2012-06-28 20:40 PDT, Ryosuke Niwa
no flags
Updated for ToT (35.79 KB, patch)
2012-06-29 00:18 PDT, Ryosuke Niwa
no flags
Fixed a merge error (35.81 KB, patch)
2012-06-29 00:21 PDT, Ryosuke Niwa
andersca: review+
andersca: commit-queue+
Ryosuke Niwa
Comment 1 2012-06-27 17:50:31 PDT
Created attachment 149834 [details] work in progress Here's my working in progress patch. It also includes my patch for the bug 90111.
Ryosuke Niwa
Comment 2 2012-06-28 19:20:16 PDT
Turned out that I can do better and just share the same cache class between two classes.
Ryosuke Niwa
Comment 3 2012-06-28 20:40:46 PDT
Ryosuke Niwa
Comment 4 2012-06-28 20:41:26 PDT
Comment on attachment 150075 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150075&action=review > Source/WebCore/ChangeLog:13 > + In DynamicNodeList, a very straight forward one-to-one mapping from old Caches member variables: s/a very straight/we have a very straight/
Ryosuke Niwa
Comment 5 2012-06-28 20:42:35 PDT
Comment on attachment 150075 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150075&action=review > Source/WebCore/html/HTMLCollection.cpp:274 > + unsigned i = 0; > for (Element* e = itemAfter(0); e; e = itemAfter(e)) { > if (checkForNameMatch(e, /* checkName */ false, name)) { > - m_cache.current = e; > + setItemCache(e, i); > return e; > } > + i++; > } > > + i = 0; > for (Element* e = itemAfter(0); e; e = itemAfter(e)) { > if (checkForNameMatch(e, /* checkName */ true, name)) { > - m_cache.current = e; > + setItemCache(e, i); > return e; > } > + i++; This includes the patch on https://bugs.webkit.org/show_bug.cgi?id=90240.
Ryosuke Niwa
Comment 6 2012-06-29 00:18:30 PDT
Created attachment 150098 [details] Updated for ToT
Ryosuke Niwa
Comment 7 2012-06-29 00:21:36 PDT
Created attachment 150099 [details] Fixed a merge error
Ryosuke Niwa
Comment 8 2012-06-29 10:44:36 PDT
Ping reviewers.
Ryosuke Niwa
Comment 9 2012-06-29 11:58:36 PDT
Note You need to log in before you can comment on or make changes to this bug.