INPUT element: Do not dispatch events in detachLayoutTree().

When a color chooser is closed, we dispatches a 'change' event asynchronously. Some tests need to be updated due to this behavior change. BUG=658535 Review-Url: https://codereview.chromium.org/2447653002 Cr-Commit-Position: refs/heads/master@{#427286}

INPUT element: Do not dispatch events in detachLayoutTree().
When a color chooser is closed, we dispatches a 'change' event asynchronously. Some tests need to be updated due to this behavior change. BUG=658535 Review-Url: https://codereview.chromium.org/2447653002 Cr-Commit-Position: refs/heads/master@{#427286}
fbe37c72 · tkent · Commit bot · 32a7028b · fbe37c72 · fbe37c72
Commit fbe37c72 authored Oct 24, 2016 by tkent Committed by Commit bot Oct 25, 2016
9 changed files
--- a/third_party/WebKit/LayoutTests/fast/forms/color/color-no-event-during-detach.html
+++ b/third_party/WebKit/LayoutTests/fast/forms/color/color-no-event-during-detach.html
+<!DOCTYPE html>
+<script src="../../../resources/testharness.js"></script>
+<script src="../../../resources/testharnessreport.js"></script>
+<script src="../resources/common.js"></script>
+<input type="color" list>
+<script>
+test(() => {
+  var color = document.querySelector('input');
+  color.setAttribute('value', '#010101');
+  document.body.offsetLeft;
+  clickElement(color);
+  color.remove();
+}, 'Detaching a layout object from a color input should not cause a DCHECK failure.');
+</script>
--- a/third_party/WebKit/LayoutTests/fast/forms/color/input-color-choose-default-value-after-set-value.html
+++ b/third_party/WebKit/LayoutTests/fast/forms/color/input-color-choose-default-value-after-set-value.html
@@ -10,11 +10,13 @@
 <input type="color" id="input" value="#ffffff">
 <script>
 description('Test if change event fires when the user selects the default value after the value was changed by JS.');
+jsTestIsAsync = true;
 var input = document.getElementById('input');
 input.onchange = function() {
    debug("onchange fired: " + input.value);
+    finishJSTest();
 };
 clickElement(input);

--- a/third_party/WebKit/LayoutTests/fast/forms/color/input-color-onchange-event-expected.txt
+++ b/third_party/WebKit/LayoutTests/fast/forms/color/input-color-onchange-event-expected.txt
@@ -7,13 +7,12 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 PASS internals.selectColorInColorChooser({}, '#ff0000'); threw exception TypeError: Failed to execute 'selectColorInColorChooser' on 'Internals': parameter 1 is not of type 'Element'..
 PASS internals.selectColorInColorChooser(document, '#ff0000'); threw exception TypeError: Failed to execute 'selectColorInColorChooser' on 'Internals': parameter 1 is not of type 'Element'..
 input event dispatched - value is:  #ff0000
+PASS input.value is "#ff0000"
 change event dispatched - value changed to #ff0000
 PASS input.value is "#ff0000"
 Change event is only dispatched, when color chooser is closed
 input event dispatched - value is:  #ff0002
-PASS onChange is 0
+PASS Change event was dispatched.
-change event dispatched - value changed to #ff0002
-PASS onChange is 1
 PASS successfullyParsed is true
 TEST COMPLETE

--- a/third_party/WebKit/LayoutTests/fast/forms/color/input-color-onchange-event.html
+++ b/third_party/WebKit/LayoutTests/fast/forms/color/input-color-onchange-event.html
@@ -8,6 +8,7 @@
 <div id="console"></div>
 <script>
 description('Test if change event fires properly when color chooser changes. Bug 66848 <br> To manually test this, click on the input color element in the top left corner and change the value from the color chooser. See if the number of "value changed" messages matches the number of times you changed the color.');
+jsTestIsAsync = true;
 var input = document.createElement('input');
 input.type = 'color';
@@ -22,11 +23,6 @@ input.style.height = '20px';
 var onChange = 0;
-input.onchange = function() {
-    debug("change event dispatched - value changed to " + input.value);
-    onChange++;
-};
 input.oninput = function() {
    debug("input event dispatched - value is:  " + input.value);
 };
@@ -39,21 +35,39 @@ eventSender.mouseUp();
 shouldThrow("internals.selectColorInColorChooser({}, '#ff0000');");
 shouldThrow("internals.selectColorInColorChooser(document, '#ff0000');");
-// input.onchange should be called
+function step1() {
-internals.selectColorInColorChooser(input, '#ff0000');
+    // input.onchange should be called
-internals.endColorChooser(input);
+    input.onchange = step2;
-// input.onchange should not be called
+    internals.selectColorInColorChooser(input, '#ff0000');
-internals.selectColorInColorChooser(input, '#ff0000');
+    internals.endColorChooser(input);
-internals.endColorChooser(input);
+    shouldBe('input.value', '"#ff0000"');
-shouldBe('input.value', '"#ff0000"');
+}
-debug('Change event is only dispatched, when color chooser is closed');
+function step2() {
-onChange = 0;
+    debug("change event dispatched - value changed to " + input.value);
-internals.selectColorInColorChooser(input, '#ff0002');
+    // input.onchange should not be called
-shouldBe('onChange', '0');
+    input.onchange = () => {
-internals.endColorChooser(input);
+        testFailed("Change event should not be dispatched.");
-shouldBe('onChange', '1');
+    };
+    internals.selectColorInColorChooser(input, '#ff0000');
+    internals.endColorChooser(input);
+    shouldBe('input.value', '"#ff0000"');
+    step3();
+}
+function step3() {
+    debug('Change event is only dispatched, when color chooser is closed');
+    input.onchange = step4;
+    internals.selectColorInColorChooser(input, '#ff0002');
+    internals.endColorChooser(input);
+}
+function step4() {
+    testPassed('Change event was dispatched.');
+    finishJSTest();
+}
+step1();
 </script>
 </body>
 </html>
