Skip to content

Commit 40c7aa4

Browse files
author
David Hyatt
committed
[CSS Parser] Eliminate in-place lowercasing in the parser.
https://bugs.webkit.org/show_bug.cgi?id=165368 Reviewed by Darin Adler. Source/WebCore: Replace the in-place lowercasing that the parser does with new mechanisms. In-place lowercasing ruins serialization and doesn't work on CSS parsed from static strings. It also has the side effect of mutating strings passed in from JavaScript like for querySelectorAll. For class/id selectors, we now check if the string is lowercase or not. If it contains uppercase ASCII characters, then we allocate the RareData for the selector. RareData now has two fields instead of one for the value, a matching value (all lowercase in quirks mode), and a serializing value (the original string). Because this is done at the CSSSelector level, the old parser has been patched as well for these cases. In addition, in-place lowercasing was done for pseudo-elements, for media query features, and for attr(). In all of these cases we do lowercase converting by first checking if it's needed. Serialization will not retain the original string in these cases, so we may want to revisit these cases in the future and apply a solution similar to what we did for selectors. * css/CSSGrammar.y.in: * css/CSSSelector.cpp: (WebCore::CSSSelector::createRareData): (WebCore::CSSSelector::selectorText): (WebCore::CSSSelector::RareData::RareData): (WebCore::CSSSelector::RareData::~RareData): * css/CSSSelector.h: (WebCore::CSSSelector::RareData::create): (WebCore::CSSSelector::setValue): (WebCore::CSSSelector::value): (WebCore::CSSSelector::serializingValue): * css/MediaQueryExp.cpp: (WebCore::MediaQueryExpression::MediaQueryExpression): * css/parser/CSSParserToken.cpp: (WebCore::convertToASCIILowercaseInPlace): Deleted. (WebCore::CSSParserToken::convertToASCIILowercaseInPlace): Deleted. * css/parser/CSSParserToken.h: * css/parser/CSSParserValues.h: (WebCore::CSSParserSelector::setValue): * css/parser/CSSPropertyParser.cpp: (WebCore::consumeAttr): * css/parser/CSSSelectorParser.cpp: (WebCore::CSSSelectorParser::consumeId): (WebCore::CSSSelectorParser::consumeClass): (WebCore::CSSSelectorParser::consumePseudo): * css/parser/MediaQueryParser.cpp: (WebCore::MediaQueryParser::readFeature): LayoutTests: * fast/media/mq-pointer-expected.txt: Canonical link: https://commits.webkit.org/182989@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@209314 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 4a21965 commit 40c7aa4

13 files changed

Lines changed: 124 additions & 76 deletions

‎LayoutTests/ChangeLog‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
1+
2016-12-04 Dave Hyatt <hyatt@apple.com>
2+
3+
[CSS Parser] Eliminate in-place lowercasing in the parser.
4+
https://bugs.webkit.org/show_bug.cgi?id=165368
5+
6+
Reviewed by Darin Adler.
7+
8+
* fast/media/mq-pointer-expected.txt:
9+
110
2016-12-04 Matt Baker <mattbaker@apple.com>
211

312
Web Inspector: Assertion Failures breakpoint should respect global Breakpoints enabled setting

‎LayoutTests/fast/media/mq-pointer-expected.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
Test the (pointer) and (hover) media features. See Bug 87403 for details.
22

33
Query "(pointer)": true
4-
Query "(pointer)": true
4+
Query "(Pointer)": true
55
Query "(pointer:none)": false
66
Query "(pointer:coarse)": false
77
Query "(pointer:coARse)": false

