Bug 1402218 - Make sure to clone our CSSDeclaration as needed when removing properties. r=emilio, a=sledru
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 28 Sep 2017 22:04:34 -0400
changeset 673667 af6b3f0fdb60d6ef99b96927396d1c2912af8d11
parent 673666 57e49b668005f60dea19b8eda37836a23b9994e0
child 673668 2f1496c1455f4a122dfe50a024da0ae297dd9673
push id82597
push userbmo:edilee@mozilla.com
push dateMon, 02 Oct 2017 17:24:38 +0000
reviewersemilio, sledru
bugs1402218
milestone57.0
Bug 1402218 - Make sure to clone our CSSDeclaration as needed when removing properties. r=emilio, a=sledru MozReview-Commit-ID: 6LRjLU3QQok
layout/style/DeclarationBlockInlines.h
layout/style/crashtests/1402218-1.html
layout/style/crashtests/crashtests.list
layout/style/nsDOMCSSDeclaration.cpp
--- a/layout/style/DeclarationBlockInlines.h
+++ b/layout/style/DeclarationBlockInlines.h
@@ -41,16 +41,25 @@ DeclarationBlock::Clone() const
 already_AddRefed<DeclarationBlock>
 DeclarationBlock::EnsureMutable()
 {
 #ifdef DEBUG
   if (IsGecko()) {
     AsGecko()->AssertNotExpanded();
   }
 #endif
+  if (IsServo() && !IsDirty()) {
+    // In stylo, the old DeclarationBlock is stored in element's rule node tree
+    // directly, to avoid new values replacing the DeclarationBlock in the tree
+    // directly, we need to copy the old one here if we haven't yet copied.
+    // As a result the new value does not replace rule node tree until traversal
+    // happens.
+    return Clone();
+  }
+
   if (!IsMutable()) {
     return Clone();
   }
   return do_AddRef(this);
 }
 
 void
 DeclarationBlock::ToString(nsAString& aString) const
new file mode 100644
--- /dev/null
+++ b/layout/style/crashtests/1402218-1.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<style>
+#wrapper::first-line {}
+</style>
+<body>
+<div id="wrapper">
+  <div id="test"></div>
+</div>
+<script>
+  document.querySelector('#test').style.position = 'absolute';
+  document.body.offsetHeight;
+  document.querySelector('#wrapper').style.color = 'green';
+  document.querySelector('#test').style.position = '';
+</script>
+</body>
--- a/layout/style/crashtests/crashtests.list
+++ b/layout/style/crashtests/crashtests.list
@@ -222,15 +222,16 @@ load 1400035.html
 load 1399546.html
 load 1400325.html
 load 1400926.html
 load 1400936-1.html
 load 1400936-2.html
 load 1401256.html
 load 1401706.html
 load 1401801.html
+load 1402218-1.html
+load 1402366.html
 load 1402472.html
-load 1402366.html
 load 1403028.html
 load 1403465.html
 load 1403592.html
 load 1403615.html
 load 1403712.html
--- a/layout/style/nsDOMCSSDeclaration.cpp
+++ b/layout/style/nsDOMCSSDeclaration.cpp
@@ -300,27 +300,17 @@ nsDOMCSSDeclaration::ModifyDeclaration(G
   }
 
   // For nsDOMCSSAttributeDeclaration, SetCSSDeclaration will lead to
   // Attribute setting code, which leads in turn to BeginUpdate.  We
   // need to start the update now so that the old rule doesn't get used
   // between when we mutate the declaration and when we set the new
   // rule (see stack in bug 209575).
   mozAutoDocConditionalContentUpdateBatch autoUpdate(DocToUpdate(), true);
-  RefPtr<DeclarationBlock> decl;
-  if (olddecl->IsServo() && !olddecl->IsDirty()) {
-    // In stylo, the old DeclarationBlock is stored in element's rule node tree
-    // directly, to avoid new values replacing the DeclarationBlock in the tree
-    // directly, we need to copy the old one here if we haven't yet copied.
-    // As a result the new value does not replace rule node tree until traversal
-    // happens.
-    decl = olddecl->Clone();
-  } else {
-    decl = olddecl->EnsureMutable();
-  }
+  RefPtr<DeclarationBlock> decl = olddecl->EnsureMutable();
 
   bool changed;
   if (decl->IsGecko()) {
     CSSParsingEnvironment geckoEnv;
     GetCSSParsingEnvironment(geckoEnv);
     if (!geckoEnv.mPrincipal) {
       return NS_ERROR_NOT_AVAILABLE;
     }