Bug 1352968 part 8 - Construct @import rule object eagerly. r=heycam
authorXidorn Quan <me@upsuper.org>
Tue, 30 May 2017 11:10:25 +1000
changeset 409278 80ccd721253248f8a4cc0ea35104e6dae66c34e3
parent 409277 74b069ba31d6a5c209d0e4e87b5fe439ac4219aa
child 409279 3a5052f5e48f162881c8f1e3387b265c3e9164bc
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1352968
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1352968 part 8 - Construct @import rule object eagerly. r=heycam MozReview-Commit-ID: HrgZnW21dHz
layout/style/ServoBindingList.h
layout/style/ServoCSSRuleList.cpp
layout/style/ServoCSSRuleList.h
layout/style/ServoImportRule.cpp
layout/style/ServoImportRule.h
layout/style/StyleSheet.h
--- a/layout/style/ServoBindingList.h
+++ b/layout/style/ServoBindingList.h
@@ -139,16 +139,18 @@ SERVO_BINDING_FUNC(Servo_StyleRule_GetSt
                    RawServoStyleRuleBorrowed rule)
 SERVO_BINDING_FUNC(Servo_StyleRule_SetStyle, void,
                    RawServoStyleRuleBorrowed rule,
                    RawServoDeclarationBlockBorrowed declarations)
 SERVO_BINDING_FUNC(Servo_StyleRule_GetSelectorText, void,
                    RawServoStyleRuleBorrowed rule, nsAString* result)
 SERVO_BINDING_FUNC(Servo_ImportRule_GetHref, void,
                    RawServoImportRuleBorrowed rule, nsAString* result)