‎Source/WebCore/ChangeLog‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,57 @@
1+
2016-12-04 Dave Hyatt <hyatt@apple.com>
2+
3+
[CSS Parser] Eliminate in-place lowercasing in the parser.
4+
https://bugs.webkit.org/show_bug.cgi?id=165368
5+
6+
Reviewed by Darin Adler.
7+
8+
Replace the in-place lowercasing that the parser does with new
9+
mechanisms. In-place lowercasing ruins serialization and doesn't
10+
work on CSS parsed from static strings. It also has the side effect
11+
of mutating strings passed in from JavaScript like for querySelectorAll.
12+
13+
For class/id selectors, we now check if the string is lowercase or not.
14+
If it contains uppercase ASCII characters, then we allocate the RareData
15+
for the selector. RareData now has two fields instead of one for the value,
16+
a matching value (all lowercase in quirks mode), and a serializing value (the
17+
original string). Because this is done at the CSSSelector level, the old
18+
parser has been patched as well for these cases.
19+
20+
In addition, in-place lowercasing was done for pseudo-elements, for
21+
media query features, and for attr(). In all of these cases we do
22+
lowercase converting by first checking if it's needed. Serialization will
23+
not retain the original string in these cases, so we may want to revisit
24+
these cases in the future and apply a solution similar to what we did for
25+
selectors.
26+
27+
* css/CSSGrammar.y.in:
28+
* css/CSSSelector.cpp:
29+
(WebCore::CSSSelector::createRareData):
30+
(WebCore::CSSSelector::selectorText):
31+
(WebCore::CSSSelector::RareData::RareData):
32+
(WebCore::CSSSelector::RareData::~RareData):
33+
* css/CSSSelector.h:
34+
(WebCore::CSSSelector::RareData::create):
35+
(WebCore::CSSSelector::setValue):
36+
(WebCore::CSSSelector::value):
37+
(WebCore::CSSSelector::serializingValue):
38+
* css/MediaQueryExp.cpp:
39+
(WebCore::MediaQueryExpression::MediaQueryExpression):
40+
* css/parser/CSSParserToken.cpp:
41+
(WebCore::convertToASCIILowercaseInPlace): Deleted.
42+
(WebCore::CSSParserToken::convertToASCIILowercaseInPlace): Deleted.
43+
* css/parser/CSSParserToken.h:
44+
* css/parser/CSSParserValues.h:
45+
(WebCore::CSSParserSelector::setValue):
46+
* css/parser/CSSPropertyParser.cpp:
47+
(WebCore::consumeAttr):
48+
* css/parser/CSSSelectorParser.cpp:
49+
(WebCore::CSSSelectorParser::consumeId):
50+
(WebCore::CSSSelectorParser::consumeClass):
51+
(WebCore::CSSSelectorParser::consumePseudo):
52+
* css/parser/MediaQueryParser.cpp:
53+
(WebCore::MediaQueryParser::readFeature):
54+
155
2016-12-04 Gyuyoung Kim <gyuyoung.kim@webkit.org>
256

357
Fix a build break on EFL since r209303.

‎Source/WebCore/css/CSSGrammar.y.in‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1214,19 +1214,15 @@ specifier:
12141214
IDSEL {
12151215
$$ = new CSSParserSelector;
12161216
$$->setMatch(CSSSelector::Id);
1217-
if (parser->m_context.mode == HTMLQuirksMode)
1218-
$1.convertToASCIILowercaseInPlace();
1219-
$$->setValue($1);
1217+
$$->setValue($1, parser->m_context.mode == HTMLQuirksMode);
12201218
}
12211219
| HEX {
12221220
if ($1[0] >= '0' && $1[0] <= '9')
12231221
$$ = nullptr;
12241222
else {
12251223
$$ = new CSSParserSelector;
12261224
$$->setMatch(CSSSelector::Id);
1227-
if (parser->m_context.mode == HTMLQuirksMode)
1228-
$1.convertToASCIILowercaseInPlace();
1229-
$$->setValue($1);
1225+
$$->setValue($1, parser->m_context.mode == HTMLQuirksMode);
12301226
}
12311227
}
12321228
| class
@@ -1238,9 +1234,7 @@ class:
12381234
'.' IDENT {
12391235
$$ = new CSSParserSelector;
12401236
$$->setMatch(CSSSelector::Class);
1241-
if (parser->m_context.mode == HTMLQuirksMode)
1242-
$2.convertToASCIILowercaseInPlace();
1243-
$$->setValue($2);
1237+
$$->setValue($2, parser->m_context.mode == HTMLQuirksMode);
12441238
}
12451239
;
12461240

‎Source/WebCore/css/CSSSelector.cpp‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ void CSSSelector::createRareData()
8484
if (m_hasRareData)
8585
return;
8686
// Move the value to the rare data stucture.
87-
m_data.m_rareData = &RareData::create(adoptRef(m_data.m_value)).leakRef();
87+
AtomicString value { adoptRef(m_data.m_value) };
88+
m_data.m_rareData = &RareData::create(WTFMove(value)).leakRef();
8889
m_hasRareData = true;
8990
}
9091

