[iOS] Editable regions causes ~1% slowdown in PLT5
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 Jun 2020 19:05:40 +0000 (19:05 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 Jun 2020 19:05:40 +0000 (19:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=213659
<rdar://problem/64361390>

Reviewed by Simon Fraser.

Source/WebCore:

Fix the slowdown by only enabling editable region when Page::editableElementsInRect is called.

There are two parts that make computing the editable region expensive:
        1. Requires traversing descendents during painting when a normal paint may be
           able to avoid this.
        2. Can cause more event region invalidations because it extends the invalidation
           criterion to include changes to element editability.

Tests: editing/editable-region/hit-test-basic-without-editable-region.html
       editing/editable-region/iframe-without-editable-region.html
       editing/editable-region/text-field-basic-without-editable-region.html

Tests: editing/editable-region/hit-test-basic-without-editable-region.html
       editing/editable-region/iframe-without-editable-region.html
       editing/editable-region/text-field-basic-without-editable-region.html

* page/Frame.cpp:
(WebCore::Frame::invalidateContentEventRegionsIfNeeded): Check if editable region is enabled.
If it is then do what we do now. Otherwise, don't invalidate the region unless we were going
to do so anyway.
* page/Page.cpp:
(WebCore::Page::setEditableRegionEnabled): Added. Update state and then invalidate
the event region in all layers.
(WebCore::Page::shouldBuildEditableRegion const): Added. Returns whether to build the
editable region: either when Page::isEditableRegionEnabled() is true or the editable
region debug overlay is enabled.
(WebCore::Page::didFinishLoad): Turn off editable region as it may not be needed on
the new page.
* page/Page.h:
(WebCore::Page::isEditableRegionEnabled const): Added.

* rendering/EventRegion.cpp:
(WebCore::EventRegion::unite):
(WebCore::EventRegion::translate):
(WebCore::EventRegion::containsEditableElementsInRect const):
(WebCore::EventRegion::dump const):
* rendering/EventRegion.h:
(WebCore::EventRegion::hasEditableRegion const):
(WebCore::EventRegion::rectsForEditableElements const):
(WebCore::EventRegion::decode):
(WebCore::EventRegion::ensureEditableRegion):
The editable region is now an Optional<>. There will only be one if ensureEditableRegion
was called, which is only when Page::isEditableRegionEnabled() returns true.

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::paintObject):
* rendering/RenderElement.cpp:
(WebCore::RenderElement::styleWillChange):
* rendering/RenderLayer.h:
* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::maintainsEventRegion const):
Only do what we do now if Page::shouldBuildEditableRegion() returns true.

(WebCore::RenderLayerBacking::updateEventRegion): Instantiate the editable region, if needed.
Painting will then populate it.

* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::applyToCompositedLayerIncludingDescendants): Added.
(WebCore::RenderLayerCompositor::invalidateEventRegionForAllLayers): Added.
(WebCore::RenderLayerCompositor::clearBackingForAllLayers): Wrote in terms of applyToCompositedLayerIncludingDescendants.
(WebCore::RenderLayerCompositor::clearBackingForLayerIncludingDescendants): Deleted.
* rendering/RenderLayerCompositor.h:
* testing/InternalSettings.cpp:
(WebCore::InternalSettings::setEditableRegionEnabled):
* testing/InternalSettings.h:
* testing/InternalSettings.idl:
Add a new internal setting for testing purposes to toggle enabling/disabling editable region.

Source/WebKit:

Fix up RemoteLayerTreeViews now that the editable region is an Optional<>. Have
the UI process message the web process to enable editable region on the first
invocation of _requestTextInputContextsInRect, which is good indicator that there
would be benefit to computing it as a typical client that calls it will call it
many times.

* UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:
(WebKit::mayContainEditableElementsInRect): Fix up the logic now that there may
be not be an editable region sent over in the last event region update. If there
isn't one then we don't know if there are editable elements in the rect of not.
So, return true, which could turn out to be a false positive if there aren't any
editable elements in the rect. The caller will have to message the web process
to find out the real answer if they want it. Just to clarify, it's OK for this
function to have false positives, but it must never have false negatives.
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::textInputContextsInRect): Enable editable region. If it's already
enabled then doing so again does nothing.

LayoutTests:

Add some tests to ensure the new setting works and the feature can be disabled. Note
that the setting is enabled by default to keep the current behavior. See the WebCore
ChangeLog entry for more details.

Update existings tests that need the editable region enabled to enable it now that it must
be explicitly enabled.

* editing/editable-region/fixed-and-absolute-contenteditable-scrolled.html:
* editing/editable-region/float-contenteditable.html:
* editing/editable-region/hit-test-basic-without-editable-region-expected.txt: Added.
* editing/editable-region/hit-test-basic-without-editable-region.html: Copied from LayoutTests/editing/editable-region/hit-test-basic.html.
* editing/editable-region/hit-test-basic.html:
* editing/editable-region/hit-test-editable-document-element.html:
* editing/editable-region/hit-test-fixed.html:
* editing/editable-region/hit-test-overlap.html:
* editing/editable-region/hit-test-textarea-empty-space.html:
* editing/editable-region/iframe-without-editable-region-expected.txt: Added.
* editing/editable-region/iframe-without-editable-region.html: Copied from LayoutTests/editing/editable-region/iframe.html.
* editing/editable-region/iframe.html:
* editing/editable-region/out-hanging-child-of-contenteditable.html:
* editing/editable-region/overflow-scroll-text-field-and-contenteditable.html:
* editing/editable-region/relative-inside-fixed-contenteditable-scrolled.html:
* editing/editable-region/relative-inside-transformed-contenteditable.html:
* editing/editable-region/search-field-basic.html:
* editing/editable-region/text-field-basic-without-editable-region-expected.txt: Added.
* editing/editable-region/text-field-basic-without-editable-region.html: Copied from LayoutTests/editing/editable-region/text-field-basic.html.
* editing/editable-region/text-field-basic.html:
* editing/editable-region/text-field-inside-composited-negative-z-index-layer.html:
* editing/editable-region/textarea-basic.html:
* editing/editable-region/transformed-scrolled-on-top-of-fixed-contenteditables.html:

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