--- a/third_party/WebKit/LayoutTests/platform/mac/fast/forms/color/input-color-onchange-event-expected.txt
+++ b/third_party/WebKit/LayoutTests/platform/mac/fast/forms/color/input-color-onchange-event-expected.txt
@@ -11,9 +11,8 @@ change event dispatched - value changed to #ff0000
 PASS input.value is "#ff0000"
 Change event is only dispatched, when color chooser is closed
 input event dispatched - value is:  #ff0002
-change event dispatched - value changed to #ff0002
+PASS Change event was dispatched.
-FAIL onChange should be 0. Was 1.
+FAIL input.value should be #ff0000. Was #ff0002.
-PASS onChange is 1
 PASS successfullyParsed is true
 TEST COMPLETE

--- a/third_party/WebKit/Source/core/html/HTMLTextFormControlElement.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLTextFormControlElement.cpp
@@ -224,6 +224,18 @@ void HTMLTextFormControlElement::dispatchFormControlChangeEvent() {
  setChangedSinceLastFormControlChangeEvent(false);
 }
+void HTMLTextFormControlElement::enqueueChangeEvent() {
+  String newValue = value();
+  if (shouldDispatchFormControlChangeEvent(m_textAsOfLastFormControlChangeEvent,
+                                           newValue)) {
+    setTextAsOfLastFormControlChangeEvent(newValue);
+    Event* event = Event::createBubble(EventTypeNames::change);
+    event->setTarget(this);
+    document().enqueueAnimationFrameEvent(event);
+  }
+  setChangedSinceLastFormControlChangeEvent(false);
+}
 void HTMLTextFormControlElement::setRangeText(const String& replacement,
                                              ExceptionState& exceptionState) {
  setRangeText(replacement, selectionStart(), selectionEnd(), "preserve",

--- a/third_party/WebKit/Source/core/html/HTMLTextFormControlElement.h
+++ b/third_party/WebKit/Source/core/html/HTMLTextFormControlElement.h
@@ -103,6 +103,7 @@ class CORE_EXPORT HTMLTextFormControlElement
  void setAutocapitalize(const AtomicString&);
  void dispatchFormControlChangeEvent() final;
+  void enqueueChangeEvent();
  virtual String value() const = 0;
  virtual void setValue(const String&,

--- a/third_party/WebKit/Source/core/html/forms/ColorInputType.cpp
+++ b/third_party/WebKit/Source/core/html/forms/ColorInputType.cpp
@@ -202,9 +202,8 @@ void ColorInputType::didChooseColor(const Color& color) {
 }
 void ColorInputType::didEndChooser() {
-  EventQueueScope scope;
  if (LayoutTheme::theme().isModalColorChooser())
-    element().dispatchFormControlChangeEvent();
+    element().enqueueChangeEvent();
  m_chooser.clear();
 }

--- a/third_party/WebKit/Source/web/WebPagePopupImpl.cpp
+++ b/third_party/WebKit/Source/web/WebPagePopupImpl.cpp
@@ -518,26 +518,27 @@ void WebPagePopupImpl::close() {
 }
 void WebPagePopupImpl::closePopup() {
-  // This function can be called in EventDispatchForbiddenScope for the main
+  {
-  // document, and the following operations dispatch some events.  It's safe
+    // This function can be called in EventDispatchForbiddenScope for the main
-  // because web authors can't listen the events.
+    // document, and the following operations dispatch some events.  It's safe
-  EventDispatchForbiddenScope::AllowUserAgentEvents allowEvents;
+    // because web authors can't listen the events.
+    EventDispatchForbiddenScope::AllowUserAgentEvents allowEvents;
-  if (m_page) {
-    toLocalFrame(m_page->mainFrame())->loader().stopAllLoaders();
+    if (m_page) {
-    PagePopupSupplement::uninstall(*toLocalFrame(m_page->mainFrame()));
+      toLocalFrame(m_page->mainFrame())->loader().stopAllLoaders();
-  }
+      PagePopupSupplement::uninstall(*toLocalFrame(m_page->mainFrame()));
-  bool closeAlreadyCalled = m_closing;
+    }
-  m_closing = true;
+    bool closeAlreadyCalled = m_closing;
+    m_closing = true;
-  destroyPage();
+    destroyPage();
-  // m_widgetClient might be 0 because this widget might be already closed.
+    // m_widgetClient might be 0 because this widget might be already closed.
-  if (m_widgetClient && !closeAlreadyCalled) {
+    if (m_widgetClient && !closeAlreadyCalled) {
-    // closeWidgetSoon() will call this->close() later.
+      // closeWidgetSoon() will call this->close() later.
-    m_widgetClient->closeWidgetSoon();
+      m_widgetClient->closeWidgetSoon();
+    }
  }
  m_popupClient->didClosePopup();
  m_webView->cleanupPagePopup();
 }