Bug 1341086 - Part 3: stylo: Add thread-safe version of nsStyleUtil::IsSignificantChild(); r?emilio
MozReview-Commit-ID: 3l8cNwDHKFl
--- a/dom/base/FragmentOrElement.cpp
+++ b/dom/base/FragmentOrElement.cpp
@@ -2067,16 +2067,22 @@ FragmentOrElement::AppendText(const char
bool
FragmentOrElement::TextIsOnlyWhitespace()
{
return false;
}
bool
+FragmentOrElement::ThreadSafeTextIsOnlyWhitespace() const
+{
+ return false;
+}
+
+bool
FragmentOrElement::HasTextForTranslation()
{
return false;
}
void
FragmentOrElement::AppendTextTo(nsAString& aResult)
{
--- a/dom/base/FragmentOrElement.h
+++ b/dom/base/FragmentOrElement.h
@@ -138,16 +138,17 @@ public:
// Need to implement this here too to avoid hiding.
nsresult SetText(const nsAString& aStr, bool aNotify)
{
return SetText(aStr.BeginReading(), aStr.Length(), aNotify);
}
virtual nsresult AppendText(const char16_t* aBuffer, uint32_t aLength,
bool aNotify) override;
virtual bool TextIsOnlyWhitespace() override;
+ virtual bool ThreadSafeTextIsOnlyWhitespace() const override;
virtual bool HasTextForTranslation() override;
virtual void AppendTextTo(nsAString& aResult) override;
MOZ_MUST_USE
virtual bool AppendTextTo(nsAString& aResult, const mozilla::fallible_t&) override;
virtual nsIContent *GetBindingParent() const override;
virtual nsXBLBinding *GetXBLBinding() const override;
virtual void SetXBLBinding(nsXBLBinding* aBinding,
nsBindingManager* aOldBindingManager = nullptr) override;
--- a/dom/base/nsGenericDOMDataNode.cpp
+++ b/dom/base/nsGenericDOMDataNode.cpp
@@ -978,16 +978,31 @@ nsGenericDOMDataNode::AppendText(const c
bool aNotify)
{
return SetTextInternal(mText.GetLength(), 0, aBuffer, aLength, aNotify);
}
bool
nsGenericDOMDataNode::TextIsOnlyWhitespace()
{
+
+ MOZ_ASSERT(NS_IsMainThread());
+ if (!ThreadSafeTextIsOnlyWhitespace()) {
+ UnsetFlags(NS_TEXT_IS_ONLY_WHITESPACE);
+ SetFlags(NS_CACHED_TEXT_IS_ONLY_WHITESPACE);
+ return false;
+ }
+
+ SetFlags(NS_CACHED_TEXT_IS_ONLY_WHITESPACE | NS_TEXT_IS_ONLY_WHITESPACE);
+ return true;
+}
+
+bool
+nsGenericDOMDataNode::ThreadSafeTextIsOnlyWhitespace() const
+{
// FIXME: should this method take content language into account?
if (mText.Is2b()) {
// The fragment contains non-8bit characters and such characters
// are never considered whitespace.
return false;
}
if (HasFlag(NS_CACHED_TEXT_IS_ONLY_WHITESPACE)) {
@@ -996,25 +1011,22 @@ nsGenericDOMDataNode::TextIsOnlyWhitespa
const char* cp = mText.Get1b();
const char* end = cp + mText.GetLength();
while (cp < end) {
char ch = *cp;
if (!dom::IsSpaceCharacter(ch)) {
- UnsetFlags(NS_TEXT_IS_ONLY_WHITESPACE);
- SetFlags(NS_CACHED_TEXT_IS_ONLY_WHITESPACE);
return false;
}
++cp;
}
- SetFlags(NS_CACHED_TEXT_IS_ONLY_WHITESPACE | NS_TEXT_IS_ONLY_WHITESPACE);
return true;
}
bool
nsGenericDOMDataNode::HasTextForTranslation()
{
if (NodeType() != nsIDOMNode::TEXT_NODE &&
NodeType() != nsIDOMNode::CDATA_SECTION_NODE) {
--- a/dom/base/nsGenericDOMDataNode.h
+++ b/dom/base/nsGenericDOMDataNode.h
@@ -131,16 +131,17 @@ public:
// Need to implement this here too to avoid hiding.
nsresult SetText(const nsAString& aStr, bool aNotify)
{
return SetText(aStr.BeginReading(), aStr.Length(), aNotify);
}
virtual nsresult AppendText(const char16_t* aBuffer, uint32_t aLength,
bool aNotify) override;
virtual bool TextIsOnlyWhitespace() override;
+ virtual bool ThreadSafeTextIsOnlyWhitespace() const final;
virtual bool HasTextForTranslation() override;
virtual void AppendTextTo(nsAString& aResult) override;
MOZ_MUST_USE
virtual bool AppendTextTo(nsAString& aResult,
const mozilla::fallible_t&) override;
virtual void SaveSubtreeState() override;
#ifdef DEBUG
--- a/dom/base/nsIContent.h
+++ b/dom/base/nsIContent.h
@@ -554,16 +554,21 @@ public:
/**
* Query method to see if the frame is nothing but whitespace
* NOTE: Always returns false for elements
*/
virtual bool TextIsOnlyWhitespace() = 0;
/**
+ * Thread-safe version of TextIsOnlyWhitespace.
+ */
+ virtual bool ThreadSafeTextIsOnlyWhitespace() const = 0;
+
+ /**
* Method to see if the text node contains data that is useful
* for a translation: i.e., it consists of more than just whitespace,
* digits and punctuation.
* NOTE: Always returns false for elements.
*/
virtual bool HasTextForTranslation() = 0;
/**
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -492,21 +492,20 @@ nscolor Gecko_GetLookAndFeelSystemColor(
}
bool
Gecko_MatchStringArgPseudo(RawGeckoElementBorrowed aElement,
CSSPseudoClassType aType,
const char16_t* aIdent,
bool* aSetSlowSelectorFlag)
{
- MOZ_ASSERT(aElement->OwnerDoc() == aElement->GetComposedDoc());
EventStates dummyMask; // mask is never read because we pass aDependence=nullptr
return nsCSSRuleProcessor::StringPseudoMatches(aElement, aType, aIdent,
aElement->OwnerDoc(), true,
- dummyMask, aSetSlowSelectorFlag, nullptr);
+ dummyMask, false, aSetSlowSelectorFlag, nullptr);
}
template <typename Implementor>
static nsIAtom*
AtomAttrValue(Implementor* aElement, nsIAtom* aName)
{
const nsAttrValue* attr = aElement->GetParsedAttr(aName);
return attr ? attr->GetAtomValue() : nullptr;
--- a/layout/style/nsCSSRuleProcessor.cpp
+++ b/layout/style/nsCSSRuleProcessor.cpp
@@ -1632,23 +1632,44 @@ StateSelectorMatches(Element* aElement,
aTreeMatchContext, aSelectorFlags, nullptr,
statesToCheck)) {
return false;
}
}
return true;
}
+// Chooses the thread safe version in Servo mode, and
+// the non-thread safe one in Gecko mode. The non thread safe one does
+// some extra caching, and is preferred when possible.
+static inline bool
+IsSignificantChildMaybeThreadSafe(const nsIContent* aContent,
+ bool aTextIsSignificant,
+ bool aWhitespaceIsSignificant,
+ bool aIsGecko)
+{
+ if (aIsGecko) {
+ auto content = const_cast<nsIContent*>(aContent);
+ return IsSignificantChild(content, aTextIsSignificant, aWhitespaceIsSignificant);
+ } else {
+ // See bug 1349100 for optimizing this
+ return nsStyleUtil::ThreadSafeIsSignificantChild(aContent,
+ aTextIsSignificant,
+ aWhitespaceIsSignificant);
+ }
+}
+
/* static */ bool
nsCSSRuleProcessor::StringPseudoMatches(const mozilla::dom::Element* aElement,
CSSPseudoClassType aPseudo,
const char16_t* aString,
const nsIDocument* aDocument,
bool aForStyling,
EventStates aStateMask,
+ bool aIsGecko,
bool* aSetSlowSelectorFlag,
bool* const aDependence)
{
MOZ_ASSERT(aSetSlowSelectorFlag);
switch (aPseudo) {
case CSSPseudoClassType::mozLocaleDir:
{
@@ -1686,31 +1707,31 @@ nsCSSRuleProcessor::StringPseudoMatches(
return false;
}
}
break;
case CSSPseudoClassType::mozEmptyExceptChildrenWithLocalname:
{
NS_ASSERTION(aString, "Must have string!");
- nsIContent *child = nullptr;
+ const nsIContent *child = nullptr;
int32_t index = -1;
if (aForStyling) {
// FIXME: This isn't sufficient to handle:
// :-moz-empty-except-children-with-localname() + E
// :-moz-empty-except-children-with-localname() ~ E
// because we don't know to restyle the grandparent of the
// inserted/removed element (as in bug 534804 for :empty).
*aSetSlowSelectorFlag = true;
}
do {
child = aElement->GetChildAt(++index);
} while (child &&
- (!IsSignificantChild(child, true, false) ||
+ (!IsSignificantChildMaybeThreadSafe(child, true, false, aIsGecko) ||
(child->GetNameSpaceID() == aElement->GetNameSpaceID() &&
child->NodeInfo()->NameAtom()->Equals(nsDependentString(aString)))));
if (child) {
return false;
}
}
break;
@@ -2152,16 +2173,17 @@ static bool SelectorMatches(Element* aEl
MOZ_ASSERT(nsCSSPseudoClasses::HasStringArg(pseudoClass->mType));
bool setSlowSelectorFlag = false;
bool matched = nsCSSRuleProcessor::StringPseudoMatches(aElement,
pseudoClass->mType,
pseudoClass->u.mString,
aTreeMatchContext.mDocument,
aTreeMatchContext.mForStyling,
aNodeMatchContext.mStateMask,
+ true,
&setSlowSelectorFlag,
aDependence);
if (setSlowSelectorFlag) {
aElement->SetFlags(NODE_HAS_SLOW_SELECTOR);
}
if (!matched) {
return false;
--- a/layout/style/nsCSSRuleProcessor.h
+++ b/layout/style/nsCSSRuleProcessor.h
@@ -147,25 +147,29 @@ public:
* @param aElement The element we are trying to match
* @param aPseudo The name of the pseudoselector
* @param aString The identifier inside the pseudoselector (cannot be null)
* @param aDocument The document
* @param aForStyling Is this matching operation for the creation of a style context?
* (For setting the slow selector flag)
* @param aStateMask Mask containing states which we should exclude.
* Ignored if aDependence is null
+ * @param aIsGecko Set if Gecko.
+ * @param aSetSlowSelectorFlag Outparam, set if the caller is
+ * supposed to set the slow selector flag.
* @param aDependence Pointer to be set to true if we ignored a state due to
* aStateMask. Can be null.
*/
static bool StringPseudoMatches(const mozilla::dom::Element* aElement,
mozilla::CSSPseudoClassType aPseudo,
const char16_t* aString,
const nsIDocument* aDocument,
bool aForStyling,
mozilla::EventStates aStateMask,
+ bool aIsGecko,
bool* aSetSlowSelectorFlag,
bool* const aDependence = nullptr);
// nsIStyleRuleProcessor
virtual void RulesMatching(ElementRuleProcessorData* aData) override;
virtual void RulesMatching(PseudoElementRuleProcessorData* aData) override;
--- a/layout/style/nsStyleUtil.cpp
+++ b/layout/style/nsStyleUtil.cpp
@@ -736,16 +736,36 @@ nsStyleUtil::IsSignificantChild(nsIConte
return true;
}
return aTextIsSignificant && isText && aChild->TextLength() != 0 &&
(aWhitespaceIsSignificant ||
!aChild->TextIsOnlyWhitespace());
}
+/* static */ bool
+nsStyleUtil::ThreadSafeIsSignificantChild(const nsIContent* aChild,
+ bool aTextIsSignificant,
+ bool aWhitespaceIsSignificant)
+{
+ NS_ASSERTION(!aWhitespaceIsSignificant || aTextIsSignificant,
+ "Nonsensical arguments");
+
+ bool isText = aChild->IsNodeOfType(nsINode::eTEXT);
+
+ if (!isText && !aChild->IsNodeOfType(nsINode::eCOMMENT) &&
+ !aChild->IsNodeOfType(nsINode::ePROCESSING_INSTRUCTION)) {
+ return true;
+ }
+
+ return aTextIsSignificant && isText && aChild->TextLength() != 0 &&
+ (aWhitespaceIsSignificant ||
+ !aChild->ThreadSafeTextIsOnlyWhitespace());
+}
+
// For a replaced element whose concrete object size is no larger than the
// element's content-box, this method checks whether the given
// "object-position" coordinate might cause overflow in its dimension.
static bool
ObjectPositionCoordMightCauseOverflow(const Position::Coord& aCoord)
{
// Any nonzero length in "object-position" can push us to overflow
// (particularly if our concrete object size is exactly the same size as the
--- a/layout/style/nsStyleUtil.h
+++ b/layout/style/nsStyleUtil.h
@@ -133,16 +133,23 @@ public:
static float ColorComponentToFloat(uint8_t aAlpha);
/*
* Does this child count as significant for selector matching?
*/
static bool IsSignificantChild(nsIContent* aChild,
bool aTextIsSignificant,
bool aWhitespaceIsSignificant);
+
+ /*
+ * Thread-safe version of IsSignificantChild()
+ */
+ static bool ThreadSafeIsSignificantChild(const nsIContent* aChild,
+ bool aTextIsSignificant,
+ bool aWhitespaceIsSignificant);
/**
* Returns true if our object-fit & object-position properties might cause
* a replaced element's contents to overflow its content-box (requiring
* clipping), or false if we can be sure that this won't happen.
*
* This lets us optimize by skipping clipping when we can tell it's
* unnecessary (particularly with the default values of these properties).
*