Bug 1640040 - Only handle updating the DOM state from TextControlState::SetValue on the outer caller. r=masayuki

The issue here is that AccessibleCaret flushes layout from the middle of
the SetValue call, because it hides the caret (see the call stack in
comment 11).

That changes the placeholder-shown state because it goes from empty to
non-empty but the SetValue call from UnbindFromFrame is not supposed to
change that state (because we're in the middle of restyling).

Call OnValueChanged later, at the outer caller instead.

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

dom/html/TextControlState.cpp

@@ -2568,6 +2568,9 @@ bool TextControlState::SetValue(const nsAString& aValue,
     aOldValue = nullptr;
   }
 
+  const bool wasHandlingSetValue =
+      mHandlingState && mHandlingState->IsHandling(TextControlAction::SetValue);
+
   ErrorResult error;
   AutoTextControlHandlingState handlingSetValue(
       *this, TextControlAction::SetValue, aValue, aOldValue, aFlags, error);
@@ -2646,25 +2649,23 @@ bool TextControlState::SetValue(const nsAString& aValue,
   }
 
   if (mTextEditor && mBoundFrame) {
-    AutoWeakFrame weakFrame(mBoundFrame);
-
     if (!SetValueWithTextEditor(handlingSetValue)) {
       return false;
     }
-
-    if (!weakFrame.IsAlive()) {
-      return true;
-    }
   } else if (!SetValueWithoutTextEditor(handlingSetValue)) {
     return false;
   }
 
-  // TODO(emilio): It seems wrong to pass ValueChangeKind::Script if
-  // BySetUserInput is in aFlags.
-  auto changeKind = (aFlags & eSetValue_Internal) ? ValueChangeKind::Internal
-                                                  : ValueChangeKind::Script;
+  // If we were handling SetValue() before, don't update the DOM state twice,
+  // just let the outer call do so.
+  if (!wasHandlingSetValue) {
+    // TODO(emilio): It seems wrong to pass ValueChangeKind::Script if
+    // BySetUserInput is in aFlags.
+    auto changeKind = (aFlags & eSetValue_Internal) ? ValueChangeKind::Internal
+                                                    : ValueChangeKind::Script;
 
-  handlingSetValue.GetTextControlElement()->OnValueChanged(changeKind);
+    handlingSetValue.GetTextControlElement()->OnValueChanged(changeKind);
+  }
   return true;
 }
 

layout/style/crashtests/1640040.html

@@ -0,0 +1,11 @@
+<script>
+window.onload = () => {
+  b.type = ""
+  document.getSelection().selectAllChildren(a)
+  b.placeholder = ""
+  b.setRangeText(String.fromCodePoint(814688))
+}
+</script>
+<menu spellcheck="true">
+<input id="b" autofocus="" type="button">
+<menu id="a">

layout/style/crashtests/crashtests.list

@@ -320,3 +320,4 @@ pref(layout.css.motion-path.enabled,true) load 1609786.html
 pref(layout.css.constructable-stylesheets.enabled,true) load 1616433.html
 pref(layout.css.constructable-stylesheets.enabled,true) load 1616407.html
 load 1639533.html
+pref(layout.accessiblecaret.enabled,true) load 1640040.html