Title: [286836] trunk
Revision
286836
Author
grao...@webkit.org
Date
2021-12-10 00:15:09 -0800 (Fri, 10 Dec 2021)

Log Message

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

Reviewed by Chris Dumez and Dean Jackson.

Source/WebCore:

Test: model-element/model-element-error-and-load-events.html

Prior to this patch, <model> elements had a "ready" promise which resolved once the resource had been loaded.
However, this promise should be used when the <model> is fully ready, and this is done on macOS and iOS asynchronously
after the resource has been loaded by the supporting ARQL framework. So we need a way to monitor success or failure of
the resource load specifically.

To that end, and matching the <img> element, we dispatch "load" and "error" events on <model> elements and add a
"complete" property to indicate whether the resource is loaded.

Meanwhile, the "ready" promise is now resolved when the model is fully loaded by the supporting framework, indicating
that further APIs are safe to use.

Since creating the support ARQL object for macOS and iOS also requires the <model> element's renderer being available,
we opt into "custom style resolve callbacks" so that we may implement didAttachRenderers() on HTMLModelElement and keep
track of renderer availability before attempting to create the ModelPlayer.

* Modules/model-element/HTMLModelElement.cpp:
(WebCore::HTMLModelElement::HTMLModelElement):
(WebCore::HTMLModelElement::create):
(WebCore::HTMLModelElement::setSourceURL):
(WebCore::HTMLModelElement::didAttachRenderers):
(WebCore::HTMLModelElement::notifyFinished):
(WebCore::HTMLModelElement::modelDidChange):
(WebCore::HTMLModelElement::createModelPlayer):
(WebCore::HTMLModelElement::didFinishLoading):
(WebCore::HTMLModelElement::didFailLoading):
(WebCore::HTMLModelElement::activeDOMObjectName const):
(WebCore::HTMLModelElement::virtualHasPendingActivity const):
* Modules/model-element/HTMLModelElement.h:
* Modules/model-element/HTMLModelElement.idl:

Tools:

Use the "load" event instead of the "ready" promise for this test which only requires monitoring
the <model> resource being loaded.

* TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm:
(TestWebKitAPI::TEST):

LayoutTests:

Remove existing tests around resource loading and recreate them in terms of "load" and "error"
events in model-element/model-element-error-and-load-events.html and in terms of the ready
promise in model-element/model-element-ready.html.

Other tests using model.ready for other purposes are also rewritten using events.

* model-element/model-element-contents-layer-updates-with-clipping.html:
* model-element/model-element-contents-layer-updates.html:
* model-element/model-element-error-and-load-events-expected.txt: Added.
* model-element/model-element-error-and-load-events.html: Added.
* model-element/model-element-graphics-layers-opacity.html:
* model-element/model-element-graphics-layers.html:
* model-element/model-element-ready-expected.txt:
* model-element/model-element-ready-load-aborted-expected.txt: Removed.
* model-element/model-element-ready-load-aborted.html: Removed.
* model-element/model-element-ready-load-failed-expected.txt: Removed.
* model-element/model-element-ready-load-failed.html: Removed.
* model-element/model-element-ready.html:
* model-element/resources/model-element-test-utils.js: Added.
(const.createModelAndSource):
* platform/ios-simulator/TestExpectations:
* platform/mac/TestExpectations:

Modified Paths

Added Paths

Removed Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (286835 => 286836)


