[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:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@288424 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 2b72a81..3deadc7 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,5 +1,36 @@
2022-01-23 Antoine Quint <graouts@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:
+
+2022-01-23 Antoine Quint <graouts@webkit.org>
+
m_lastStyleChangeEventStyle null ptr deref for accelerated CSS Animation with no duration and an implicit keyframe
https://bugs.webkit.org/show_bug.cgi?id=235394
<rdar://problem/87701738>
diff --git a/LayoutTests/model-element/model-element-contents-layer-updates-with-clipping.html b/LayoutTests/model-element/model-element-contents-layer-updates-with-clipping.html
index e89f268..6341b7a 100644
--- a/LayoutTests/model-element/model-element-contents-layer-updates-with-clipping.html
+++ b/LayoutTests/model-element/model-element-contents-layer-updates-with-clipping.html
@@ -2,43 +2,43 @@
<html>
<body>
<model id="model" style="border-radius: 5px">
- <source src="resources/heart.usdz">
+ <source>
</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 = "resources/cube.usdz";
- 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();
- });
+ };
+
+ model.addEventListener("load", modelDidLoadFirstSource, { once: true });
+ model.addEventListener("error", event => {
+ layers.textContent = `Failed. First model did not load.`;
+ window.testRunner?.notifyDone();
+ }, { once: true });
+
+ source.src = "resources/heart.usdz";
+
</script>
</body>
</html>
diff --git a/LayoutTests/model-element/model-element-contents-layer-updates.html b/LayoutTests/model-element/model-element-contents-layer-updates.html
index 9f13a64..c99210a 100644
--- a/LayoutTests/model-element/model-element-contents-layer-updates.html
+++ b/LayoutTests/model-element/model-element-contents-layer-updates.html
@@ -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 = "resources/cube.usdz";
- 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>
diff --git a/LayoutTests/model-element/model-element-error-and-load-events-expected.txt b/LayoutTests/model-element/model-element-error-and-load-events-expected.txt
new file mode 100644
index 0000000..8fbf908
--- /dev/null
+++ b/LayoutTests/model-element/model-element-error-and-load-events-expected.txt
@@ -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.
+
diff --git a/LayoutTests/model-element/model-element-error-and-load-events.html b/LayoutTests/model-element/model-element-error-and-load-events.html
new file mode 100644
index 0000000..09a5bfc
--- /dev/null
+++ b/LayoutTests/model-element/model-element-error-and-load-events.html
@@ -0,0 +1,52 @@
+<!doctype html>
+<meta charset="utf-8">
+<title><model> load and error events</title>
+<script src="../resources/testharness.js"></script>
+<script src="../resources/testharnessreport.js"></script>
+<script src="resources/model-element-test-utils.js"></script>
+<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 = "resources/heart.usdz";
+ source.remove();
+}, `<model> dispatches an "error" event when its resource load is aborted before completion.`);
+
+model_load_test("error", source => {
+ source.src = "resources/does-not-exist.usdz";
+}, `<model> dispatches an "error" event when its specified resource does not exist.`);
+
+model_load_test("load", source => {
+ source.src = "resources/cube.usdz";
+}, `<model> dispatches a "load" event when its resource is successfully loaded.`);
+
+</script>
+</body>
diff --git a/LayoutTests/model-element/model-element-graphics-layers-opacity.html b/LayoutTests/model-element/model-element-graphics-layers-opacity.html
index 4981e6d..a6132f9 100644
--- a/LayoutTests/model-element/model-element-graphics-layers-opacity.html
+++ b/LayoutTests/model-element/model-element-graphics-layers-opacity.html
@@ -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");
-
- model.ready.then(value => {
- if (window.testRunner)
- layers.innerText = window.internals.platformLayerTreeAsText(model);
+ const modelDidLoad = () => {
+ layers.innerText = window.internals?.platformLayerTreeAsText(model) ?? "This test requires testRunner.";
model.remove();
- }, reason => {
- layers.textContent = `Failed. Model did not load: ${reason}`;
- }).finally(() => {
- if (window.testRunner)
- testRunner.notifyDone();
- });
+ 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();
+ });
+ }
</script>
</body>
</html>
diff --git a/LayoutTests/model-element/model-element-graphics-layers.html b/LayoutTests/model-element/model-element-graphics-layers.html
index f09383e..af1a21a 100644
--- a/LayoutTests/model-element/model-element-graphics-layers.html
+++ b/LayoutTests/model-element/model-element-graphics-layers.html
@@ -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");
-
- model.ready.then(value => {
- if (window.testRunner)
- layers.innerText = window.internals.layerTreeAsText(document);
+ const modelDidLoad = () => {
+ layers.innerText = window.internals?.layerTreeAsText(document) ?? "This test requires testRunner.";
model.remove();
- }, reason => {
- layers.textContent = `Failed. Model did not load: ${reason}`;
- }).finally(() => {
- if (window.testRunner)
- testRunner.notifyDone();
- });
+ 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();
+ });
+ }
</script>
</body>
</html>
diff --git a/LayoutTests/model-element/model-element-ready-expected.txt b/LayoutTests/model-element/model-element-ready-expected.txt
index f87b245..87f2976 100644
--- a/LayoutTests/model-element/model-element-ready-expected.txt
+++ b/LayoutTests/model-element/model-element-ready-expected.txt
@@ -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.
+
diff --git a/LayoutTests/model-element/model-element-ready-load-aborted-expected.txt b/LayoutTests/model-element/model-element-ready-load-aborted-expected.txt
deleted file mode 100644
index f87b245..0000000
--- a/LayoutTests/model-element/model-element-ready-load-aborted-expected.txt
+++ /dev/null
@@ -1,3 +0,0 @@
-This test passes if you see the word "Passed" below:
-
-Passed
diff --git a/LayoutTests/model-element/model-element-ready-load-aborted.html b/LayoutTests/model-element/model-element-ready-load-aborted.html
deleted file mode 100644
index ca3ca11..0000000
--- a/LayoutTests/model-element/model-element-ready-load-aborted.html
+++ /dev/null
@@ -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 = "resources/heart.usdz";
-
- model.removeChild(source);
-</script>
-</body>
-</html>
diff --git a/LayoutTests/model-element/model-element-ready-load-failed-expected.txt b/LayoutTests/model-element/model-element-ready-load-failed-expected.txt
deleted file mode 100644
index f87b245..0000000
--- a/LayoutTests/model-element/model-element-ready-load-failed-expected.txt
+++ /dev/null
@@ -1,3 +0,0 @@
-This test passes if you see the word "Passed" below:
-
-Passed
diff --git a/LayoutTests/model-element/model-element-ready-load-failed.html b/LayoutTests/model-element/model-element-ready-load-failed.html
deleted file mode 100644
index 13e83ac..0000000
--- a/LayoutTests/model-element/model-element-ready-load-failed.html
+++ /dev/null
@@ -1,31 +0,0 @@
-<!DOCTYPE html><!-- webkit-test-runner [ ModelElementEnabled=true ] -->
-<html>
-<body>
-<model id="model">
- <source src="resources/does-not-exist.usdz">
-</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>
diff --git a/LayoutTests/model-element/model-element-ready.html b/LayoutTests/model-element/model-element-ready.html
index e80624a..839b698 100644
--- a/LayoutTests/model-element/model-element-ready.html
+++ b/LayoutTests/model-element/model-element-ready.html
@@ -1,28 +1,38 @@
-<!DOCTYPE html><!-- webkit-test-runner [ ModelElementEnabled=true ] -->
-<html>
+<!doctype html>
+<meta charset="utf-8">
+<title><model> ready promise</title>
+<script src="../resources/testharness.js"></script>
+<script src="../resources/testharnessreport.js"></script>
+<script src="resources/model-element-test-utils.js"></script>
<body>
-<model id="model">
- <source src="resources/heart.usdz">
-</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>
diff --git a/LayoutTests/model-element/resources/model-element-test-utils.js b/LayoutTests/model-element/resources/model-element-test-utils.js
new file mode 100644
index 0000000..0a534cc
--- /dev/null
+++ b/LayoutTests/model-element/resources/model-element-test-utils.js
@@ -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 = src;
+ model.appendChild(source);
+
+ test.add_cleanup(() => model.remove());
+
+ return [model, source];
+};
diff --git a/LayoutTests/platform/ios-simulator/TestExpectations b/LayoutTests/platform/ios-simulator/TestExpectations
index d40465b..073c980 100644
--- a/LayoutTests/platform/ios-simulator/TestExpectations
+++ b/LayoutTests/platform/ios-simulator/TestExpectations
@@ -136,3 +136,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
diff --git a/LayoutTests/platform/mac/TestExpectations b/LayoutTests/platform/mac/TestExpectations
index c3ba857..f172013 100644
--- a/LayoutTests/platform/mac/TestExpectations
+++ b/LayoutTests/platform/mac/TestExpectations
@@ -2276,7 +2276,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 ]
@@ -2421,6 +2421,9 @@
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 ]
+
# OT-SVG is not implemented on Catalina.
[ Catalina ] fast/text/otsvg-canvas.html [ ImageOnlyFailure ]
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 1f3b712..3d3cb64 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,5 +1,45 @@
2022-01-23 Antoine Quint <graouts@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:
+
+2022-01-23 Antoine Quint <graouts@webkit.org>
+
m_lastStyleChangeEventStyle null ptr deref for accelerated CSS Animation with no duration and an implicit keyframe
https://bugs.webkit.org/show_bug.cgi?id=235394
<rdar://problem/87701738>
diff --git a/Source/WebCore/Modules/model-element/HTMLModelElement.cpp b/Source/WebCore/Modules/model-element/HTMLModelElement.cpp
index a641f810..98359ff 100644
--- a/Source/WebCore/Modules/model-element/HTMLModelElement.cpp
+++ b/Source/WebCore/Modules/model-element/HTMLModelElement.cpp
@@ -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 SharedBuffer& buffer)
@@ -196,6 +213,8 @@
if (resource.loadFailedOrCanceled()) {
m_data.reset();
+ queueTaskToDispatchEvent(*this, TaskSource::DOMManipulation, Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No));
+
invalidateResourceHandleAndUpdateRenderer();
m_readyPromise->reject(Exception { NetworkError });
@@ -205,9 +224,9 @@
m_dataComplete = true;
m_model = Model::create(m_data.takeAsContiguous().get(), resource.mimeType(), resource.url());
- invalidateResourceHandleAndUpdateRenderer();
+ queueTaskToDispatchEvent(*this, TaskSource::DOMManipulation, Event::create(eventNames().loadEvent, Event::CanBubble::No, Event::IsCancelable::No));
- m_readyPromise->resolve(*this);
+ invalidateResourceHandleAndUpdateRenderer();
modelDidChange();
}
@@ -216,25 +235,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);
}
@@ -254,11 +282,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()
@@ -551,6 +582,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()
{
diff --git a/Source/WebCore/Modules/model-element/HTMLModelElement.h b/Source/WebCore/Modules/model-element/HTMLModelElement.h
index 0d3bdb7..3dcac28 100644
--- a/Source/WebCore/Modules/model-element/HTMLModelElement.h
+++ b/Source/WebCore/Modules/model-element/HTMLModelElement.h
@@ -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 SharedBuffer&) 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;
};
diff --git a/Source/WebCore/Modules/model-element/HTMLModelElement.idl b/Source/WebCore/Modules/model-element/HTMLModelElement.idl
index 3ff6ce9..45b8fd2 100644
--- a/Source/WebCore/Modules/model-element/HTMLModelElement.idl
+++ b/Source/WebCore/Modules/model-element/HTMLModelElement.idl
@@ -30,6 +30,7 @@
] interface HTMLModelElement : HTMLElement {
[URL] readonly attribute USVString currentSrc;
+ readonly attribute boolean complete;
readonly attribute Promise<HTMLModelElement> ready;
undefined enterFullscreen();
diff --git a/Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm b/Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm
index 868c1e5..53418b9 100644
--- a/Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm
+++ b/Source/WebKit/UIProcess/Cocoa/ModelElementControllerCocoa.mm
@@ -164,31 +164,35 @@
// FIXME: Why is this not just using normal URL -> NSURL conversion?
auto url = adoptNS([[NSURL alloc] initFileURLWithPath:fileURL.fileSystemPath()]);
+ auto handler = CompletionHandlerWithFinalizer<void(Expected<std::pair<String, uint32_t>, WebCore::ResourceError>)>(WTFMove(completionHandler), [url] (Function<void(Expected<std::pair<String, uint32_t>, WebCore::ResourceError>)>& completionHandler) {
+ completionHandler(makeUnexpected(WebCore::ResourceError { WebCore::ResourceError::Type::General }));
+ });
+
RELEASE_ASSERT(isMainRunLoop());
- [preview setupRemoteConnectionWithCompletionHandler:makeBlockPtr([weakThis = WeakPtr { *this }, preview, uuid = WTFMove(uuid), url = WTFMove(url), completionHandler = WTFMove(completionHandler)] (NSError *contextError) mutable {
+ [preview setupRemoteConnectionWithCompletionHandler:makeBlockPtr([weakThis = WeakPtr { *this }, preview, uuid = WTFMove(uuid), url = WTFMove(url), handler = WTFMove(handler)] (NSError *contextError) mutable {
if (contextError) {
LOG(ModelElement, "Unable to create remote connection for uuid %s: %@.", uuid.utf8().data(), contextError.localizedDescription);
- callOnMainRunLoop([weakThis = WTFMove(weakThis), completionHandler = WTFMove(completionHandler), error = WebCore::ResourceError { contextError }] () mutable {
+ callOnMainRunLoop([weakThis = WTFMove(weakThis), handler = WTFMove(handler), error = WebCore::ResourceError { contextError }] () mutable {
if (!weakThis)
return;
- completionHandler(makeUnexpected(error));
+ handler(makeUnexpected(error));
});
return;
}
LOG(ModelElement, "Established remote connection with UUID %s.", uuid.utf8().data());
- [preview preparePreviewOfFileAtURL:url.get() completionHandler:makeBlockPtr([weakThis = WTFMove(weakThis), preview, uuid = WTFMove(uuid), url = WTFMove(url), completionHandler = WTFMove(completionHandler)] (NSError *loadError) mutable {
+ [preview preparePreviewOfFileAtURL:url.get() completionHandler:makeBlockPtr([weakThis = WTFMove(weakThis), preview, uuid = WTFMove(uuid), url = WTFMove(url), handler = WTFMove(handler)] (NSError *loadError) mutable {
if (loadError) {
LOG(ModelElement, "Unable to load file for uuid %s: %@.", uuid.utf8().data(), loadError.localizedDescription);
- callOnMainRunLoop([weakThis = WTFMove(weakThis), completionHandler = WTFMove(completionHandler), error = WebCore::ResourceError { loadError }] () mutable {
+ callOnMainRunLoop([weakThis = WTFMove(weakThis), handler = WTFMove(handler), error = WebCore::ResourceError { loadError }] () mutable {
if (!weakThis)
return;
- completionHandler(makeUnexpected(error));
+ handler(makeUnexpected(error));
});
return;
}
@@ -196,11 +200,11 @@
LOG(ModelElement, "Loaded file with UUID %s.", uuid.utf8().data());
auto contextId = [preview contextId];
- callOnMainRunLoop([weakThis = WTFMove(weakThis), uuid = WTFMove(uuid), completionHandler = WTFMove(completionHandler), contextId] () mutable {
+ callOnMainRunLoop([weakThis = WTFMove(weakThis), uuid = WTFMove(uuid), handler = WTFMove(handler), contextId] () mutable {
if (!weakThis)
return;
- completionHandler(std::make_pair(uuid, contextId));
+ handler(std::make_pair(uuid, contextId));
});
}).get()];
}).get()];
diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index fee9bba..46ba280 100644
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,17 @@
+2022-01-23 Antoine Quint <graouts@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):
+
2022-01-22 Carlos Garcia Campos <cgarcia@igalia.com>
[GTK][a11y] Stop registering the tree when clients are connected with ATSPI
diff --git a/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm b/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm
index 1898c3d..3985116 100644
--- a/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm
+++ b/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm
@@ -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://cube.usdz'></model><script>document.getElementsByTagName('model')[0].ready.then(() => { window.webkit.messageHandlers.modelLoading.postMessage('READY') });</script>"];
+ [webView synchronouslyLoadHTMLString:@"<model><source src='model://cube.usdz'></model><script>document.querySelector('model').addEventListener('load', event => window.webkit.messageHandlers.modelLoading.postMessage('READY'));</script>"];
while (![messageHandler didLoadModel])
Util::spinRunLoop();
