NEW 145002
Fix the incorrectness that AtomicString could have symbolic StringImpl
https://bugs.webkit.org/show_bug.cgi?id=145002
Summary Fix the incorrectness that AtomicString could have symbolic StringImpl
Yusuke Suzuki
Reported 2015-05-14 08:44:07 PDT
In issue 144848, AtomicStringImpl's incorrectness is resolved. But AtomicString's incorrectness still remains. There are several plans. 1. Introduce UniquedString, AtomicString, SymbolString and replace the current AtomicString with UniquedString The simplest plan. Introduce UniquedString and SymbolString as the same to UniquedStringImpl and SymbolImpl. And replace the current AtomicString use with UniquedString. Pros: + Simple, easy. Cons: + Introduce more and more classes for strings :( 2. Drop AtomicString and use RefPtr<UniquedStringImpl> / Ref<UniquedStringImpl> directly Drop the AtomicString use cases completely and use RefPtr<UniquedStringImpl> / Ref<UniquedStringImpl> instead. Pros: + We can drop the class for strings! (AtomicString) Cons: + Since there's code using `operator==` for AtomicString, rewriting is not mechanical. (We need to consider about PassRefPtr<UniquedStringImpl>...) + WTFString remains. In both, IDL change is involved.
Attachments
Darin Adler
Comment 1 2015-05-19 08:32:18 PDT
(In reply to comment #0) > + Since there's code using `operator==` for AtomicString, rewriting is not > mechanical. I don’t understand the problem here. One of the properties of AtomicString is that == is the same operation for AtomicString, for AtomicStringImpl*, and for RefPtr<AtomicStringImpl>. Unlike String, where String's == compares the characters, but StringImpl* and RefPtr<StringImpl> are not the same.
Darin Adler
Comment 2 2015-05-19 08:34:28 PDT
Another plus of using RefPtr directly is that it will be more natural to pass types like UniquedStringImpl& and UniquedStringImpl*, rather than const RefPtr<UniquedStringImpl>&, and the former is more efficient. In fact, we would eventually rename UniquedStringImpl to UniquedString, once we figure out what to do about the names of WTF::String and WTF::StringImpl. I think we might at some point name WTF::String something crazy like RefPtrString and give StringImpl the simpler name String.
Note You need to log in before you can comment on or make changes to this bug.