Bug 1368922 - Set mIsDirty atomically. r=hiro
authorCameron McCormack <cam@mcc.id.au>
Wed, 20 Sep 2017 11:30:08 +0800
changeset 433795 1bd488e6c4d64dc1674a95891e8f89f1982ebc62
parent 433794 b3619f0787cdd1ece6ff805e0540c99fdef82dc7
child 433796 3045288dcb2ebd8106da35ed5067f0edb3f7fb71
push id1567
push userjlorenzo@mozilla.com
push dateThu, 02 Nov 2017 12:36:05 +0000
treeherdermozilla-release@e512c14a0406 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershiro
bugs1368922
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1368922 - Set mIsDirty atomically. r=hiro MozReview-Commit-ID: Ei3zCECVRFf
js/src/devtools/rootAnalysis/analyzeHeapWrites.js
layout/style/DeclarationBlock.h
--- a/js/src/devtools/rootAnalysis/analyzeHeapWrites.js
+++ b/js/src/devtools/rootAnalysis/analyzeHeapWrites.js
@@ -30,16 +30,17 @@ function checkExternalFunction(entry)
         "strlen",
         "Servo_ComputedValues_EqualCustomProperties",
         /Servo_DeclarationBlock_GetCssText/,
         "Servo_GetArcStringData",
         /nsIFrame::AppendOwnedAnonBoxes/,
         // Assume that atomic accesses are threadsafe.
         /^__atomic_fetch_/,
         /^__atomic_load_/,
+        /^__atomic_store_/,
         /^__atomic_thread_fence/,
     ];
     if (entry.matches(whitelist))
         return;
 
     // memcpy and memset are safe if the target pointer is threadsafe.
     const simpleWrites = [
         "memcpy",
@@ -470,19 +471,16 @@ function ignoreContents(entry)
 
         // Unable to analyze safety of linked list initialization.
         "Gecko_NewCSSValueSharedList",
         "Gecko_CSSValue_InitSharedList",
 
         // Unable to trace through dataflow, but straightforward if inspected.
         "Gecko_NewNoneTransform",
 
-        // Bug 1368922
-        "Gecko_UnsetDirtyStyleAttr",
-
         // Bug 1400438
         "Gecko_AppendMozBorderColors",
 
         // Need main thread assertions or other fixes.
         /EffectCompositor::GetServoAnimationRule/,
         /LookAndFeel::GetColor/,
     ];
     if (entry.matches(whitelist))
--- a/layout/style/DeclarationBlock.h
+++ b/layout/style/DeclarationBlock.h
@@ -7,16 +7,17 @@
 /*
  * representation of a declaration block in a CSS stylesheet, or of
  * a style attribute
  */
 
 #ifndef mozilla_DeclarationBlock_h
 #define mozilla_DeclarationBlock_h
 
+#include "mozilla/Atomics.h"
 #include "mozilla/ServoUtils.h"
 #include "mozilla/StyleBackendType.h"
 
 #include "nsCSSPropertyID.h"
 
 class nsHTMLCSSStyleSheet;
 
 namespace mozilla {
@@ -28,18 +29,18 @@ class Declaration;
 class Rule;
 } // namespace css
 
 class DeclarationBlock
 {
 protected:
   explicit DeclarationBlock(StyleBackendType aType)
     : mImmutable(false)
+    , mType(aType)
     , mIsDirty(false)
-    , mType(aType)
   {
     mContainer.mRaw = 0;
   }
 
   DeclarationBlock(const DeclarationBlock& aCopy)
     : DeclarationBlock(aCopy.mType) {}
 
 public:
@@ -149,17 +150,28 @@ private:
 
     // The nsHTMLCSSStyleSheet that is responsible for this declaration.
     // Only non-null for style attributes.
     nsHTMLCSSStyleSheet* mHTMLCSSStyleSheet;
   } mContainer;
 
   // set when declaration put in the rule tree;
   bool mImmutable;
-  // True if this declaration has not been restyled after modified.
-  bool mIsDirty;
 
   const StyleBackendType mType;
+
+  // True if this declaration has not been restyled after modified.
+  //
+  // Since we can clear this flag from style worker threads, we use an Atomic.
+  //
+  // Note that although a single DeclarationBlock can be shared between
+  // different rule nodes (due to the style="" attribute cache), whenever a
+  // DeclarationBlock has its mIsDirty flag set to true, we always clone it to
+  // a unique object first. So when we clear this flag during Servo traversal,
+  // we know that we are clearing it on a DeclarationBlock that has a single
+  // reference, and there is no problem with another user of the same
+  // DeclarationBlock thinking that it is not dirty.
+  Atomic<bool, MemoryOrdering::Relaxed> mIsDirty;
 };
 
 } // namespace mozilla
 
 #endif // mozilla_DeclarationBlock_h