Bug 1352968 part 8 - Construct @import rule object eagerly. r=heycam draft
authorXidorn Quan <me@upsuper.org>
Mon, 08 May 2017 10:23:47 +1000
changeset 586233 cedc65c6376373d54e623dc5cfadaa2de92909cb
parent 586232 1895bd31f6c88e1be8c710910dec6e773d1a233d
child 586234 17633a6e12bf4f9651d35ae0c40fa2b6bca2faa0
push id61334
push userxquan@mozilla.com
push dateTue, 30 May 2017 01:07:29 +0000
reviewersheycam
bugs1352968
milestone55.0a1
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
@@ -138,16 +138,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