--- trunk/LayoutTests/ChangeLog	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/LayoutTests/ChangeLog	2021-12-10 08:15:09 UTC (rev 286836)
@@ -1,3 +1,34 @@
+2021-12-09  Antoine Quint  <grao...@webkit.org>
+
+        [Model] Add load and error events to distinguish resource load from model readiness
+        https://bugs.webkit.org/show_bug.cgi?id=233706
+        rdar://85922697
+
+        Reviewed by Chris Dumez and Dean Jackson.
+
+        Remove existing tests around resource loading and recreate them in terms of "load" and "error"
+        events in model-element/model-element-error-and-load-events.html and in terms of the ready
+        promise in model-element/model-element-ready.html.
+
+        Other tests using model.ready for other purposes are also rewritten using events. 
+
+        * model-element/model-element-contents-layer-updates-with-clipping.html:
+        * model-element/model-element-contents-layer-updates.html:
+        * model-element/model-element-error-and-load-events-expected.txt: Added.
+        * model-element/model-element-error-and-load-events.html: Added.
+        * model-element/model-element-graphics-layers-opacity.html:
+        * model-element/model-element-graphics-layers.html:
+        * model-element/model-element-ready-expected.txt:
+        * model-element/model-element-ready-load-aborted-expected.txt: Removed.
+        * model-element/model-element-ready-load-aborted.html: Removed.
+        * model-element/model-element-ready-load-failed-expected.txt: Removed.
+        * model-element/model-element-ready-load-failed.html: Removed.
+        * model-element/model-element-ready.html:
+        * model-element/resources/model-element-test-utils.js: Added.
+        (const.createModelAndSource):
+        * platform/ios-simulator/TestExpectations:
+        * platform/mac/TestExpectations:
+
 2021-12-09  Arcady Goldmints-Orlov  <agoldmi...@igalia.com>
 
         [GLIB] Update test expectations and baselines. Unreviewed test gardening.

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


--- trunk/LayoutTests/model-element/model-element-contents-layer-updates-with-clipping.html	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/LayoutTests/model-element/model-element-contents-layer-updates-with-clipping.html	2021-12-10 08:15:09 UTC (rev 286836)
@@ -6,39 +6,40 @@
 </model>
 <pre id="layers"></pre>
 <script>
-    let layers = document.getElementById("layers");
-    let source = document.getElementsByTagName("source")[0];
+    window.testRunner?.waitUntilDone();
+    window.testRunner?.dumpAsText();
 
-    if (window.testRunner) {
-        testRunner.waitUntilDone();
-        testRunner.dumpAsText();
-    } else
-        layers.textContent = "This test requires testRunner.";
+    const layers = document.getElementById("layers");
+    const source = document.querySelector("source");
+    const model = document.getElementById("model");
 
-    let model = document.getElementById("model");
-
-    model.ready.then(value => {
+    const modelDidLoadFirstSource = () => {
         layers.textContent = "Before Changing Source:\n";
-        layers.textContent += window.internals.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS);
+        layers.textContent += window.internals?.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS) ?? "This test requires testRunner.";
 
+        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 = ""
-        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();
-    });
+    }
+
+    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 });
+    }
 </script>
 </body>
 </html>

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


--- trunk/LayoutTests/model-element/model-element-contents-layer-updates.html	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/LayoutTests/model-element/model-element-contents-layer-updates.html	2021-12-10 08:15:09 UTC (rev 286836)
@@ -6,39 +6,40 @@
 </model>
 <pre id="layers"></pre>
 <script>
-    let layers = document.getElementById("layers");
-    let source = document.getElementsByTagName("source")[0];
+    window.testRunner?.waitUntilDone();
+    window.testRunner?.dumpAsText();
 
-    if (window.testRunner) {
-        testRunner.waitUntilDone();
-        testRunner.dumpAsText();
-    } else
-        layers.textContent = "This test requires testRunner.";
+    const layers = document.getElementById("layers");
+    const source = document.querySelector("source");
+    const model = document.getElementById("model");
 
-    let model = document.getElementById("model");
-
-    model.ready.then(value => {
+    const modelDidLoadFirstSource = () => {
         layers.textContent = "Before Changing Source:\n";
-        layers.textContent += window.internals.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS);
+        layers.textContent += window.internals?.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS) ?? "This test requires testRunner.";
 
+        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 = ""
-        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();
-    });
+    }
+
+    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 });
+    }
 </script>
 </body>
 </html>

Added: trunk/LayoutTests/model-element/model-element-error-and-load-events-expected.txt (0 => 286836)