42 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/editable-region/fixed-and-absolute-contenteditable-scrolled.html
LayoutTests/editing/editable-region/float-contenteditable.html
LayoutTests/editing/editable-region/hit-test-basic-without-editable-region-expected.txt [new file with mode: 0644]
LayoutTests/editing/editable-region/hit-test-basic-without-editable-region.html [new file with mode: 0644]
LayoutTests/editing/editable-region/hit-test-basic.html
LayoutTests/editing/editable-region/hit-test-editable-document-element.html
LayoutTests/editing/editable-region/hit-test-fixed.html
LayoutTests/editing/editable-region/hit-test-overlap.html
LayoutTests/editing/editable-region/hit-test-textarea-empty-space.html
LayoutTests/editing/editable-region/iframe-without-editable-region-expected.txt [new file with mode: 0644]
LayoutTests/editing/editable-region/iframe-without-editable-region.html [new file with mode: 0644]
LayoutTests/editing/editable-region/iframe.html
LayoutTests/editing/editable-region/out-hanging-child-of-contenteditable.html
LayoutTests/editing/editable-region/overflow-scroll-text-field-and-contenteditable.html
LayoutTests/editing/editable-region/relative-inside-fixed-contenteditable-scrolled.html
LayoutTests/editing/editable-region/relative-inside-transformed-contenteditable.html
LayoutTests/editing/editable-region/search-field-basic.html
LayoutTests/editing/editable-region/text-field-basic-without-editable-region-expected.txt [new file with mode: 0644]
LayoutTests/editing/editable-region/text-field-basic-without-editable-region.html [new file with mode: 0644]
LayoutTests/editing/editable-region/text-field-basic.html
LayoutTests/editing/editable-region/text-field-inside-composited-negative-z-index-layer.html
LayoutTests/editing/editable-region/textarea-basic.html
LayoutTests/editing/editable-region/transformed-scrolled-on-top-of-fixed-contenteditables.html
Source/WebCore/ChangeLog
Source/WebCore/page/Frame.cpp
Source/WebCore/page/Page.cpp
Source/WebCore/page/Page.h
Source/WebCore/rendering/EventRegion.cpp
Source/WebCore/rendering/EventRegion.h
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderLayer.h
Source/WebCore/rendering/RenderLayerBacking.cpp
Source/WebCore/rendering/RenderLayerCompositor.cpp
Source/WebCore/rendering/RenderLayerCompositor.h
Source/WebCore/testing/InternalSettings.cpp
Source/WebCore/testing/InternalSettings.h
Source/WebCore/testing/InternalSettings.idl
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

index 2949eda..b190c30 100644 (file)
@@ -1,3 +1,42 @@
+2020-06-30  Daniel Bates  <dabates@apple.com>
+
+        [iOS] Editable regions causes ~1% slowdown in PLT5
+        https://bugs.webkit.org/show_bug.cgi?id=213659
+        <rdar://problem/64361390>
+
+        Reviewed by Simon Fraser.
+
+        Add some tests to ensure the new setting works and the feature can be disabled. Note
+        that the setting is enabled by default to keep the current behavior. See the WebCore
+        ChangeLog entry for more details.
+
+        Update existings tests that need the editable region enabled to enable it now that it must
+        be explicitly enabled.
+
+        * editing/editable-region/fixed-and-absolute-contenteditable-scrolled.html:
+        * editing/editable-region/float-contenteditable.html:
+        * editing/editable-region/hit-test-basic-without-editable-region-expected.txt: Added.
+        * editing/editable-region/hit-test-basic-without-editable-region.html: Copied from LayoutTests/editing/editable-region/hit-test-basic.html.
+        * editing/editable-region/hit-test-basic.html:
+        * editing/editable-region/hit-test-editable-document-element.html:
+        * editing/editable-region/hit-test-fixed.html:
+        * editing/editable-region/hit-test-overlap.html:
+        * editing/editable-region/hit-test-textarea-empty-space.html:
+        * editing/editable-region/iframe-without-editable-region-expected.txt: Added.
+        * editing/editable-region/iframe-without-editable-region.html: Copied from LayoutTests/editing/editable-region/iframe.html.
+        * editing/editable-region/iframe.html:
+        * editing/editable-region/out-hanging-child-of-contenteditable.html:
+        * editing/editable-region/overflow-scroll-text-field-and-contenteditable.html:
+        * editing/editable-region/relative-inside-fixed-contenteditable-scrolled.html:
+        * editing/editable-region/relative-inside-transformed-contenteditable.html:
+        * editing/editable-region/search-field-basic.html:
+        * editing/editable-region/text-field-basic-without-editable-region-expected.txt: Added.
+        * editing/editable-region/text-field-basic-without-editable-region.html: Copied from LayoutTests/editing/editable-region/text-field-basic.html.
+        * editing/editable-region/text-field-basic.html:
+        * editing/editable-region/text-field-inside-composited-negative-z-index-layer.html:
+        * editing/editable-region/textarea-basic.html:
+        * editing/editable-region/transformed-scrolled-on-top-of-fixed-contenteditables.html:
+
 2020-06-30  Antoine Quint  <graouts@webkit.org>
 
         [iOS] Test landed flaky: webanimations/accelerated-animation-with-easing.html
