Just something that I think makes the code easier to follow. Also, adding a few docs here and there.

Comment on attachment 8883359 [details] Bug 1378190: Try to make ServoRestyleManager easier to follow. https://reviewboard.mozilla.org/r/154244/#review159380 Thanks, looks good. As my old chef housemate used to say, "clean as you go". :-) ::: layout/base/ServoRestyleManager.cpp:997 (Diff revision 1) > + // We assume that id and class attributes are referenced from selectors, and > + // thus record them. Nit: Maybe "IDs and classes are used in selectors" rather than "id and class attributes are referenced from selectors", since the latter sounds a bit like it's talking about attribute selectors. ::: layout/base/ServoRestyleManager.cpp:1000 (Diff revision 1) > + // TODO(emilio): We keep a filter of the ids in use somewhere in the StyleSet, > + // presumably we could try to filter the old and new id. Yeah, we could do this. Although I think modifying id="" attributes on existing elements is not super common. ::: layout/base/ServoRestyleManager.cpp:1083 (Diff revision 1) > if (aAttribute == nsGkAtoms::style) { > - PostRestyleEvent(aElement, eRestyle_StyleAttribute, nsChangeHint(0)); > + restyleHint |= eRestyle_StyleAttribute; > + } else if (AttributeChangeRequiresSubtreeRestyle(*aElement, aAttribute)) { > + restyleHint |= eRestyle_Subtree; Nit: I think the structure with the "else if" here assumes that AttributeChangeRequiresSubtreeRestyle won't return true for style="". That is the case, but it seems like we should avoid relying on that. We could swap the order of these two things (since the eRestyle_Subtree will subsume the eRestyle_StyleAttribute work), but it is nice to check style="" first I suppose. Not sure I have a good suggestion that makes the code look as neat as you have it here. :-)

Comment on attachment 8883359 [details] Bug 1378190: Try to make ServoRestyleManager easier to follow. https://reviewboard.mozilla.org/r/154244/#review159386 ::: layout/base/ServoRestyleManager.cpp:997 (Diff revision 1) > + // We assume that id and class attributes are referenced from selectors, and > + // thus record them. Agreed, updated the comment :) ::: layout/base/ServoRestyleManager.cpp:1083 (Diff revision 1) > if (aAttribute == nsGkAtoms::style) { > - PostRestyleEvent(aElement, eRestyle_StyleAttribute, nsChangeHint(0)); > + restyleHint |= eRestyle_StyleAttribute; > + } else if (AttributeChangeRequiresSubtreeRestyle(*aElement, aAttribute)) { > + restyleHint |= eRestyle_Subtree; Yeah, I also noticed this... But it seems unlikely that we'd start doing subtree restyles for style attribute changes, so I think it's ok.

Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/ac434d1aa45d Try to make ServoRestyleManager easier to follow. r=heycam