Title: [229393] trunk
Revision
229393
Author
za...@apple.com
Date
2018-03-07 19:51:33 -0800 (Wed, 07 Mar 2018)

Log Message

Invalid innerTextRenderer in RenderTextControlSingleLine::styleDidChange()
https://bugs.webkit.org/show_bug.cgi?id=183385
<rdar://problem/38085397>

Reviewed by Antti Koivisto.

Source/WebCore:

When HTMLInputElement::updateType() is called with a dirty value, we eagerly change the m_inputType first
and then we take care of the dirty value by calling setAttributeWithoutSynchronization().
With a DOMSubtreeModified event listener attached, setAttributeWithoutSynchronization() can end up running some
layout code (offsetHeight) with a renderer - m_inputType mismatch.

This patch ensures that we don't change the m_inputType until after we finished setting the new value.

Test: fast/DOM/HTMLInputElement/input-value-and-type-change-crash.html

* html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::updateType):

LayoutTests:

* fast/DOM/HTMLInputElement/input-value-and-type-change-crash-expected.txt: Added.
* fast/DOM/HTMLInputElement/input-value-and-type-change-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (229392 => 229393)


--- trunk/LayoutTests/ChangeLog	2018-03-08 03:31:01 UTC (rev 229392)
+++ trunk/LayoutTests/ChangeLog	2018-03-08 03:51:33 UTC (rev 229393)
@@ -1,3 +1,14 @@
+2018-03-07  Zalan Bujtas  <za...@apple.com>
+
+        Invalid innerTextRenderer in RenderTextControlSingleLine::styleDidChange()
+        https://bugs.webkit.org/show_bug.cgi?id=183385
+        <rdar://problem/38085397>
+
+        Reviewed by Antti Koivisto.
+
+        * fast/DOM/HTMLInputElement/input-value-and-type-change-crash-expected.txt: Added.
+        * fast/DOM/HTMLInputElement/input-value-and-type-change-crash.html: Added.
+
 2018-03-06  Ryan Haddad  <ryanhad...@apple.com>
 
         Update TestExpectations for media/modern-media-controls/fullscreen-support/fullscreen-support-press.html.

Added: trunk/LayoutTests/fast/DOM/HTMLInputElement/input-value-and-type-change-crash-expected.txt (0 => 229393)


--- trunk/LayoutTests/fast/DOM/HTMLInputElement/input-value-and-type-change-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/DOM/HTMLInputElement/input-value-and-type-change-crash-expected.txt	2018-03-08 03:51:33 UTC (rev 229393)
@@ -0,0 +1 @@
+ Pass if no crash.

Added: trunk/LayoutTests/fast/DOM/HTMLInputElement/input-value-and-type-change-crash.html (0 => 229393)


--- trunk/LayoutTests/fast/DOM/HTMLInputElement/input-value-and-type-change-crash.html	                        (rev 0)
+++ trunk/LayoutTests/fast/DOM/HTMLInputElement/input-value-and-type-change-crash.html	2018-03-08 03:51:33 UTC (rev 229393)
@@ -0,0 +1,16 @@
+<input id=input>
+Pass if no crash.
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+    
+function eventhandler() {
+    document.body.offsetHeight;
+}
+
+input.addEventListener("DOMSubtreeModified", eventhandler);
+document.execCommand("selectAll", true);
+
+input.value = "foobar";
+input.type = "button";
+</script>

Modified: trunk/Source/WebCore/ChangeLog (229392 => 229393)


--- trunk/Source/WebCore/ChangeLog	2018-03-08 03:31:01 UTC (rev 229392)
+++ trunk/Source/WebCore/ChangeLog	2018-03-08 03:51:33 UTC (rev 229393)
@@ -1,3 +1,23 @@
+2018-03-07  Zalan Bujtas  <za...@apple.com>
+
+        Invalid innerTextRenderer in RenderTextControlSingleLine::styleDidChange()
+        https://bugs.webkit.org/show_bug.cgi?id=183385
+        <rdar://problem/38085397>
+
+        Reviewed by Antti Koivisto.
+
+        When HTMLInputElement::updateType() is called with a dirty value, we eagerly change the m_inputType first
+        and then we take care of the dirty value by calling setAttributeWithoutSynchronization().
+        With a DOMSubtreeModified event listener attached, setAttributeWithoutSynchronization() can end up running some
+        layout code (offsetHeight) with a renderer - m_inputType mismatch.
+
+        This patch ensures that we don't change the m_inputType until after we finished setting the new value.
+
+        Test: fast/DOM/HTMLInputElement/input-value-and-type-change-crash.html
+
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::updateType):
+
 2018-03-07  Chris Dumez  <cdu...@apple.com>
 
         Get rid of custom bindings for History's replaceState() / pushState()

Modified: trunk/Source/WebCore/html/HTMLInputElement.cpp (229392 => 229393)


--- trunk/Source/WebCore/html/HTMLInputElement.cpp	2018-03-08 03:31:01 UTC (rev 229392)
+++ trunk/Source/WebCore/html/HTMLInputElement.cpp	2018-03-08 03:51:33 UTC (rev 229393)
@@ -491,10 +491,16 @@
     removeFromRadioButtonGroup();
 
     bool didStoreValue = m_inputType->storesValueSeparateFromAttribute();
+    bool willStoreValue = newType->storesValueSeparateFromAttribute();
     bool neededSuspensionCallback = needsSuspensionCallback();
     bool didRespectHeightAndWidth = m_inputType->shouldRespectHeightAndWidthAttributes();
     bool wasSuccessfulSubmitButtonCandidate = m_inputType->canBeSuccessfulSubmitButton();
 
+    if (didStoreValue && !willStoreValue && hasDirtyValue()) {
+        setAttributeWithoutSynchronization(valueAttr, m_valueIfDirty);
+        m_valueIfDirty = String();
+    }
+
     m_inputType->destroyShadowSubtree();
 
     m_inputType = WTFMove(newType);
@@ -503,12 +509,6 @@
 
     setNeedsWillValidateCheck();
 
-    bool willStoreValue = m_inputType->storesValueSeparateFromAttribute();
-
-    if (didStoreValue && !willStoreValue && hasDirtyValue()) {
-        setAttributeWithoutSynchronization(valueAttr, m_valueIfDirty);
-        m_valueIfDirty = String();
-    }
     if (!didStoreValue && willStoreValue)
         m_valueIfDirty = sanitizeValue(attributeWithoutSynchronization(valueAttr));
     else
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to