Do not immediately execute an event if another event is ongoing
<rdar://86153015>
Created attachment 446644 [details] patch
Comment on attachment 446644 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=446644&action=review > LayoutTests/ChangeLog:3 > + Do not immediately execute an event if another event is ongoing The title doesn't match the patch. Should be something like: The load & error events of an <img> element should be dispatched asynchronously.
Comment on attachment 446644 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=446644&action=review > LayoutTests/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=234113 Please add the radar number on the next line: <rdar://86153015>
Comment on attachment 446644 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=446644&action=review > Source/WebCore/html/HTMLImageLoader.cpp:64 > + element().document().queueTaskToDispatchEvent(TaskSource::DOMManipulation, WTFMove(event)); I was wrong, despite us setting the event target, Document::queueTaskToDispatchEvent() will dispatch the event at the Document (not the image element). You'll likely need to do something like this instead: ``` element.document().eventLoop().queueTask(TaskSource::DOMManipulation, [element = Ref { element() }, errorOccurred] { element.dispatchEvent(Event::create(errorOccurred ? eventNames().errorEvent : eventNames().loadEvent, Event::CanBubble::No, Event::IsCancelable::No)); }); ```
<rdar://85926354> Radar number should be this one instead.
Created attachment 446794 [details] patch
Comment on attachment 446794 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=446794&action=review > Source/WebCore/html/HTMLImageLoader.cpp:65 > + element().document().eventLoop().queueTask(TaskSource::DOMManipulation, [element = Ref { element() }, errorOccurred] { Many tests are failing. I think the issue is that the page load event can now fire *before* an image's load/error event. We likely need to delay the firing of the page's load event somehow. Or maybe we need to make something asynchronous earlier in the call stack instead. Do you have a stack trace of this event getting called synchronously in the context of your test? It would help determinate how to best fix this.
Comment on attachment 446794 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=446794&action=review >> Source/WebCore/html/HTMLImageLoader.cpp:65 >> + element().document().eventLoop().queueTask(TaskSource::DOMManipulation, [element = Ref { element() }, errorOccurred] { > > Many tests are failing. I think the issue is that the page load event can now fire *before* an image's load/error event. We likely need to delay the firing of the page's load event somehow. Or maybe we need to make something asynchronous earlier in the call stack instead. > Do you have a stack trace of this event getting called synchronously in the context of your test? It would help determinate how to best fix this. Yes, I think it is likely one of the callers need to be fixed instead. You can see that some callers already call this function asynchronously (e.g. via `loadEventSender().dispatchEventSoon(*this);`) so adding a queueTask() here delays it one run loop iteration further.
Created attachment 447049 [details] patch
Created attachment 447051 [details] patch
Comment on attachment 447051 [details] patch r=me assuming the bots are happy.
Committed r287029 (245232@main): <https://commits.webkit.org/245232@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 447051 [details].