REGRESSION (273804@main): [ macOS wk1 Debug ] http/tests/media/video-throttled-load-metadata.html is a constant crash

[jyavenard]

e70021f

REGRESSION (273804@main): [ macOS wk1 Debug ] http/tests/media/video-throttled-load-metadata.html is a constant crash
https://bugs.webkit.org/show_bug.cgi?id=268480
rdar://122025990

Reviewed by Youenn Fablet.

273804@main incorrectly removed the delayed task removal to perform the operation immediately.
However, this method is called while iterating the HashTable from which we are removing the task causing
the iterator to become invalid.
Re-introduce the delayed operation, add comment in code.

Covered by existing tests.

* LayoutTests/platform/mac-wk1/TestExpectations:
* Source/WebCore/platform/network/cocoa/RangeResponseGenerator.mm:
(WebCore::RangeResponseGenerator::giveResponseToTaskIfBytesInRangeReceived):

Canonical link: https://commits.webkit.org/273892@main

cad1c56

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim
✅ 🛠 🧪 merge	✅ 🛠 watch
	✅ 🛠 watch-sim

[webkit-early-warning-system]

EWS run on current version of this PR (hash cad1c56)

Details

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim
✅ 🛠 🧪 merge	✅ 🛠 watch
	✅ 🛠 watch-sim

[youennf]

LGTM.
Quick question with the model though: why do we need to remove it once consumed?
Could we remove it from the map before calling giveResponseToTasksWithFinishedRanges? And pass ownership to giveResponseToTasksWithFinishedRanges?

[jyavenard]

LGTM. Quick question with the model though: why do we need to remove it once consumed? Could we remove it from the map before calling giveResponseToTasksWithFinishedRanges? And pass ownership to giveResponseToTasksWithFinishedRanges?

TBH, I don't know. The extent of my involvement with the RangeResponseGenerator has been to make it thread-safe. I haven't touched to the basic logic.

As it is, the task is removed from the map only once all the requested data has been delivered. And sometimes, the data is delivered asynchronously to the PlatformMediaResource.

So we can't always determine when it's okay to remove it.

[webkit-commit-queue]

Committed 273892@main (e70021f): https://commits.webkit.org/273892@main

Reviewed commits have been landed. Closing PR #23644 and removing active labels.