index 3b35dbd..044a7a5 100644 (file)
@@ -1,5 +1,11 @@
 <!DOCTYPE html>
 <html>
+<head>
+<script>
+if (window.internals)
+    internals.settings.setEditableRegionEnabled(true);
+</script>
+</head>
 <body style="height: 4096px">
 <div style="position: fixed; border: 1px solid black; width: 100%; height: 40px" contenteditable="true"></div>
 <div style="position: absolute; border: 1px solid black; height: 256px; width: 256px; top: 1000px; left: 200px" contenteditable="true"></div>
index d37334d..3f4b2c7 100644 (file)
@@ -1,5 +1,11 @@
 <!DOCTYPE html>
 <html>
+<head>
+<script>
+if (window.internals)
+    internals.settings.setEditableRegionEnabled(true);
+</script>
+</head>
 <body>
 <div style="float: right; border: 1px solid black; width: 50px; height: 40px" contenteditable="true"></div>
 <pre id="results"></pre>
diff --git a/LayoutTests/editing/editable-region/hit-test-basic-without-editable-region-expected.txt b/LayoutTests/editing/editable-region/hit-test-basic-without-editable-region-expected.txt
new file mode 100644 (file)
index 0000000..8e66f5d
--- /dev/null
@@ -0,0 +1,11 @@
+Hit tests for editable elements inside the black outlined rectangle when the editable region is disabled. There should be more false positives when disabled.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS (x = 100, y = 100, width = 202, height = 202) contains editable elements.
+PASS (x = 0, y = 0, width = 50, height = 50) contains editable elements.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/editing/editable-region/hit-test-basic-without-editable-region.html b/LayoutTests/editing/editable-region/hit-test-basic-without-editable-region.html
new file mode 100644 (file)
index 0000000..cc34bfb
--- /dev/null
@@ -0,0 +1,52 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test.js"></script>
+<script src="../../resources/ui-helper.js"></script>
+<script src="resources/hit-test-utilities.js"></script>
+<style>
+#test-container {
+    position: absolute;
+    top: 100px;
+    left: 100px;
+    border: 1px solid black;
+    width: 200px;
+    height: 200px;
+}
+</style>
+</head>
+<body>
+<div id="test-container">
+    <input>
+</div>
+<script>
+window.jsTestIsAsync = true;
+
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+if (window.internals)
+    internals.settings.setEditableRegionEnabled(false);
+
+async function runTest()
+{
+    if (!window.testRunner) {
+        testFailed("Must be run in WebKitTestRunner.");
+        return;
+    }
+
+    let testContainer = document.getElementById("test-container");
+    await shouldHaveEditableElementsInRectForElement(testContainer);
+
+    // Without editable region enabled this rect is thought to contain something editable even though it doesn't.
+    await shouldHaveEditableElementsInRect(0, 0, 50, 50);
+
+    document.body.removeChild(testContainer);
+    finishJSTest();
+}
+
+description("Hit tests for editable elements inside the black outlined rectangle when the editable region is disabled. There should be more false positives when disabled.");
+runTest();
+</script>
+</body>
+</html>
index 823e7cf..10e66c4 100644 (file)
@@ -25,6 +25,9 @@ window.jsTestIsAsync = true;
 if (window.testRunner)
     testRunner.dumpAsText();
 
