Title: [286425] trunk
Revision
286425
Author
commit-qu...@webkit.org
Date
2021-12-02 07:21:12 -0800 (Thu, 02 Dec 2021)

Log Message

Add a fast path for empty string to setInnerHTML()
https://bugs.webkit.org/show_bug.cgi?id=233647

Patch by Alexey Shvayka <ashva...@apple.com> on 2021-12-02
Reviewed by Geoff Garen.

LayoutTests/imported/w3c:

* web-platform-tests/html/semantics/scripting-1/the-template-element/innerhtml-on-templates/innerhtml-expected.txt:
* web-platform-tests/html/semantics/scripting-1/the-template-element/innerhtml-on-templates/innerhtml.html:

Source/WebCore:

This patch adds a fast path for `element.innerHTML = ""`, which is a common idiom for
removing all children from an element, while ensuring <template> contents is modified
rather than its children [1], and that mutation records are enqueued.

Although there are quite a few insertion modes [1], parsing empty string creates
additional elements only inside <html> container (please see "Anything else" clauses).

Bypassing parser instantiation results in 3.2x progression of the attached microbenchmark.
Both Gecko and Blink has this optimization, which was reported to progress Speedometer2
score (especially Vanilla / jQuery / Preact subtests).

Our `textContent` and `innerText` setters are already fast enough for empty strings.

[1] https://w3c.github.io/DOM-Parsing/#dom-innerhtml-innerhtml (setting, step 3)
[2] https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-inhtml

No behavior change.

* dom/Element.cpp:
(WebCore::Element::setInnerHTML):
* dom/ShadowRoot.cpp:
(WebCore::ShadowRoot::setInnerHTML):

Modified Paths

Diff

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (286424 => 286425)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2021-12-02 14:26:22 UTC (rev 286424)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2021-12-02 15:21:12 UTC (rev 286425)
@@ -1,3 +1,13 @@
+2021-12-02  Alexey Shvayka  <ashva...@apple.com>
+
+        Add a fast path for empty string to setInnerHTML()
+        https://bugs.webkit.org/show_bug.cgi?id=233647
+
+        Reviewed by Geoff Garen.
+
+        * web-platform-tests/html/semantics/scripting-1/the-template-element/innerhtml-on-templates/innerhtml-expected.txt:
+        * web-platform-tests/html/semantics/scripting-1/the-template-element/innerhtml-on-templates/innerhtml.html:
+
 2021-12-02  Youenn Fablet  <you...@apple.com>
 
         Add support for NavigationPreloadManager

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-template-element/innerhtml-on-templates/innerhtml-expected.txt (286424 => 286425)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-template-element/innerhtml-on-templates/innerhtml-expected.txt	2021-12-02 14:26:22 UTC (rev 286424)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-template-element/innerhtml-on-templates/innerhtml-expected.txt	2021-12-02 15:21:12 UTC (rev 286425)
@@ -1,5 +1,6 @@
 
 PASS innerHTML of template element replaces all referenced by the content attribute
+PASS innerHTML of template element replaces all referenced by the content attribute. Test empty HTML string
 PASS innerHTML of template element replaces all referenced by the content attribute. Test nested template
 PASS innerHTML of template element replaces all referenced by the content attribute. Test loading of HTML document from a file
 

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-template-element/innerhtml-on-templates/innerhtml.html (286424 => 286425)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-template-element/innerhtml-on-templates/innerhtml.html	2021-12-02 14:26:22 UTC (rev 286424)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/html/semantics/scripting-1/the-template-element/innerhtml-on-templates/innerhtml.html	2021-12-02 15:21:12 UTC (rev 286425)
@@ -35,7 +35,26 @@
 }, 'innerHTML of template element replaces all referenced by the content attribute');
 
 
