Title: [286857] trunk
Revision
286857
Author
commit-qu...@webkit.org
Date
2021-12-10 09:28:10 -0800 (Fri, 10 Dec 2021)

Log Message

Unreviewed, reverting r286836.
https://bugs.webkit.org/show_bug.cgi?id=234153

some tests are flaky on iOS and some are crashing on macOS

Reverted changeset:

"[Model] Add load and error events to distinguish resource
load from model readiness"
https://bugs.webkit.org/show_bug.cgi?id=233706
https://commits.webkit.org/r286836

Modified Paths

Added Paths

Removed Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (286856 => 286857)


--- trunk/LayoutTests/ChangeLog	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/LayoutTests/ChangeLog	2021-12-10 17:28:10 UTC (rev 286857)
@@ -1,3 +1,17 @@
+2021-12-10  Commit Queue  <commit-qu...@webkit.org>
+
+        Unreviewed, reverting r286836.
+        https://bugs.webkit.org/show_bug.cgi?id=234153
+
+        some tests are flaky on iOS and some are crashing on macOS
+
+        Reverted changeset:
+
+        "[Model] Add load and error events to distinguish resource
+        load from model readiness"
+        https://bugs.webkit.org/show_bug.cgi?id=233706
+        https://commits.webkit.org/r286836
+
 2021-12-10  Chris Dumez  <cdu...@apple.com>
 
         Radio buttons with no form owner are not grouped

Modified: trunk/LayoutTests/model-element/model-element-contents-layer-updates-with-clipping.html (286856 => 286857)


--- trunk/LayoutTests/model-element/model-element-contents-layer-updates-with-clipping.html	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/LayoutTests/model-element/model-element-contents-layer-updates-with-clipping.html	2021-12-10 17:28:10 UTC (rev 286857)
@@ -6,40 +6,39 @@
 </model>
 <pre id="layers"></pre>
 <script>
-    window.testRunner?.waitUntilDone();
-    window.testRunner?.dumpAsText();
+    let layers = document.getElementById("layers");
+    let source = document.getElementsByTagName("source")[0];
 
-    const layers = document.getElementById("layers");
-    const source = document.querySelector("source");
-    const model = document.getElementById("model");
+    if (window.testRunner) {
+        testRunner.waitUntilDone();
+        testRunner.dumpAsText();
+    } else
+        layers.textContent = "This test requires testRunner.";
 
-    const modelDidLoadFirstSource = () => {
+    let model = document.getElementById("model");
+
+    model.ready.then(value => {
         layers.textContent = "Before Changing Source:\n";
-        layers.textContent += window.internals?.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS) ?? "This test requires testRunner.";
+        layers.textContent += window.internals.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS);
 
-        model.addEventListener("load", event => {
-            layers.textContent += "After Changing Source:\n";
-            layers.textContent += window.internals?.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS) ?? "This test requires testRunner.";
-            window.testRunner?.notifyDone();
-        }, { once: true });
-
-        model.addEventListener("error", event => {
-            layers.textContent = `Failed. Second model did not load.`;
-            window.testRunner?.notifyDone();
-        }, { once: true });
-
         source.src = ""
-    }
-
-    if (model.complete)
-        modelDidLoadFirstSource();
-    else {
-        model.addEventListener("load", modelDidLoadFirstSource, { once: true });
-        model.addEventListener("error", event => {
-            layers.textContent = `Failed. First model did not load.`;
-            window.testRunner?.notifyDone();
-        }, { once: true });
-    }
+        model.ready.then(value => {
+            if (window.testRunner) {
+                layers.textContent += "After Changing Source:\n";
+                layers.textContent += window.internals.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS);
+            }
+        }, reason => {
+            layers.textContent = `Failed. Second model did not load: ${reason}`;
+        }).finally(() => { 
+            if (window.testRunner)
+                testRunner.notifyDone();
+        });
+        
+    }, reason => {
+        layers.textContent = `Failed. First model did not load: ${reason}`;
+        if (window.testRunner)
+            testRunner.notifyDone();
+    });
 </script>
 </body>
 </html>

Modified: trunk/LayoutTests/model-element/model-element-contents-layer-updates.html (286856 => 286857)


