[webkit-reviews] review requested: [Bug 30187] Implicit AtomicString(char*) considered harmful : [Attachment 44166] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Dec 2 11:23:19 PST 2009


Jens Alfke <snej at chromium.org> has asked  for review:
Bug 30187: Implicit AtomicString(char*) considered harmful
https://bugs.webkit.org/show_bug.cgi?id=30187

Attachment 44166: patch
https://bugs.webkit.org/attachment.cgi?id=44166&action=review

------- Additional Comments from Jens Alfke <snej at chromium.org>
Here's the rest of the patch. This finally makes the AtomicString(const char*)
constructor explicit.
It does two other nontrivial things to support this change:
1. Adds a HashMap::remove that takes an alternate key type, so HTTPHeaderMap
can use it for char*.
2. Adds an HTTPHeaders namespace containing a bunch of string constants like
ContentType, so we don't have to use literal C strings for these all over the
code.

There are also little changes in lots of source files, to replace C string
literals with either
a. AtomicString constants (like emptyAtom)
b. HTTPHeaders constants
c. New static AtomicString constants defined using DEFINE_STATIC_LOCAL
d. Explicit AtomicString("...") construction, in places where the time/space
tradeoff didn't seem to make c. worthwhile
e. Calls to new method variants that explicitly take char*, such as
Element::setCStringAttribute.


More information about the webkit-reviews mailing list