Bug 1292432 part 5 - Unify completeness check and security check to StyleSheet. r?heycam draft
authorXidorn Quan <me@upsuper.org>
Tue, 04 Oct 2016 11:08:11 +1100
changeset 423109 7a4f37d8084da8cc6d645723fde6f23bb63bb10f
parent 423108 68e1d8db6039e0ac683f1426f45ce400e1c9a37d
child 423110 c74cdd5b46c00501503de663ee29cd3410f1baf6
push id31799
push userxquan@mozilla.com
push dateMon, 10 Oct 2016 07:37:57 +0000
reviewersheycam
bugs1292432
milestone52.0a1
Bug 1292432 part 5 - Unify completeness check and security check to StyleSheet. r?heycam MozReview-Commit-ID: CmQ2Q9UrLAA
layout/style/CSSStyleSheet.cpp
layout/style/StyleSheet.cpp
layout/style/StyleSheet.h
--- a/layout/style/CSSStyleSheet.cpp
+++ b/layout/style/CSSStyleSheet.cpp
@@ -1680,26 +1680,17 @@ CSSStyleSheet::GetCssRules(nsIDOMCSSRule
   rules.forget(aCssRules);
   return rv.StealNSResult();
 }
 
 CSSRuleList*
 CSSStyleSheet::GetCssRules(const Maybe<nsIPrincipal*>& aSubjectPrincipal,
                            ErrorResult& aRv)
 {
-  // No doing this on incomplete sheets!
-  if (!mInner->mComplete) {
-    aRv.Throw(NS_ERROR_DOM_INVALID_ACCESS_ERR);
-    return nullptr;
-  }
-  
-  //-- Security check: Only scripts whose principal subsumes that of the
-  //   style sheet can access rule collections.
-  SubjectSubsumesInnerPrincipal(aSubjectPrincipal, aRv);
-  if (NS_WARN_IF(aRv.Failed())) {
+  if (!AreRulesAvailable(aSubjectPrincipal, aRv)) {
     return nullptr;
   }
 
   // OK, security check passed, so get the rule collection
   if (!mRuleCollection) {
     mRuleCollection = new CSSRuleListImpl(this);
   }
 
@@ -1717,23 +1708,19 @@ CSSStyleSheet::InsertRule(const nsAStrin
   return rv.StealNSResult();
 }
 
 uint32_t
 CSSStyleSheet::InsertRule(const nsAString& aRule, uint32_t aIndex,
                           const Maybe<nsIPrincipal*>& aSubjectPrincipal,
                           ErrorResult& aRv)
 {
-  //-- Security check: Only scripts whose principal subsumes that of the
-  //   style sheet can modify rule collections.
-  SubjectSubsumesInnerPrincipal(aSubjectPrincipal, aRv);
-  if (NS_WARN_IF(aRv.Failed())) {
+  if (!AreRulesAvailable(aSubjectPrincipal, aRv)) {
     return 0;
   }
-
   return InsertRuleInternal(aRule, aIndex, aRv);
 }
 
 static bool
 RuleHasPendingChildSheet(css::Rule *cssRule)
 {
   nsCOMPtr<nsIDOMCSSImportRule> importRule(do_QueryInterface(cssRule));
   NS_ASSERTION(importRule, "Rule which has type IMPORT_RULE and does not implement nsIDOMCSSImportRule!");
@@ -1743,21 +1730,17 @@ RuleHasPendingChildSheet(css::Rule *cssR
   return cssSheet != nullptr && !cssSheet->IsComplete();
 }
 
 uint32_t
 CSSStyleSheet::InsertRuleInternal(const nsAString& aRule,
                                   uint32_t aIndex,
                                   ErrorResult& aRv)
 {
-  // No doing this if the sheet is not complete!
-  if (!mInner->mComplete) {
-    aRv.Throw(NS_ERROR_DOM_INVALID_ACCESS_ERR);
-    return 0;
-  }
+  MOZ_ASSERT(mInner->mComplete);
 
   WillDirty();
   
   if (aIndex > uint32_t(mInner->mOrderedRules.Count())) {
     aRv.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR);
     return 0;
   }
   
@@ -1875,26 +1858,17 @@ CSSStyleSheet::DeleteRule(uint32_t aInde
   return rv.StealNSResult();
 }
 
 void
 CSSStyleSheet::DeleteRule(uint32_t aIndex,
                           const Maybe<nsIPrincipal*>& aSubjectPrincipal,
                           ErrorResult& aRv)
 {
-  // No doing this if the sheet is not complete!
-  if (!mInner->mComplete) {
-    aRv.Throw(NS_ERROR_DOM_INVALID_ACCESS_ERR);
-    return;
-  }
-
-  //-- Security check: Only scripts whose principal subsumes that of the
-  //   style sheet can modify rule collections.
-  SubjectSubsumesInnerPrincipal(aSubjectPrincipal, aRv);
-  if (NS_WARN_IF(aRv.Failed())) {
+  if (!AreRulesAvailable(aSubjectPrincipal, aRv)) {
     return;
   }
 
   // XXX TBI: handle @rule types
   mozAutoDocUpdate updateBatch(mDocument, UPDATE_STYLE, true);
     
   WillDirty();
 
--- a/layout/style/StyleSheet.cpp
+++ b/layout/style/StyleSheet.cpp
@@ -171,9 +171,27 @@ StyleSheet::SubjectSubsumesInnerPrincipa
 
   WillDirty();
 
   info.mPrincipal = aSubjectPrincipal.value();
 
   DidDirty();
 }
 
+bool
+StyleSheet::AreRulesAvailable(const Maybe<nsIPrincipal*>& aSubjectPrincipal,
+                              ErrorResult& aRv)
+{
+  // Rules are not available on incomplete sheets.
+  if (!SheetInfo().mComplete) {
+    aRv.Throw(NS_ERROR_DOM_INVALID_ACCESS_ERR);
+    return false;
+  }
+  //-- Security check: Only scripts whose principal subsumes that of the
+  //   style sheet can access rule collections.
+  SubjectSubsumesInnerPrincipal(aSubjectPrincipal, aRv);
+  if (NS_WARN_IF(aRv.Failed())) {
+    return false;
+  }
+  return true;
+}
+
 } // namespace mozilla
--- a/layout/style/StyleSheet.h
+++ b/layout/style/StyleSheet.h
@@ -154,16 +154,22 @@ public:
   inline void DidDirty();
 
 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
+  // completely loaded.
+  bool AreRulesAvailable(const Maybe<nsIPrincipal*>& aSubjectPrincipal,
+                         ErrorResult& aRv);
+
 protected:
   // Return success if the subject principal subsumes the principal of our
   // inner, error otherwise.  This will also succeed if the subject has
   // UniversalXPConnect or if access is allowed by CORS.  In the latter case,
   // it will set the principal of the inner to the subject principal.
   void SubjectSubsumesInnerPrincipal(const Maybe<nsIPrincipal*>& aSubjectPrincipal,
                                      ErrorResult& aRv);