--- trunk/LayoutTests/model-element/model-element-contents-layer-updates.html	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/LayoutTests/model-element/model-element-contents-layer-updates.html	2021-12-10 17:28:10 UTC (rev 286857)
@@ -6,40 +6,39 @@
 </model>
 <pre id="layers"></pre>
 <script>
-    window.testRunner?.waitUntilDone();
-    window.testRunner?.dumpAsText();
+    let layers = document.getElementById("layers");
+    let source = document.getElementsByTagName("source")[0];
 
-    const layers = document.getElementById("layers");
-    const source = document.querySelector("source");
-    const model = document.getElementById("model");
+    if (window.testRunner) {
+        testRunner.waitUntilDone();
+        testRunner.dumpAsText();
+    } else
+        layers.textContent = "This test requires testRunner.";
 
-    const modelDidLoadFirstSource = () => {
+    let model = document.getElementById("model");
+
+    model.ready.then(value => {
         layers.textContent = "Before Changing Source:\n";
-        layers.textContent += window.internals?.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS) ?? "This test requires testRunner.";
+        layers.textContent += window.internals.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS);
 
-        model.addEventListener("load", event => {
-            layers.textContent += "After Changing Source:\n";
-            layers.textContent += window.internals?.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS) ?? "This test requires testRunner.";
-            window.testRunner?.notifyDone();
-        }, { once: true });
-
-        model.addEventListener("error", event => {
-            layers.textContent = `Failed. Second model did not load.`;
-            window.testRunner?.notifyDone();
-        }, { once: true });
-
         source.src = ""
-    }
-
-    if (model.complete)
-        modelDidLoadFirstSource();
-    else {
-        model.addEventListener("load", modelDidLoadFirstSource, { once: true });
-        model.addEventListener("error", event => {
-            layers.textContent = `Failed. First model did not load.`;
-            window.testRunner?.notifyDone();
-        }, { once: true });
-    }
+        model.ready.then(value => {
+            if (window.testRunner) {
+                layers.textContent += "After Changing Source:\n";
+                layers.textContent += window.internals.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS);
+            }
+        }, reason => {
+            layers.textContent = `Failed. Second model did not load: ${reason}`;
+        }).finally(() => { 
+            if (window.testRunner)
+                testRunner.notifyDone();
+        });
+        
+    }, reason => {
+        layers.textContent = `Failed. First model did not load: ${reason}`;
+        if (window.testRunner)
+            testRunner.notifyDone();
+    });
 </script>
 </body>
 </html>

Deleted: trunk/LayoutTests/model-element/model-element-error-and-load-events-expected.txt (286856 => 286857)


--- trunk/LayoutTests/model-element/model-element-error-and-load-events-expected.txt	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/LayoutTests/model-element/model-element-error-and-load-events-expected.txt	2021-12-10 17:28:10 UTC (rev 286857)
@@ -1,5 +0,0 @@
-
-PASS <model> dispatches an "error" event when its resource load is aborted before completion.
-PASS <model> dispatches an "error" event when its specified resource does not exist.
-PASS <model> dispatches a "load" event when its resource is successfully loaded.
-

Deleted: trunk/LayoutTests/model-element/model-element-error-and-load-events.html (286856 => 286857)


--- trunk/LayoutTests/model-element/model-element-error-and-load-events.html	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/LayoutTests/model-element/model-element-error-and-load-events.html	2021-12-10 17:28:10 UTC (rev 286857)
@@ -1,52 +0,0 @@
-<!doctype html>
-<meta charset="utf-8">
-<title>&lt;model> load and error events</title>
-<script src=""
-<script src=""
-<script src=""
-<body>
-<script>
-'use strict';
-
-const model_load_test = (expectedEvent, executor, description) => {
-    promise_test(t => {
-        return new Promise((resolve, reject) => {
-            const [model, source] = createModelAndSource(t);
-
-            const handleEvent = event => {
-                if (event.type === "load")
-                    assert_true(model.complete, `model.complete is true upon receiving the "load" event.`);
-                else if (event.type === "error")
-                    assert_false(model.complete, `model.complete is false upon receiving the "error" event.`);
-
-                if (event.type === expectedEvent)
-                    resolve();
-                else
-                    reject(`received unexpected event: ${event.type}`);
-            }
-
-            assert_false(model.complete, "model.complete is false before the load is initiated.");
-
-            model.addEventListener("load", handleEvent);
-            model.addEventListener("error", handleEvent);
-
-            executor(source);
-        });
-    }, description);
-};
-
-model_load_test("error", source => {
-    source.src = ""
-    source.remove();
-}, `<model> dispatches an "error" event when its resource load is aborted before completion.`);
-
-model_load_test("error", source => {
-    source.src = ""
-}, `<model> dispatches an "error" event when its specified resource does not exist.`);
-
-model_load_test("load", source => {
-    source.src = ""
-}, `<model> dispatches a "load" event when its resource is successfully loaded.`);
-
-</script>
-</body>