@@ -406,10 +407,10 @@ String CSSSelector::selectorText(const String& rightSide) const
406407
while (true) {
407408
if (cs->match() == CSSSelector::Id) {
408409
str.append('#');
409-
serializeIdentifier(cs->value(), str);
410+
serializeIdentifier(cs->serializingValue(), str);
410411
} else if (cs->match() == CSSSelector::Class) {
411412
str.append('.');
412-
serializeIdentifier(cs->value(), str);
413+
serializeIdentifier(cs->serializingValue(), str);
413414
} else if (cs->match() == CSSSelector::PseudoClass) {
414415
switch (cs->pseudoClassType()) {
415416
#if ENABLE(FULLSCREEN_API)
@@ -659,7 +660,7 @@ String CSSSelector::selectorText(const String& rightSide) const
659660
break;
660661
default:
661662
str.appendLiteral("::");
662-
str.append(cs->value());
663+
str.append(cs->serializingValue());
663664
}
664665
} else if (cs->isAttributeSelector()) {
665666
str.append('[');
@@ -696,7 +697,7 @@ String CSSSelector::selectorText(const String& rightSide) const
696697
break;
697698
}
698699
if (cs->match() != CSSSelector::Set) {
699-
serializeString(cs->value(), str);
700+
serializeString(cs->serializingValue(), str);
700701
if (cs->attributeValueMatchingIsCaseInsensitive())
701702
str.appendLiteral(" i]");
702703
else
@@ -820,8 +821,9 @@ int CSSSelector::nthB() const
820821
return m_data.m_rareData->m_b;
821822
}
822823

823-
CSSSelector::RareData::RareData(PassRefPtr<AtomicStringImpl> value)
824-
: m_value(value.leakRef())
824+
CSSSelector::RareData::RareData(AtomicString&& value)
825+
: m_matchingValue(value)
826+
, m_serializingValue(value)
825827
, m_a(0)
826828
, m_b(0)
827829
, m_attribute(anyQName())
@@ -831,8 +833,6 @@ CSSSelector::RareData::RareData(PassRefPtr<AtomicStringImpl> value)
831833

832834
CSSSelector::RareData::~RareData()
833835
{
834-
if (m_value)
835-
m_value->deref();
836836
}
837837

838838
// a helper function for parsing nth-arguments

‎Source/WebCore/css/CSSSelector.h‎

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -233,14 +233,15 @@ namespace WebCore {
233233
const AtomicString& tagLowercaseLocalName() const;
234234

235235
const AtomicString& value() const;
236+
const AtomicString& serializingValue() const;
236237
const QualifiedName& attribute() const;
237238
const AtomicString& attributeCanonicalLocalName() const;
238239
const AtomicString& argument() const { return m_hasRareData ? m_data.m_rareData->m_argument : nullAtom; }
239240
bool attributeValueMatchingIsCaseInsensitive() const;
240241
const Vector<AtomicString>* langArgumentList() const { return m_hasRareData ? m_data.m_rareData->m_langArgumentList.get() : nullptr; }
241242
const CSSSelectorList* selectorList() const { return m_hasRareData ? m_data.m_rareData->m_selectorList.get() : nullptr; }
242243

243-
void setValue(const AtomicString&);
244+
void setValue(const AtomicString&, bool matchLowerCase = false);
244245

245246
// FIXME-NEWPARSER: These two methods can go away once the old parser is gone.
246247
void setAttribute(const QualifiedName&, bool);
@@ -348,13 +349,18 @@ namespace WebCore {
348349
CSSSelector& operator=(const CSSSelector&);
349350

350351
struct RareData : public RefCounted<RareData> {
351-
static Ref<RareData> create(PassRefPtr<AtomicStringImpl> value) { return adoptRef(*new RareData(value)); }
352+
static Ref<RareData> create(AtomicString&& value) { return adoptRef(*new RareData(WTFMove(value))); }
352353
~RareData();
353354

354355
bool parseNth();
355356
bool matchNth(int count);
356357

357-
AtomicStringImpl* m_value; // Plain pointer to keep things uniform with the union.
358+
// For quirks mode, class and id are case-insensitive. In the case where uppercase
359+
// letters are used in quirks mode, |m_matchingValue| holds the lowercase class/id
360+
// and |m_serializingValue| holds the original string.
361+
AtomicString m_matchingValue;
362+
AtomicString m_serializingValue;
363+
358364
int m_a; // Used for :nth-*
359365
int m_b; // Used for :nth-*
360366
QualifiedName m_attribute; // used for attribute selector
@@ -364,7 +370,7 @@ namespace WebCore {
364370
std::unique_ptr<CSSSelectorList> m_selectorList; // Used for :matches() and :not().
365371

366372
private:
367-
RareData(PassRefPtr<AtomicStringImpl> value);
373+
RareData(AtomicString&& value);
368374
};
369375
void createRareData();
370376

@@ -459,21 +465,24 @@ inline bool CSSSelector::isAttributeSelector() const
459465
|| match() == CSSSelector::End;
460466
}
461467

462-
inline void CSSSelector::setValue(const AtomicString& value)
468+
inline void CSSSelector::setValue(const AtomicString& value, bool matchLowerCase)
463469
{
464470
ASSERT(match() != Tag);
471+
AtomicString matchingValue = matchLowerCase ? value.convertToASCIILowercase() : value;
472+
if (!m_hasRareData && matchingValue != value)
473+
createRareData();
474+
465475
// Need to do ref counting manually for the union.
466-
if (m_hasRareData) {
467-
if (m_data.m_rareData->m_value)
468-
m_data.m_rareData->m_value->deref();
469-
m_data.m_rareData->m_value = value.impl();
470-
m_data.m_rareData->m_value->ref();
476+
if (!m_hasRareData) {
477+
if (m_data.m_value)
478+
m_data.m_value->deref();
479+
m_data.m_value = value.impl();
480+
m_data.m_value->ref();
471481
return;
472482
}
473-
if (m_data.m_value)
474-
m_data.m_value->deref();
475-
m_data.m_value = value.impl();
476-
m_data.m_value->ref();
483+
484+
m_data.m_rareData->m_matchingValue = WTFMove(matchingValue);
485+
m_data.m_rareData->m_serializingValue = value;
477486
}
478487

479488
inline CSSSelector::CSSSelector()
@@ -567,9 +576,21 @@ inline const AtomicString& CSSSelector::tagLowercaseLocalName() const
567576
inline const AtomicString& CSSSelector::value() const
568577
{
569578
ASSERT(match() != Tag);
579+
if (m_hasRareData)
580+
return m_data.m_rareData->m_matchingValue;
581+
582+
// AtomicString is really just an AtomicStringImpl* so the cast below is safe.
583+
return *reinterpret_cast<const AtomicString*>(&m_data.m_value);
584+
}
585+
586+
inline const AtomicString& CSSSelector::serializingValue() const
587+
{
588+
ASSERT(match() != Tag);
589+
if (m_hasRareData)
590+
return m_data.m_rareData->m_serializingValue;
591+
570592
// AtomicString is really just an AtomicStringImpl* so the cast below is safe.
571-
// FIXME: Perhaps call sites could be changed to accept AtomicStringImpl?
572-
return *reinterpret_cast<const AtomicString*>(m_hasRareData ? &m_data.m_rareData->m_value : &m_data.m_value);
593+
return *reinterpret_cast<const AtomicString*>(&m_data.m_value);
573594
}
574595

575596
inline void CSSSelector::setAttributeValueMatchingIsCaseInsensitive(bool isCaseInsensitive)

‎Source/WebCore/css/MediaQueryExp.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ MediaQueryExpression::MediaQueryExpression(const AtomicString& mediaFeature, CSS
289289
}
290290

291291
MediaQueryExpression::MediaQueryExpression(const String& feature, const Vector<CSSParserToken, 4>& tokenList)
292-
: m_mediaFeature(feature)
292+
: m_mediaFeature(feature.convertToASCIILowercase())
293293
, m_isValid(false)
294294
{
295295
// Create value for media query expression that must have 1 or more values.

‎Source/WebCore/css/parser/CSSParserToken.cpp‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -476,23 +476,4 @@ void CSSParserToken::serialize(StringBuilder& builder) const
476476
}
477477
}
478478

479-
template<typename CharacterType> ALWAYS_INLINE static void convertToASCIILowercaseInPlace(CharacterType* characters, unsigned length)
480-
{
481-
for (unsigned i = 0; i < length; ++i)
482-
characters[i] = toASCIILower(characters[i]);
483-
}
484-
485-
// FIXME-NEWPARSER: Would like to get rid of this operation. Blink uses HTMLParser static lowercase
486-
// string hashing, but we don't have that code in our HTMLParser.
487-
void CSSParserToken::convertToASCIILowercaseInPlace()
488-
{
489-
if (!hasStringBacking())
490-
return;
491-
492-
if (m_valueIs8Bit)
493-
WebCore::convertToASCIILowercaseInPlace(static_cast<LChar*>(m_valueDataCharRaw), m_valueLength);
494-
else
495-
WebCore::convertToASCIILowercaseInPlace(static_cast<UChar*>(m_valueDataCharRaw), m_valueLength);
496-
}
497-
498479
} // namespace WebCore

‎Source/WebCore/css/parser/CSSParserToken.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,6 @@ class CSSParserToken {
121121
return StringView(static_cast<const UChar*>(m_valueDataCharRaw), m_valueLength);
122122
}
123123

124-
void convertToASCIILowercaseInPlace();
125-
126124
UChar delimiter() const;
127125
NumericSign numericSign() const;
128126
NumericValueType numericValueType() const;

‎Source/WebCore/css/parser/CSSParserValues.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ class CSSParserSelector {
218218

219219
std::unique_ptr<CSSSelector> releaseSelector() { return WTFMove(m_selector); }
220220

221-
void setValue(const AtomicString& value) { m_selector->setValue(value); }
221+
void setValue(const AtomicString& value, bool matchLowerCase = false) { m_selector->setValue(value, matchLowerCase); }
222222

223223
// FIXME-NEWPARSER: These two methods can go away once old parser is gone.
224224
void setAttribute(const QualifiedName& value, bool isCaseInsensitive) { m_selector->setAttribute(value, isCaseInsensitive); }

0 commit comments

Comments
 (0)