[CSS] Minor cleanups in CSS variables handling
Created attachment 202091 [details] Patch
Comment on attachment 202091 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=202091&action=review > Source/WebCore/css/CSSParser.cpp:-407 > - ASSERT(m_length >= position + length); > - > - RefPtr<StringImpl> result; > - > - if (is8Bit()) { > - result = StringImpl::create(characters8() + position, length); > - } else { > - result = StringImpl::create(characters16() + position, length); > - } > - > - return AtomicString(result); Here’s the correct way to write this: ASSERT(m_length >= length); ASSERT(m_length >= position + length); if (is8Bit()) return AtomicString(characters8() + position, length); return AtomicString(characters16() + position, length); There is no reason to call StringImpl::create, and in fact that costs memory allocation and deallocation when the string already exists in the atomic string table. > Source/WebCore/css/CSSParser.cpp:3313 > - AtomicString variableName = name.substring(prefixLength, name.length() - prefixLength); > + AtomicString variableName(String(name).impl(), prefixLength, name.length() - prefixLength); Like the old code, this does unnecessary memory allocation, so you should do what I suggested instead.
(In reply to comment #2) > ASSERT(m_length >= length); Isn't that implicit below? > ASSERT(m_length >= position + length);
Created attachment 202105 [details] Patch
Comment on attachment 202105 [details] Patch Sorry, I should have caught this when you were landing the first patch.
Committed r150302: <http://trac.webkit.org/changeset/150302>