--- trunk/LayoutTests/model-element/model-element-error-and-load-events-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/model-element/model-element-error-and-load-events-expected.txt	2021-12-10 08:15:09 UTC (rev 286836)
@@ -0,0 +1,5 @@
+
+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.
+

Added: trunk/LayoutTests/model-element/model-element-error-and-load-events.html (0 => 286836)


--- trunk/LayoutTests/model-element/model-element-error-and-load-events.html	                        (rev 0)
+++ trunk/LayoutTests/model-element/model-element-error-and-load-events.html	2021-12-10 08:15:09 UTC (rev 286836)
@@ -0,0 +1,52 @@
+<!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 (286835 => 286836)


--- trunk/LayoutTests/model-element/model-element-graphics-layers-opacity.html	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/LayoutTests/model-element/model-element-graphics-layers-opacity.html	2021-12-10 08:15:09 UTC (rev 286836)
@@ -13,26 +13,27 @@
 </model>
 <pre id="layers"></pre>
 <script>
-    let layers = document.getElementById("layers");
+    window.testRunner?.waitUntilDone();
+    window.testRunner?.dumpAsText();
 
-    if (window.testRunner) {
-        testRunner.waitUntilDone();
-        testRunner.dumpAsText();
-    } else
-        layers.textContent = "This test requires testRunner.";
+    const layers = document.getElementById("layers");
+    const model = document.getElementById("model");
 
-    let model = document.getElementById("model");
+    const modelDidLoad = () => {
+        layers.innerText = window.internals?.platformLayerTreeAsText(model) ?? "This test requires testRunner.";
+        model.remove();
+        window.testRunner?.notifyDone();
+    };
 
-    model.ready.then(value => {
-        if (window.testRunner)
-            layers.innerText = window.internals.platformLayerTreeAsText(model);
-        model.remove();
-    }, reason => {
-        layers.textContent = `Failed. Model did not load: ${reason}`;
-    }).finally(() => { 
-        if (window.testRunner)
-            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();
+        });
+    }
 </script>
 </body>
 </html>

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


--- trunk/LayoutTests/model-element/model-element-graphics-layers.html	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/LayoutTests/model-element/model-element-graphics-layers.html	2021-12-10 08:15:09 UTC (rev 286836)
@@ -6,26 +6,27 @@
 </model>
 <pre id="layers"></pre>
 <script>
-    let layers = document.getElementById("layers");
+    window.testRunner?.waitUntilDone();
+    window.testRunner?.dumpAsText();
 
-    if (window.testRunner) {
-        testRunner.waitUntilDone();
-        testRunner.dumpAsText();
-    } else
-        layers.textContent = "This test requires testRunner.";
+    const layers = document.getElementById("layers");
+    const model = document.getElementById("model");
 
-    let model = document.getElementById("model");
+    const modelDidLoad = () => {
+        layers.innerText = window.internals?.layerTreeAsText(document) ?? "This test requires testRunner.";
+        model.remove();
+        window.testRunner?.notifyDone();
+    }
 
-    model.ready.then(value => {
-        if (window.testRunner)
-            layers.innerText = window.internals.layerTreeAsText(document);
-        model.remove();
-    }, reason => {
-        layers.textContent = `Failed. Model did not load: ${reason}`;
-    }).finally(() => { 
-        if (window.testRunner)
-            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();
+        });
+    }
 </script>
 </body>
 </html>

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


--- trunk/LayoutTests/model-element/model-element-ready-expected.txt	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/LayoutTests/model-element/model-element-ready-expected.txt	2021-12-10 08:15:09 UTC (rev 286836)
@@ -1,3 +1,5 @@
-This test passes if you see the word "Passed" below:
 
-Passed
+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.
+

Deleted: trunk/LayoutTests/model-element/model-element-ready-load-aborted-expected.txt (286835 => 286836)


--- trunk/LayoutTests/model-element/model-element-ready-load-aborted-expected.txt	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/LayoutTests/model-element/model-element-ready-load-aborted-expected.txt	2021-12-10 08:15:09 UTC (rev 286836)
@@ -1,3 +0,0 @@
-This test passes if you see the word "Passed" below:
-
-Passed

