Clean up handling of CSSOM modifications to rules that are non-child descendants of a sheet. (Bug 435442, patch 3) r=bzbarsky

This fixes handling of a number of cases:
 1. Modifications to a style rule inside a group rule now trigger
    GetRuleCascades.
 2. A sheet that has been (potentially) modified is reliably marked as
    such, and thus never cloned.

content/html/document/src/nsHTMLFragmentContentSink.cpp

@@ -1172,7 +1172,8 @@ nsHTMLParanoidFragmentSink::CloseContainer(const nsHTMLTag aTag)
                             0, PR_FALSE);
           // Mark the sheet as complete.
           if (NS_SUCCEEDED(rv)) {
-            sheet->SetModified(PR_FALSE);
+            NS_ABORT_IF_FALSE(!sheet->IsModified(),
+                              "should not get marked modified during parsing");
             sheet->SetComplete();
           }
           if (NS_SUCCEEDED(rv)) {

layout/style/Loader.cpp

@@ -1694,8 +1694,8 @@ Loader::DoSheetComplete(SheetLoadData* aLoadData, nsresult aStatus,
   // Go through and deal with the whole linked list.
   SheetLoadData* data = aLoadData;
   while (data) {
-
-    data->mSheet->SetModified(PR_FALSE); // it's clean
+    NS_ABORT_IF_FALSE(!data->mSheet->IsModified(),
+                      "should not get marked modified during parsing");
     data->mSheet->SetComplete();
     if (data->mMustNotify && (data->mObserver || !mObservers.IsEmpty())) {
       // Don't notify here so we don't trigger script.  Remember the

layout/style/nsCSSRules.cpp

@@ -583,9 +583,7 @@ GroupRule::AppendStyleRule(nsICSSRule* aRule)
   aRule->SetStyleSheet(mSheet);
   aRule->SetParentRule(this);
   if (mSheet) {
-    // XXXldb Shouldn't we be using |WillDirty| and |DidDirty| (and
-    // shouldn't |SetModified| be removed?
-    mSheet->SetModified(PR_TRUE);
+    mSheet->SetModifiedByChildRule();
   }
 }
 

layout/style/nsCSSStyleSheet.cpp

@@ -97,8 +97,6 @@ protected:
   virtual ~CSSRuleListImpl();
 
   nsCSSStyleSheet*  mStyleSheet;
-public:
-  PRBool              mRulesAccessed;
 };
 
 CSSRuleListImpl::CSSRuleListImpl(nsCSSStyleSheet *aStyleSheet)
@@ -106,7 +104,6 @@ CSSRuleListImpl::CSSRuleListImpl(nsCSSStyleSheet *aStyleSheet)
   // Not reference counted to avoid circular references.
   // The style sheet will tell us when its going away.
   mStyleSheet = aStyleSheet;
-  mRulesAccessed = PR_FALSE;
 }
 
 CSSRuleListImpl::~CSSRuleListImpl()
@@ -155,7 +152,6 @@ CSSRuleListImpl::GetItemAt(PRUint32 aIndex, nsresult* aResult)
 
       result = mStyleSheet->GetStyleRuleAt(aIndex, *getter_AddRefs(rule));
       if (rule) {
-        mRulesAccessed = PR_TRUE; // signal to never share rules again
         return rule->GetDOMRuleWeak(aResult);
       }
       if (result == NS_ERROR_ILLEGAL_VALUE) {
@@ -1022,15 +1018,14 @@ nsCSSStyleSheet::nsCSSStyleSheet(const nsCSSStyleSheet& aCopy,
     mDocument(aDocumentToUse),
     mOwningNode(aOwningNodeToUse),
     mDisabled(aCopy.mDisabled),
-    mDirty(PR_FALSE),
+    mDirty(aCopy.mDirty),
     mInner(aCopy.mInner),
     mRuleProcessors(nsnull)
 {
 
   mInner->AddSheet(this);
 
-  if (aCopy.mRuleCollection && 
-      aCopy.mRuleCollection->mRulesAccessed) {  // CSSOM's been there, force full copy now
+  if (mDirty) { // CSSOM's been there, force full copy now
     NS_ASSERTION(mInner->mComplete, "Why have rules been accessed on an incomplete sheet?");
     // FIXME: handle failure?
     EnsureUniqueInner();
@@ -1432,6 +1427,8 @@ nsCSSStyleSheet::StyleSheetCount() const
 nsCSSStyleSheet::EnsureUniqueInnerResult
 nsCSSStyleSheet::EnsureUniqueInner()
 {
+  mDirty = PR_TRUE;
+
   NS_ABORT_IF_FALSE(mInner->mSheets.Length() != 0,
                     "unexpected number of outers");
   if (mInner->mSheets.Length() == 1) {
@@ -1564,8 +1561,9 @@ nsCSSStyleSheet::WillDirty()
 void
 nsCSSStyleSheet::DidDirty()
 {
+  NS_ABORT_IF_FALSE(!mInner->mComplete || mDirty,
+                    "caller must have called WillDirty()");
   ClearRuleCascades();
-  mDirty = PR_TRUE;
 }
 
 nsresult

layout/style/nsCSSStyleSheet.h

@@ -210,7 +210,12 @@ public:
                                           nsIDOMNode* aCloneOwningNode) const;
 
   PRBool IsModified() const { return mDirty; }
-  void SetModified(PRBool aModified) { mDirty = aModified; }
+
+  void SetModifiedByChildRule() {
+    NS_ASSERTION(mDirty,
+                 "sheet must be marked dirty before handing out child rules");
+    DidDirty();
+  }
 
   nsresult AddRuleProcessor(nsCSSRuleProcessor* aProcessor);
   nsresult DropRuleProcessor(nsCSSRuleProcessor* aProcessor);