Diff
Modified: trunk/Source/WebCore/ChangeLog (234824 => 234825)
--- trunk/Source/WebCore/ChangeLog 2018-08-13 23:15:05 UTC (rev 234824)
+++ trunk/Source/WebCore/ChangeLog 2018-08-13 23:36:09 UTC (rev 234825)
@@ -1,3 +1,46 @@
+2018-08-13 Alex Christensen <[email protected]>
+
+ Make CSSSelectorList a little more sane
+ https://bugs.webkit.org/show_bug.cgi?id=188539
+
+ Reviewed by Simon Fraser.
+
+ This patch does four things:
+ 1. Use a UniqueArray<CSSSelector> instead of a raw pointer and manually calling destructors.
+ 2. Use move semantics a little bit better.
+ 3. Add a CSSSelectorList&& to the StyleRule and StyleRulePage because every time we create either
+ one of those objects we call a setter to give it a CSSSelectorList. That's what constructor arguments are for.
+ 4. Don't use CSSSelectorList.componentCount(), which iterates all components, to determine if it's empty.
+ Use first() instead.
+
+ * css/CSSPageRule.cpp:
+ (WebCore::CSSPageRule::setSelectorText):
+ * css/CSSSelectorList.cpp:
+ (WebCore::CSSSelectorList::CSSSelectorList):
+ (WebCore::CSSSelectorList::componentCount const):
+ (WebCore::CSSSelectorList::listSize const):
+ (WebCore::CSSSelectorList::operator=):
+ (WebCore::CSSSelectorList::deleteSelectors): Deleted.
+ * css/CSSSelectorList.h:
+ (WebCore::CSSSelectorList::CSSSelectorList):
+ (WebCore::CSSSelectorList::first const):
+ (WebCore::CSSSelectorList::indexOfNextSelectorAfter const):
+ (WebCore::CSSSelectorList::~CSSSelectorList): Deleted.
+ (WebCore::CSSSelectorList::adoptSelectorArray): Deleted.
+ (WebCore::CSSSelectorList::hasOneSelector const): Deleted.
+ * css/CSSStyleRule.cpp:
+ (WebCore::CSSStyleRule::setSelectorText):
+ * css/StyleRule.cpp:
+ (WebCore::StyleRule::StyleRule):
+ (WebCore::StyleRule::createForSplitting):
+ (WebCore::StyleRulePage::StyleRulePage):
+ * css/StyleRule.h:
+ * css/parser/CSSParserImpl.cpp:
+ (WebCore::CSSParserImpl::consumePageRule):
+ (WebCore::CSSParserImpl::consumeStyleRule):
+ * css/parser/CSSSelectorParser.cpp:
+ (WebCore::CSSSelectorParser::consumePseudo):
+
2018-08-13 Ali Juma <[email protected]>
[IntersectionObserver] Validate threshold values
Modified: trunk/Source/WebCore/css/CSSPageRule.cpp (234824 => 234825)
--- trunk/Source/WebCore/css/CSSPageRule.cpp 2018-08-13 23:15:05 UTC (rev 234824)
+++ trunk/Source/WebCore/css/CSSPageRule.cpp 2018-08-13 23:36:09 UTC (rev 234825)
@@ -77,7 +77,7 @@
CSSStyleSheet::RuleMutationScope mutationScope(this);
- m_pageRule->wrapperAdoptSelectorList(selectorList);
+ m_pageRule->wrapperAdoptSelectorList(WTFMove(selectorList));
}
String CSSPageRule::cssText() const
Modified: trunk/Source/WebCore/css/CSSSelectorList.cpp (234824 => 234825)
--- trunk/Source/WebCore/css/CSSSelectorList.cpp 2018-08-13 23:15:05 UTC (rev 234824)
+++ trunk/Source/WebCore/css/CSSSelectorList.cpp 2018-08-13 23:36:09 UTC (rev 234825)
@@ -37,15 +37,14 @@
unsigned otherComponentCount = other.componentCount();
ASSERT_WITH_SECURITY_IMPLICATION(otherComponentCount);
- m_selectorArray = reinterpret_cast<CSSSelector*>(fastMalloc(sizeof(CSSSelector) * otherComponentCount));
+ m_selectorArray = makeUniqueArray<CSSSelector>(otherComponentCount);
for (unsigned i = 0; i < otherComponentCount; ++i)
new (NotNull, &m_selectorArray[i]) CSSSelector(other.m_selectorArray[i]);
}
CSSSelectorList::CSSSelectorList(CSSSelectorList&& other)
- : m_selectorArray(other.m_selectorArray)
+ : m_selectorArray(WTFMove(other.m_selectorArray))
{
- other.m_selectorArray = nullptr;
}
CSSSelectorList::CSSSelectorList(Vector<std::unique_ptr<CSSParserSelector>>&& selectorVector)
@@ -58,7 +57,7 @@
++flattenedSize;
}
ASSERT(flattenedSize);
- m_selectorArray = reinterpret_cast<CSSSelector*>(fastMalloc(sizeof(CSSSelector) * flattenedSize));
+ m_selectorArray = makeUniqueArray<CSSSelector>(flattenedSize);
size_t arrayIndex = 0;
for (size_t i = 0; i < selectorVector.size(); ++i) {
CSSParserSelector* current = selectorVector[i].get();
@@ -87,10 +86,10 @@
{
if (!m_selectorArray)
return 0;
- CSSSelector* current = m_selectorArray;
+ CSSSelector* current = m_selectorArray.get();
while (!current->isLastInSelectorList())
++current;
- return (current - m_selectorArray) + 1;
+ return (current - m_selectorArray.get()) + 1;
}
unsigned CSSSelectorList::listSize() const
@@ -98,7 +97,7 @@
if (!m_selectorArray)
return 0;
unsigned size = 1;
- CSSSelector* current = m_selectorArray;
+ CSSSelector* current = m_selectorArray.get();
while (!current->isLastInSelectorList()) {
if (current->isLastInTagHistory())
++size;
@@ -109,29 +108,10 @@
CSSSelectorList& CSSSelectorList::operator=(CSSSelectorList&& other)
{
- deleteSelectors();
- m_selectorArray = other.m_selectorArray;
- other.m_selectorArray = nullptr;
-
+ m_selectorArray = WTFMove(other.m_selectorArray);
return *this;
}
-void CSSSelectorList::deleteSelectors()
-{
- if (!m_selectorArray)
- return;
-
- CSSSelector* selectorArray = m_selectorArray;
- m_selectorArray = nullptr;
-
- bool isLastSelector = false;
- for (CSSSelector* s = selectorArray; !isLastSelector; ++s) {
- isLastSelector = s->isLastInSelectorList();
- s->~CSSSelector();
- }
- fastFree(selectorArray);
-}
-
String CSSSelectorList::selectorsText() const
{
StringBuilder result;
Modified: trunk/Source/WebCore/css/CSSSelectorList.h (234824 => 234825)
--- trunk/Source/WebCore/css/CSSSelectorList.h 2018-08-13 23:15:05 UTC (rev 234824)
+++ trunk/Source/WebCore/css/CSSSelectorList.h 2018-08-13 23:36:09 UTC (rev 234825)
@@ -27,6 +27,7 @@
#include "CSSSelector.h"
#include <memory>
+#include <wtf/UniqueArray.h>
namespace WebCore {
@@ -35,19 +36,16 @@
class CSSSelectorList {
WTF_MAKE_FAST_ALLOCATED;
public:
- CSSSelectorList() : m_selectorArray(0) { }
+ CSSSelectorList() = default;
CSSSelectorList(const CSSSelectorList&);
CSSSelectorList(CSSSelectorList&&);
CSSSelectorList(Vector<std::unique_ptr<CSSParserSelector>>&&);
+ CSSSelectorList(UniqueArray<CSSSelector>&& array)
+ : m_selectorArray(WTFMove(array)) { }
- ~CSSSelectorList() { deleteSelectors(); }
-
- void adoptSelectorArray(CSSSelector* selectors) { ASSERT(!m_selectorArray); m_selectorArray = selectors; }
-
bool isValid() const { return !!m_selectorArray; }
- const CSSSelector* first() const { return m_selectorArray; }
+ const CSSSelector* first() const { return m_selectorArray.get(); }
static const CSSSelector* next(const CSSSelector*);
- bool hasOneSelector() const { return m_selectorArray && !next(m_selectorArray); }
const CSSSelector* selectorAt(size_t index) const { return &m_selectorArray[index]; }
size_t indexOfNextSelectorAfter(size_t index) const
@@ -56,7 +54,7 @@
current = next(current);
if (!current)
return notFound;
- return current - m_selectorArray;
+ return current - m_selectorArray.get();
}
bool selectorsNeedNamespaceResolution();
@@ -75,7 +73,7 @@
// End of a multipart selector is indicated by m_isLastInTagHistory bit in the last item.
// End of the array is indicated by m_isLastInSelectorList bit in the last item.
- CSSSelector* m_selectorArray;
+ UniqueArray<CSSSelector> m_selectorArray;
};
inline const CSSSelector* CSSSelectorList::next(const CSSSelector* current)
Modified: trunk/Source/WebCore/css/CSSStyleRule.cpp (234824 => 234825)
--- trunk/Source/WebCore/css/CSSStyleRule.cpp 2018-08-13 23:15:05 UTC (rev 234824)
+++ trunk/Source/WebCore/css/CSSStyleRule.cpp 2018-08-13 23:36:09 UTC (rev 234825)
@@ -102,7 +102,7 @@
CSSStyleSheet::RuleMutationScope mutationScope(this);
- m_styleRule->wrapperAdoptSelectorList(selectorList);
+ m_styleRule->wrapperAdoptSelectorList(WTFMove(selectorList));
if (hasCachedSelectorText()) {
selectorTextCache().remove(this);
Modified: trunk/Source/WebCore/css/StyleRule.cpp (234824 => 234825)
--- trunk/Source/WebCore/css/StyleRule.cpp 2018-08-13 23:15:05 UTC (rev 234824)
+++ trunk/Source/WebCore/css/StyleRule.cpp 2018-08-13 23:36:09 UTC (rev 234825)
@@ -181,9 +181,10 @@
return sizeof(StyleRule) + sizeof(CSSSelector) + StyleProperties::averageSizeInBytes();
}
-StyleRule::StyleRule(Ref<StylePropertiesBase>&& properties, bool hasDocumentSecurityOrigin)
+StyleRule::StyleRule(Ref<StylePropertiesBase>&& properties, bool hasDocumentSecurityOrigin, CSSSelectorList&& selectors)
: StyleRuleBase(Style, hasDocumentSecurityOrigin)
, m_properties(WTFMove(properties))
+ , m_selectorList(WTFMove(selectors))
{
}
@@ -213,13 +214,11 @@
Ref<StyleRule> StyleRule::createForSplitting(const Vector<const CSSSelector*>& selectors, Ref<StyleProperties>&& properties, bool hasDocumentSecurityOrigin)
{
ASSERT_WITH_SECURITY_IMPLICATION(!selectors.isEmpty());
- CSSSelector* selectorListArray = reinterpret_cast<CSSSelector*>(fastMalloc(sizeof(CSSSelector) * selectors.size()));
+ auto selectorListArray = makeUniqueArray<CSSSelector>(selectors.size());
for (unsigned i = 0; i < selectors.size(); ++i)
new (NotNull, &selectorListArray[i]) CSSSelector(*selectors.at(i));
selectorListArray[selectors.size() - 1].setLastInSelectorList();
- auto rule = StyleRule::create(WTFMove(properties), hasDocumentSecurityOrigin);
- rule.get().parserAdoptSelectorArray(selectorListArray);
- return rule;
+ return StyleRule::create(WTFMove(properties), hasDocumentSecurityOrigin, WTFMove(selectorListArray));
}
Vector<RefPtr<StyleRule>> StyleRule::splitIntoMultipleRulesWithMaximumSelectorComponentCount(unsigned maxCount) const
@@ -248,9 +247,10 @@
return rules;
}
-StyleRulePage::StyleRulePage(Ref<StyleProperties>&& properties)
+StyleRulePage::StyleRulePage(Ref<StyleProperties>&& properties, CSSSelectorList&& selectors)
: StyleRuleBase(Page)
, m_properties(WTFMove(properties))
+ , m_selectorList(WTFMove(selectors))
{
}
Modified: trunk/Source/WebCore/css/StyleRule.h (234824 => 234825)
--- trunk/Source/WebCore/css/StyleRule.h 2018-08-13 23:15:05 UTC (rev 234824)
+++ trunk/Source/WebCore/css/StyleRule.h 2018-08-13 23:36:09 UTC (rev 234825)
@@ -118,9 +118,9 @@
class StyleRule final : public StyleRuleBase {
WTF_MAKE_FAST_ALLOCATED;
public:
- static Ref<StyleRule> create(Ref<StylePropertiesBase>&& properties, bool hasDocumentSecurityOrigin)
+ static Ref<StyleRule> create(Ref<StylePropertiesBase>&& properties, bool hasDocumentSecurityOrigin, CSSSelectorList&& selectors)
{
- return adoptRef(*new StyleRule(WTFMove(properties), hasDocumentSecurityOrigin));
+ return adoptRef(*new StyleRule(WTFMove(properties), hasDocumentSecurityOrigin, WTFMove(selectors)));
}
~StyleRule();
@@ -133,7 +133,7 @@
using StyleRuleBase::hasDocumentSecurityOrigin;
- void wrapperAdoptSelectorList(CSSSelectorList& selectors)
+ void wrapperAdoptSelectorList(CSSSelectorList&& selectors)
{
m_selectorList = WTFMove(selectors);
#if ENABLE(CSS_SELECTOR_JIT)
@@ -140,7 +140,6 @@
m_compiledSelectors = nullptr;
#endif
}
- void parserAdoptSelectorArray(CSSSelector* selectors) { m_selectorList.adoptSelectorArray(selectors); }
Ref<StyleRule> copy() const { return adoptRef(*new StyleRule(*this)); }
@@ -162,7 +161,7 @@
static unsigned averageSizeInBytes();
private:
- StyleRule(Ref<StylePropertiesBase>&&, bool hasDocumentSecurityOrigin);
+ StyleRule(Ref<StylePropertiesBase>&&, bool hasDocumentSecurityOrigin, CSSSelectorList&&);
StyleRule(const StyleRule&);
static Ref<StyleRule> createForSplitting(const Vector<const CSSSelector*>&, Ref<StyleProperties>&&, bool hasDocumentSecurityOrigin);
@@ -200,7 +199,7 @@
class StyleRulePage final : public StyleRuleBase {
public:
- static Ref<StyleRulePage> create(Ref<StyleProperties>&& properties) { return adoptRef(*new StyleRulePage(WTFMove(properties))); }
+ static Ref<StyleRulePage> create(Ref<StyleProperties>&& properties, CSSSelectorList&& selectors) { return adoptRef(*new StyleRulePage(WTFMove(properties), WTFMove(selectors))); }
~StyleRulePage();
@@ -208,12 +207,12 @@
const StyleProperties& properties() const { return m_properties; }
MutableStyleProperties& mutableProperties();
- void wrapperAdoptSelectorList(CSSSelectorList& selectors) { m_selectorList = WTFMove(selectors); }
+ void wrapperAdoptSelectorList(CSSSelectorList&& selectors) { m_selectorList = WTFMove(selectors); }
Ref<StyleRulePage> copy() const { return adoptRef(*new StyleRulePage(*this)); }
private:
- explicit StyleRulePage(Ref<StyleProperties>&&);
+ explicit StyleRulePage(Ref<StyleProperties>&&, CSSSelectorList&&);
StyleRulePage(const StyleRulePage&);
Ref<StyleProperties> m_properties;
Modified: trunk/Source/WebCore/css/parser/CSSParserImpl.cpp (234824 => 234825)
--- trunk/Source/WebCore/css/parser/CSSParserImpl.cpp 2018-08-13 23:15:05 UTC (rev 234824)
+++ trunk/Source/WebCore/css/parser/CSSParserImpl.cpp 2018-08-13 23:36:09 UTC (rev 234825)
@@ -668,9 +668,7 @@
consumeDeclarationList(block, StyleRule::Style);
- RefPtr<StyleRulePage> page = StyleRulePage::create(createStyleProperties(m_parsedProperties, m_context.mode));
- page->wrapperAdoptSelectorList(selectorList);
- return page;
+ return StyleRulePage::create(createStyleProperties(m_parsedProperties, m_context.mode), WTFMove(selectorList));
}
// FIXME-NEWPARSER: Support "apply"
@@ -726,7 +724,6 @@
if (!selectorList.isValid())
return nullptr; // Parse error, invalid selector list
- RefPtr<StyleRule> rule;
if (m_observerWrapper)
observeSelectors(*m_observerWrapper, prelude);
@@ -738,16 +735,12 @@
CSSParserTokenRange blockCopy = block;
blockCopy.consumeWhitespace();
if (!blockCopy.atEnd()) {
- rule = StyleRule::create(createDeferredStyleProperties(block), m_context.hasDocumentSecurityOrigin);
- rule->wrapperAdoptSelectorList(selectorList);
- return rule;
+ return StyleRule::create(createDeferredStyleProperties(block), m_context.hasDocumentSecurityOrigin, WTFMove(selectorList));
}
}
consumeDeclarationList(block, StyleRule::Style);
- rule = StyleRule::create(createStyleProperties(m_parsedProperties, m_context.mode), m_context.hasDocumentSecurityOrigin);
- rule->wrapperAdoptSelectorList(selectorList);
- return rule;
+ return StyleRule::create(createStyleProperties(m_parsedProperties, m_context.mode), m_context.hasDocumentSecurityOrigin, WTFMove(selectorList));
}
void CSSParserImpl::consumeDeclarationList(CSSParserTokenRange range, StyleRule::Type ruleType)
Modified: trunk/Source/WebCore/css/parser/CSSSelectorParser.cpp (234824 => 234825)
--- trunk/Source/WebCore/css/parser/CSSSelectorParser.cpp 2018-08-13 23:15:05 UTC (rev 234824)
+++ trunk/Source/WebCore/css/parser/CSSSelectorParser.cpp 2018-08-13 23:36:09 UTC (rev 234825)
@@ -530,7 +530,7 @@
DisallowPseudoElementsScope scope(this);
std::unique_ptr<CSSSelectorList> selectorList = std::unique_ptr<CSSSelectorList>(new CSSSelectorList());
*selectorList = consumeComplexSelectorList(block);
- if (!selectorList->componentCount() || !block.atEnd())
+ if (!selectorList->first() || !block.atEnd())
return nullptr;
selector->setSelectorList(WTFMove(selectorList));
return selector;
@@ -559,7 +559,7 @@
block.consumeWhitespace();
std::unique_ptr<CSSSelectorList> selectorList = std::unique_ptr<CSSSelectorList>(new CSSSelectorList());
*selectorList = consumeComplexSelectorList(block);
- if (!selectorList->componentCount() || !block.atEnd())
+ if (!selectorList->first() || !block.atEnd())
return nullptr;
selector->setSelectorList(WTFMove(selectorList));
}
@@ -577,7 +577,7 @@
case CSSSelector::PseudoClassMatches: {
std::unique_ptr<CSSSelectorList> selectorList = std::unique_ptr<CSSSelectorList>(new CSSSelectorList());
*selectorList = consumeComplexSelectorList(block);
- if (!selectorList->componentCount() || !block.atEnd())
+ if (!selectorList->first() || !block.atEnd())
return nullptr;
selector->setSelectorList(WTFMove(selectorList));
return selector;
@@ -586,7 +586,7 @@
case CSSSelector::PseudoClassHost: {
std::unique_ptr<CSSSelectorList> selectorList = std::unique_ptr<CSSSelectorList>(new CSSSelectorList());
*selectorList = consumeCompoundSelectorList(block);
- if (!selectorList->componentCount() || !block.atEnd())
+ if (!selectorList->first() || !block.atEnd())
return nullptr;
selector->setSelectorList(WTFMove(selectorList));
return selector;
