⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 286857 in webkit


Ignore:
Timestamp:
Dec 10, 2021, 9:28:10 AM (5 years ago)
Author:
commit-queue@webkit.org
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

Location:
trunk
Files:
4 added
3 deleted
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r286855 r286857  
     12021-12-10  Commit Queue  <commit-queue@webkit.org>
     2
     3        Unreviewed, reverting r286836.
     4        https://bugs.webkit.org/show_bug.cgi?id=234153
     5
     6        some tests are flaky on iOS and some are crashing on macOS
     7
     8        Reverted changeset:
     9
     10        "[Model] Add load and error events to distinguish resource
     11        load from model readiness"
     12        https://bugs.webkit.org/show_bug.cgi?id=233706
     13        https://commits.webkit.org/r286836
     14
    1152021-12-10  Chris Dumez  <cdumez@apple.com>
    216

  • trunk/LayoutTests/model-element/model-element-contents-layer-updates-with-clipping.html

    r286836 r286857  
    77<pre id="layers"></pre>
    88<script>
    9     window.testRunner?.waitUntilDone();
    10     window.testRunner?.dumpAsText();
     9    let layers = document.getElementById("layers");
     10    let source = document.getElementsByTagName("source")[0];
    1111
    12     const layers = document.getElementById("layers");
    13     const source = document.querySelector("source");
    14     const model = document.getElementById("model");
     12    if (window.testRunner) {
     13        testRunner.waitUntilDone();
     14        testRunner.dumpAsText();
     15    } else
     16        layers.textContent = "This test requires testRunner.";
    1517
    16     const modelDidLoadFirstSource = () => {
     18    let model = document.getElementById("model");
     19
     20    model.ready.then(value => {
    1721        layers.textContent = "Before Changing Source:\n";
    18         layers.textContent += window.internals?.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS) ?? "This test requires testRunner.";
    19 
    20         model.addEventListener("load", event => {
    21             layers.textContent += "After Changing Source:\n";
    22             layers.textContent += window.internals?.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS) ?? "This test requires testRunner.";
    23             window.testRunner?.notifyDone();
    24         }, { once: true });
    25 
    26         model.addEventListener("error", event => {
    27             layers.textContent = `Failed. Second model did not load.`;
    28             window.testRunner?.notifyDone();
    29         }, { once: true });
     22        layers.textContent += window.internals.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS);
    3023
    3124        source.src = "resources/cube.usdz";
    32     }
    33 
    34     if (model.complete)
    35         modelDidLoadFirstSource();
    36     else {
    37         model.addEventListener("load", modelDidLoadFirstSource, { once: true });
    38         model.addEventListener("error", event => {
    39             layers.textContent = `Failed. First model did not load.`;
    40             window.testRunner?.notifyDone();
    41         }, { once: true });
    42     }
     25        model.ready.then(value => {
     26            if (window.testRunner) {
     27                layers.textContent += "After Changing Source:\n";
     28                layers.textContent += window.internals.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS);
     29            }
     30        }, reason => {
     31            layers.textContent = `Failed. Second model did not load: ${reason}`;
     32        }).finally(() => {
     33            if (window.testRunner)
     34                testRunner.notifyDone();
     35        });
     36       
     37    }, reason => {
     38        layers.textContent = `Failed. First model did not load: ${reason}`;
     39        if (window.testRunner)
     40            testRunner.notifyDone();
     41    });
    4342</script>
    4443</body>

  • trunk/LayoutTests/model-element/model-element-contents-layer-updates.html

    r286836 r286857  
    77<pre id="layers"></pre>
    88<script>
    9     window.testRunner?.waitUntilDone();
    10     window.testRunner?.dumpAsText();
     9    let layers = document.getElementById("layers");
     10    let source = document.getElementsByTagName("source")[0];
    1111
    12     const layers = document.getElementById("layers");
    13     const source = document.querySelector("source");
    14     const model = document.getElementById("model");
     12    if (window.testRunner) {
     13        testRunner.waitUntilDone();
     14        testRunner.dumpAsText();
     15    } else
     16        layers.textContent = "This test requires testRunner.";
    1517
    16     const modelDidLoadFirstSource = () => {
     18    let model = document.getElementById("model");
     19
     20    model.ready.then(value => {
    1721        layers.textContent = "Before Changing Source:\n";
    18         layers.textContent += window.internals?.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS) ?? "This test requires testRunner.";
    19 
    20         model.addEventListener("load", event => {
    21             layers.textContent += "After Changing Source:\n";
    22             layers.textContent += window.internals?.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS) ?? "This test requires testRunner.";
    23             window.testRunner?.notifyDone();
    24         }, { once: true });
    25 
    26         model.addEventListener("error", event => {
    27             layers.textContent = `Failed. Second model did not load.`;
    28             window.testRunner?.notifyDone();
    29         }, { once: true });
     22        layers.textContent += window.internals.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS);
    3023
    3124        source.src = "resources/cube.usdz";
    32     }
    33 
    34     if (model.complete)
    35         modelDidLoadFirstSource();
    36     else {
    37         model.addEventListener("load", modelDidLoadFirstSource, { once: true });
    38         model.addEventListener("error", event => {
    39             layers.textContent = `Failed. First model did not load.`;
    40             window.testRunner?.notifyDone();
    41         }, { once: true });
    42     }
     25        model.ready.then(value => {
     26            if (window.testRunner) {
     27                layers.textContent += "After Changing Source:\n";
     28                layers.textContent += window.internals.platformLayerTreeAsText(model, window.internals.PLATFORM_LAYER_TREE_INCLUDE_MODELS);
     29            }
     30        }, reason => {
     31            layers.textContent = `Failed. Second model did not load: ${reason}`;
     32        }).finally(() => {
     33            if (window.testRunner)
     34                testRunner.notifyDone();
     35        });
     36       
     37    }, reason => {
     38        layers.textContent = `Failed. First model did not load: ${reason}`;
     39        if (window.testRunner)
     40            testRunner.notifyDone();
     41    });
    4342</script>
    4443</body>

  • trunk/LayoutTests/model-element/model-element-graphics-layers-opacity.html

    r286836 r286857  
    1414<pre id="layers"></pre>
    1515<script>
    16     window.testRunner?.waitUntilDone();
    17     window.testRunner?.dumpAsText();
     16    let layers = document.getElementById("layers");
    1817
    19     const layers = document.getElementById("layers");
    20     const model = document.getElementById("model");
     18    if (window.testRunner) {
     19        testRunner.waitUntilDone();
     20        testRunner.dumpAsText();
     21    } else
     22        layers.textContent = "This test requires testRunner.";
    2123
    22     const modelDidLoad = () => {
    23         layers.innerText = window.internals?.platformLayerTreeAsText(model) ?? "This test requires testRunner.";
     24    let model = document.getElementById("model");
     25
     26    model.ready.then(value => {
     27        if (window.testRunner)
     28            layers.innerText = window.internals.platformLayerTreeAsText(model);
    2429        model.remove();
    25         window.testRunner?.notifyDone();
    26     };
    27 
    28     if (model.complete)
    29         modelDidLoad();
    30     else {
    31         model.addEventListener("load", modelDidLoad);
    32         model.addEventListener("error", event => {
    33             layers.textContent = `Failed. Model did not load.`;
    34             window.testRunner?.notifyDone();
    35         });
    36     }
     30    }, reason => {
     31        layers.textContent = `Failed. Model did not load: ${reason}`;
     32    }).finally(() => {
     33        if (window.testRunner)
     34            testRunner.notifyDone();
     35    });
    3736</script>
    3837</body>

  • trunk/LayoutTests/model-element/model-element-graphics-layers.html

    r286836 r286857  
    77<pre id="layers"></pre>
    88<script>
    9     window.testRunner?.waitUntilDone();
    10     window.testRunner?.dumpAsText();
     9    let layers = document.getElementById("layers");
    1110
    12     const layers = document.getElementById("layers");
    13     const model = document.getElementById("model");
     11    if (window.testRunner) {
     12        testRunner.waitUntilDone();
     13        testRunner.dumpAsText();
     14    } else
     15        layers.textContent = "This test requires testRunner.";
    1416
    15     const modelDidLoad = () => {
    16         layers.innerText = window.internals?.layerTreeAsText(document) ?? "This test requires testRunner.";
     17    let model = document.getElementById("model");
     18
     19    model.ready.then(value => {
     20        if (window.testRunner)
     21            layers.innerText = window.internals.layerTreeAsText(document);
    1722        model.remove();
    18         window.testRunner?.notifyDone();
    19     }
    20 
    21     if (model.complete)
    22         modelDidLoad();
    23     else {
    24         model.addEventListener("load", modelDidLoad);
    25         model.addEventListener("error", event => {
    26             layers.textContent = `Failed. Model did not load.`;
    27             window.testRunner?.notifyDone();
    28         });
    29     }
     23    }, reason => {
     24        layers.textContent = `Failed. Model did not load: ${reason}`;
     25    }).finally(() => {
     26        if (window.testRunner)
     27            testRunner.notifyDone();
     28    });
    3029</script>
    3130</body>

  • trunk/LayoutTests/model-element/model-element-ready-expected.txt

    r286836 r286857  
     1This test passes if you see the word "Passed" below:
    12
    2 PASS <model> rejects the ready promise when provided with an unknown resoure.
    3 PASS <model> rejects the ready promise when its resource load is aborted.
    4 PASS <model> resolves the ready promise when provided with a known resource.
    5 
     3Passed

  • trunk/LayoutTests/model-element/model-element-ready.html

    r286836 r286857  
    1 <!doctype html>
    2 <meta charset="utf-8">
    3 <title>&lt;model> ready promise</title>
    4 <script src="../resources/testharness.js"></script>
    5 <script src="../resources/testharnessreport.js"></script>
    6 <script src="resources/model-element-test-utils.js"></script>
     1<!DOCTYPE html><!-- webkit-test-runner [ ModelElementEnabled=true ] -->
     2<html>
    73<body>
     4<model id="model">
     5    <source src="resources/heart.usdz">
     6</model>
     7<p>This test passes if you see the word "Passed" below:</p>
     8<p id="result">Failed</p>
    89<script>
    9 'use strict';
     10    if (window.testRunner) {
     11        testRunner.waitUntilDone();
     12        testRunner.dumpAsText();
     13    }
    1014
    11 promise_test(async t => {
    12     const [model, source] = createModelAndSource(t, "resources/does-not-exist.usdz");
    13     return model.ready.then(
    14         value => assert_unreached("Unexpected ready promise resolution."),
    15         reason => assert_true(reason.toString().includes("NetworkError"), "The ready promise is rejected with a NetworkError.")
    16     );
    17 }, `<model> rejects the ready promise when provided with an unknown resoure.`);
     15    let result = document.getElementById("result");
     16    let model = document.getElementById("model");
    1817
    19 promise_test(async t => {
    20     const [model, source] = createModelAndSource(t, "resources/heart.usdz");
    21     const modelReady = model.ready;
    22 
    23     source.remove();
    24     assert_not_equals(model.ready, modelReady, "Removing the <source> child resets the ready promise.");
    25 
    26     return modelReady.then(
    27         value => assert_unreached("Unexpected ready promise resolution."),
    28         reason => assert_true(reason.toString().includes("AbortError"), "The ready promise is rejected with a NetworkError.")
    29     );
    30 }, `<model> rejects the ready promise when its resource load is aborted.`);
    31 
    32 promise_test(async t => {
    33     const [model, source] = createModelAndSource(t, "resources/cube.usdz");
    34     await model.ready;
    35 }, `<model> resolves the ready promise when provided with a known resource.`);
    36 
     18    model.ready.then(value => {
     19        result.textContent = `Passed`;
     20    }, reason => {
     21        result.textContent = `Failed. Model did not load: ${reason}`;
     22    }).finally(() => {
     23        if (window.testRunner)
     24            testRunner.notifyDone();
     25    });
    3726</script>
    3827</body>
     28</html>

  • trunk/LayoutTests/platform/ios-simulator/TestExpectations

    r286836 r286857  
    153153
    154154webkit.org/b/223949 crypto/crypto-random-values-oom.html [ Pass Timeout ]
    155 
    156 # This test relies on ARQL APIs which are not available in the Simulator
    157 model-element/model-element-ready.html [ Skip ]

  • trunk/LayoutTests/platform/mac/TestExpectations

    r286836 r286857  
    22832283[ Monterey ] imported/w3c/web-platform-tests/fetch/connection-pool/network-partition-key.html [ Failure ]
    22842284
    2285 # webkit.org/b/228200 Setting multiple test expectations for Monterey on OpenSource:
     2285# webkit.org/b/228200 Setting multiple test expectations for Monetery on OpenSource:
    22862286[ Monterey ] model-element/model-element-graphics-layers-opacity.html [ Pass Failure ]
    22872287[ Monterey Debug arm64 ] imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-restartIce.https.html [ Pass Failure Crash ]
    … …  
    24292429
    24302430webkit.org/b/221230 [ BigSur+ ] imported/w3c/web-platform-tests/media-source/mediasource-addsourcebuffer.html [ Pass Failure ]
    2431 
    2432 # <model> tests involving the ready promise can only work on Monterey and up
    2433 [ Catalina BigSur ] model-element/model-element-ready.html [ Skip ]

  • trunk/Source/WebCore/ChangeLog

    r286855 r286857  
     12021-12-10  Commit Queue  <commit-queue@webkit.org>
     2
     3        Unreviewed, reverting r286836.
     4        https://bugs.webkit.org/show_bug.cgi?id=234153
     5
     6        some tests are flaky on iOS and some are crashing on macOS
     7
     8        Reverted changeset:
     9
     10        "[Model] Add load and error events to distinguish resource
     11        load from model readiness"
     12        https://bugs.webkit.org/show_bug.cgi?id=233706
     13        https://commits.webkit.org/r286836
     14
    1152021-12-10  Chris Dumez  <cdumez@apple.com>
    216

  • trunk/Source/WebCore/Modules/model-element/HTMLModelElement.cpp

    r286836 r286857  
    6666HTMLModelElement::HTMLModelElement(const QualifiedName& tagName, Document& document)
    6767    : HTMLElement(tagName, document)
    68     , ActiveDOMObject(document)
    6968    , m_readyPromise { makeUniqueRef<ReadyPromise>(*this, &HTMLModelElement::readyPromiseResolve) }
    7069{
    71     setHasCustomStyleResolveCallbacks();
    7270}
    7371
    … …  
    8280Ref<HTMLModelElement> HTMLModelElement::create(const QualifiedName& tagName, Document& document)
    8381{
    84     auto model = adoptRef(*new HTMLModelElement(tagName, document));
    85     model->suspendIfNeeded();
    86     return model;
     82    return adoptRef(*new HTMLModelElement(tagName, document));
    8783}
    8884
    … …  
    136132
    137133    m_readyPromise = makeUniqueRef<ReadyPromise>(*this, &HTMLModelElement::readyPromiseResolve);
    138     m_shouldCreateModelPlayerUponRendererAttachment = false;
    139 
    140     if (m_sourceURL.isEmpty()) {
    141         queueTaskToDispatchEvent(*this, TaskSource::DOMManipulation, Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No));
    142         return;
    143     }
     134
     135    if (m_sourceURL.isEmpty())
     136        return;
    144137
    145138    ResourceLoaderOptions options = CachedResourceLoader::defaultCachedResourceOptions();
    … …  
    153146    auto resource = document().cachedResourceLoader().requestModelResource(WTFMove(request));
    154147    if (!resource.has_value()) {
    155         queueTaskToDispatchEvent(*this, TaskSource::DOMManipulation, Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No));
    156148        m_readyPromise->reject(Exception { NetworkError });
    157149        return;
    … …  
    182174{
    183175    return createRenderer<RenderModel>(*this, WTFMove(style));
    184 }
    185 
    186 void HTMLModelElement::didAttachRenderers()
    187 {
    188     if (!m_shouldCreateModelPlayerUponRendererAttachment)
    189         return;
    190 
    191     m_shouldCreateModelPlayerUponRendererAttachment = false;
    192     createModelPlayer();
    193176}
    194177
    … …  
    215198        m_data = nullptr;
    216199
    217         queueTaskToDispatchEvent(*this, TaskSource::DOMManipulation, Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No));
    218 
    219200        invalidateResourceHandleAndUpdateRenderer();
    220201
    … …  
    226207    m_model = Model::create(m_data.releaseNonNull().get(), resource.mimeType(), resource.url());
    227208
    228     queueTaskToDispatchEvent(*this, TaskSource::DOMManipulation, Event::create(eventNames().loadEvent, Event::CanBubble::No, Event::IsCancelable::No));
    229 
    230209    invalidateResourceHandleAndUpdateRenderer();
    231210
     211    m_readyPromise->resolve(*this);
     212
    232213    modelDidChange();
    233214}
    … …  
    237218void HTMLModelElement::modelDidChange()
    238219{
    239     auto* page = document().page();
    240     if (!page) {
    241         m_readyPromise->reject(Exception { AbortError });
    242         return;
    243     }
     220    // FIXME: For the early returns here, we should probably inform the page that things have
     221    // failed to render. For the case of no-renderer, we should probably also build the model
     222    // when/if a renderer is created.
     223
     224    auto page = document().page();
     225    if (!page)
     226        return;
    244227
    245228    auto* renderer = this->renderer();
    246     if (!renderer) {
    247         m_shouldCreateModelPlayerUponRendererAttachment = true;
    248         return;
    249     }
    250 
    251     createModelPlayer();
    252 }
    253 
    254 void HTMLModelElement::createModelPlayer()
    255 {
    256     ASSERT(document().page());
    257     m_modelPlayer = document().page()->modelPlayerProvider().createModelPlayer(*this);
    258     if (!m_modelPlayer) {
    259         m_readyPromise->reject(Exception { AbortError });
    260         return;
    261     }
     229    if (!renderer)
     230        return;
     231
     232    m_modelPlayer = page->modelPlayerProvider().createModelPlayer(*this);
     233    if (!m_modelPlayer)
     234        return;
    262235
    263236    // FIXME: We need to tell the player if the size changes as well, so passing this
    264237    // in with load probably doesn't make sense.
    265     ASSERT(renderer());
    266     auto size = renderer()->absoluteBoundingBoxRect(false).size();
     238    auto size = renderer->absoluteBoundingBoxRect(false).size();
    267239    m_modelPlayer->load(*m_model, size);
    268240}
    … …  
    284256    if (auto* renderer = this->renderer())
    285257        renderer->updateFromElement();
    286 
    287     m_readyPromise->resolve(*this);
    288258}
    289259
    … …  
    291261{
    292262    ASSERT_UNUSED(modelPlayer, &modelPlayer == m_modelPlayer);
    293     m_readyPromise->reject(Exception { AbortError });
    294263}
    295264
    … …  
    584553}
    585554
    586 const char* HTMLModelElement::activeDOMObjectName() const
    587 {
    588     return "HTMLModelElement";
    589 }
    590 
    591 bool HTMLModelElement::virtualHasPendingActivity() const
    592 {
    593     // We need to ensure the JS wrapper is kept alive if a load is in progress and we may yet dispatch
    594     // "load" or "error" events, ie. as long as we have a resource, meaning we are in the process of loading.
    595     return m_resource;
    596 }
    597 
    598555#if PLATFORM(COCOA)
    599556Vector<RetainPtr<id>> HTMLModelElement::accessibilityChildren()

  • trunk/Source/WebCore/Modules/model-element/HTMLModelElement.h

    r286836 r286857  
    2828#if ENABLE(MODEL_ELEMENT)
    2929
    30 #include "ActiveDOMObject.h"
    3130#include "CachedRawResource.h"
    3231#include "CachedRawResourceClient.h"
    … …  
    5150template<typename IDLType> class DOMPromiseProxyWithResolveCallback;
    5251
    53 class HTMLModelElement final : public HTMLElement, private CachedRawResourceClient, public ModelPlayerClient, public ActiveDOMObject {
     52class HTMLModelElement final : public HTMLElement, private CachedRawResourceClient, public ModelPlayerClient {
    5453    WTF_MAKE_ISO_ALLOCATED(HTMLModelElement);
    5554public:
    … …  
    5958    void sourcesChanged();
    6059    const URL& currentSrc() const { return m_sourceURL; }
    61     bool complete() const { return m_dataComplete; }
    6260
    6361    // MARK: DOM Functions and Attributes
    … …  
    109107    void setSourceURL(const URL&);
    110108    void modelDidChange();
    111     void createModelPlayer();
    112109
    113110    HTMLModelElement& readyPromiseResolve();
    114 
    115     // ActiveDOMObject
    116     const char* activeDOMObjectName() const final;
    117     bool virtualHasPendingActivity() const final;
    118111
    119112    // DOM overrides.
    … …  
    122115    // Rendering overrides.
    123116    RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) final;
    124     void didAttachRenderers() final;
    125117
    126118    // CachedRawResourceClient overrides.
    … …  
    147139    bool m_dataComplete { false };
    148140    bool m_isDragging { false };
    149     bool m_shouldCreateModelPlayerUponRendererAttachment { false };
    150141
    151142    RefPtr<ModelPlayer> m_modelPlayer;

  • trunk/Source/WebCore/Modules/model-element/HTMLModelElement.idl

    r286836 r286857  
    3131    [URL] readonly attribute USVString currentSrc;
    3232
    33     readonly attribute boolean complete;
    3433    readonly attribute Promise<HTMLModelElement> ready;
    3534

  • trunk/Tools/ChangeLog

    r286838 r286857  
     12021-12-10  Commit Queue  <commit-queue@webkit.org>
     2
     3        Unreviewed, reverting r286836.
     4        https://bugs.webkit.org/show_bug.cgi?id=234153
     5
     6        some tests are flaky on iOS and some are crashing on macOS
     7
     8        Reverted changeset:
     9
     10        "[Model] Add load and error events to distinguish resource
     11        load from model readiness"
     12        https://bugs.webkit.org/show_bug.cgi?id=233706
     13        https://commits.webkit.org/r286836
     14
    1152021-12-10  Kimmo Kinnunen  <kkinnunen@apple.com>
    216

  • trunk/Tools/TestWebKitAPI/Tests/ios/DragAndDropTestsIOS.mm

    r286836 r286857  
    21982198   
    21992199    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500) configuration:configuration.get()]);
    2200     [webView synchronouslyLoadHTMLString:@"<model><source src='model://cube.usdz'></model><script>document.querySelector('model').addEventListener('load', event => window.webkit.messageHandlers.modelLoading.postMessage('READY'));</script>"];
     2200    [webView synchronouslyLoadHTMLString:@"<model><source src='model://cube.usdz'></model><script>document.getElementsByTagName('model')[0].ready.then(() => { window.webkit.messageHandlers.modelLoading.postMessage('READY') });</script>"];
    22012201
    22022202    while (![messageHandler didLoadModel])
Note: See TracChangeset for help on using the changeset viewer.