Modified: trunk/LayoutTests/model-element/model-element-graphics-layers-opacity.html (286856 => 286857)


--- trunk/LayoutTests/model-element/model-element-graphics-layers-opacity.html	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/LayoutTests/model-element/model-element-graphics-layers-opacity.html	2021-12-10 17:28:10 UTC (rev 286857)
@@ -13,27 +13,26 @@
 </model>
 <pre id="layers"></pre>
 <script>
-    window.testRunner?.waitUntilDone();
-    window.testRunner?.dumpAsText();
+    let layers = document.getElementById("layers");
 
-    const layers = document.getElementById("layers");
-    const model = document.getElementById("model");
+    if (window.testRunner) {
+        testRunner.waitUntilDone();
+        testRunner.dumpAsText();
+    } else
+        layers.textContent = "This test requires testRunner.";
 
-    const modelDidLoad = () => {
-        layers.innerText = window.internals?.platformLayerTreeAsText(model) ?? "This test requires testRunner.";
+    let model = document.getElementById("model");
+
+    model.ready.then(value => {
+        if (window.testRunner)
+            layers.innerText = window.internals.platformLayerTreeAsText(model);
         model.remove();
-        window.testRunner?.notifyDone();
-    };
-
-    if (model.complete)
-        modelDidLoad();
-    else {
-        model.addEventListener("load", modelDidLoad);
-        model.addEventListener("error", event => {
-            layers.textContent = `Failed. Model did not load.`;
-            window.testRunner?.notifyDone();
-        });
-    }
+    }, reason => {
+        layers.textContent = `Failed. Model did not load: ${reason}`;
+    }).finally(() => { 
+        if (window.testRunner)
+            testRunner.notifyDone();
+    });
 </script>
 </body>
 </html>

Modified: trunk/LayoutTests/model-element/model-element-graphics-layers.html (286856 => 286857)


--- trunk/LayoutTests/model-element/model-element-graphics-layers.html	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/LayoutTests/model-element/model-element-graphics-layers.html	2021-12-10 17:28:10 UTC (rev 286857)
@@ -6,27 +6,26 @@
 </model>
 <pre id="layers"></pre>
 <script>
-    window.testRunner?.waitUntilDone();
-    window.testRunner?.dumpAsText();
+    let layers = document.getElementById("layers");
 
-    const layers = document.getElementById("layers");
-    const model = document.getElementById("model");
+    if (window.testRunner) {
+        testRunner.waitUntilDone();
+        testRunner.dumpAsText();
+    } else
+        layers.textContent = "This test requires testRunner.";
 
-    const modelDidLoad = () => {
-        layers.innerText = window.internals?.layerTreeAsText(document) ?? "This test requires testRunner.";
+    let model = document.getElementById("model");
+
+    model.ready.then(value => {
+        if (window.testRunner)
+            layers.innerText = window.internals.layerTreeAsText(document);
         model.remove();
-        window.testRunner?.notifyDone();
-    }
-
-    if (model.complete)
-        modelDidLoad();
-    else {
-        model.addEventListener("load", modelDidLoad);
-        model.addEventListener("error", event => {
-            layers.textContent = `Failed. Model did not load.`;
-            window.testRunner?.notifyDone();
-        });
-    }
+    }, reason => {
+        layers.textContent = `Failed. Model did not load: ${reason}`;
+    }).finally(() => { 
+        if (window.testRunner)
+            testRunner.notifyDone();
+    });
 </script>
 </body>
 </html>

Modified: trunk/LayoutTests/model-element/model-element-ready-expected.txt (286856 => 286857)