Deleted: trunk/LayoutTests/model-element/model-element-ready-load-aborted.html (286835 => 286836)


--- trunk/LayoutTests/model-element/model-element-ready-load-aborted.html	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/LayoutTests/model-element/model-element-ready-load-aborted.html	2021-12-10 08:15:09 UTC (rev 286836)
@@ -1,36 +0,0 @@
-<!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>

Deleted: trunk/LayoutTests/model-element/model-element-ready-load-failed-expected.txt (286835 => 286836)


--- trunk/LayoutTests/model-element/model-element-ready-load-failed-expected.txt	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/LayoutTests/model-element/model-element-ready-load-failed-expected.txt	2021-12-10 08:15:09 UTC (rev 286836)
@@ -1,3 +0,0 @@
-This test passes if you see the word "Passed" below:
-
-Passed

Deleted: trunk/LayoutTests/model-element/model-element-ready-load-failed.html (286835 => 286836)


--- trunk/LayoutTests/model-element/model-element-ready-load-failed.html	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/LayoutTests/model-element/model-element-ready-load-failed.html	2021-12-10 08:15:09 UTC (rev 286836)
@@ -1,31 +0,0 @@
-<!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 (286835 => 286836)


--- trunk/LayoutTests/model-element/model-element-ready.html	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/LayoutTests/model-element/model-element-ready.html	2021-12-10 08:15:09 UTC (rev 286836)
@@ -1,28 +1,38 @@
-<!DOCTYPE html><!-- webkit-test-runner [ ModelElementEnabled=true ] -->
-<html>
+<!doctype html>
+<meta charset="utf-8">
+<title>&lt;model> ready promise</title>
+<script src=""
+<script src=""
+<script src=""
 <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();
-    }
+'use strict';
 
-    let result = document.getElementById("result");
-    let model = document.getElementById("model");
+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.`);
 
-    model.ready.then(value => {
-        result.textContent = `Passed`;
-    }, reason => {
-        result.textContent = `Failed. Model did not load: ${reason}`;
-    }).finally(() => { 
-        if (window.testRunner)
-            testRunner.notifyDone();
-    });
+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.`);
+
 </script>
 </body>
-</html>

Added: trunk/LayoutTests/model-element/resources/model-element-test-utils.js (0 => 286836)


--- trunk/LayoutTests/model-element/resources/model-element-test-utils.js	                        (rev 0)
+++ trunk/LayoutTests/model-element/resources/model-element-test-utils.js	2021-12-10 08:15:09 UTC (rev 286836)
@@ -0,0 +1,14 @@
+
+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 (286835 => 286836)


--- trunk/LayoutTests/platform/ios-simulator/TestExpectations	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/LayoutTests/platform/ios-simulator/TestExpectations	2021-12-10 08:15:09 UTC (rev 286836)
@@ -152,3 +152,6 @@
 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 (286835 => 286836)


--- trunk/LayoutTests/platform/mac/TestExpectations	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/LayoutTests/platform/mac/TestExpectations	2021-12-10 08:15:09 UTC (rev 286836)
@@ -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 Monetery on OpenSource:
+# webkit.org/b/228200 Setting multiple test expectations for Monterey 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,3 +2428,6 @@
 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 (286835 => 286836)