+SERVO_BINDING_FUNC(Servo_ImportRule_GetSheet, const RawServoStyleSheet*,
+                   RawServoImportRuleBorrowed rule)
 SERVO_BINDING_FUNC(Servo_Keyframe_GetKeyText, void,
                    RawServoKeyframeBorrowed keyframe, nsAString* result)
 // Returns whether it successfully changes the key text.
 SERVO_BINDING_FUNC(Servo_Keyframe_SetKeyText, bool,
                    RawServoKeyframeBorrowed keyframe, const nsACString* text)
 SERVO_BINDING_FUNC(Servo_Keyframe_GetStyle, RawServoDeclarationBlockStrong,
                    RawServoKeyframeBorrowed keyframe)
 SERVO_BINDING_FUNC(Servo_Keyframe_SetStyle, void,
--- a/layout/style/ServoCSSRuleList.cpp
+++ b/layout/style/ServoCSSRuleList.cpp
@@ -3,39 +3,65 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* representation of CSSRuleList for stylo */
 
 #include "mozilla/ServoCSSRuleList.h"
 
+#include "mozilla/IntegerRange.h"
 #include "mozilla/ServoBindings.h"
 #include "mozilla/ServoDocumentRule.h"
 #include "mozilla/ServoImportRule.h"
 #include "mozilla/ServoKeyframesRule.h"
 #include "mozilla/ServoMediaRule.h"
 #include "mozilla/ServoNamespaceRule.h"
 #include "mozilla/ServoPageRule.h"
 #include "mozilla/ServoStyleRule.h"
+#include "mozilla/ServoStyleSheet.h"
 #include "mozilla/ServoSupportsRule.h"
 #include "nsCSSCounterStyleRule.h"
 #include "nsCSSFontFaceRule.h"
 
 namespace mozilla {
 
 ServoCSSRuleList::ServoCSSRuleList(already_AddRefed<ServoCssRules> aRawRules,
                                    ServoStyleSheet* aDirectOwnerStyleSheet)
   : mStyleSheet(aDirectOwnerStyleSheet)
   , mRawRules(aRawRules)
 {
   Servo_CssRules_ListTypes(mRawRules, &mRules);
-  // XXX We may want to eagerly create object for import rule, so that
-  //     we don't lose the reference to child stylesheet when our own
-  //     stylesheet goes away.
+  // Only top level rule list can have @import rules.
+  if (aDirectOwnerStyleSheet) {
+    nsDataHashtable<nsPtrHashKey<const RawServoStyleSheet>,
+                    ServoStyleSheet*> stylesheets;
+    aDirectOwnerStyleSheet->EnumerateChildSheets(
+      [&stylesheets](StyleSheet* child) {
+        ServoStyleSheet* servoSheet = child->AsServo();
+        const RawServoStyleSheet* rawSheet = servoSheet->RawSheet();
+        MOZ_ASSERT(!stylesheets.Get(rawSheet, nullptr),
+                   "Multiple child sheets with same raw sheet?");
+        stylesheets.Put(rawSheet, servoSheet);
+      });
+    for (auto i : IntegerRange(mRules.Length())) {
+      if (mRules[i] != nsIDOMCSSRule::IMPORT_RULE) {
+        // Only @charset can be put before @import rule, but @charset
+        // rules don't have corresponding object, so if a rule is not
+        // @import rule, there is definitely no @import rule after it.
+        break;
+      }
+      ConstructImportRule(i, [&stylesheets](const RawServoStyleSheet* raw) {
+        // Child sheets may not correctly cloned for stylo, which is
+        // bug 1367213. That makes it possible to fail to get a style
+        // sheet for the raw sheet here.
+        return stylesheets.GetAndRemove(raw).valueOr(nullptr);
+      });
+    }
+  }
 }
 
 // QueryInterface implementation for ServoCSSRuleList
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(ServoCSSRuleList)
 NS_INTERFACE_MAP_END_INHERITING(dom::CSSRuleList)
 
 NS_IMPL_ADDREF_INHERITED(ServoCSSRuleList, dom::CSSRuleList)
 NS_IMPL_RELEASE_INHERITED(ServoCSSRuleList, dom::CSSRuleList)
@@ -96,17 +122,16 @@ ServoCSSRuleList::GetRule(uint32_t aInde
         RefPtr<RawServo##name_##Rule> rule =                                \
           Servo_CssRules_Get##name_##RuleAt(                                \
               mRawRules, aIndex, &line, &column                             \
           ).Consume();                                                      \
         ruleObj = new Servo##name_##Rule(rule.forget(), line, column);      \
         break;                                                              \
       }
       CASE_RULE(STYLE, Style)
-      CASE_RULE(IMPORT, Import)
       CASE_RULE(KEYFRAMES, Keyframes)
       CASE_RULE(MEDIA, Media)
       CASE_RULE(NAMESPACE, Namespace)
       CASE_RULE(PAGE, Page)
       CASE_RULE(SUPPORTS, Supports)
       CASE_RULE(DOCUMENT, Document)
 #undef CASE_RULE
       // For @font-face and @counter-style rules, the function returns
@@ -116,16 +141,22 @@ ServoCSSRuleList::GetRule(uint32_t aInde
       case nsIDOMCSSRule::FONT_FACE_RULE: {
         ruleObj = Servo_CssRules_GetFontFaceRuleAt(mRawRules, aIndex);
         break;
       }
       case nsIDOMCSSRule::COUNTER_STYLE_RULE: {
         ruleObj = Servo_CssRules_GetCounterStyleRuleAt(mRawRules, aIndex);
         break;
       }
+      case nsIDOMCSSRule::IMPORT_RULE:
+        // Currently ConstructImportRule may fail to construct an import
+        // rule eagerly. See comment in that function. This should be
+        // converted into an assertion when those bugs get fixed.
+        NS_WARNING("stylo: this @import rule was not constructed");
+        return nullptr;
       case nsIDOMCSSRule::KEYFRAME_RULE:
         MOZ_ASSERT_UNREACHABLE("keyframe rule cannot be here");
         return nullptr;
       default:
         NS_WARNING("stylo: not implemented yet");
         return nullptr;
     }
     ruleObj->SetStyleSheet(mStyleSheet);
@@ -179,33 +210,82 @@ ServoCSSRuleList::DropAllRules()
 void
 ServoCSSRuleList::DropReference()
 {
   mStyleSheet = nullptr;
   mParentRule = nullptr;
   DropAllRules();
 }
 
+template<typename ChildSheetGetter>
+void
+ServoCSSRuleList::ConstructImportRule(uint32_t aIndex, ChildSheetGetter aGetter)
+{
+  MOZ_ASSERT(mRules[aIndex] == nsIDOMCSSRule::IMPORT_RULE);
+
+  uint32_t line, column;
+  RefPtr<RawServoImportRule> rawRule =
+    Servo_CssRules_GetImportRuleAt(mRawRules, aIndex,
+                                   &line, &column).Consume();
+  const RawServoStyleSheet*
+    rawChildSheet = Servo_ImportRule_GetSheet(rawRule);
+  ServoStyleSheet* childSheet = aGetter(rawChildSheet);
+  if (!childSheet) {
+    // There are cases that we cannot get the child sheet currently.
+    // See comments in callsites of this function. This should become
+    // an assertion after bug 1367213 and bug 1368381 get fixed.
+    NS_WARNING("stylo: fail to get child sheet for @import rule");
+    return;
+  }
+  RefPtr<ServoImportRule>
+    ruleObj = new ServoImportRule(Move(rawRule), childSheet, line, column);
+  MOZ_ASSERT(!childSheet->GetOwnerRule(),
+             "Child sheet is already owned by another rule?");
+  MOZ_ASSERT(childSheet->GetParentSheet() == mStyleSheet,
+             "Not a child sheet of the owner of the rule?");
+  childSheet->SetOwnerRule(ruleObj);
+  mRules[aIndex] = CastToUint(ruleObj.forget().take());
+}
+
 nsresult
 ServoCSSRuleList::InsertRule(const nsAString& aRule, uint32_t aIndex)
 {
   MOZ_ASSERT(mStyleSheet, "Caller must ensure that "
              "the list is not unlinked from stylesheet");
   NS_ConvertUTF16toUTF8 rule(aRule);
   bool nested = !!mParentRule;
   css::Loader* loader = nullptr;
   if (nsIDocument* doc = mStyleSheet->GetAssociatedDocument()) {
     loader = doc->CSSLoader();
   }
   uint16_t type;
   nsresult rv = Servo_CssRules_InsertRule(mRawRules, mStyleSheet->RawSheet(),
                                           &rule, aIndex, nested,
                                           loader, mStyleSheet, &type);
-  if (!NS_FAILED(rv)) {
-    mRules.InsertElementAt(aIndex, type);
+  if (NS_FAILED(rv)) {
+    return rv;
+  }
+  mRules.InsertElementAt(aIndex, type);
+  if (type == nsIDOMCSSRule::IMPORT_RULE) {
+    MOZ_ASSERT(!nested, "@import rule cannot be nested");
+    ConstructImportRule(aIndex, [this](const RawServoStyleSheet* raw) {
+      StyleSheet* sheet = mStyleSheet->GetMostRecentlyAddedChildSheet();
+      MOZ_ASSERT(sheet, "Should have at least one "
+                 "child stylesheet after inserting @import rule");
+      ServoStyleSheet* servoSheet = sheet->AsServo();
+      // This should always be that case, but currently ServoStyleSheet
+      // may be reused and the reused stylesheet doesn't refer to the
+      // right raw sheet, which is bug 1368381. This should be converted
+      // to an assertion after that bug gets fixed.
+      if (servoSheet->RawSheet() == raw) {
+        NS_WARNING("New child sheet should always be prepended to the list");
+        return static_cast<ServoStyleSheet*>(nullptr);
+      }
+      return servoSheet;
+    });
   }
   return rv;
 }
 
 nsresult
 ServoCSSRuleList::DeleteRule(uint32_t aIndex)
 {
   nsresult rv = Servo_CssRules_DeleteRule(mRawRules, aIndex);
--- a/layout/style/ServoCSSRuleList.h
+++ b/layout/style/ServoCSSRuleList.h
@@ -70,16 +70,19 @@ private:
     return reinterpret_cast<css::Rule*>(aInt);
   }
 
   template<typename Func>
   void EnumerateInstantiatedRules(Func aCallback);
 
   void DropAllRules();
 
+  template<typename ChildSheetGetter>
+  inline void ConstructImportRule(uint32_t aIndex, ChildSheetGetter aGetter);
+
   // mStyleSheet may be nullptr when it drops the reference to us.
   ServoStyleSheet* mStyleSheet = nullptr;
   // mParentRule is nullptr if it isn't a nested rule list.
   css::GroupRule* mParentRule = nullptr;
   RefPtr<ServoCssRules> mRawRules;
   // Array stores either a number indicating rule type, or a pointer to
   // css::Rule. If the value is less than kMaxRuleType, the given rule
   // instance has not been constructed, and the value means the type
--- a/layout/style/ServoImportRule.cpp
+++ b/layout/style/ServoImportRule.cpp
@@ -9,20 +9,21 @@
 #include "mozilla/ServoImportRule.h"
 
 #include "mozilla/ServoBindings.h"
 #include "mozilla/ServoStyleSheet.h"
 
 namespace mozilla {
 
 ServoImportRule::ServoImportRule(RefPtr<RawServoImportRule> aRawRule,
+                                 ServoStyleSheet* aSheet,
                                  uint32_t aLine, uint32_t aColumn)
   : CSSImportRule(aLine, aColumn)
   , mRawRule(Move(aRawRule))
-  , mChildSheet(nullptr)
+  , mChildSheet(aSheet)
 {
 }
 
 ServoImportRule::~ServoImportRule()
 {
   if (mChildSheet) {
     mChildSheet->SetOwnerRule(nullptr);
   }
--- a/layout/style/ServoImportRule.h
+++ b/layout/style/ServoImportRule.h
@@ -16,16 +16,17 @@ namespace mozilla {
 
 class ServoStyleSheet;
 class ServoMediaList;
 
 class ServoImportRule final : public dom::CSSImportRule
 {
 public:
   ServoImportRule(RefPtr<RawServoImportRule> aRawRule,
+                  ServoStyleSheet* aSheet,
                   uint32_t aLine, uint32_t aColumn);
 
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(ServoImportRule, dom::CSSImportRule)
 
   // unhide since nsIDOMCSSImportRule has its own GetStyleSheet
   using dom::CSSImportRule::GetStyleSheet;
 
--- a/layout/style/StyleSheet.h
+++ b/layout/style/StyleSheet.h
@@ -165,16 +165,21 @@ public:
   void SetOwnerRule(dom::CSSImportRule* aOwnerRule) {
     mOwnerRule = aOwnerRule; /* Not ref counted */
   }
   dom::CSSImportRule* GetOwnerRule() const { return mOwnerRule; }
 
   void PrependStyleSheet(StyleSheet* aSheet);
 
   StyleSheet* GetFirstChild() const;
+  StyleSheet* GetMostRecentlyAddedChildSheet() const {
+    // New child sheet can only be prepended into the linked list of
+    // child sheets, so the most recently added one is always the first.
+    return GetFirstChild();
+  }
 
   // Principal() never returns a null pointer.
   inline nsIPrincipal* Principal() const;
   /**
    * SetPrincipal should be called on all sheets before parsing into them.
    * This can only be called once with a non-null principal.  Calling this with
    * a null pointer is allowed and is treated as a no-op.
    */
@@ -248,16 +253,23 @@ public:
 
   void AddStyleSet(const StyleSetHandle& aStyleSet);
   void DropStyleSet(const StyleSetHandle& aStyleSet);
 
   nsresult DeleteRuleFromGroup(css::GroupRule* aGroup, uint32_t aIndex);
   nsresult InsertRuleIntoGroup(const nsAString& aRule,
                                css::GroupRule* aGroup, uint32_t aIndex);
 
+  template<typename Func>
+  void EnumerateChildSheets(Func aCallback) {
+    for (StyleSheet* child = GetFirstChild(); child; child = child->mNext) {
+      aCallback(child);
+    }
+  }
+
 private:
   // Get a handle to the various stylesheet bits which live on the 'inner' for
   // gecko stylesheets and live on the StyleSheet for Servo stylesheets.
   inline StyleSheetInfo& SheetInfo();
   inline const StyleSheetInfo& SheetInfo() const;
 
   // Check if the rules are available for read and write.
   // It does the security check as well as whether the rules have been