--- trunk/LayoutTests/model-element/model-element-ready-expected.txt	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/LayoutTests/model-element/model-element-ready-expected.txt	2021-12-10 17:28:10 UTC (rev 286857)
@@ -1,5 +1,3 @@
+This test passes if you see the word "Passed" below:
 
-PASS <model> rejects the ready promise when provided with an unknown resoure.
-PASS <model> rejects the ready promise when its resource load is aborted.
-PASS <model> resolves the ready promise when provided with a known resource.
-
+Passed

Added: trunk/LayoutTests/model-element/model-element-ready-load-aborted-expected.txt (0 => 286857)


--- trunk/LayoutTests/model-element/model-element-ready-load-aborted-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/model-element/model-element-ready-load-aborted-expected.txt	2021-12-10 17:28:10 UTC (rev 286857)
@@ -0,0 +1,3 @@
+This test passes if you see the word "Passed" below:
+
+Passed

Added: trunk/LayoutTests/model-element/model-element-ready-load-aborted.html (0 => 286857)


--- trunk/LayoutTests/model-element/model-element-ready-load-aborted.html	                        (rev 0)
+++ trunk/LayoutTests/model-element/model-element-ready-load-aborted.html	2021-12-10 17:28:10 UTC (rev 286857)
@@ -0,0 +1,36 @@
+<!DOCTYPE html><!-- webkit-test-runner [ ModelElementEnabled=true ] -->
+<html>
+<body>
+<model id="model">
+    <source id="model-source">
+</model>
+<p>This test passes if you see the word "Passed" below:</p>
+<p id="result">Failed</p>
+<script>
+    if (window.testRunner) {
+        testRunner.waitUntilDone();
+        testRunner.dumpAsText();
+    }
+
+    let result = document.getElementById("result");
+    let model = document.getElementById("model");
+    let source = document.getElementById("model-source");
+
+    model.ready.then(value => {
+        result.textContent = "Failed. Model should not have loaded, but did.";
+    }, reason => {
+        if (reason.name == "AbortError")
+            result.textContent = `Passed`;
+        else
+            result.textContent = `Failed. Wrong error type: ${reason}.`;
+    }).finally(() => { 
+        if (window.testRunner)
+            testRunner.notifyDone();
+    });
+
+    source.src = ""
+
+    model.removeChild(source);
+</script>
+</body>
+</html>

Added: trunk/LayoutTests/model-element/model-element-ready-load-failed-expected.txt (0 => 286857)


--- trunk/LayoutTests/model-element/model-element-ready-load-failed-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/model-element/model-element-ready-load-failed-expected.txt	2021-12-10 17:28:10 UTC (rev 286857)
@@ -0,0 +1,3 @@
+This test passes if you see the word "Passed" below:
+
+Passed

Added: trunk/LayoutTests/model-element/model-element-ready-load-failed.html (0 => 286857)


--- trunk/LayoutTests/model-element/model-element-ready-load-failed.html	                        (rev 0)
+++ trunk/LayoutTests/model-element/model-element-ready-load-failed.html	2021-12-10 17:28:10 UTC (rev 286857)
@@ -0,0 +1,31 @@
+<!DOCTYPE html><!-- webkit-test-runner [ ModelElementEnabled=true ] -->
+<html>
+<body>
+<model id="model">
+    <source src=""
+</model>
+<p>This test passes if you see the word "Passed" below:</p>
+<p id="result">Failed</p>
+<script>
+    if (window.testRunner) {
+        testRunner.waitUntilDone();
+        testRunner.dumpAsText();
+    }
+
+    let result = document.getElementById("result");
+    let model = document.getElementById("model");
+
+    model.ready.then(value => {
+        result.textContent = "Failed. Model should not have loaded, but did.";
+    }, reason => {
+        if (reason.name == "NetworkError")
+            result.textContent = `Passed`;
+        else
+            result.textContent = `Failed. Wrong error type: ${reason}.`;
+    }).finally(() => { 
+        if (window.testRunner)
+            testRunner.notifyDone();
+    });
+</script>
+</body>
+</html>

Modified: trunk/LayoutTests/model-element/model-element-ready.html (286856 => 286857)


