Avoid style resolution when clearing focused element.
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 Sep 2017 18:23:22 +0000 (18:23 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 18 Sep 2017 18:23:22 +0000 (18:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=176224
<rdar://problem/34206409>

Reviewed by Zalan Bujtas.

Source/WebCore:

Test: fast/dom/focus-style-resolution.html

* dom/Document.cpp:
(WebCore::Document::setFocusedElement):

    Don't do synchronous style resolution with FocusRemovalEventsMode::DoNotDispatch.
    Style resolution may dispatch events.

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

    Move resolveStyleIfNeeded call to setFocusedElement. It is the only client for didBlur.

LayoutTests:

* fast/dom/focus-style-resolution-expected.txt: Added.
* fast/dom/focus-style-resolution.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@222167 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/dom/focus-style-resolution-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/focus-style-resolution.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/html/HTMLInputElement.cpp

index 184d87b..11b4f71 100644 (file)
@@ -1,5 +1,16 @@
 2017-09-18  Antti Koivisto  <antti@apple.com>
 
+        Avoid style resolution when clearing focused element.
+        https://bugs.webkit.org/show_bug.cgi?id=176224
+        <rdar://problem/34206409>
+
+        Reviewed by Zalan Bujtas.
+
+        * fast/dom/focus-style-resolution-expected.txt: Added.
+        * fast/dom/focus-style-resolution.html: Added.
+
+2017-09-18  Antti Koivisto  <antti@apple.com>
+
         Try to unflake a test.
 
         Unreviewed.
diff --git a/LayoutTests/fast/dom/focus-style-resolution-expected.txt b/LayoutTests/fast/dom/focus-style-resolution-expected.txt
new file mode 100644 (file)
index 0000000..c47940d
--- /dev/null
@@ -0,0 +1,2 @@
+This test passes if it doesn't assert or crash.
+  
diff --git a/LayoutTests/fast/dom/focus-style-resolution.html b/LayoutTests/fast/dom/focus-style-resolution.html
new file mode 100644 (file)
index 0000000..6a13918
--- /dev/null
@@ -0,0 +1,31 @@
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+function eventhandler1() {
+txt.appendChild(kg);
+}
+
+function eventhandler2() {
+anim.appendChild(kg);
+}
+
+function eventhandler3() {
+table.scrollIntoView(true);
+testRunner.notifyDone();
+}
+
+</script>
+This test passes if it doesn't assert or crash.
+<table id="table"></table>
+<form>
+<input id="kg" autofocus="autofocus">
+</form>
+<svg>
+<animate id="anim" attributeName="text-anchor" from="middle" to="inherit" onbegin="eventhandler1()" />
+<text id="txt" onload="eventhandler3()">
+<font color="white"></font>
+<select onfocus="eventhandler2()" autofocus="autofocus">
+<textarea>a</textarea>
+<iframe onload="eventhandler1()"></iframe>
index cdf6e14..a2151d1 100644 (file)
@@ -1,5 +1,26 @@
 2017-09-18  Antti Koivisto  <antti@apple.com>
 
+        Avoid style resolution when clearing focused element.
+        https://bugs.webkit.org/show_bug.cgi?id=176224
+        <rdar://problem/34206409>
+
+        Reviewed by Zalan Bujtas.
+
+        Test: fast/dom/focus-style-resolution.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::setFocusedElement):
+
+            Don't do synchronous style resolution with FocusRemovalEventsMode::DoNotDispatch.
+            Style resolution may dispatch events.
+
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::didBlur):
+
+            Move resolveStyleIfNeeded call to setFocusedElement. It is the only client for didBlur.
+
+2017-09-18  Antti Koivisto  <antti@apple.com>
+
         Rolling out the previous to land again with a test.
 
         * dom/Document.cpp:
index fc96f1e..7c2725a 100644 (file)
@@ -3840,8 +3840,14 @@ bool Document::setFocusedElement(Element* element, FocusDirection direction, Foc
                 view()->setFocus(false);
         }
 
-        if (is<HTMLInputElement>(oldFocusedElement.get()))
+        if (is<HTMLInputElement>(oldFocusedElement.get())) {
+            // HTMLInputElement::didBlur just scrolls text fields back to the beginning.
+            // FIXME: This could be done asynchronusly.
+            // Updating style may dispatch events due to PostResolutionCallback
+            if (eventsMode == FocusRemovalEventsMode::Dispatch)
+                updateStyleIfNeeded();
             downcast<HTMLInputElement>(*oldFocusedElement).didBlur();
+        }
     }
 
     if (newFocusedElement && newFocusedElement->isFocusable()) {
@@ -3915,7 +3921,10 @@ bool Document::setFocusedElement(Element* element, FocusDirection direction, Foc
         page()->chrome().focusedElementChanged(m_focusedElement.get());
 
 SetFocusedNodeDone:
-    updateStyleIfNeeded();
+    // Updating style may dispatch events due to PostResolutionCallback
+    // FIXME: Why is synchronous style update needed here at all?
+    if (eventsMode == FocusRemovalEventsMode::Dispatch)
+        updateStyleIfNeeded();
     return !focusChangeBlocked;
 }
 
index 40bf52f..f5d36af 100644 (file)
@@ -1122,9 +1122,6 @@ void HTMLInputElement::didDispatchClickEvent(Event& event, const InputElementCli
 
 void HTMLInputElement::didBlur()
 {
-    // We need to update style here, rather than in InputType itself, since style recomputation may fire events
-    // that could change the input's type.
-    document().updateStyleIfNeeded();
     m_inputType->elementDidBlur();
 }