Bug 1356305 - stylo: Initialize system metrics before traversing; r?bholley r?sfink draft
authorManish Goregaokar <manishearth@gmail.com>
Fri, 14 Apr 2017 09:28:25 +0800
changeset 566088 d3d4cfe0451b00f5e3486bf9366099768732bd95
parent 565261 a34919b3d942cfd4f0737d432742b0ffa9929389
child 566093 3cf6ed6f3f73a50b555bd2bf4f46e61f2d78bc62
push id55081
push userbmo:manishearth@gmail.com
push dateThu, 20 Apr 2017 20:26:35 +0000
reviewersbholley, sfink
bugs1356305
milestone55.0a1
Bug 1356305 - stylo: Initialize system metrics before traversing; r?bholley r?sfink MozReview-Commit-ID: IOeT4qSHTy5
layout/style/ServoBindings.cpp
layout/style/ServoStyleSet.cpp
layout/style/nsCSSRuleProcessor.cpp
layout/style/nsCSSRuleProcessor.h
taskcluster/scripts/builder/hazard-analysis.sh
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -645,17 +645,17 @@ bool
 Gecko_MatchStringArgPseudo(RawGeckoElementBorrowed aElement,
                            CSSPseudoClassType aType,
                            const char16_t* aIdent,
                            bool* aSetSlowSelectorFlag)
 {
   EventStates dummyMask; // mask is never read because we pass aDependence=nullptr
   return nsCSSRuleProcessor::StringPseudoMatches(aElement, aType, aIdent,
                                                  aElement->OwnerDoc(), true,
-                                                 dummyMask, false, aSetSlowSelectorFlag, nullptr);
+                                                 dummyMask, aSetSlowSelectorFlag, nullptr);
 }
 
 nsIAtom*
 Gecko_GetXMLLangValue(RawGeckoElementBorrowed aElement)
 {
   nsString string;
   if (aElement->GetAttr(kNameSpaceID_XML, nsGkAtoms::lang, string)) {
     return NS_Atomize(string).take();
--- a/layout/style/ServoStyleSet.cpp
+++ b/layout/style/ServoStyleSet.cpp
@@ -11,16 +11,17 @@
 #include "mozilla/ServoRestyleManager.h"
 #include "mozilla/dom/AnonymousContent.h"
 #include "mozilla/dom/ChildIterator.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/ElementInlines.h"
 #include "mozilla/dom/KeyframeEffectReadOnly.h"
 #include "nsCSSAnonBoxes.h"
 #include "nsCSSPseudoElements.h"
+#include "nsCSSRuleProcessor.h"
 #include "nsDeviceContext.h"
 #include "nsHTMLStyleSheet.h"
 #include "nsIDocumentInlines.h"
 #include "nsPrintfCString.h"
 #include "nsStyleContext.h"
 #include "nsStyleSet.h"
 
 using namespace mozilla;
@@ -240,16 +241,18 @@ ServoStyleSet::ResolveMappedAttrDeclarat
   mPresContext->Document()->ResolveScheduledSVGPresAttrs();
 }
 
 void
 ServoStyleSet::PreTraverseSync()
 {
   ResolveMappedAttrDeclarationBlocks();
 
+  nsCSSRuleProcessor::InitSystemMetrics();
+
   // This is lazily computed and pseudo matching needs to access
   // it so force computation early.
   mPresContext->Document()->GetDocumentState();
 
   // Ensure that the @font-face data is not stale
   mPresContext->Document()->GetUserFontSet();
 }
 
--- a/layout/style/nsCSSRuleProcessor.cpp
+++ b/layout/style/nsCSSRuleProcessor.cpp
@@ -42,16 +42,17 @@
 #include "nsCSSRules.h"
 #include "nsCSSFontFaceRule.h"
 #include "nsStyleSet.h"
 #include "mozilla/dom/Element.h"
 #include "nsNthIndexCache.h"
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/EventStates.h"
 #include "mozilla/Preferences.h"
+#include "mozilla/ServoStyleSet.h"
 #include "mozilla/LookAndFeel.h"
 #include "mozilla/Likely.h"
 #include "mozilla/OperatorNewExtensions.h"
 #include "mozilla/TypedEnumBits.h"
 #include "RuleProcessorCache.h"
 #include "nsIDOMMutationEvent.h"
 
 using namespace mozilla;
@@ -1056,23 +1057,25 @@ nsCSSRuleProcessor::ClearSheets()
 
 /* static */ void
 nsCSSRuleProcessor::Startup()
 {
   Preferences::AddBoolVarCache(&gSupportVisitedPseudo, VISITED_PSEUDO_PREF,
                                true);
 }
 
-static bool
-InitSystemMetrics()
+/* static */ void
+nsCSSRuleProcessor::InitSystemMetrics()
 {
-  NS_ASSERTION(!sSystemMetrics, "already initialized");
+  if (sSystemMetrics)
+    return;
+
+  MOZ_ASSERT(NS_IsMainThread());
 
   sSystemMetrics = new nsTArray< nsCOMPtr<nsIAtom> >;
-  NS_ENSURE_TRUE(sSystemMetrics, false);
 
   /***************************************************************************
    * ANY METRICS ADDED HERE SHOULD ALSO BE ADDED AS MEDIA QUERIES IN         *
    * nsMediaFeatures.cpp                                                     *
    ***************************************************************************/
 
   int32_t metricResult =
     LookAndFeel::GetInt(LookAndFeel::eIntID_ScrollArrowStyle);
@@ -1188,18 +1191,16 @@ InitSystemMetrics()
         sSystemMetrics->AppendElement(nsGkAtoms::windows_theme_zune);
         break;
       case LookAndFeel::eWindowsTheme_Generic:
         sSystemMetrics->AppendElement(nsGkAtoms::windows_theme_generic);
         break;
     }
   }
 #endif
