While I was checking bug 1361938, I found a couple of problem in animation-only restyle. 1) RESTYLE_STYLE_ATTRIBUTE seems to be also processed in animation-only restyle 2) RESTYLE_SMIL, RESTYLE_CSS_TRANSITIONS and RESTYLE_CSS_ANIMATIONS look exclusive, we should use interscts() instead of contains() 3) TraversalFlags.for_animation_only() should also use intersetcs() instead of contains() https://treeherder.mozilla.org/#/jobs?repo=try&revision=624d0982d97cafbed1fd3f1e80b299fbcb6e93dd

Comment on attachment 8870569 [details] Bug 1367225 - Animation-only restyle should be also processed in all the kinds of traversal. https://reviewboard.mozilla.org/r/142026/#review145694 ::: servo/components/style/traversal.rs:50 (Diff revision 1) > } > > impl TraversalFlags { > /// Returns true if the traversal is for animation-only restyles. > pub fn for_animation_only(&self) -> bool { > - self.contains(ANIMATION_ONLY) > + self.intersects(ANIMATION_ONLY) Hmmm... I'm very confused about this patch (and the previous ones too). Does this make a difference? `intersects` is defined as: self.bits & flags.bits != 0 While `contains` is: self.bits & flags.bits == flags.bits For flags that are only one bit, that shouldn't make a difference, should it?

(In reply to Emilio Cobos Álvarez [:emilio] from comment #5) > Comment on attachment 8870569 [details] > Bug 1367225 - Animation-only restyle should be also processed in all the > kinds of traversal. > > https://reviewboard.mozilla.org/r/142026/#review145694 > > ::: servo/components/style/traversal.rs:50 > (Diff revision 1) > > } > > > > impl TraversalFlags { > > /// Returns true if the traversal is for animation-only restyles. > > pub fn for_animation_only(&self) -> bool { > > - self.contains(ANIMATION_ONLY) > > + self.intersects(ANIMATION_ONLY) > > Hmmm... I'm very confused about this patch (and the previous ones too). Does > this make a difference? > > `intersects` is defined as: > > self.bits & flags.bits != 0 > > While `contains` is: > > self.bits & flags.bits == flags.bits > > For flags that are only one bit, that shouldn't make a difference, should it? Oh, you are right. I was just referring the bitflags document. There is no difference. But it's really confusing. I think we should use intersetcs() there just in case that if someone tries to add another flag in contains().

Comment on attachment 8870566 [details] Bug 1367225 - Make replace_rules returning boolean. https://reviewboard.mozilla.org/r/142020/#review145796 ::: servo/components/style/matching.rs:967 (Diff revision 1) > new_values, > pseudo) > } > > /// Updates the rule nodes without re-running selector matching, using just > - /// the rule tree. Returns RulesChanged which indicates whether the rule nodes changed > + /// the rule tree. Returns true if an important rule was replaced. (Nit: s/important/!important/)

I don't understand the change from intersects to contains. Don't we want the more restrictive 'contains' instead?

Comment on attachment 8870567 [details] Bug 1367225 - Don't process style attribute changes in animation-only restyle. https://reviewboard.mozilla.org/r/142022/#review145798

(In reply to Brian Birtles (:birtles) from comment #8) > I don't understand the change from intersects to contains. Don't we want the > more restrictive 'contains' instead? Ok, I will drop them, it's confusing.

I am going to include these changes into a PR for bug 1366631 to avoid an extra PR in the servo's CI queue. https://treeherder.mozilla.org/#/jobs?repo=try&revision=ef94d0ae936c0a4ad8978a093cba8e18bf93c830