1627275 - ScriptLoader::OnStreamComplete never returns a failure

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Honza Bambas (:mayhemer)]

Please see this searchfox. The only caller to ScriptLoader::OnStreamComplete is ScriptLoadHandler::OnStreamComplete which in case of a failure should clear the cache and propagate the error to its caller.

But apparently ScriptLoader::OnStreamComplete can never return a failure.

This (with patch for bug 1618292 applied) leads to sending load event notification to a <link preload> tag regardless it's expected to get error notification when the resource doesn't exist on the server (we get 404).

Baku, you've been involved in this code in the past. Should ScriptLoader::OnStreamComplete return rv instead?

Comment 1

[Andrea Marchesini [:baku]]

Baku, you've been involved in this code in the past. Should ScriptLoader::OnStreamComplete return rv instead?

Yes, it seems totally safe to expose rv. And we already check the return value here:
https://searchfox.org/mozilla-central/rev/4ccefc3181f9d237ef4ca8bd17b4e7c101ddf7b5/dom/script/ScriptLoadHandler.cpp#405

Comment 2

[Honza Bambas (:mayhemer)]

Attachment: Bug 1627275 - Let ScriptLoader::OnStreamComplete return an error when verification or processing of the script fails, r=baku

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/17f170a040e9
Let ScriptLoader::OnStreamComplete return an error when verification or processing of the script fails, r=baku

Comment 4

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/17f170a040e9