Bug 91046 - invalidateNodeListsCacheAfterAttributeChanged should dynamically figure out which attribute needs invalidation
Summary: invalidateNodeListsCacheAfterAttributeChanged should dynamically figure out w...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks: 90326
  Show dependency treegraph
 
Reported: 2012-07-11 20:55 PDT by Ryosuke Niwa
Modified: 2012-07-31 05:01 PDT (History)
8 users (show)

See Also:


Attachments
Cleanup (30.69 KB, patch)
2012-07-11 21:50 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Renamed shouldInvalidateNodeList to shouldInvalidateDynamicSubtreeNodeList (30.82 KB, patch)
2012-07-11 21:56 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Fixed a bug (31.69 KB, patch)
2012-07-11 22:35 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
Fixed build for microdata (32.26 KB, patch)
2012-07-11 22:52 PDT, Ryosuke Niwa
andersca: review+
Details | Formatted Diff | Diff
backtrace (1.68 KB, text/plain)
2012-07-25 07:01 PDT, Thiago Marcos P. Santos
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 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.
Comment 1 Ryosuke Niwa 2012-07-11 21:50:48 PDT
Created attachment 151854 [details]
Cleanup
Comment 2 Ryosuke Niwa 2012-07-11 21:56:52 PDT
Created attachment 151855 [details]
Renamed shouldInvalidateNodeList to shouldInvalidateDynamicSubtreeNodeList
Comment 3 Ryosuke Niwa 2012-07-11 22:35:13 PDT
Created attachment 151859 [details]
Fixed a bug
Comment 4 Ryosuke Niwa 2012-07-11 22:37:52 PDT
This is one of the most important patches in this refactoring effort.
Comment 5 Gyuyoung Kim 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
Comment 6 Ryosuke Niwa 2012-07-11 22:52:29 PDT
Created attachment 151863 [details]
Fixed build for microdata
Comment 7 Ryosuke Niwa 2012-07-12 13:31:14 PDT
Committed r122498: <http://trac.webkit.org/changeset/122498>
Comment 8 Thiago Marcos P. Santos 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?
Comment 9 Ryosuke Niwa 2012-07-25 10:05:10 PDT
Please file a new bug instead of commenting here.
Comment 10 Dominik Röttsches (drott) 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.