Bug 1358056 - Fix stylesheet handling of associated documents in various edge cases. r=heycam, a=gchang
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 24 Apr 2017 15:44:19 -0400
changeset 396063 cb989af0ee3725738387a32935fa8cb7816efb65
parent 396062 a13f60339131a212a7d259f33e1b68cf918da422
child 396064 076868035c7498c29dcdefb9d28b6ed55d52fe83
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam, gchang
bugs1358056
milestone54.0
Bug 1358056 - Fix stylesheet handling of associated documents in various edge cases. r=heycam, a=gchang
layout/style/CSSStyleSheet.cpp
layout/style/StyleSheet.cpp
--- a/layout/style/CSSStyleSheet.cpp
+++ b/layout/style/CSSStyleSheet.cpp
@@ -1093,17 +1093,17 @@ CSSStyleSheet::ReparseSheet(const nsAStr
     }
   }
 
   // nuke child sheets list and current namespace map
   for (StyleSheet* child = GetFirstChild(); child; ) {
     NS_ASSERTION(child->mParent == this, "Child sheet is not parented to this!");
     StyleSheet* next = child->mNext;
     child->mParent = nullptr;
-    child->mDocument = nullptr;
+    child->SetAssociatedDocument(nullptr, NotOwnedByDocument);
     child->mNext = nullptr;
     child = next;
   }
   SheetInfo().mFirstChild = nullptr;
   Inner()->mNameSpaceMap = nullptr;
 
   uint32_t lineNumber = 1;
   if (mOwningNode) {
--- a/layout/style/StyleSheet.cpp
+++ b/layout/style/StyleSheet.cpp
@@ -80,17 +80,22 @@ StyleSheet::UnlinkInner()
   // don't want to do any addrefing in the process, just to make sure
   // we don't confuse the cycle collector (though on the face of it,
   // addref/release pairs during unlink should probably be ok).
   RefPtr<StyleSheet> child;
   child.swap(SheetInfo().mFirstChild);
   while (child) {
     MOZ_ASSERT(child->mParent == this, "We have a unique inner!");
     child->mParent = nullptr;
-    child->mDocument = nullptr;
+    // We (and child) might still think we're owned by a document, because
+    // unlink order is non-deterministic, so the document's unlink, which would
+    // tell us it does't own us anymore, may not have happened yet.  But if
+    // we're being unlinked, clearly we're not owned by a document anymore
+    // conceptually!
+    child->SetAssociatedDocument(nullptr, NotOwnedByDocument);
 
     RefPtr<StyleSheet> next;
     // Null out child->mNext, but don't let it die yet
     next.swap(child->mNext);
     // Switch to looking at the old value of child->mNext next iteration
     child.swap(next);
     // "next" is now our previous value of child; it'll get released
     // as we loop around.
@@ -411,16 +416,19 @@ StyleSheet::UnparentChildren()
 {
   // XXXbz this is a little bogus; see the XXX comment where we
   // declare mFirstChild in StyleSheetInfo.
   for (StyleSheet* child = GetFirstChild();
        child;
        child = child->mNext) {
     if (child->mParent == this) {
       child->mParent = nullptr;
+      MOZ_ASSERT(child->mDocumentAssociationMode == NotOwnedByDocument,
+                 "How did we get to the destructor, exactly, if we're owned "
+                 "by a document?");
       child->mDocument = nullptr;
     }
   }
 }
 
 void
 StyleSheet::SubjectSubsumesInnerPrincipal(nsIPrincipal& aSubjectPrincipal,
                                           ErrorResult& aRv)
@@ -521,17 +529,17 @@ StyleSheet::AppendStyleSheet(StyleSheet*
   while (*tail) {
     tail = &(*tail)->mNext;
   }
   *tail = aSheet;
 
   // This is not reference counted. Our parent tells us when
   // it's going away.
   aSheet->mParent = this;
-  aSheet->mDocument = mDocument;
+  aSheet->SetAssociatedDocument(mDocument, mDocumentAssociationMode);
   DidDirty();
 }
 
 size_t
 StyleSheet::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const
 {
   size_t n = 0;
   const StyleSheet* s = this;