-
-  return true;
 }
 
 /* static */ void
 nsCSSRuleProcessor::FreeSystemMetrics()
 {
   delete sSystemMetrics;
   sSystemMetrics = nullptr;
 }
@@ -1208,28 +1209,25 @@ nsCSSRuleProcessor::FreeSystemMetrics()
 nsCSSRuleProcessor::Shutdown()
 {
   FreeSystemMetrics();
 }
 
 /* static */ bool
 nsCSSRuleProcessor::HasSystemMetric(nsIAtom* aMetric)
 {
-  if (!sSystemMetrics && !InitSystemMetrics()) {
-    return false;
-  }
+  nsCSSRuleProcessor::InitSystemMetrics();
   return sSystemMetrics->IndexOf(aMetric) != sSystemMetrics->NoIndex;
 }
 
 #ifdef XP_WIN
 /* static */ uint8_t
 nsCSSRuleProcessor::GetWindowsThemeIdentifier()
 {
-  if (!sSystemMetrics)
-    InitSystemMetrics();
+  nsCSSRuleProcessor::InitSystemMetrics();
   return sWinThemeId;
 }
 #endif
 
 /* static */
 EventStates
 nsCSSRuleProcessor::GetContentState(Element* aElement, const TreeMatchContext& aTreeMatchContext)
 {
@@ -1634,54 +1632,52 @@ StateSelectorMatches(Element* aElement,
 }
 
 // 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)
+                                  bool aWhitespaceIsSignificant)
 {
-  if (aIsGecko) {
-    auto content = const_cast<nsIContent*>(aContent);
-    return IsSignificantChild(content, aTextIsSignificant, aWhitespaceIsSignificant);
-  } else {
+  if (ServoStyleSet::IsInServoTraversal()) {
     // See bug 1349100 for optimizing this
     return nsStyleUtil::ThreadSafeIsSignificantChild(aContent,
                                                      aTextIsSignificant,
                                                      aWhitespaceIsSignificant);
+  } else {
+    auto content = const_cast<nsIContent*>(aContent);
+    return IsSignificantChild(content, 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:
       {
         bool docIsRTL;
-        if (aIsGecko) {
+        if (ServoStyleSet::IsInServoTraversal()) {
+          docIsRTL = aDocument->ThreadSafeGetDocumentState()
+                              .HasState(NS_DOCUMENT_STATE_RTL_LOCALE);
+        } else {
           auto doc = const_cast<nsIDocument*>(aDocument);
           docIsRTL = doc->GetDocumentState()
                         .HasState(NS_DOCUMENT_STATE_RTL_LOCALE);
-        } else {
-          docIsRTL = aDocument->ThreadSafeGetDocumentState()
-                              .HasState(NS_DOCUMENT_STATE_RTL_LOCALE);
         }
 
         nsDependentString dirString(aString);
 
         if (dirString.EqualsLiteral("rtl")) {
           if (!docIsRTL) {
             return false;
           }
@@ -1717,17 +1713,17 @@ nsCSSRuleProcessor::StringPseudoMatches(
           //   :-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 &&
-                  (!IsSignificantChildMaybeThreadSafe(child, true, false, aIsGecko) ||
+                  (!IsSignificantChildMaybeThreadSafe(child, true, false) ||
                   (child->GetNameSpaceID() == aElement->GetNameSpaceID() &&
                     child->NodeInfo()->NameAtom()->Equals(nsDependentString(aString)))));
         if (child) {
           return false;
         }
       }
       break;
 
@@ -2163,17 +2159,16 @@ 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
@@ -77,16 +77,17 @@ public:
 
   NS_DECL_CYCLE_COLLECTING_ISUPPORTS
   NS_DECL_CYCLE_COLLECTION_CLASS(nsCSSRuleProcessor)
 
 public:
   nsresult ClearRuleCascades();
 
   static void Startup();
+  static void InitSystemMetrics();
   static void Shutdown();
   static void FreeSystemMetrics();
   static bool HasSystemMetric(nsIAtom* aMetric);
 
   /*
    * Returns true if the given aElement matches one of the
    * selectors in aSelectorList.  Note that this method will assume
    * the given aElement is not a relevant link.  aSelectorList must not
@@ -147,29 +148,27 @@ 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/taskcluster/scripts/builder/hazard-analysis.sh
+++ b/taskcluster/scripts/builder/hazard-analysis.sh
@@ -146,16 +146,16 @@ function check_hazards () {
     echo "TinderboxPrint: heap write hazards<br/>$NUM_WRITE_HAZARDS"
 
     if [ $NUM_HAZARDS -gt 0 ]; then
         echo "TEST-UNEXPECTED-FAIL $NUM_HAZARDS rooting hazards detected" >&2
         echo "TinderboxPrint: documentation<br/><a href='https://wiki.mozilla.org/Javascript:Hazard_Builds#Diagnosing_a_rooting_hazards_failure'>static rooting hazard analysis failures</a>, visit \"Inspect Task\" link for hazard details"
         exit 1
     fi
 
-    NUM_ALLOWED_WRITE_HAZARDS=9
+    NUM_ALLOWED_WRITE_HAZARDS=8
     if [ $NUM_WRITE_HAZARDS -gt $NUM_ALLOWED_WRITE_HAZARDS ]; then
         echo "TEST-UNEXPECTED-FAIL $NUM_WRITE_HAZARDS heap write hazards detected out of $NUM_ALLOWED_WRITE_HAZARDS allowed" >&2
         echo "TinderboxPrint: documentation<br/><a href='https://wiki.mozilla.org/Javascript:Hazard_Builds#Diagnosing_a_heap_write_hazard_failure'>heap write hazard analysis failures</a>, visit \"Inspect Task\" link for hazard details"
         exit 1
     fi
     )
 }