Another round.

MozReview-Commit-ID: 732bV0X80Gk

MozReview-Commit-ID: 5DdcKgyADbE

MozReview-Commit-ID: 1SJlS4PwBoy

MozReview-Commit-ID: LnYYbx742X1

Comment on attachment 8842221 [details] [diff] [review] Part 1 - Don't write to undisplayed contents map cache during servo traversal. v1 Review of attachment 8842221 [details] [diff] [review]: ----------------------------------------------------------------- Gah. I'm dubious about this cache being useful at all FWIW, I can't think of a place where we'd repeteadly look up the same node twice here. Perhaps we should look at removing them, but that's fine as a followup I guess.

Comment on attachment 8842224 [details] [diff] [review] Part 3 - Use threadsafe style struct accessors in CalcStyleDifference when analyzing visited-link style contexts. v1 Review of attachment 8842224 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/style/nsStyleContext.cpp @@ +1212,5 @@ > // not having a style-if-visited), but not the other way around. > #define STYLE_FIELD(name_) thisVisStruct->name_ != otherVisStruct->name_ || > #define STYLE_STRUCT(name_, fields_) \ > if (!change && PeekStyle##name_()) { \ > + const nsStyle##name_* thisVisStruct = thisVis->ThreadsafeStyle##name_(); \ Regarding this... This works, but should we do this anyway? As far as I know we don't reach here, since we don't ever call SetStyleIfVisited for Servo style contexts. Perhaps we could just assert we're not a servo style context / that we're on main-thread / that we aren't in traversal for now?

(In reply to Emilio Cobos Álvarez [:emilio] from comment #7) > Comment on attachment 8842224 [details] [diff] [review] > Part 3 - Use threadsafe style struct accessors in CalcStyleDifference when > analyzing visited-link style contexts. v1 > > Review of attachment 8842224 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: layout/style/nsStyleContext.cpp > @@ +1212,5 @@ > > // not having a style-if-visited), but not the other way around. > > #define STYLE_FIELD(name_) thisVisStruct->name_ != otherVisStruct->name_ || > > #define STYLE_STRUCT(name_, fields_) \ > > if (!change && PeekStyle##name_()) { \ > > + const nsStyle##name_* thisVisStruct = thisVis->ThreadsafeStyle##name_(); \ > > Regarding this... This works, but should we do this anyway? > > As far as I know we don't reach here, since we don't ever call > SetStyleIfVisited for Servo style contexts. Perhaps we could just assert > we're not a servo style context / that we're on main-thread / that we aren't > in traversal for now? That seems like a reasonable, lower-risk approach. I'll go ahead and do that.

Pushed by bholley@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5b8c22b505a4 Don't write to undisplayed contents map cache during servo traversal. r=emilio https://hg.mozilla.org/integration/autoland/rev/2d3c0b4c3e4a Use threadsafe style struct accessor in assertion. r=emilio https://hg.mozilla.org/integration/autoland/rev/042624a21b86 Assert against the servo traversal when analyzing visited-link style contexts. r=emilio https://hg.mozilla.org/integration/autoland/rev/a79c630604eb Assert against the servo traversal when serializing gecko declarations. r=emilio

https://hg.mozilla.org/mozilla-central/rev/5b8c22b505a4 https://hg.mozilla.org/mozilla-central/rev/2d3c0b4c3e4a https://hg.mozilla.org/mozilla-central/rev/042624a21b86 https://hg.mozilla.org/mozilla-central/rev/a79c630604eb