--- trunk/LayoutTests/model-element/model-element-ready.html	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/LayoutTests/model-element/model-element-ready.html	2021-12-10 17:28:10 UTC (rev 286857)
@@ -1,38 +1,28 @@
-<!doctype html>
-<meta charset="utf-8">
-<title>&lt;model> ready promise</title>
-<script src=""
-<script src=""
-<script src=""
+<!DOCTYPE html><!-- webkit-test-runner [ ModelElementEnabled=true ] -->
+<html>
 <body>
+<model id="model">
+    <source src=""
+</model>
+<p>This test passes if you see the word "Passed" below:</p>
+<p id="result">Failed</p>
 <script>
-'use strict';
+    if (window.testRunner) {
+        testRunner.waitUntilDone();
+        testRunner.dumpAsText();
+    }
 
-promise_test(async t => {
-    const [model, source] = createModelAndSource(t, "resources/does-not-exist.usdz");
-    return model.ready.then(
-        value => assert_unreached("Unexpected ready promise resolution."),
-        reason => assert_true(reason.toString().includes("NetworkError"), "The ready promise is rejected with a NetworkError.")
-    );
-}, `<model> rejects the ready promise when provided with an unknown resoure.`);
+    let result = document.getElementById("result");
+    let model = document.getElementById("model");
 
-promise_test(async t => {
-    const [model, source] = createModelAndSource(t, "resources/heart.usdz");
-    const modelReady = model.ready;
-
-    source.remove();
-    assert_not_equals(model.ready, modelReady, "Removing the <source> child resets the ready promise.");
-
-    return modelReady.then(
-        value => assert_unreached("Unexpected ready promise resolution."),
-        reason => assert_true(reason.toString().includes("AbortError"), "The ready promise is rejected with a NetworkError.")
-    );
-}, `<model> rejects the ready promise when its resource load is aborted.`);
-
-promise_test(async t => {
-    const [model, source] = createModelAndSource(t, "resources/cube.usdz");
-    await model.ready;
-}, `<model> resolves the ready promise when provided with a known resource.`);
-
+    model.ready.then(value => {
+        result.textContent = `Passed`;
+    }, reason => {
+        result.textContent = `Failed. Model did not load: ${reason}`;
+    }).finally(() => { 
+        if (window.testRunner)
+            testRunner.notifyDone();
+    });
 </script>
 </body>
+</html>

Deleted: trunk/LayoutTests/model-element/resources/model-element-test-utils.js (286856 => 286857)


--- trunk/LayoutTests/model-element/resources/model-element-test-utils.js	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/LayoutTests/model-element/resources/model-element-test-utils.js	2021-12-10 17:28:10 UTC (rev 286857)
@@ -1,14 +0,0 @@
-
-const createModelAndSource = (test, src) => {
-    const model = document.createElement("model");
-    document.body.appendChild(model);
-
-    const source = document.createElement("source");
-    if (src)
-        source.src = ""
-    model.appendChild(source);
-
-    test.add_cleanup(() => model.remove());
-
-    return [model, source];
-};

Modified: trunk/LayoutTests/platform/ios-simulator/TestExpectations (286856 => 286857)


--- trunk/LayoutTests/platform/ios-simulator/TestExpectations	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/LayoutTests/platform/ios-simulator/TestExpectations	2021-12-10 17:28:10 UTC (rev 286857)
@@ -152,6 +152,3 @@
 http/tests/security/webaudio-render-remote-audio-allowed-crossorigin-redirect.html [ Pass Timeout ]
 
 webkit.org/b/223949 crypto/crypto-random-values-oom.html [ Pass Timeout ]
-
-# This test relies on ARQL APIs which are not available in the Simulator
-model-element/model-element-ready.html [ Skip ]
\ No newline at end of file

Modified: trunk/LayoutTests/platform/mac/TestExpectations (286856 => 286857)


--- trunk/LayoutTests/platform/mac/TestExpectations	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/LayoutTests/platform/mac/TestExpectations	2021-12-10 17:28:10 UTC (rev 286857)
@@ -2282,7 +2282,7 @@
 # webkit.org/b/228200 Adjusting test expectations for Monterey on Open Source <rdar://80344138>:
 [ Monterey ] imported/w3c/web-platform-tests/fetch/connection-pool/network-partition-key.html [ Failure ]
 