--- trunk/Source/WebCore/ChangeLog	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/Source/WebCore/ChangeLog	2021-12-10 08:15:09 UTC (rev 286836)
@@ -1,3 +1,43 @@
+2021-12-09  Antoine Quint  <grao...@webkit.org>
+
+        [Model] Add load and error events to distinguish resource load from model readiness
+        https://bugs.webkit.org/show_bug.cgi?id=233706
+        rdar://85922697
+
+        Reviewed by Chris Dumez and Dean Jackson.
+
+        Test: model-element/model-element-error-and-load-events.html
+
+        Prior to this patch, <model> elements had a "ready" promise which resolved once the resource had been loaded.
+        However, this promise should be used when the <model> is fully ready, and this is done on macOS and iOS asynchronously
+        after the resource has been loaded by the supporting ARQL framework. So we need a way to monitor success or failure of
+        the resource load specifically.
+
+        To that end, and matching the <img> element, we dispatch "load" and "error" events on <model> elements and add a
+        "complete" property to indicate whether the resource is loaded.
+
+        Meanwhile, the "ready" promise is now resolved when the model is fully loaded by the supporting framework, indicating
+        that further APIs are safe to use.
+
+        Since creating the support ARQL object for macOS and iOS also requires the <model> element's renderer being available,
+        we opt into "custom style resolve callbacks" so that we may implement didAttachRenderers() on HTMLModelElement and keep
+        track of renderer availability before attempting to create the ModelPlayer.
+
+        * Modules/model-element/HTMLModelElement.cpp:
+        (WebCore::HTMLModelElement::HTMLModelElement):
+        (WebCore::HTMLModelElement::create):
+        (WebCore::HTMLModelElement::setSourceURL):
+        (WebCore::HTMLModelElement::didAttachRenderers):
+        (WebCore::HTMLModelElement::notifyFinished):
+        (WebCore::HTMLModelElement::modelDidChange):
+        (WebCore::HTMLModelElement::createModelPlayer):
+        (WebCore::HTMLModelElement::didFinishLoading):
+        (WebCore::HTMLModelElement::didFailLoading):
+        (WebCore::HTMLModelElement::activeDOMObjectName const):
+        (WebCore::HTMLModelElement::virtualHasPendingActivity const):
+        * Modules/model-element/HTMLModelElement.h:
+        * Modules/model-element/HTMLModelElement.idl:
+
 2021-12-10  Said Abou-Hallawa  <s...@apple.com>
 
         [GPU Process] [Filters] Enable Filter rendering in GPU Process

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


--- trunk/Source/WebCore/Modules/model-element/HTMLModelElement.cpp	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/Source/WebCore/Modules/model-element/HTMLModelElement.cpp	2021-12-10 08:15:09 UTC (rev 286836)
@@ -65,8 +65,10 @@
 
 HTMLModelElement::HTMLModelElement(const QualifiedName& tagName, Document& document)
     : HTMLElement(tagName, document)
+    , ActiveDOMObject(document)
     , m_readyPromise { makeUniqueRef<ReadyPromise>(*this, &HTMLModelElement::readyPromiseResolve) }
 {
+    setHasCustomStyleResolveCallbacks();
 }
 
 HTMLModelElement::~HTMLModelElement()
@@ -79,7 +81,9 @@
 
 Ref<HTMLModelElement> HTMLModelElement::create(const QualifiedName& tagName, Document& document)
 {
-    return adoptRef(*new HTMLModelElement(tagName, document));
+    auto model = adoptRef(*new HTMLModelElement(tagName, document));
+    model->suspendIfNeeded();
+    return model;
 }
 
 RefPtr<Model> HTMLModelElement::model() const
@@ -131,9 +135,12 @@
         m_readyPromise->reject(Exception { AbortError });
 
     m_readyPromise = makeUniqueRef<ReadyPromise>(*this, &HTMLModelElement::readyPromiseResolve);
+    m_shouldCreateModelPlayerUponRendererAttachment = false;
 
-    if (m_sourceURL.isEmpty())
+    if (m_sourceURL.isEmpty()) {
+        queueTaskToDispatchEvent(*this, TaskSource::DOMManipulation, Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No));
         return;
+    }
 
     ResourceLoaderOptions options = CachedResourceLoader::defaultCachedResourceOptions();
     options.destination = FetchOptions::Destination::Model;
@@ -145,6 +152,7 @@
 
     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;
     }
@@ -175,6 +183,15 @@
     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)
@@ -197,6 +214,8 @@
     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 });
@@ -206,10 +225,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();
 }
 
