Xidorn Quan [:xidorn] UTC+11 Assignee
	Description • 8 years ago

Sometimes people miss the "IMPORTANT NOTE" in nsChangeHint and forget to update the name list in RestyleManagerBase::ChangeHintToString, which may cause issues when debugging.

Luckily sometimes this can be caught by Coverity (when nsChangeHint_Hints_NoHandledForDescendants is updated), and that's why I found they are out-of-sync currently. But we are not always lucky, so we need a more robust way to catch that.

	Markus Stange [:mstange]
	Comment 3 • 8 years ago

Thanks for catching that.

	David Baron :dbaron:
	Comment 4 • 8 years ago
mozreview-review

Comment on attachment 8781089 [details]
Bug 1295111 part 1 - Add UpdateBackgroundPosition hint to ChangeHintToString.

https://reviewboard.mozilla.org/r/71596/#review69196

	David Baron :dbaron:
	Comment 5 • 8 years ago
mozreview-review

Comment on attachment 8781090 [details]
Bug 1295111 part 2 - Add static_assert to ensure that is updated properly.

https://reviewboard.mozilla.org/r/71598/#review69198

::: layout/base/nsChangeHint.h:209
(Diff revision 1)
>    // IMPORTANT NOTE: When adding new hints, consider whether you need to
>    // add them to NS_HintsNotHandledForDescendantsIn() below.  Please also
>    // add them to RestyleManager::ChangeHintToString.

Even though it's just a few lines below, please update this comment to say to modify nsChangeHint_AllHints.

::: layout/style/nsStyleContext.cpp:1224
(Diff revision 1)
>        hint |= nsChangeHint_RepaintFrame;
>      }
>    }
>  
> +  MOZ_ASSERT(NS_IsHintSubset(hint, nsChangeHint_AllHints),
> +             "Added any new hint without bumping AllHints?");

"any new hint" -> "a new hint"

	David Baron :dbaron:
	Comment 6 • 8 years ago
mozreview-review

Comment on attachment 8781090 [details]
Bug 1295111 part 2 - Add static_assert to ensure that is updated properly.

https://reviewboard.mozilla.org/r/71598/#review69202

Oops, I hit the new "Publish" button when I meant to hit "Finish" instead.

Thanks for fixing this.

	Pulsebot
	Comment 9 • 8 years ago

Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/df6e1c83af8d
part 1 - Add UpdateBackgroundPosition hint to ChangeHintToString. r=dbaron
https://hg.mozilla.org/integration/autoland/rev/7f8b1c2a0802
part 2 - Add static_assert to ensure that is updated properly. r=dbaron

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 10 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/df6e1c83af8d
https://hg.mozilla.org/mozilla-central/rev/7f8b1c2a0802