1847813 - Infinite scrolling broken on https://reddit.com/r/toronto on recent nightly builds

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Thomas Wisniewski [:twisniewski] [on PTO until Sep.9]]

As reported on webcompat.com, the scrolling on https://reddit.com/r/toronto is broken on nightly builds. When one scrolls down, the normally-gray "loading article" area never updates (though I sometimes see a brief flicker, the loading indicator remains).

Mozregression suggests that bug 1842798 caused the regression.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1842798

:jonco, since you are the author of the regressor, bug 1842798, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[Jon Coppeard (:jonco)]

The problem here is similar to that addressed by https://phabricator.services.mozilla.com/D183876 in the regressing bug 1842798 but for loading static imports rather than dynamic imports.

According to the spec both ContinueDynamicImport and ContinueModuleLoading work in terms of a promise that is resolved or rejected when module dependencies have been fetched and compiled but not linked or executed. These are called from FinishLoadingImportedModule.

ContinueModuleLoading is defined here: https://tc39.es/ecma262/#sec-ContinueModuleLoading

For static imports the HTML integration waits for this promise to be resolved in step 6 of "fetch the descendants of and link a module script" before linking the module: https://html.spec.whatwg.org/#fetch-the-descendants-of-and-link-a-module-script

This is that part that's not happening.

Comment 3

[Jon Coppeard (:jonco)]

For context, here's the stack when the error is thrown:

#01: InnerModuleLinking(JSContext*, JS::Handle<js::ModuleObject*>, JS::MutableHandle<JS::GCVector<js::ModuleObject*, 0ul, js::SystemAllocPolicy>>, unsigned long, unsigned long*)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0x65b2b58]
#02: InnerModuleLinking(JSContext*, JS::Handle<js::ModuleObject*>, JS::MutableHandle<JS::GCVector<js::ModuleObject*, 0ul, js::SystemAllocPolicy>>, unsigned long, unsigned long*)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0x65b2650]
#03: js::ModuleLink(JSContext*, JS::Handle<js::ModuleObject*>)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0x65ab970]
#04: JS::ModuleLink(JSContext*, JS::Handle<JSObject*>)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0x65ab7f0]
#05: JS::loader::ModuleLoaderBase::InstantiateModuleGraph(JS::loader::ModuleLoadRequest*)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0xc2f5f0]
#06: mozilla::dom::ScriptLoader::ProcessRequest(JS::loader::ScriptLoadRequest*)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0x4673418]
#07: mozilla::dom::ScriptLoader::ProcessPendingRequests()[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0x46647a4]
#08: mozilla::dom::ModuleLoader::OnModuleLoadComplete(JS::loader::ModuleLoadRequest*)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0x4663e24]
#09: JS::loader::ModuleLoadRequest::LoadFinished()[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0xc26b90]
#10: JS::loader::ModuleLoadRequest::DependenciesLoaded()[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0xc26f04]
#11: JS::loader::ModuleLoadRequest::ChildLoadComplete(bool)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0xc25b78]
#12: JS::loader::ModuleLoadRequest::DependenciesLoaded()[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0xc26efc]
#13: JS::loader::ModuleLoaderBase::StartFetchingModuleDependencies(JS::loader::ModuleLoadRequest*)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0xc266f0]
#14: JS::loader::ModuleLoadRequest::ModuleLoaded()[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0xc26008]
#15: JS::loader::ModuleLoaderBase::WaitForModuleFetch(JS::loader::ModuleLoadRequest*)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0xc2b3fc]
#16: JS::loader::ModuleLoaderBase::StartOrRestartModuleLoad(JS::loader::ModuleLoadRequest*, JS::loader::ModuleLoaderBase::RestartRequest)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0xc2b0c8]
#17: JS::loader::ModuleLoaderBase::StartFetchingModuleAndDependencies(JS::loader::ModuleLoadRequest*, nsIURI*)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0xc2d8fc]
#18: JS::loader::ModuleLoaderBase::StartFetchingModuleDependencies(JS::loader::ModuleLoadRequest*)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0xc266d8]
#19: JS::loader::ModuleLoadRequest::ModuleLoaded()[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0xc26008]
#20: JS::loader::ModuleLoaderBase::WaitForModuleFetch(JS::loader::ModuleLoadRequest*)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0xc2b3fc]
#21: JS::loader::ModuleLoaderBase::StartOrRestartModuleLoad(JS::loader::ModuleLoadRequest*, JS::loader::ModuleLoaderBase::RestartRequest)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0xc2b0c8]
#22: JS::loader::ModuleLoaderBase::StartDynamicImport(JS::loader::ModuleLoadRequest*)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0xc2aeb8]
#23: JS::loader::ModuleLoaderBase::HostImportModuleDynamically(JSContext*, JS::Handle<JS::Value>, JS::Handle<JSObject*>, JS::Handle<JSObject*>)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0xc2926c]
#24: js::StartDynamicModuleImport(JSContext*, JS::Handle<JSScript*>, JS::Handle<JS::Value>, JS::Handle<JS::Value>)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0x63be604]
#25: js::Interpret(JSContext*, js::RunState&)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0x630fcf0]
#26: js::RunScript(JSContext*, js::RunState&)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0x62f70bc]
#27: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0x62f7db4]
#28: js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0x62f9424]
#29: js::jit::InvokeFunction(JSContext*, JS::Handle<JSObject*>, bool, bool, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0x6e5f5b0]
#30: js::jit::InvokeFromInterpreterStub(JSContext*, js::jit::InterpreterStubExitFrameLayout*)[/Users/jon/work/modules/optdebug-build/toolkit/library/build/XUL +0x6e5fbe8]

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1847813 - Process loaded modules asynchronously in DOM module loader r?smaug

Based on the explanation on the bug, the solution would seem to be to call the
ModuleLoaderBase::OnModuleLoadComplete hook from a microtask. This causes a ton
of test failures.

Instead this patch changes the DOM script loader to implement this hook with a
runnable. This fixes the problem (verified manually) and all tests pass.

(Replacing the runnable with a microtask just in the DOM script loader also
does not work and results in many test failures. I'm still trying to understand
why.)

The changes to ContinueDynamicImport are removed because they are also on this
path and we don't need a microtask as well.

This takes us closer to the state before bug 1842798 landed when all the module
loader used MozPromise internally and completions like this happened via
runnables.

I was not able to immediately come up with a test for this.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9d4c13dd5f58
Process loaded modules asynchronously in DOM module loader r=smaug

Comment 6

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/9d4c13dd5f58

Comment 12

[Monica Chiorean]

I was able to reproduce the issue on Mac 12.6 using FF build 118.0a1(20230808212319).
Verified as fixed on Mac 12.6/Win10x64 using FF builds 118.0b2 and 119.a1.