RESOLVED FIXED 120490
XMLSerializer-attribute-namespace-prefix-conflicts can't produce reliable results
https://bugs.webkit.org/show_bug.cgi?id=120490
Summary XMLSerializer-attribute-namespace-prefix-conflicts can't produce reliable res...
Dean Jackson
Reported 2013-08-29 14:07:39 PDT
The test fast/dom/XMLSerializer-attribute-namespace-prefix-conflicts.html creates some duplicated namespaces and serializes the output. Since the unique namespace prefixes are based on an incrementing number, the actual prefixes are not reliably identical across runs. The expected result can't simply be the text dump of a single run. I'm going to mark the test as [ PASS FAIL ] for now.
Attachments
Patch (3.03 KB, patch)
2013-08-30 19:27 PDT, Rob Buis
no flags
Patch (2.76 KB, patch)
2013-08-31 09:16 PDT, Rob Buis
no flags
Dean Jackson
Comment 1 2013-08-29 14:11:40 PDT
TestExpectations updated in https://trac.webkit.org/r154842
Rob Buis
Comment 2 2013-08-29 14:38:29 PDT
This seems to be a Debug only problem. Will have a look.
Rob Buis
Comment 3 2013-08-29 19:27:42 PDT
(In reply to comment #2) > This seems to be a Debug only problem. Will have a look. I found a possible fix, will likely provide a patch tomorrow.
Dean Jackson
Comment 4 2013-08-30 16:28:44 PDT
I'm skipping the tests for now. Please unskip in the patch. Thanks!
Dean Jackson
Comment 5 2013-08-30 16:34:04 PDT
Rob Buis
Comment 6 2013-08-30 19:27:05 PDT
Rob Buis
Comment 7 2013-08-30 19:31:55 PDT
I debugged this a bit. It seems fastMalloc can return identical pointers for strings with differing hash values in StringImpl::createUninitializedInternalNonEmpty(). Specifically what happens on the test is that NS1 is not found so it is written out. Then for the next element, NS2 is tried but it already exists in a ancestor element. Then NS3, NS4 etc. all seem to reuse the same internal StringImpl pointer (for already existing NS2) even though the strings are obviously different,. I wonder if I am using the API wrong or if this is a bug...
Dean Jackson
Comment 8 2013-08-30 19:35:42 PDT
Comment on attachment 210178 [details] Patch This looks fine to me. However, we should probably check if there is a bug in AtomicString. cc-ing Ben.
Ryosuke Niwa
Comment 9 2013-08-30 20:30:48 PDT
Comment on attachment 210178 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=210178&action=review > Source/WebCore/ChangeLog:8 > + Avoid toAtomicString which sometimes returns the same internal AtomicStringImpl* for two different strings. That definitely sounds like a serious bug!
Ryosuke Niwa
Comment 10 2013-08-30 20:32:57 PDT
(In reply to comment #7) > I debugged this a bit. It seems fastMalloc can return identical pointers for strings with differing hash values in StringImpl::createUninitializedInternalNonEmpty(). Specifically what happens on the test is that NS1 is not found so it is written out. Then for the next element, NS2 is tried but it already exists in a ancestor element. Then NS3, NS4 etc. all seem to reuse the same internal StringImpl pointer (for already existing NS2) even though the strings are obviously different,. I wonder if I am using the API wrong or if this is a bug... Maybe you're not retaining each AtomicString?
Darin Adler
Comment 11 2013-08-31 08:16:11 PDT
(In reply to comment #10) > Maybe you're not retaining each AtomicString? Sounds likely. We can definitely get the same AtomicStringImpl twice if the first one is deallocated before the second one is allocated.
Darin Adler
Comment 12 2013-08-31 08:17:07 PDT
Comment on attachment 210178 [details] Patch This patch is definitely wrong. It covers up a bug elsewhere.
Rob Buis
Comment 13 2013-08-31 09:16:47 PDT
WebKit Commit Bot
Comment 14 2013-08-31 14:04:57 PDT
Comment on attachment 210194 [details] Patch Clearing flags on attachment: 210194 Committed r154932: <http://trac.webkit.org/changeset/154932>
WebKit Commit Bot
Comment 15 2013-08-31 14:05:00 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.