+test(function () {
+    var doc = newHTMLDocument();
+    var template = doc.createElement('template');
 
+    var div1 = doc.createElement('div');
+    div1.setAttribute('id', 'div1');
+    template.content.appendChild(div1);
+
+    assert_not_equals(template.content.querySelector('#div1'), null,
+            'Element should present in template content');
+
+    template.innerHTML = '';
+
+    assert_false(template.content.hasChildNodes(),
+            'Template content should be removed by innerHTML');
+
+}, 'innerHTML of template element replaces all referenced by the content attribute. '
+    + 'Test empty HTML string');
+
+
 test(function () {
     var doc = newHTMLDocument();
     var template = doc.createElement('template');

Modified: trunk/Source/WebCore/ChangeLog (286424 => 286425)


--- trunk/Source/WebCore/ChangeLog	2021-12-02 14:26:22 UTC (rev 286424)
+++ trunk/Source/WebCore/ChangeLog	2021-12-02 15:21:12 UTC (rev 286425)
@@ -1,3 +1,33 @@
+2021-12-02  Alexey Shvayka  <ashva...@apple.com>
+
+        Add a fast path for empty string to setInnerHTML()
+        https://bugs.webkit.org/show_bug.cgi?id=233647
+
+        Reviewed by Geoff Garen.
+
+        This patch adds a fast path for `element.innerHTML = ""`, which is a common idiom for
+        removing all children from an element, while ensuring <template> contents is modified
+        rather than its children [1], and that mutation records are enqueued.
+
+        Although there are quite a few insertion modes [1], parsing empty string creates
+        additional elements only inside <html> container (please see "Anything else" clauses).
+
+        Bypassing parser instantiation results in 3.2x progression of the attached microbenchmark.
+        Both Gecko and Blink has this optimization, which was reported to progress Speedometer2
+        score (especially Vanilla / jQuery / Preact subtests).
+
+        Our `textContent` and `innerText` setters are already fast enough for empty strings.
+
+        [1] https://w3c.github.io/DOM-Parsing/#dom-innerhtml-innerhtml (setting, step 3)
+        [2] https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-inhtml
+
+        No behavior change.
+
+        * dom/Element.cpp:
+        (WebCore::Element::setInnerHTML):
+        * dom/ShadowRoot.cpp:
+        (WebCore::ShadowRoot::setInnerHTML):
+
 2021-12-02  Kimmo Kinnunen  <kkinnu...@apple.com>
 
         GraphicsContextGLANGLE should not have Cocoa Mac specific display reconfiguration code

Modified: trunk/Source/WebCore/dom/Element.cpp (286424 => 286425)


--- trunk/Source/WebCore/dom/Element.cpp	2021-12-02 14:26:22 UTC (rev 286424)
+++ trunk/Source/WebCore/dom/Element.cpp	2021-12-02 15:21:12 UTC (rev 286425)
@@ -31,6 +31,7 @@
 #include "AttributeChangeInvalidation.h"
 #include "CSSParser.h"
 #include "ChildChangeInvalidation.h"
+#include "ChildListMutationScope.h"
 #include "Chrome.h"
 #include "ChromeClient.h"
 #include "ClassChangeInvalidation.h"
@@ -3266,10 +3267,6 @@
 
 ExceptionOr<void> Element::setInnerHTML(const String& html)
 {
-    auto fragment = createFragmentForInnerOuterHTML(*this, html, AllowScriptingContent);
-    if (fragment.hasException())
-        return fragment.releaseException();
-
     ContainerNode* container;
     if (!is<HTMLTemplateElement>(*this))
         container = this;
@@ -3276,6 +3273,18 @@
     else
         container = &downcast<HTMLTemplateElement>(*this).content();
 
+    // Parsing empty string creates additional elements only inside <html> container
+    // https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-inhtml
+    if (html.isEmpty() && !is<HTMLHtmlElement>(*container)) {
+        ChildListMutationScope mutation(*container);
+        container->removeChildren();
+        return { };
+    }
+
+    auto fragment = createFragmentForInnerOuterHTML(*this, html, AllowScriptingContent);
+    if (fragment.hasException())
+        return fragment.releaseException();
+
     return replaceChildrenWithFragment(*container, fragment.releaseReturnValue());
 }
 

Modified: trunk/Source/WebCore/dom/ShadowRoot.cpp (286424 => 286425)


--- trunk/Source/WebCore/dom/ShadowRoot.cpp	2021-12-02 14:26:22 UTC (rev 286424)
+++ trunk/Source/WebCore/dom/ShadowRoot.cpp	2021-12-02 15:21:12 UTC (rev 286425)
@@ -29,6 +29,7 @@
 #include "ShadowRoot.h"
 
 #include "CSSStyleSheet.h"
+#include "ChildListMutationScope.h"
 #include "ElementInlines.h"
 #include "ElementTraversal.h"
 #include "HTMLParserIdioms.h"
@@ -182,6 +183,12 @@
 
 ExceptionOr<void> ShadowRoot::setInnerHTML(const String& markup)
 {
+    if (markup.isEmpty()) {
+        ChildListMutationScope mutation(*this);
+        removeChildren();
+        return { };
+    }
+
     auto fragment = createFragmentForInnerOuterHTML(*host(), markup, AllowScriptingContent);
     if (fragment.hasException())
         return fragment.releaseException();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to