@@ -217,25 +236,34 @@
 
 void HTMLModelElement::modelDidChange()
 {
-    // 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)
+    auto* page = document().page();
+    if (!page) {
+        m_readyPromise->reject(Exception { AbortError });
         return;
+    }
 
     auto* renderer = this->renderer();
-    if (!renderer)
+    if (!renderer) {
+        m_shouldCreateModelPlayerUponRendererAttachment = true;
         return;
+    }
 
-    m_modelPlayer = page->modelPlayerProvider().createModelPlayer(*this);
-    if (!m_modelPlayer)
+    createModelPlayer();
+}
+
+void HTMLModelElement::createModelPlayer()
+{
+    ASSERT(document().page());
+    m_modelPlayer = document().page()->modelPlayerProvider().createModelPlayer(*this);
+    if (!m_modelPlayer) {
+        m_readyPromise->reject(Exception { AbortError });
         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.
-    auto size = renderer->absoluteBoundingBoxRect(false).size();
+    ASSERT(renderer());
+    auto size = renderer()->absoluteBoundingBoxRect(false).size();
     m_modelPlayer->load(*m_model, size);
 }
 
@@ -255,11 +283,14 @@
 
     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()
@@ -552,6 +583,18 @@
     });
 }
 
+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 (286835 => 286836)


--- trunk/Source/WebCore/Modules/model-element/HTMLModelElement.h	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/Source/WebCore/Modules/model-element/HTMLModelElement.h	2021-12-10 08:15:09 UTC (rev 286836)
@@ -27,6 +27,7 @@
 
 #if ENABLE(MODEL_ELEMENT)
 
+#include "ActiveDOMObject.h"
 #include "CachedRawResource.h"
 #include "CachedRawResourceClient.h"
 #include "CachedResourceHandle.h"
@@ -49,7 +50,7 @@
 template<typename IDLType> class DOMPromiseDeferred;
 template<typename IDLType> class DOMPromiseProxyWithResolveCallback;
 
-class HTMLModelElement final : public HTMLElement, private CachedRawResourceClient, public ModelPlayerClient {
+class HTMLModelElement final : public HTMLElement, private CachedRawResourceClient, public ModelPlayerClient, public ActiveDOMObject {
     WTF_MAKE_ISO_ALLOCATED(HTMLModelElement);
 public:
     static Ref<HTMLModelElement> create(const QualifiedName&, Document&);
@@ -57,6 +58,7 @@
 
     void sourcesChanged();
     const URL& currentSrc() const { return m_sourceURL; }
+    bool complete() const { return m_dataComplete; }
 
     // MARK: DOM Functions and Attributes
 
@@ -106,14 +108,20 @@
 
     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;
@@ -138,6 +146,7 @@
     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 (286835 => 286836)


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

Modified: trunk/Tools/ChangeLog (286835 => 286836)


--- trunk/Tools/ChangeLog	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/Tools/ChangeLog	2021-12-10 08:15:09 UTC (rev 286836)
@@ -1,3 +1,17 @@
+2021-12-09  Antoine Quint  <grao...@webkit.org>
+
+        [Model] Add load and error events to distinguish resource load from model readiness
+        https://bugs.webkit.org/show_bug.cgi?id=233706
+        rdar://85922697
+
+        Reviewed by Chris Dumez and Dean Jackson.
+
+        Use the "load" event instead of the "ready" promise for this test which only requires monitoring
+        the <model> resource being loaded.
+
+        * TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm:
+        (TestWebKitAPI::TEST):
+
 2021-12-09  Aditya Keerthi  <akeer...@apple.com>
 
         [iOS] Add SPI to enable find interactions on WKWebView

Modified: trunk/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm (286835 => 286836)


--- trunk/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm	2021-12-10 08:11:25 UTC (rev 286835)
+++ trunk/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm	2021-12-10 08:15:09 UTC (rev 286836)
@@ -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.getElementsByTagName('model')[0].ready.then(() => { window.webkit.messageHandlers.modelLoading.postMessage('READY') });</script>"];
+    [webView synchronouslyLoadHTMLString:@"<model><source src=''></model><script>document.querySelector('model').addEventListener('load', event => 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