+if (window.internals)
+    internals.settings.setEditableRegionEnabled(true);
+
 async function runTest()
 {
     if (!window.testRunner) {
index 6c6b60a..0b5a81a 100644 (file)
@@ -9,6 +9,9 @@
 <script>
 window.jsTestIsAsync = true;
 
+if (window.internals)
+    internals.settings.setEditableRegionEnabled(true);
+
 async function runTest()
 {
     if (!window.testRunner) {
index d2b4434..9a51a9c 100644 (file)
@@ -22,6 +22,9 @@ window.jsTestIsAsync = true;
 if (window.testRunner)
     testRunner.dumpAsText();
 
+if (window.internals)
+    internals.settings.setEditableRegionEnabled(true);
+
 async function runTest()
 {
     if (!window.testRunner) {
index 310b25f..c7ed84e 100644 (file)
@@ -69,6 +69,9 @@ window.jsTestIsAsync = true;
 if (window.testRunner)
     testRunner.dumpAsText();
 
+if (window.internals)
+    internals.settings.setEditableRegionEnabled(true);
+
 async function runTest()
 {
     if (!window.testRunner) {
index 9df51a7..cfd404e 100644 (file)
@@ -25,6 +25,9 @@ window.jsTestIsAsync = true;
 if (window.testRunner)
     testRunner.dumpAsText();
 
+if (window.internals)
+    internals.settings.setEditableRegionEnabled(true);
+
 async function runTest()
 {
     if (!window.testRunner) {
diff --git a/LayoutTests/editing/editable-region/iframe-without-editable-region-expected.txt b/LayoutTests/editing/editable-region/iframe-without-editable-region-expected.txt
new file mode 100644 (file)
index 0000000..b40fafa
--- /dev/null
@@ -0,0 +1,54 @@
+
+(GraphicsLayer
+  (anchor 0.00 0.00)
+  (bounds 800.00 4112.00)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 4112.00)
+      (contentsOpaque 1)
+      (drawsContent 1)
+      (backgroundColor #FFFFFF)
+      (children 1
+        (GraphicsLayer
+          (position 8.00 8.00)
+          (bounds 304.00 304.00)
+          (drawsContent 1)
+          (children 1
+            (GraphicsLayer
+              (position 2.00 2.00)
+              (children 1
+                (GraphicsLayer
+                  (bounds origin 0.00 200.00)
+                  (anchor 0.00 0.00)
+                  (bounds 300.00 300.00)
+                  (children 1
+                    (GraphicsLayer
+                      (anchor 0.00 0.00)
+                      (children 1
+                        (GraphicsLayer
+                          (anchor 0.00 0.00)
+                          (bounds 310.00 670.00)
+                          (children 1
+                            (GraphicsLayer
+                              (bounds 310.00 670.00)
+                              (drawsContent 1)
+                              (children 1
+                                (GraphicsLayer
+                                )
+                              )
+                            )
+                          )
+                        )
+                      )
+                    )
+                  )
+                )
+              )
+            )
+          )
+        )
+      )
+    )
+  )
+)
+
diff --git a/LayoutTests/editing/editable-region/iframe-without-editable-region.html b/LayoutTests/editing/editable-region/iframe-without-editable-region.html
new file mode 100644 (file)
index 0000000..a263b9d
--- /dev/null
@@ -0,0 +1,59 @@
+<!DOCTYPE html>
+<html>
+<body style="height: 4096px">
+<iframe id="testFrame" height="300" srcdoc="
+<style>
+.scroller {
+    margin-top: 400px;
+    width: 300px;
+    height: 300px;
+    overflow: scroll;
+    border: 1px solid black;
+}
+.contents {
+    width: 500px;
+    height: 500px;
+}
+.editable {
+    -webkit-user-modify: read-write;
+    border: 2px solid blue;
+}   
+</style>
+<p>iframe</p>
+<div class='editable box' style='margin-top: 250px;'>
+</div>
+<div class='scroller' style='margin-top: 50px;'>
+    <div class='contents'>
+        Overflow scroll in iframe
+        <div class='editable box' style='margin-top: 50px;'>
+        </div>
+    </div>
+</div>
+<script>
+window.parent.scrollIFrameAndDumpRegions();
+</script>
+">
+</iframe>
+<pre id="results"></pre>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+if (window.internals) {
+    internals.settings.setAsyncFrameScrollingEnabled(true);
+    internals.settings.setEditableRegionEnabled(false);
+}
+
+function scrollIFrameAndDumpRegions()
+{
+    testFrame.contentWindow.scrollTo(0, 200);
+    if (window.internals)
+       results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
+   if (window.testRunner)
+        testRunner.notifyDone();
+}
+</script>
+</body>
+</html>
index 8708491..a7ee4e8 100644 (file)
@@ -41,8 +41,10 @@ if (window.testRunner) {
     testRunner.waitUntilDone();
 }
 
-if (window.internals)
+if (window.internals) {
     internals.settings.setAsyncFrameScrollingEnabled(true);
+    internals.settings.setEditableRegionEnabled(true);
+}
 
 function scrollIFrameAndDumpRegions()
 {
index 92c3579..dc8573d 100644 (file)
@@ -35,6 +35,8 @@
 if (window.testRunner)
     testRunner.dumpAsText();
 if (window.internals)
+    internals.settings.setEditableRegionEnabled(true);
+if (window.internals)
     results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
 </script>
 </body>
index ad29458..4596700 100644 (file)
@@ -27,6 +27,8 @@
 if (window.testRunner)
     testRunner.dumpAsText();
 if (window.internals)
+    internals.settings.setEditableRegionEnabled(true);
+if (window.internals)
     results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
 </script>
 </body>
index 197af32..0b8a60b 100644 (file)
@@ -11,6 +11,8 @@ window.scrollTo(0, 1024);
 if (window.testRunner)
     testRunner.dumpAsText();
 if (window.internals)
+    internals.settings.setEditableRegionEnabled(true);
+if (window.internals)
     results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
 </script>
 </body>
index 519f299..47e71cd 100644 (file)
@@ -20,6 +20,8 @@
 if (window.testRunner)
     testRunner.dumpAsText();
 if (window.internals)
+    internals.settings.setEditableRegionEnabled(true);
+if (window.internals)
     results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
 </script>
 </body>
index 6923137..eb3dfb9 100644 (file)
@@ -7,6 +7,8 @@
 if (window.testRunner)
     testRunner.dumpAsText();
 if (window.internals)
+    internals.settings.setEditableRegionEnabled(true);
+if (window.internals)
     results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
 </script>
 </body>
diff --git a/LayoutTests/editing/editable-region/text-field-basic-without-editable-region-expected.txt b/LayoutTests/editing/editable-region/text-field-basic-without-editable-region-expected.txt
new file mode 100644 (file)
index 0000000..881fe5d
--- /dev/null
@@ -0,0 +1,14 @@
+
+(GraphicsLayer
+  (anchor 0.00 0.00)
+  (bounds 800.00 600.00)
+  (children 1
+    (GraphicsLayer
+      (bounds 800.00 600.00)
+      (contentsOpaque 1)
+      (drawsContent 1)
+      (backgroundColor #FFFFFF)
+    )
+  )
+)
+
diff --git a/LayoutTests/editing/editable-region/text-field-basic-without-editable-region.html b/LayoutTests/editing/editable-region/text-field-basic-without-editable-region.html
new file mode 100644 (file)
index 0000000..3bbb01e
--- /dev/null
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<body>
+<input type="text">
+<pre id="results"></pre>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+if (window.internals)
+    internals.settings.setEditableRegionEnabled(true);
+if (window.internals) {
+    internals.settings.setEditableRegionEnabled(false);
+    results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
+}
+</script>
+</body>
+</html>
index 84a8019..b2c03cd 100644 (file)
@@ -7,6 +7,8 @@
 if (window.testRunner)
     testRunner.dumpAsText();
 if (window.internals)
+    internals.settings.setEditableRegionEnabled(true);
+if (window.internals)
     results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
 </script>
 </body>
index b4e4d8d..01e1e3a 100644 (file)
@@ -19,6 +19,8 @@
 if (window.testRunner)
     testRunner.dumpAsText();
 if (window.internals)
+    internals.settings.setEditableRegionEnabled(true);
+if (window.internals)
     results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
 </script>
 </body>
index 1a189a9..69fe762 100644 (file)
@@ -7,6 +7,8 @@
 if (window.testRunner)
     testRunner.dumpAsText();
 if (window.internals)
+    internals.settings.setEditableRegionEnabled(true);
+if (window.internals)
     results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
 </script>
 </body>
index bbc7ff9..fc3aad9 100644 (file)
@@ -22,6 +22,8 @@ window.scrollTo(0, 120);
 if (window.testRunner)
     testRunner.dumpAsText();
 if (window.internals)
+    internals.settings.setEditableRegionEnabled(true);
+if (window.internals)
     results.textContent = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_EVENT_REGION | internals.LAYER_TREE_INCLUDES_ROOT_LAYER_PROPERTIES);
 </script>
 </body>
index e59ca1a..0a0adcc 100644 (file)
@@ -1,3 +1,79 @@
+2020-06-30  Daniel Bates  <dabates@apple.com>
+
+        [iOS] Editable regions causes ~1% slowdown in PLT5
+        https://bugs.webkit.org/show_bug.cgi?id=213659
+        <rdar://problem/64361390>
+
+        Reviewed by Simon Fraser.
+
+        Fix the slowdown by only enabling editable region when Page::editableElementsInRect is called.
+
+        There are two parts that make computing the editable region expensive:
+                1. Requires traversing descendents during painting when a normal paint may be
+                   able to avoid this.
+                2. Can cause more event region invalidations because it extends the invalidation
+                   criterion to include changes to element editability.
+
+        Tests: editing/editable-region/hit-test-basic-without-editable-region.html
+               editing/editable-region/iframe-without-editable-region.html
+               editing/editable-region/text-field-basic-without-editable-region.html
+
+        Tests: editing/editable-region/hit-test-basic-without-editable-region.html
+               editing/editable-region/iframe-without-editable-region.html
+               editing/editable-region/text-field-basic-without-editable-region.html
+
+        * page/Frame.cpp:
+        (WebCore::Frame::invalidateContentEventRegionsIfNeeded): Check if editable region is enabled.
+        If it is then do what we do now. Otherwise, don't invalidate the region unless we were going
+        to do so anyway.
+        * page/Page.cpp:
+        (WebCore::Page::setEditableRegionEnabled): Added. Update state and then invalidate
+        the event region in all layers.
+        (WebCore::Page::shouldBuildEditableRegion const): Added. Returns whether to build the
+        editable region: either when Page::isEditableRegionEnabled() is true or the editable
+        region debug overlay is enabled.
+        (WebCore::Page::didFinishLoad): Turn off editable region as it may not be needed on
+        the new page.
+        * page/Page.h:
+        (WebCore::Page::isEditableRegionEnabled const): Added.
+
+        * rendering/EventRegion.cpp:
+        (WebCore::EventRegion::unite):
+        (WebCore::EventRegion::translate):
+        (WebCore::EventRegion::containsEditableElementsInRect const):
+        (WebCore::EventRegion::dump const):
+        * rendering/EventRegion.h:
+        (WebCore::EventRegion::hasEditableRegion const):
+        (WebCore::EventRegion::rectsForEditableElements const):
+        (WebCore::EventRegion::decode):
+        (WebCore::EventRegion::ensureEditableRegion):
+        The editable region is now an Optional<>. There will only be one if ensureEditableRegion
+        was called, which is only when Page::isEditableRegionEnabled() returns true.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::paintObject):
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::styleWillChange):
+        * rendering/RenderLayer.h:
+        * rendering/RenderLayerBacking.cpp:
+        (WebCore::RenderLayerBacking::maintainsEventRegion const):
+        Only do what we do now if Page::shouldBuildEditableRegion() returns true.
+
+        (WebCore::RenderLayerBacking::updateEventRegion): Instantiate the editable region, if needed.
+        Painting will then populate it.
+
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::applyToCompositedLayerIncludingDescendants): Added.
+        (WebCore::RenderLayerCompositor::invalidateEventRegionForAllLayers): Added.
+        (WebCore::RenderLayerCompositor::clearBackingForAllLayers): Wrote in terms of applyToCompositedLayerIncludingDescendants.
+        (WebCore::RenderLayerCompositor::clearBackingForLayerIncludingDescendants): Deleted.
+        * rendering/RenderLayerCompositor.h:
+        * testing/InternalSettings.cpp:
+        (WebCore::InternalSettings::setEditableRegionEnabled):
+        * testing/InternalSettings.h:
+        * testing/InternalSettings.idl:
+        Add a new internal setting for testing purposes to toggle enabling/disabling editable region.
+
 2020-06-30  Peng Liu  <peng.liu6@apple.com>
 
         Scrunching a video to PiP can result in broken animation and leave Safari in a bad state
index d04fa0c..ff1a27e 100644 (file)
@@ -313,7 +313,7 @@ void Frame::invalidateContentEventRegionsIfNeeded()
     hasTouchActionElements = m_doc->mayHaveElementsWithNonAutoTouchAction();
 #endif
 #if ENABLE(EDITABLE_REGION)
-    hasEditableElements = m_doc->mayHaveEditableElements();
+    hasEditableElements = m_doc->mayHaveEditableElements() && m_page->shouldBuildEditableRegion();
 #endif
     // FIXME: This needs to look at wheel event handlers too.
     if (!hasTouchActionElements && !hasEditableElements)
index e4fecd6..8ed7aff 100644 (file)
@@ -921,6 +921,31 @@ void Page::unmarkAllTextMatches()
     });
 }
 
+#if ENABLE(EDITABLE_REGION)
+
+void Page::setEditableRegionEnabled(bool enabled)
+{
+    if (m_isEditableRegionEnabled == enabled)
+        return;
+    m_isEditableRegionEnabled = enabled;
+    auto frameView = makeRefPtr(mainFrame().view());
+    if (!frameView)
+        return;
+    if (auto* renderView = frameView->renderView())
+        renderView->compositor().invalidateEventRegionForAllLayers();
+}
+
+#endif
+
+#if ENABLE(EDITABLE_REGION)
+
+bool Page::shouldBuildEditableRegion() const
+{
+    return m_isEditableRegionEnabled || m_settings->visibleDebugOverlayRegions() & EditableElementRegion;
+}
+
+#endif
+
 Vector<Ref<Element>> Page::editableElementsInRect(const FloatRect& searchRectInRootViewCoordinates) const
 {
     auto frameView = makeRefPtr(mainFrame().view());
@@ -1187,6 +1212,9 @@ void Page::didStartProvisionalLoad()
 
 void Page::didFinishLoad()
 {
+#if ENABLE(EDITABLE_REGION)
+    m_isEditableRegionEnabled = false;
+#endif
     resetRelevantPaintedObjectCounter();
 
     if (m_performanceMonitor)
index ac20644..f7d227e 100644 (file)
@@ -742,6 +742,12 @@ public:
 
     void configureLoggingChannel(const String&, WTFLogChannelState, WTFLogLevel);
 
+#if ENABLE(EDITABLE_REGION)
+    bool shouldBuildEditableRegion() const;
+    bool isEditableRegionEnabled() const { return m_isEditableRegionEnabled; }
+    WEBCORE_EXPORT void setEditableRegionEnabled(bool = true);
+#endif
+
     WEBCORE_EXPORT Vector<Ref<Element>> editableElementsInRect(const FloatRect&) const;
 
 #if ENABLE(DEVICE_ORIENTATION) && PLATFORM(IOS_FAMILY)
@@ -1003,6 +1009,10 @@ private:
     bool m_hasResourceLoadClient { false };
     bool m_delegatesScaling { false };
 
+#if ENABLE(EDITABLE_REGION)
+    bool m_isEditableRegionEnabled { false };
+#endif
+
     UserInterfaceLayoutDirection m_userInterfaceLayoutDirection { UserInterfaceLayoutDirection::LTR };
     
     // For testing.
index 63cb23d..87394ed 100644 (file)
@@ -130,8 +130,8 @@ void EventRegion::unite(const Region& region, const RenderStyle& style, bool ove
 #endif
 
 #if ENABLE(EDITABLE_REGION)
-    if (overrideUserModifyIsEditable || style.userModify() != UserModify::ReadOnly)
-        m_editableRegion.unite(region);
+    if (m_editableRegion && (overrideUserModifyIsEditable || style.userModify() != UserModify::ReadOnly))
+        m_editableRegion->unite(region);
 #else
     UNUSED_PARAM(overrideUserModifyIsEditable);
 #endif
@@ -156,7 +156,8 @@ void EventRegion::translate(const IntSize& offset)
 #endif
 
 #if ENABLE(EDITABLE_REGION)
-    m_editableRegion.translate(offset);
+    if (m_editableRegion)
+        m_editableRegion->translate(offset);
 #endif
 }
 
@@ -283,10 +284,12 @@ const Region& EventRegion::eventListenerRegionForType(EventListenerRegionType ty
 #endif // ENABLE(WHEEL_EVENT_REGIONS)
 
 #if ENABLE(EDITABLE_REGION)
+
 bool EventRegion::containsEditableElementsInRect(const IntRect& rect) const
 {
-    return m_editableRegion.intersects(rect);
+    return m_editableRegion && m_editableRegion->intersects(rect);
 }
+
 #endif
 
 void EventRegion::dump(TextStream& ts) const
@@ -322,8 +325,8 @@ void EventRegion::dump(TextStream& ts) const
 #endif
 
 #if ENABLE(EDITABLE_REGION)
-    if (!m_editableRegion.isEmpty()) {
-        ts << indent << "(editable region" << m_editableRegion;
+    if (m_editableRegion && !m_editableRegion->isEmpty()) {
+        ts << indent << "(editable region" << *m_editableRegion;
         ts << indent << ")\n";
     }
 #endif
index 501243e..6c4fae6 100644 (file)
@@ -87,8 +87,10 @@ public:
 #endif
 
 #if ENABLE(EDITABLE_REGION)
+    void ensureEditableRegion();
+    bool hasEditableRegion() const { return m_editableRegion.hasValue(); }
     WEBCORE_EXPORT bool containsEditableElementsInRect(const IntRect&) const;
-    Vector<IntRect, 1> rectsForEditableElements() const { return m_editableRegion.rects(); }
+    Vector<IntRect, 1> rectsForEditableElements() const { return m_editableRegion ? m_editableRegion->rects() : Vector<IntRect, 1> { }; }
 #endif
 
     template<class Encoder> void encode(Encoder&) const;
@@ -113,7 +115,7 @@ private:
     Region m_nonPassiveWheelEventListenerRegion;
 #endif
 #if ENABLE(EDITABLE_REGION)
-    Region m_editableRegion;
+    Optional<Region> m_editableRegion;
 #endif
 };
 
@@ -152,7 +154,7 @@ Optional<EventRegion> EventRegion::decode(Decoder& decoder)
 #endif
 
 #if ENABLE(EDITABLE_REGION)
-    Optional<Region> editableRegion;
+    Optional<Optional<Region>> editableRegion;
     decoder >> editableRegion;
     if (!editableRegion)
         return WTF::nullopt;
@@ -173,4 +175,14 @@ bool EventRegion::decode(Decoder& decoder, EventRegion& eventRegion)
     return true;
 }
 
+#if ENABLE(EDITABLE_REGION)
+
+inline void EventRegion::ensureEditableRegion()
+{
+    if (!m_editableRegion)
+        m_editableRegion.emplace();
+}
+
+#endif
+
 }
index 373c796..bb487a7 100644 (file)
@@ -1264,7 +1264,7 @@ void RenderBlock::paintObject(PaintInfo& paintInfo, const LayoutPoint& paintOffs
         // though it's actually the inner text element of the control that is editable.
         // So, no need to traverse to find the inner text element in this case.
         if (!isTextControl())
-            needsTraverseDescendants |= document().mayHaveEditableElements();
+            needsTraverseDescendants |= document().mayHaveEditableElements() && page().shouldBuildEditableRegion();
 #endif
         if (!needsTraverseDescendants)
             return;
index 7d68cdb..1821a30 100644 (file)
@@ -749,7 +749,7 @@ void RenderElement::styleWillChange(StyleDifference diff, const RenderStyle& new
             bool wasEditable = m_style.userModify() != UserModify::ReadOnly;
             bool isEditable = newStyle.userModify() != UserModify::ReadOnly;
             if (wasEditable != isEditable)
-                return true;
+                return page().shouldBuildEditableRegion();
 #endif
             return false;
         };
index 03eb61d..4735dab 100644 (file)
@@ -927,7 +927,7 @@ public:
 
     // Invalidation can fail if there is no enclosing compositing layer (e.g. nested iframe)
     // or the layer does not maintain an event region.
-    enum class EventRegionInvalidationReason { Paint, Style, NonCompositedFrame };
+    enum class EventRegionInvalidationReason { Paint, SettingDidChange, Style, NonCompositedFrame };
     bool invalidateEventRegion(EventRegionInvalidationReason);
 
     String debugDescription() const final;
index 2376b62..a1a3842 100644 (file)
@@ -1708,7 +1708,7 @@ bool RenderLayerBacking::maintainsEventRegion() const
         return true;
 #endif
 #if ENABLE(EDITABLE_REGION)
-    if (renderer().document().mayHaveEditableElements())
+    if (renderer().document().mayHaveEditableElements() && renderer().page().shouldBuildEditableRegion())
         return true;
 #endif
 #if !PLATFORM(IOS_FAMILY)
@@ -1735,6 +1735,10 @@ void RenderLayerBacking::updateEventRegion()
     auto updateEventRegionForLayer = [&](GraphicsLayer& graphicsLayer) {
         GraphicsContext nullContext(nullptr);
         EventRegion eventRegion;
+#if ENABLE(EDITABLE_REGION)
+        if (renderer().page().shouldBuildEditableRegion())
+            eventRegion.ensureEditableRegion();
+#endif
         auto eventRegionContext = eventRegion.makeContext();
         auto layerOffset = graphicsLayer.scrollOffset() - roundedIntSize(graphicsLayer.offsetFromRenderer());
 
index 3ef1cfe..e58e1f6 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2009, 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2009-2020 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -2357,18 +2357,25 @@ void RenderLayerCompositor::setIsInWindow(bool isInWindow)
     }
 }
 
-void RenderLayerCompositor::clearBackingForLayerIncludingDescendants(RenderLayer& layer)
+template<typename ApplyFunctionType>
+void RenderLayerCompositor::applyToCompositedLayerIncludingDescendants(RenderLayer& layer, const ApplyFunctionType& function)
 {
     if (layer.isComposited())
-        layer.clearBacking();
-
+        function(layer);
     for (auto* childLayer = layer.firstChild(); childLayer; childLayer = childLayer->nextSibling())
-        clearBackingForLayerIncludingDescendants(*childLayer);
+        applyToCompositedLayerIncludingDescendants(*childLayer, function);
+}
+
+void RenderLayerCompositor::invalidateEventRegionForAllLayers()
+{
+    applyToCompositedLayerIncludingDescendants(*m_renderView.layer(), [](auto& layer) {
+        layer.invalidateEventRegion(RenderLayer::EventRegionInvalidationReason::SettingDidChange);
+    });
 }
 
 void RenderLayerCompositor::clearBackingForAllLayers()
 {
-    clearBackingForLayerIncludingDescendants(*m_renderView.layer());
+    applyToCompositedLayerIncludingDescendants(*m_renderView.layer(), [](auto& layer) { layer.clearBacking(); });
 }
 
 void RenderLayerCompositor::updateRootLayerPosition()
index 42de3be..00737bc 100644 (file)
@@ -282,6 +282,7 @@ public:
     void setIsInWindow(bool);
 
     void clearBackingForAllLayers();
+    void invalidateEventRegionForAllLayers();
     
     void layerBecameComposited(const RenderLayer&);
     void layerBecameNonComposited(const RenderLayer&);
@@ -417,7 +418,7 @@ private:
     bool updateBacking(RenderLayer&, RequiresCompositingData&, BackingSharingState* = nullptr, BackingRequired = BackingRequired::Unknown);
     bool updateLayerCompositingState(RenderLayer&, const RenderLayer* compositingAncestor, RequiresCompositingData&, BackingSharingState&);
 
-    void clearBackingForLayerIncludingDescendants(RenderLayer&);
+    template<typename ApplyFunctionType> void applyToCompositedLayerIncludingDescendants(RenderLayer&, const ApplyFunctionType&);
 
     // Repaint this and its child layers.
     void recursiveRepaintLayer(RenderLayer&);
index bbbb4a4..4615163 100644 (file)
@@ -441,6 +441,18 @@ ExceptionOr<void> InternalSettings::setTextAutosizingUsesIdempotentMode(bool ena
     return { };
 }
 
+ExceptionOr<void> InternalSettings::setEditableRegionEnabled(bool enabled)
+{
+    if (!m_page)
+        return Exception { InvalidAccessError };
+#if ENABLE(EDITABLE_REGION)
+    m_page->setEditableRegionEnabled(enabled);
+#else
+    UNUSED_PARAM(enabled);
+#endif
+    return { };
+}
+
 ExceptionOr<void> InternalSettings::setMediaTypeOverride(const String& mediaType)
 {
     if (!m_page)
index a1b9709..4785e68 100644 (file)
@@ -109,6 +109,8 @@ public:
 
     using FrameFlatteningValue = FrameFlattening;
     ExceptionOr<void> setFrameFlattening(FrameFlatteningValue);
+
+    ExceptionOr<void> setEditableRegionEnabled(bool);
     
     static void setAllowsAnySSLCertificate(bool);
 
index 36887fc..c15f3be 100644 (file)
@@ -122,5 +122,7 @@ enum FontLoadTimingOverride { "Block", "Swap", "Failure" };
     void setShouldDeactivateAudioSession(boolean shouldDeactivate);
     
     void setStorageAccessAPIPerPageScopeEnabled(boolean enabled);
+
+    [MayThrowException] void setEditableRegionEnabled(boolean enabled);
 };
 
index 863dc37..1782755 100644 (file)
@@ -1,3 +1,29 @@
+2020-06-30  Daniel Bates  <dabates@apple.com>
+
+        [iOS] Editable regions causes ~1% slowdown in PLT5
+        https://bugs.webkit.org/show_bug.cgi?id=213659
+        <rdar://problem/64361390>
+
+        Reviewed by Simon Fraser.
+
+        Fix up RemoteLayerTreeViews now that the editable region is an Optional<>. Have
+        the UI process message the web process to enable editable region on the first
+        invocation of _requestTextInputContextsInRect, which is good indicator that there
+        would be benefit to computing it as a typical client that calls it will call it
+        many times.
+
+        * UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:
+        (WebKit::mayContainEditableElementsInRect): Fix up the logic now that there may
+        be not be an editable region sent over in the last event region update. If there
+        isn't one then we don't know if there are editable elements in the rect of not.
+        So, return true, which could turn out to be a false positive if there aren't any
+        editable elements in the rect. The caller will have to message the web process
+        to find out the real answer if they want it. Just to clarify, it's OK for this
+        function to have false positives, but it must never have false negatives.
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::textInputContextsInRect): Enable editable region. If it's already
+        enabled then doing so again does nothing.
+
 2020-06-30  Peng Liu  <peng.liu6@apple.com>
 
         Scrunching a video to PiP can result in broken animation and leave Safari in a bad state
index 52a8a75..b22c341 100644 (file)
@@ -120,6 +120,7 @@ bool mayContainEditableElementsInRect(UIView *rootView, const WebCore::FloatRect
     collectDescendantViewsInRect(viewsInRect, rootView, rect);
     if (viewsInRect.isEmpty())
         return false;
+    bool possiblyHasEditableElements = true;
     for (auto *view : WTF::makeReversedRange(viewsInRect)) {
         if (![view isKindOfClass:WKCompositingView.class])
             continue;
@@ -129,10 +130,13 @@ bool mayContainEditableElementsInRect(UIView *rootView, const WebCore::FloatRect
         WebCore::IntRect rectToTest { [view convertRect:rect fromView:rootView] };
         if (node->eventRegion().containsEditableElementsInRect(rectToTest))
             return true;
-        if (node->eventRegion().contains(rectToTest))
+        bool hasEditableRegion = node->eventRegion().hasEditableRegion();
+        if (hasEditableRegion && node->eventRegion().contains(rectToTest))
             return false;
+        if (hasEditableRegion)
+            possiblyHasEditableElements = false;
     }
-    return false;
+    return possiblyHasEditableElements;
 }
 
 #endif // ENABLE(EDITABLE_REGION)
index d61168e..7b8102c 100644 (file)
@@ -4318,6 +4318,9 @@ void WebPage::textInputContextsInRect(FloatRect searchRect, CompletionHandler<vo
         return context;
     });
     completionHandler(contexts);
+#if ENABLE(EDITABLE_REGION)
+    m_page->setEditableRegionEnabled();
+#endif
 }
 
 void WebPage::focusTextInputContextAndPlaceCaret(const ElementContext& elementContext, const IntPoint& point, CompletionHandler<void(bool)>&& completionHandler)