Diff
Modified: trunk/Source/WTF/ChangeLog (146701 => 146702)
--- trunk/Source/WTF/ChangeLog 2013-03-23 01:19:35 UTC (rev 146701)
+++ trunk/Source/WTF/ChangeLog 2013-03-23 01:35:34 UTC (rev 146702)
@@ -1,5 +1,40 @@
2013-03-22 Benjamin Poulain <[email protected]>
+ Remove 2 bad branches from StringHash::equal() and CaseFoldingHash::equal()
+ https://bugs.webkit.org/show_bug.cgi?id=113003
+
+ Reviewed by Eric Seidel.
+
+ StringHash::equal() and CaseFoldingHash::equal() were both testing for
+ the nullity of the two input pointers. The catch is: neither traits handle
+ null pointers, and any client code would have crashed on hash(), before equal()
+ is called.
+ Consequently, the two branches had a pass rate of zero when called from a HashMap code.
+
+ The function is also never inlined because it is too big (the code of equal() for characters
+ is always inlined, causing the function to be quite big).
+
+ This patch introduces two new functions in the StringImpl API: equalNonNull() and
+ equalIgnoringCaseNonNull(). Those functions are similar to their equal() equivalent
+ but make the assumtion the input is never null.
+
+ The functions are used for StringHash to avoid the useless branches.
+
+ * wtf/text/StringHash.h:
+ (WTF::StringHash::equal):
+ (WTF::CaseFoldingHash::equal):
+ * wtf/text/StringImpl.cpp:
+ (WTF::stringImplContentEqual):
+ (WTF::equal):
+ (WTF::equalNonNull):
+ (WTF::equalIgnoringCase):
+ (WTF::equalIgnoringCaseNonNull):
+ (WTF::equalIgnoringNullity):
+ * wtf/text/StringImpl.h:
+ (WTF::equalIgnoringCase):
+
+2013-03-22 Benjamin Poulain <[email protected]>
+
Name correctly the argument of StringImpl::setIsAtomic()
https://bugs.webkit.org/show_bug.cgi?id=113000
Modified: trunk/Source/WTF/wtf/text/StringHash.h (146701 => 146702)
--- trunk/Source/WTF/wtf/text/StringHash.h 2013-03-23 01:19:35 UTC (rev 146701)
+++ trunk/Source/WTF/wtf/text/StringHash.h 2013-03-23 01:35:34 UTC (rev 146702)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2006, 2007, 2008, 2012 Apple Inc. All rights reserved
+ * Copyright (C) 2006, 2007, 2008, 2012, 2013 Apple Inc. All rights reserved
* Copyright (C) Research In Motion Limited 2009. All rights reserved.
*
* This library is free software; you can redistribute it and/or
@@ -43,29 +43,9 @@
struct StringHash {
static unsigned hash(StringImpl* key) { return key->hash(); }
- static bool equal(const StringImpl* a, const StringImpl* b)
+ static inline bool equal(const StringImpl* a, const StringImpl* b)
{
- if (a == b)
- return true;
- if (!a || !b)
- return false;
-
- unsigned aLength = a->length();
- unsigned bLength = b->length();
- if (aLength != bLength)
- return false;
-
- if (a->is8Bit()) {
- if (b->is8Bit())
- return WTF::equal(a->characters8(), b->characters8(), aLength);
-
- return WTF::equal(a->characters8(), b->characters16(), aLength);
- }
-
- if (b->is8Bit())
- return WTF::equal(a->characters16(), b->characters8(), aLength);
-
- return WTF::equal(a->characters16(), b->characters16(), aLength);
+ return equalNonNull(a, b);
}
static unsigned hash(const RefPtr<StringImpl>& key) { return key->hash(); }
@@ -112,27 +92,9 @@
return CaseFoldingHash::hash(reinterpret_cast<const LChar*>(data), length);
}
- static bool equal(const StringImpl* a, const StringImpl* b)
+ static inline bool equal(const StringImpl* a, const StringImpl* b)
{
- if (a == b)
- return true;
- if (!a || !b)
- return false;
- unsigned length = a->length();
- if (length != b->length())
- return false;
-
- if (a->is8Bit()) {
- if (b->is8Bit())
- return equalIgnoringCase(a->characters8(), b->characters8(), length);
-
- return equalIgnoringCase(b->characters16(), a->characters8(), length);
- }
-
- if (b->is8Bit())
- return equalIgnoringCase(a->characters16(), b->characters8(), length);
-
- return equalIgnoringCase(a->characters16(), b->characters16(), length);
+ return equalIgnoringCaseNonNull(a, b);
}
static unsigned hash(const RefPtr<StringImpl>& key)
Modified: trunk/Source/WTF/wtf/text/StringImpl.cpp (146701 => 146702)
--- trunk/Source/WTF/wtf/text/StringImpl.cpp 2013-03-23 01:19:35 UTC (rev 146701)
+++ trunk/Source/WTF/wtf/text/StringImpl.cpp 2013-03-23 01:35:34 UTC (rev 146702)
@@ -1667,9 +1667,34 @@
return newImpl.release();
}
+static inline bool stringImplContentEqual(const StringImpl* a, const StringImpl* b)
+{
+ unsigned aLength = a->length();
+ unsigned bLength = b->length();
+ if (aLength != bLength)
+ return false;
+
+ if (a->is8Bit()) {
+ if (b->is8Bit())
+ return equal(a->characters8(), b->characters8(), aLength);
+
+ return equal(a->characters8(), b->characters16(), aLength);
+ }
+
+ if (b->is8Bit())
+ return equal(a->characters16(), b->characters8(), aLength);
+
+ return equal(a->characters16(), b->characters16(), aLength);
+}
+
bool equal(const StringImpl* a, const StringImpl* b)
{
- return StringHash::equal(a, b);
+ if (a == b)
+ return true;
+ if (!a || !b)
+ return false;
+
+ return stringImplContentEqual(a, b);
}
bool equal(const StringImpl* a, const LChar* b, unsigned length)
@@ -1736,12 +1761,26 @@
return equal(a->characters16(), b, length);
}
-bool equalIgnoringCase(StringImpl* a, StringImpl* b)
+bool equalNonNull(const StringImpl* a, const StringImpl* b)
{
+ ASSERT(a && b);
+ if (a == b)
+ return true;
+
+ return stringImplContentEqual(a, b);
+}
+
+bool equalIgnoringCase(const StringImpl* a, const StringImpl* b)
+{
+ if (a == b)
+ return true;
+ if (!a || !b)
+ return false;
+
return CaseFoldingHash::equal(a, b);
}
-bool equalIgnoringCase(StringImpl* a, const LChar* b)
+bool equalIgnoringCase(const StringImpl* a, const LChar* b)
{
if (!a)
return !b;
@@ -1795,16 +1834,36 @@
return equal && !b[length];
}
+bool equalIgnoringCaseNonNull(const StringImpl* a, const StringImpl* b)
+{
+ ASSERT(a && b);
+ if (a == b)
+ return true;
+
+ unsigned length = a->length();
+ if (length != b->length())
+ return false;
+
+ if (a->is8Bit()) {
+ if (b->is8Bit())
+ return equalIgnoringCase(a->characters8(), b->characters8(), length);
+
+ return equalIgnoringCase(b->characters16(), a->characters8(), length);
+ }
+
+ if (b->is8Bit())
+ return equalIgnoringCase(a->characters16(), b->characters8(), length);
+
+ return equalIgnoringCase(a->characters16(), b->characters16(), length);
+}
+
bool equalIgnoringNullity(StringImpl* a, StringImpl* b)
{
- if (StringHash::equal(a, b))
- return true;
if (!a && b && !b->length())
return true;
if (!b && a && !a->length())
return true;
-
- return false;
+ return equal(a, b);
}
WTF::Unicode::Direction StringImpl::defaultWritingDirection(bool* hasStrongDirectionality)
Modified: trunk/Source/WTF/wtf/text/StringImpl.h (146701 => 146702)
--- trunk/Source/WTF/wtf/text/StringImpl.h 2013-03-23 01:19:35 UTC (rev 146701)
+++ trunk/Source/WTF/wtf/text/StringImpl.h 2013-03-23 01:35:34 UTC (rev 146702)
@@ -844,6 +844,7 @@
inline bool equal(const LChar* a, StringImpl* b) { return equal(b, a); }
inline bool equal(const char* a, StringImpl* b) { return equal(b, reinterpret_cast<const LChar*>(a)); }
WTF_EXPORT_STRING_API bool equal(const StringImpl*, const UChar*, unsigned);
+WTF_EXPORT_STRING_API bool equalNonNull(const StringImpl* a, const StringImpl* b);
// Do comparisons 8 or 4 bytes-at-a-time on architectures where it's safe.
#if CPU(X86_64)
@@ -1089,9 +1090,9 @@
return true;
}
-WTF_EXPORT_STRING_API bool equalIgnoringCase(StringImpl*, StringImpl*);
-WTF_EXPORT_STRING_API bool equalIgnoringCase(StringImpl*, const LChar*);
-inline bool equalIgnoringCase(const LChar* a, StringImpl* b) { return equalIgnoringCase(b, a); }
+WTF_EXPORT_STRING_API bool equalIgnoringCase(const StringImpl*, const StringImpl*);
+WTF_EXPORT_STRING_API bool equalIgnoringCase(const StringImpl*, const LChar*);
+inline bool equalIgnoringCase(const LChar* a, const StringImpl* b) { return equalIgnoringCase(b, a); }
WTF_EXPORT_STRING_API bool equalIgnoringCase(const LChar*, const LChar*, unsigned);
WTF_EXPORT_STRING_API bool equalIgnoringCase(const UChar*, const LChar*, unsigned);
inline bool equalIgnoringCase(const UChar* a, const char* b, unsigned length) { return equalIgnoringCase(a, reinterpret_cast<const LChar*>(b), length); }
@@ -1103,6 +1104,7 @@
ASSERT(length >= 0);
return !Unicode::umemcasecmp(a, b, length);
}
+WTF_EXPORT_STRING_API bool equalIgnoringCaseNonNull(const StringImpl*, const StringImpl*);
WTF_EXPORT_STRING_API bool equalIgnoringNullity(StringImpl*, StringImpl*);
@@ -1337,6 +1339,7 @@
using WTF::StringImpl;
using WTF::equal;
+using WTF::equalNonNull;
using WTF::TextCaseSensitivity;
using WTF::TextCaseSensitive;
using WTF::TextCaseInsensitive;
Modified: trunk/Source/WebCore/ChangeLog (146701 => 146702)
--- trunk/Source/WebCore/ChangeLog 2013-03-23 01:19:35 UTC (rev 146701)
+++ trunk/Source/WebCore/ChangeLog 2013-03-23 01:35:34 UTC (rev 146702)
@@ -1,3 +1,17 @@
+2013-03-22 Benjamin Poulain <[email protected]>
+
+ Remove 2 bad branches from StringHash::equal() and CaseFoldingHash::equal()
+ https://bugs.webkit.org/show_bug.cgi?id=113003
+
+ Reviewed by Eric Seidel.
+
+ Fix two unfortunate use of StringHash and use the correct StringImpl function.
+
+ * html/parser/HTMLParserIdioms.cpp:
+ (WebCore::threadSafeEqual):
+ * html/parser/HTMLTreeBuilderSimulator.cpp:
+ (WebCore::tokenExitsSVG):
+
2013-03-22 Andy Estes <[email protected]>
Set the cache partition property on CFURLRequests
Modified: trunk/Source/WebCore/html/parser/HTMLParserIdioms.cpp (146701 => 146702)
--- trunk/Source/WebCore/html/parser/HTMLParserIdioms.cpp 2013-03-23 01:19:35 UTC (rev 146701)
+++ trunk/Source/WebCore/html/parser/HTMLParserIdioms.cpp 2013-03-23 01:35:34 UTC (rev 146702)
@@ -284,7 +284,7 @@
return true;
if (a->hash() != b->hash())
return false;
- return StringHash::equal(a, b);
+ return equalNonNull(a, b);
}
bool threadSafeMatch(const QualifiedName& a, const QualifiedName& b)
Modified: trunk/Source/WebCore/html/parser/HTMLTreeBuilderSimulator.cpp (146701 => 146702)
--- trunk/Source/WebCore/html/parser/HTMLTreeBuilderSimulator.cpp 2013-03-23 01:19:35 UTC (rev 146701)
+++ trunk/Source/WebCore/html/parser/HTMLTreeBuilderSimulator.cpp 2013-03-23 01:35:34 UTC (rev 146702)
@@ -95,10 +95,7 @@
static bool tokenExitsSVG(const CompactHTMLToken& token)
{
// FIXME: It's very fragile that we special case foreignObject here to be case-insensitive.
- // FIXME: Using CaseFoldingHash::equal instead of equalIgnoringCase, as equalIgnoringCase
- // wants non-const StringImpl* (even though it never modifies them).
- // https://bugs.webkit.org/show_bug.cgi?id=111892 is for fixing equalIgnoringCase.
- return CaseFoldingHash::equal(token.data().asStringImpl(), SVGNames::foreignObjectTag.localName().impl());
+ return equalIgnoringCaseNonNull(token.data().asStringImpl(), SVGNames::foreignObjectTag.localName().impl());
}
static bool tokenExitsMath(const CompactHTMLToken& token)