-# webkit.org/b/228200 Setting multiple test expectations for Monterey on OpenSource:
+# webkit.org/b/228200 Setting multiple test expectations for Monetery on OpenSource:
 [ Monterey ] model-element/model-element-graphics-layers-opacity.html [ Pass Failure ]
 [ Monterey Debug arm64 ] imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-restartIce.https.html [ Pass Failure Crash ]
 
@@ -2428,6 +2428,3 @@
 webkit.org/b/226826 [ Monterey ] http/tests/ssl/applepay/ApplePayButton.html [ Failure ]
 
 webkit.org/b/221230 [ BigSur+ ] imported/w3c/web-platform-tests/media-source/mediasource-addsourcebuffer.html [ Pass Failure ]
-
-# <model> tests involving the ready promise can only work on Monterey and up
-[ Catalina BigSur ] model-element/model-element-ready.html [ Skip ]

Modified: trunk/Source/WebCore/ChangeLog (286856 => 286857)


--- trunk/Source/WebCore/ChangeLog	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/Source/WebCore/ChangeLog	2021-12-10 17:28:10 UTC (rev 286857)
@@ -1,3 +1,17 @@
+2021-12-10  Commit Queue  <commit-qu...@webkit.org>
+
+        Unreviewed, reverting r286836.
+        https://bugs.webkit.org/show_bug.cgi?id=234153
+
+        some tests are flaky on iOS and some are crashing on macOS
+
+        Reverted changeset:
+
+        "[Model] Add load and error events to distinguish resource
+        load from model readiness"
+        https://bugs.webkit.org/show_bug.cgi?id=233706
+        https://commits.webkit.org/r286836
+
 2021-12-10  Chris Dumez  <cdu...@apple.com>
 
         Radio buttons with no form owner are not grouped

Modified: trunk/Source/WebCore/Modules/model-element/HTMLModelElement.cpp (286856 => 286857)


--- trunk/Source/WebCore/Modules/model-element/HTMLModelElement.cpp	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/Source/WebCore/Modules/model-element/HTMLModelElement.cpp	2021-12-10 17:28:10 UTC (rev 286857)
@@ -65,10 +65,8 @@
 
 HTMLModelElement::HTMLModelElement(const QualifiedName& tagName, Document& document)
     : HTMLElement(tagName, document)
-    , ActiveDOMObject(document)
     , m_readyPromise { makeUniqueRef<ReadyPromise>(*this, &HTMLModelElement::readyPromiseResolve) }
 {
-    setHasCustomStyleResolveCallbacks();
 }
 
 HTMLModelElement::~HTMLModelElement()
@@ -81,9 +79,7 @@
 
 Ref<HTMLModelElement> HTMLModelElement::create(const QualifiedName& tagName, Document& document)
 {
-    auto model = adoptRef(*new HTMLModelElement(tagName, document));
-    model->suspendIfNeeded();
-    return model;
+    return adoptRef(*new HTMLModelElement(tagName, document));
 }
 
 RefPtr<Model> HTMLModelElement::model() const
@@ -135,12 +131,9 @@
         m_readyPromise->reject(Exception { AbortError });
 
     m_readyPromise = makeUniqueRef<ReadyPromise>(*this, &HTMLModelElement::readyPromiseResolve);
-    m_shouldCreateModelPlayerUponRendererAttachment = false;
 
-    if (m_sourceURL.isEmpty()) {
-        queueTaskToDispatchEvent(*this, TaskSource::DOMManipulation, Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No));
+    if (m_sourceURL.isEmpty())
         return;
-    }
 
     ResourceLoaderOptions options = CachedResourceLoader::defaultCachedResourceOptions();
     options.destination = FetchOptions::Destination::Model;
@@ -152,7 +145,6 @@
 
     auto resource = document().cachedResourceLoader().requestModelResource(WTFMove(request));
     if (!resource.has_value()) {
-        queueTaskToDispatchEvent(*this, TaskSource::DOMManipulation, Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No));
         m_readyPromise->reject(Exception { NetworkError });
         return;
     }
@@ -183,15 +175,6 @@
     return createRenderer<RenderModel>(*this, WTFMove(style));
 }
 
-void HTMLModelElement::didAttachRenderers()
-{
-    if (!m_shouldCreateModelPlayerUponRendererAttachment)
-        return;
-
-    m_shouldCreateModelPlayerUponRendererAttachment = false;
-    createModelPlayer();
-}
-
 // MARK: - CachedRawResourceClient
 
 void HTMLModelElement::dataReceived(CachedResource& resource, const uint8_t* data, int dataLength)
