Bug 91046

Summary: invalidateNodeListsCacheAfterAttributeChanged should dynamically figure out which attribute needs invalidation
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: DOMAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, arko, d-r, kling, koivisto, ojan, sam, s.choi
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 90326    
Attachments:
Description Flags
Cleanup
none
Renamed shouldInvalidateNodeList to shouldInvalidateDynamicSubtreeNodeList
none
Fixed a bug
none
Fixed build for microdata
andersca: review+
backtrace none

Ryosuke Niwa
Reported 2012-07-11 20:55:10 PDT
Right now invalidateNodeListsCacheAfterAttributeChanged has a hard coded list of attributes to watch out but this is problematic if wanted to also support HTMLCollection here because HTMLCollection depends on more attributes. Also, we currently invalidate node lists when an attribute that's relevant to any node list type regardless of whether such a node list exists in the document or not. We can do better by remembering which node list types are present in the document, and avoiding the invalidation altogether when some attribute changes and node lists that care about the attribute doesn't exist in the document.
Attachments
Cleanup (30.69 KB, patch)
2012-07-11 21:50 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Renamed shouldInvalidateNodeList to shouldInvalidateDynamicSubtreeNodeList (30.82 KB, patch)
2012-07-11 21:56 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Fixed a bug (31.69 KB, patch)
2012-07-11 22:35 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Fixed build for microdata (32.26 KB, patch)
2012-07-11 22:52 PDT, Ryosuke Niwa 		andersca: review+
DetailsFormatted DiffDiff
backtrace (1.68 KB, text/plain)
2012-07-25 07:01 PDT, Thiago Marcos P. Santos 		no flags
Details
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2012-07-11 21:50:48 PDT
Created attachment 151854 [details] Cleanup
Ryosuke Niwa
Comment 2 2012-07-11 21:56:52 PDT
Created attachment 151855 [details] Renamed shouldInvalidateNodeList to shouldInvalidateDynamicSubtreeNodeList
Ryosuke Niwa
Comment 3 2012-07-11 22:35:13 PDT
Created attachment 151859 [details] Fixed a bug
Ryosuke Niwa
Comment 4 2012-07-11 22:37:52 PDT
This is one of the most important patches in this refactoring effort.
Gyuyoung Kim
Comment 5 2012-07-11 22:46:10 PDT
Comment on attachment 151859 [details] Fixed a bug Attachment 151859 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13221081
Ryosuke Niwa
Comment 6 2012-07-11 22:52:29 PDT
Created attachment 151863 [details] Fixed build for microdata
Ryosuke Niwa
Comment 7 2012-07-12 13:31:14 PDT
Committed r122498: <http://trac.webkit.org/changeset/122498>
Thiago Marcos P. Santos
Comment 8 2012-07-25 07:01:36 PDT
Created attachment 154342 [details] backtrace I'm hitting this assertion when running a internal test suite we have here. Unfortunately I could not create a test yet that maps exactly what triggers this assertion, but what the test suit does is: - It has a frameset with two frames, one for the test (A) another one for progress (B). - Loads a test case on the frame A. - Does some checks to find out when the test is done on the frame A. - Manipulates the DOM inside the progress frame B. - Replaces the src from the frame A with a new test case. - Looks like the crash happens at the moment the src from frame A is replaced. I could reproduce the crash on Qt too running the same test suite. Any insight?
Ryosuke Niwa
Comment 9 2012-07-25 10:05:10 PDT
Please file a new bug instead of commenting here.
Dominik Röttsches (drott)
Comment 10 2012-07-31 05:01:38 PDT
(In reply to comment #9) > Please file a new bug instead of commenting here. Moved to bug 92742.
Note You need to log in before you can comment on or make changes to this bug.