Bug 6228 – DOMString/DOMStringImpl/AtomicString need enhancements before replacing QString

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Eric Seidel (no email)]

DOMString/DOMStringImpl/AtomicString need enhancements before replacing QString

The attached patch adds find(), contains(), startsWith() and endsWith() functionality to the string classes.  I 
have another patch which uses these throughout CSSStyleSelector and other locations in WebKit which I'll 
attach separately.

[Eric Seidel (no email)]

Created attachment 5266 [details]
Adds find(), contains(), startsWith(), endsWith() to the DOM string classes

[Eric Seidel (no email)]

Created attachment 5267 [details]
Use new string methods in khtml/css

I separated out khtml/css since that's where the bulk of my changes went.

[Eric Seidel (no email)]

Created attachment 5268 [details]
Other string uses (including some unrelated cleanup)

[Eric Seidel (no email)]

Woohoo!  Autovicki says these patches yeild another .3ms improvement (out of 133.0)!

[Maciej Stachowiak]

Comment on attachment 5267 [details]
Use new string methods in khtml/css

+    bool important = priority.find("important", 0, false) != -1;

Replace this with startsWith for clarity maybe?

-	     refPos = path.find("#", 0);
+	     refPos = path.find("#");

r- because of this apparent bug (or possible undocumented fix) otherwise looks
fine.

[Maciej Stachowiak]

Comment on attachment 5268 [details]
Other string uses (including some unrelated cleanup)

+	     if (str.find("url", 0,  false ) == 0)

replace with startsWith?

otherwise r=me

[Darin Adler]

Comment on attachment 5266 [details]
Adds find(), contains(), startsWith(), endsWith() to the DOM string classes

In the future, the startsWith algorithm should not use find, because it only
needs to compare once at the start of the string and find will go on to search
the rest of the string. The things that find does to optimize that search are
not helpful for startsWith.

The patch has a number of tab characters in it. They should be converted to
spaces before it's landed.

I think that dom_string.h's version of contains and startsWith should call
contains and startsWith in the underlying DOMStringImpl and we should have a
contains function in DOMStringImpl. In general, all the functions should be
present in DOMStringImpl and then reflected in DOMString as needed.

DOMStringImpl::find has a comment still mentions QString.

[Darin Adler]

Comment on attachment 5268 [details]
Other string uses (including some unrelated cleanup)

Looking at these searches, it occurs to me that ", false" is a pretty poor way
to indicate a case insensitive search. We should consider alternate API for
that.

+	     if (str.find("url", 0,  false ) == 0)

The above is a case of "startsWith" and would be clearer if we called it that
way.

+	     if (str.length() && str[0] == '=')

And this is startsWith too (although a character, not a string).

[Darin Adler]

Comment on attachment 5267 [details]
Use new string methods in khtml/css

In parseUASheet, we can make the DOMString from the char* without decoding
because we know it has all ASCII characters in it. Unlike QString, that
function does not work for non-ASCII Latin-1 characters. So I think it's worth
a comment and adding an assertion to DOMStringImpl::DOMStringImpl(const char *)
to check for non-ASCII characters.

[Eric Seidel (no email)]

Landed.