@@ -214,8 +197,6 @@
     if (resource.loadFailedOrCanceled()) {
         m_data = nullptr;
 
-        queueTaskToDispatchEvent(*this, TaskSource::DOMManipulation, Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No));
-
         invalidateResourceHandleAndUpdateRenderer();
 
         m_readyPromise->reject(Exception { NetworkError });
@@ -225,10 +206,10 @@
     m_dataComplete = true;
     m_model = Model::create(m_data.releaseNonNull().get(), resource.mimeType(), resource.url());
 
-    queueTaskToDispatchEvent(*this, TaskSource::DOMManipulation, Event::create(eventNames().loadEvent, Event::CanBubble::No, Event::IsCancelable::No));
-
     invalidateResourceHandleAndUpdateRenderer();
 
+    m_readyPromise->resolve(*this);
+
     modelDidChange();
 }
 
@@ -236,34 +217,25 @@
 
 void HTMLModelElement::modelDidChange()
 {
-    auto* page = document().page();
-    if (!page) {
-        m_readyPromise->reject(Exception { AbortError });
+    // FIXME: For the early returns here, we should probably inform the page that things have
+    // failed to render. For the case of no-renderer, we should probably also build the model
+    // when/if a renderer is created.
+
+    auto page = document().page();
+    if (!page)
         return;
-    }
 
     auto* renderer = this->renderer();
-    if (!renderer) {
-        m_shouldCreateModelPlayerUponRendererAttachment = true;
+    if (!renderer)
         return;
-    }
 
-    createModelPlayer();
-}
-
-void HTMLModelElement::createModelPlayer()
-{
-    ASSERT(document().page());
-    m_modelPlayer = document().page()->modelPlayerProvider().createModelPlayer(*this);
-    if (!m_modelPlayer) {
-        m_readyPromise->reject(Exception { AbortError });
+    m_modelPlayer = page->modelPlayerProvider().createModelPlayer(*this);
+    if (!m_modelPlayer)
         return;
-    }
 
     // FIXME: We need to tell the player if the size changes as well, so passing this
     // in with load probably doesn't make sense.
-    ASSERT(renderer());
-    auto size = renderer()->absoluteBoundingBoxRect(false).size();
+    auto size = renderer->absoluteBoundingBoxRect(false).size();
     m_modelPlayer->load(*m_model, size);
 }
 
@@ -283,14 +255,11 @@
 
     if (auto* renderer = this->renderer())
         renderer->updateFromElement();
-
-    m_readyPromise->resolve(*this);
 }
 
 void HTMLModelElement::didFailLoading(ModelPlayer& modelPlayer, const ResourceError&)
 {
     ASSERT_UNUSED(modelPlayer, &modelPlayer == m_modelPlayer);
-    m_readyPromise->reject(Exception { AbortError });
 }
 
 GraphicsLayer::PlatformLayerID HTMLModelElement::platformLayerID()
@@ -583,18 +552,6 @@
     });
 }
 
