We've been whitelisting CalcStyleDifferenceInternal for heap write hazards. In bug 1380133 CalcStyleDifferenceInternal was removed (and its work moved up into CalcStyleDifference) revealing some hazards. For now, we're bumping the allowed hazards to 4 (in bug 1382956) but we should fix them. The first I've encountered is URLValueData::MightHaveRef, which does some lazily initialization of mMightHaveRef. That's not safe OMT.

Comment on attachment 8889116 [details] Bug 1382964 - Part 1: Don't cache URLValueData::mMightHaveRef when in a traversal. https://reviewboard.mozilla.org/r/160138/#review165498 ::: layout/style/nsCSSValue.cpp:2909 (Diff revision 1) > - return false; > + if (mMightHaveRef.isNothing()) { > + mMightHaveRef.emplace(result); > + } `mMightHaveRef = Some(result);` might be enough. ::: layout/style/nsCSSValue.cpp:2920 (Diff revision 1) > - // ::MightHaveRef is O(N), use it only use it only when MightHaveRef is > - // called. > - mMightHaveRef.emplace(::MightHaveRef(mString)); > + bool result = ::MightHaveRef(mString); > + if (!ServoStyleSet::IsInServoTraversal()) { > + // Can only cache the result if we're not on a style worker thread. > + mMightHaveRef.emplace(result); > + } I guess this is fine... but could be improved. We can probably make the field atomic so that it is safe to operate during traversal. That may require some new class in mfbt I guess... so can be done separately.

Comment on attachment 8889117 [details] Bug 1382964 - Part 2: Assert we're on the main thread in nsCSSValueTokenStream::operator==. https://reviewboard.mozilla.org/r/160140/#review165500

Comment on attachment 8889118 [details] Bug 1382964 - Part 3: Add assertion to help heap write analysis. https://reviewboard.mozilla.org/r/160142/#review165502

Comment on attachment 8889119 [details] Bug 1382964 - Part 4: Remove now unused heap write hazard whitelist entry for CalcStyleDifferenceInternal. https://reviewboard.mozilla.org/r/160144/#review165504 ::: commit-message-adb37:1 (Diff revision 1) > +Bug 1382964 - Part 4: Now now unused heap write hazard whitelist entry for CalcStyleDifferenceInternal. r?xidorn Now **remove** unused heap write hazard?

Comment on attachment 8889120 [details] Bug 1382964 - Part 5: Restore allowed heap write hazards to 3. https://reviewboard.mozilla.org/r/160146/#review165506

Pushed by cmccormack@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0fed5a7719c1 Part 1: Don't cache URLValueData::mMightHaveRef when in a traversal. r=xidorn https://hg.mozilla.org/integration/autoland/rev/65fb1f76ed5b Part 2: Assert we're on the main thread in nsCSSValueTokenStream::operator==. r=xidorn https://hg.mozilla.org/integration/autoland/rev/b9d6e44a276f Part 3: Add assertion to help heap write analysis. r=xidorn https://hg.mozilla.org/integration/autoland/rev/0f27092c9bd4 Part 4: Remove now unused heap write hazard whitelist entry for CalcStyleDifferenceInternal. r=xidorn https://hg.mozilla.org/integration/autoland/rev/b1e6304c5427 Part 5: Restore allowed heap write hazards to 3. r=xidorn

https://hg.mozilla.org/mozilla-central/rev/0fed5a7719c1 https://hg.mozilla.org/mozilla-central/rev/65fb1f76ed5b https://hg.mozilla.org/mozilla-central/rev/b9d6e44a276f https://hg.mozilla.org/mozilla-central/rev/0f27092c9bd4 https://hg.mozilla.org/mozilla-central/rev/b1e6304c5427