Bug 1722662 - Make Element::{Add,Remove}States handle change notifica…

…tions correctly. r=smaug

The style system uses the changed bits to compute the old state, so if
it's inaccurate it might cause styles to be incorrectly invalidated.

This causes issues because with the next patch the autofill jsm
calls removeManuallyManagedStates(AUTOFILL), then
addManuallyManagedStates(AUTOFILL | AUTOFILL_PREVIEW), and if the input
didn't have AUTOFILL before we'd incorrectly detect it as not changing
with the next patch.

Also make them not virtual anymore since nobody overrides them. An
alternative to this would be to assert that we don't yet have the state
we're adding (or that we have the state we're removing), and handle it
in the callers. But this is a bit more convenient.

Differential Revision: https://phabricator.services.mozilla.com/D122013

dom/base/Element.cpp

@@ -344,8 +344,11 @@ EventStates Element::IntrinsicState() const {
 }
 
 void Element::NotifyStateChange(EventStates aStates) {
-  Document* doc = GetComposedDoc();
-  if (doc) {
+  if (aStates.IsEmpty()) {
+    return;
+  }
+
+  if (Document* doc = GetComposedDoc()) {
     nsAutoScriptBlocker scriptBlocker;
     doc->ContentStateChanged(this, aStates);
   }

dom/base/Element.h

@@ -705,19 +705,21 @@ class Element : public FragmentOrElement {
   // These will handle setting up script blockers when they notify, so no need
   // to do it in the callers unless desired.  States passed here must only be
   // those in EXTERNALLY_MANAGED_STATES.
-  virtual void AddStates(EventStates aStates) {
+  void AddStates(EventStates aStates) {
     MOZ_ASSERT(!aStates.HasAtLeastOneOfStates(INTRINSIC_STATES),
                "Should only be adding externally-managed states here");
+    EventStates old = mState;
     AddStatesSilently(aStates);
-    NotifyStateChange(aStates);
+    NotifyStateChange(old ^ mState);
   }
-  virtual void RemoveStates(EventStates aStates) {
+  void RemoveStates(EventStates aStates) {
     MOZ_ASSERT(!aStates.HasAtLeastOneOfStates(INTRINSIC_STATES),
                "Should only be removing externally-managed states here");
+    EventStates old = mState;
     RemoveStatesSilently(aStates);
-    NotifyStateChange(aStates);
+    NotifyStateChange(old ^ mState);
   }
-  virtual void ToggleStates(EventStates aStates, bool aNotify) {
+  void ToggleStates(EventStates aStates, bool aNotify) {
     MOZ_ASSERT(!aStates.HasAtLeastOneOfStates(INTRINSIC_STATES),
                "Should only be removing externally-managed states here");
     mState ^= aStates;