-const char* HTMLModelElement::activeDOMObjectName() const
-{
-    return "HTMLModelElement";
-}
-
-bool HTMLModelElement::virtualHasPendingActivity() const
-{
-    // We need to ensure the JS wrapper is kept alive if a load is in progress and we may yet dispatch
-    // "load" or "error" events, ie. as long as we have a resource, meaning we are in the process of loading.
-    return m_resource;
-}
-
 #if PLATFORM(COCOA)
 Vector<RetainPtr<id>> HTMLModelElement::accessibilityChildren()
 {

Modified: trunk/Source/WebCore/Modules/model-element/HTMLModelElement.h (286856 => 286857)


--- trunk/Source/WebCore/Modules/model-element/HTMLModelElement.h	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/Source/WebCore/Modules/model-element/HTMLModelElement.h	2021-12-10 17:28:10 UTC (rev 286857)
@@ -27,7 +27,6 @@
 
 #if ENABLE(MODEL_ELEMENT)
 
-#include "ActiveDOMObject.h"
 #include "CachedRawResource.h"
 #include "CachedRawResourceClient.h"
 #include "CachedResourceHandle.h"
@@ -50,7 +49,7 @@
 template<typename IDLType> class DOMPromiseDeferred;
 template<typename IDLType> class DOMPromiseProxyWithResolveCallback;
 
-class HTMLModelElement final : public HTMLElement, private CachedRawResourceClient, public ModelPlayerClient, public ActiveDOMObject {
+class HTMLModelElement final : public HTMLElement, private CachedRawResourceClient, public ModelPlayerClient {
     WTF_MAKE_ISO_ALLOCATED(HTMLModelElement);
 public:
     static Ref<HTMLModelElement> create(const QualifiedName&, Document&);
@@ -58,7 +57,6 @@
 
     void sourcesChanged();
     const URL& currentSrc() const { return m_sourceURL; }
-    bool complete() const { return m_dataComplete; }
 
     // MARK: DOM Functions and Attributes
 
@@ -108,20 +106,14 @@
 
     void setSourceURL(const URL&);
     void modelDidChange();
-    void createModelPlayer();
 
     HTMLModelElement& readyPromiseResolve();
 
-    // ActiveDOMObject
-    const char* activeDOMObjectName() const final;
-    bool virtualHasPendingActivity() const final;
-
     // DOM overrides.
     void didMoveToNewDocument(Document& oldDocument, Document& newDocument) final;
 
     // Rendering overrides.
     RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) final;
-    void didAttachRenderers() final;
 
     // CachedRawResourceClient overrides.
     void dataReceived(CachedResource&, const uint8_t* data, int dataLength) final;
@@ -146,7 +138,6 @@
     UniqueRef<ReadyPromise> m_readyPromise;
     bool m_dataComplete { false };
     bool m_isDragging { false };
-    bool m_shouldCreateModelPlayerUponRendererAttachment { false };
 
     RefPtr<ModelPlayer> m_modelPlayer;
 };

Modified: trunk/Source/WebCore/Modules/model-element/HTMLModelElement.idl (286856 => 286857)


--- trunk/Source/WebCore/Modules/model-element/HTMLModelElement.idl	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/Source/WebCore/Modules/model-element/HTMLModelElement.idl	2021-12-10 17:28:10 UTC (rev 286857)
@@ -30,7 +30,6 @@
 ] interface HTMLModelElement : HTMLElement {
     [URL] readonly attribute USVString currentSrc;
 
-    readonly attribute boolean complete;
     readonly attribute Promise<HTMLModelElement> ready;
 
     undefined enterFullscreen();

Modified: trunk/Tools/ChangeLog (286856 => 286857)


--- trunk/Tools/ChangeLog	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/Tools/ChangeLog	2021-12-10 17:28:10 UTC (rev 286857)
@@ -1,3 +1,17 @@
+2021-12-10  Commit Queue  <commit-qu...@webkit.org>
+
+        Unreviewed, reverting r286836.
+        https://bugs.webkit.org/show_bug.cgi?id=234153
+
+        some tests are flaky on iOS and some are crashing on macOS
+
+        Reverted changeset:
+
+        "[Model] Add load and error events to distinguish resource
+        load from model readiness"
+        https://bugs.webkit.org/show_bug.cgi?id=233706
+        https://commits.webkit.org/r286836
+
 2021-12-10  Kimmo Kinnunen  <kkinnu...@apple.com>
 
         IOSurface memory attribution is hard to use in constructors

Modified: trunk/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm (286856 => 286857)


--- trunk/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm	2021-12-10 17:25:22 UTC (rev 286856)
+++ trunk/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm	2021-12-10 17:28:10 UTC (rev 286857)
@@ -2197,7 +2197,7 @@
     [configuration setURLSchemeHandler:handler.get() forURLScheme:@"model"];
     
     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration.get()]);
-    [webView synchronouslyLoadHTMLString:@"<model><source src=''></model><script>document.querySelector('model').addEventListener('load', event => window.webkit.messageHandlers.modelLoading.postMessage('READY'));</script>"];
+    [webView synchronouslyLoadHTMLString:@"<model><source src=''></model><script>document.getElementsByTagName('model')[0].ready.then(() => { window.webkit.messageHandlers.modelLoading.postMessage('READY') });</script>"];
 
     while (![messageHandler didLoadModel])
         Util::spinRunLoop();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to