Restoring control state should dispatch 'input' and 'change' events only if values are changed

This CL is a follow-up of crrev.com/727843, which caused a regression
seen in the local-NTP.

* HTMLTextAreaElement and HTMLSelectElement:
  Call QueueInputAndChangeEvents() only if their values are changed.

* HTMLInputElement and input type implementations:
  InputTypeView::RestoreFormControlState() returns a boolean flag to ask to
  call QueueInputAndChangeEvents().

Bug: 1038655
Change-Id: Ib68077516d1d6c032dbc2599abbcd541c281f4de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1983692
Commit-Queue: Kent Tamura <tkent@chromium.org>
Reviewed-by: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729334}

third_party/blink/renderer/core/html/forms/base_checkable_input_type.cc

@@ -55,9 +55,11 @@ FormControlState BaseCheckableInputType::SaveFormControlState() const {
   return FormControlState(GetElement().checked() ? "on" : "off");
 }
 
-void BaseCheckableInputType::RestoreFormControlState(
+bool BaseCheckableInputType::RestoreFormControlState(
     const FormControlState& state) {
+  bool old_checked = GetElement().checked();
   GetElement().setChecked(state[0] == "on");
+  return old_checked != GetElement().checked();
 }
 
 void BaseCheckableInputType::AppendToFormData(FormData& form_data) const {

third_party/blink/renderer/core/html/forms/base_checkable_input_type.h

@@ -57,7 +57,7 @@ class BaseCheckableInputType : public InputType, public InputTypeView {
  private:
   InputTypeView* CreateView() override;
   FormControlState SaveFormControlState() const final;
-  void RestoreFormControlState(const FormControlState&) final;
+  bool RestoreFormControlState(const FormControlState&) final;
   void AppendToFormData(FormData&) const final;
   void HandleKeypressEvent(KeyboardEvent&) final;
   bool CanSetStringValue() const final;

third_party/blink/renderer/core/html/forms/file_input_type.cc

@@ -116,9 +116,11 @@ FormControlState FileInputType::SaveFormControlState() const {
   return state;
 }
 
-void FileInputType::RestoreFormControlState(const FormControlState& state) {
+bool FileInputType::RestoreFormControlState(const FormControlState& state) {
+  // Check if the state is broken.
+  // See File::AppendToControlState() and File::CreateFromControlState().
   if (state.ValueSize() % 3)
-    return;
+    return false;
   HeapVector<Member<File>> file_vector =
       CreateFilesFrom<File*, HeapVector<Member<File>>>(
           state, &File::CreateFromControlState);
@@ -126,6 +128,9 @@ void FileInputType::RestoreFormControlState(const FormControlState& state) {
   for (const auto& file : file_vector)
     file_list->Append(file);
   SetFilesAndDispatchEvents(file_list);
+  // This function always returns false because SetFilesAndDispatchEvents()
+  // dispatches events if files are changed.
+  return false;
 }
 
 void FileInputType::AppendToFormData(FormData& form_data) const {

third_party/blink/renderer/core/html/forms/file_input_type.h

@@ -67,7 +67,7 @@ class CORE_EXPORT FileInputType final : public InputType,
   InputTypeView* CreateView() override;
   const AtomicString& FormControlType() const override;
   FormControlState SaveFormControlState() const override;
-  void RestoreFormControlState(const FormControlState&) override;
+  bool RestoreFormControlState(const FormControlState&) override;
   void AppendToFormData(FormData&) const override;
   bool ValueMissing(const String&) const override;
   String ValueMissingText() const override;

third_party/blink/renderer/core/html/forms/html_input_element.cc

@@ -584,9 +584,10 @@ FormControlState HTMLInputElement::SaveFormControlState() const {
 }
 
 void HTMLInputElement::RestoreFormControlState(const FormControlState& state) {
-  input_type_view_->RestoreFormControlState(state);
-  state_restored_ = true;
-  QueueInputAndChangeEvents();
+  if (input_type_view_->RestoreFormControlState(state)) {
+    state_restored_ = true;
+    QueueInputAndChangeEvents();
+  }
 }
 
 bool HTMLInputElement::CanStartSelection() const {

third_party/blink/renderer/core/html/forms/html_select_element.cc

@@ -1187,6 +1187,13 @@ void HTMLSelectElement::RestoreFormControlState(const FormControlState& state) {
   if (items_size == 0)
     return;
 
+  Vector<bool> old_selection(items_size);
+  for (wtf_size_t i = 0; i < items_size; ++i) {
+    if (auto* option = DynamicTo<HTMLOptionElement>(items[i].Get()))
+      old_selection[i] = option->Selected();
+    else
+      old_selection[i] = false;
+  }
   SelectOption(nullptr, kDeselectOtherOptionsFlag);
 
   // The saved state should have at least one value and an index.
@@ -1237,7 +1244,15 @@ void HTMLSelectElement::RestoreFormControlState(const FormControlState& state) {
   }
 
   SetNeedsValidityCheck();
-  QueueInputAndChangeEvents();
+
+  for (wtf_size_t i = 0; i < items_size; ++i) {
+    if (auto* option = DynamicTo<HTMLOptionElement>(items[i].Get())) {
+      if (old_selection[i] != option->Selected()) {
+        QueueInputAndChangeEvents();
+        break;
+      }
+    }
+  }
 }
 
 void HTMLSelectElement::ParseMultipleAttribute(const AtomicString& value) {

third_party/blink/renderer/core/html/forms/html_text_area_element.cc

@@ -94,6 +94,8 @@ FormControlState HTMLTextAreaElement::SaveFormControlState() const {
 
 void HTMLTextAreaElement::RestoreFormControlState(
     const FormControlState& state) {
+  if (EqualIgnoringNullity(value(), state[0]))
+    return;
   // We don't add kDispatchInputAndChangeEvent to setValue(), and we
   // post tasks to dispatch events instead. This function can be called
   // while we should not dispatch any events.

third_party/blink/renderer/core/html/forms/input_type_view.cc

@@ -177,8 +177,10 @@ FormControlState InputTypeView::SaveFormControlState() const {
   return FormControlState(current_value);
 }
 
-void InputTypeView::RestoreFormControlState(const FormControlState& state) {
+bool InputTypeView::RestoreFormControlState(const FormControlState& state) {
+  String old_value = GetElement().value();
   GetElement().setValue(state[0]);
+  return !EqualIgnoringNullity(old_value, GetElement().value());
 }
 
 bool InputTypeView::HasBadInput() const {

third_party/blink/renderer/core/html/forms/input_type_view.h

@@ -128,7 +128,9 @@ class CORE_EXPORT InputTypeView : public GarbageCollectedMixin {
   virtual void EnsurePrimaryContent() {}
   virtual bool HasFallbackContent() const { return false; }
   virtual FormControlState SaveFormControlState() const;
-  virtual void RestoreFormControlState(const FormControlState&);
+  // Should return |true| if the value is changed and the callsite needs to
+  // dispatch 'input' and 'change' events.
+  virtual bool RestoreFormControlState(const FormControlState&);
 
   // Validation functions
   virtual bool HasBadInput() const;

third_party/blink/renderer/core/html/forms/multiple_fields_temporal_input_type_view.cc

@@ -527,16 +527,18 @@ void MultipleFieldsTemporalInputTypeView::ReadonlyAttributeChanged() {
     edit->ReadOnlyStateChanged();
 }
 
-void MultipleFieldsTemporalInputTypeView::RestoreFormControlState(
+bool MultipleFieldsTemporalInputTypeView::RestoreFormControlState(
     const FormControlState& state) {
   DateTimeEditElement* edit = GetDateTimeEditElement();
   if (!edit)
-    return;
+    return false;
+  String old_value = GetElement().value();
   DateTimeFieldsState date_time_fields_state =
       DateTimeFieldsState::RestoreFormControlState(state);
   edit->SetValueAsDateTimeFieldsState(date_time_fields_state);
   GetElement().SetNonAttributeValue(input_type_->SanitizeValue(edit->Value()));
   UpdateClearButtonVisibility();
+  return !EqualIgnoringNullity(old_value, GetElement().value());
 }
 
 FormControlState MultipleFieldsTemporalInputTypeView::SaveFormControlState()

third_party/blink/renderer/core/html/forms/multiple_fields_temporal_input_type_view.h

@@ -107,7 +107,7 @@ class MultipleFieldsTemporalInputTypeView final
   void MinOrMaxAttributeChanged() final;
   void ReadonlyAttributeChanged() final;
   void RequiredAttributeChanged() final;
-  void RestoreFormControlState(const FormControlState&) final;
+  bool RestoreFormControlState(const FormControlState&) final;
   FormControlState SaveFormControlState() const final;
   void DidSetValue(const String&, bool value_changed) final;
   void StepAttributeChanged() final;

third_party/blink/renderer/core/html/forms/password_input_type.cc

@@ -69,9 +69,10 @@ FormControlState PasswordInputType::SaveFormControlState() const {
   return FormControlState();
 }
 
-void PasswordInputType::RestoreFormControlState(const FormControlState&) {
+bool PasswordInputType::RestoreFormControlState(const FormControlState&) {
   // Should never save/restore password fields.
   NOTREACHED();
+  return false;
 }
 
 bool PasswordInputType::ShouldRespectListAttribute() {

third_party/blink/renderer/core/html/forms/password_input_type.h

@@ -47,7 +47,7 @@ class PasswordInputType final : public BaseTextInputType {
   const AtomicString& FormControlType() const override;
   bool ShouldSaveAndRestoreFormControlState() const override;
   FormControlState SaveFormControlState() const override;
-  void RestoreFormControlState(const FormControlState&) override;
+  bool RestoreFormControlState(const FormControlState&) override;
   bool ShouldRespectListAttribute() override;
 
   bool NeedsContainer() const override;

third_party/blink/web_tests/fast/forms/resources/state-restore-dynamic-controls-frame.html

 <!DOCTYPE html>
 <script>
 document.addEventListener('change', e => {
-  parent.inputOrChangeEvents.push('change/' + e.target.type);
+  parent.inputOrChangeEvents.push('change/' + e.target.id);
 });
 document.addEventListener('input', e => {
-  parent.inputOrChangeEvents.push('input/' + e.target.type);
+  parent.inputOrChangeEvents.push('input/' + e.target.id);
 });
 </script>
 <div></div>
-<input type=checkbox disabled>
-<input type=text disabled>
-<textarea></textarea>
-<select><option>1<option>2</select>
+<input type=checkbox disabled id="checkbox-disabled">
+<input type=text disabled id="text-disabled">
+<input type=checkbox id="checkbox-unmodified">
+<input type=text id="text-unmodified">
+<input type=file id="file-modified">
+<input type=file id="file-unmodified">
+<input type=date id="date-modified">
+<input type=date id="date-unmodified">
+<textarea id="textarea-modified"></textarea>
+<textarea id="textarea-unmodified">abc
+def</textarea>
+<select id="select-modified"><option>1<option>2</select>
+<select id="select-unmodified" multiple>
+<option selected>1</option>
+<option>2</option>
+<option selected>3</option>
+</select>
 <script>
-document.querySelector('div').innerHTML = '<input type=checkbox><input type=text>'
+document.querySelector('div').innerHTML = '<input type=checkbox id="checkbox-modified"><input type=text id="text-modified">';
 </script>

third_party/blink/web_tests/fast/forms/state-restore-dynamic-controls.html

@@ -2,6 +2,17 @@
 <script src="../../resources/testharness.js"></script>
 <script src="../../resources/testharnessreport.js"></script>
 <script src="../../external/wpt/html/interaction/focus/the-autofocus-attribute/resources/utils.js"></script>
+<script src="file/resources/file-drag-common.js"></script>
+<style>
+/* Make the top-left corner of the iframe is equal to the top-left corner
+   of the body.  It's required for dragFilesOntoInput(). */
+body {
+  margin: 0;
+}
+iframe {
+  border: none;
+}
+</style>
 <body>
 <iframe src="resources/state-restore-dynamic-controls-frame.html"></iframe>
 <script>
@@ -16,14 +27,16 @@ promise_test(async t => {
   container.firstChild.click();
   container.lastChild.focus();
   eventSender.keyDown('z');
-  doc.querySelector('textarea').focus();
+  dragFilesOntoInput(doc.querySelector('#file-modified'), ['foo.txt']);
+  doc.querySelector('#date-modified').value = '2020-01-08';
+  doc.querySelector('#textarea-modified').focus();
   eventSender.keyDown('y');
-  doc.querySelector('select').focus();
+  doc.querySelector('#select-modified').focus();
   eventSender.keyDown('2');
   assert_true(container.firstChild.checked, 'sanity check for a checkbox');
   assert_equals(container.lastChild.value, 'z', 'sanity check for a text field');
-  assert_equals(doc.querySelector('textarea').value, 'y', 'sanity check for a textarea');
-  assert_equals(doc.querySelector('select').value, '2', 'sanity check for a select');
+  assert_equals(doc.querySelector('#textarea-modified').value, 'y', 'sanity check for a textarea');
+  assert_equals(doc.querySelector('#select-modified').value, '2', 'sanity check for a select');
 
   // Flush asynchronous input/change events
   await timeOut(t, 0);
@@ -50,7 +63,11 @@ promise_test(async t => {
   // Wait for asynchronous input/change events
   await timeOut(t, 0);
   assert_array_equals(inputOrChangeEvents, [
-      'input/checkbox', 'change/checkbox', 'input/text', 'change/text',
-      'input/textarea', 'change/textarea', 'input/select-one', 'change/select-one']);
+      'input/file-modified', 'change/file-modified',
+      'input/checkbox-modified', 'change/checkbox-modified',
+      'input/text-modified', 'change/text-modified',
+      'input/date-modified', 'change/date-modified',
+      'input/textarea-modified', 'change/textarea-modified',
+      'input/select-modified', 'change/select-modified']);
 }, 'Control states should be restored correctly even if controls were inserted before existing controls.');
 </script>