Eager evaluation shouldn't evaluate ReadableStream.values()
Categories
(DevTools :: Console, defect, P2)
Tracking
(firefox125 verified, firefox126 verified)
People
(Reporter: mattias, Assigned: arai)
References
Details
Attachments
(2 files)
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:122.0) Gecko/20100101 Firefox/122.0
Steps to reproduce:
- Open the Console tab of Firefox DevTools.
- Type
readable = new ReadableStream()
, then press Enter. - Type
readable.values()
without pressing Enter.
Actual results:
DevTools shows the following autocomplete hint: "ReadableStream AsyncIterator { }"
When you then press Enter, it throws an error:
Uncaught TypeError: Asynchronous iterator initialization steps for ReadableStream failed: Cannot get a new reader for a readable stream already locked by another reader.
I assume this happens because the autocomplete hint already created a ReadableStream AsyncIterator, which has the side effect of locking the stream.
Expected results:
The debugger should recognize that calling readable.values()
has side effects, and should not show an autocomplete hint.
After you press Enter, it should construct a valid ReadableStream AsyncIterator.
Comment 1•10 months ago
|
||
The Bugbug bot thinks this bug should belong to the 'DevTools::Debugger' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.
Comment 2•10 months ago
|
||
Thanks for the report Mattias, I can reproduce easily
Updated•10 months ago
|
Comment 3•10 months ago
|
||
Nicolas will have a look at this one.
Comment 4•10 months ago
|
||
I'm not sure what's happening, but the eager function call gets "validated" because we return true
from https://searchfox.org/mozilla-central/rev/9cd4ea81e27db6b767f1d9bbbcf47da238dd64fa/devtools/server/actors/webconsole/eval-with-debugger.js#584,609-611
function nativeIsEagerlyEvaluateable(fn) {
...
const natives = gSideEffectFreeNatives.get(fn.name);
return natives && natives.some(n => fn.isSameNative(n));
}
Here, fn.name
is values
, so I tried to remove values
from https://searchfox.org/mozilla-central/rev/9cd4ea81e27db6b767f1d9bbbcf47da238dd64fa/devtools/server/actors/webconsole/webidl-pure-allowlist.js#50-55,84-86
FormData: {
prototype: ["entries", "keys", "values"],
},
Headers: {
prototype: ["entries", "keys", "values"],
},
...
URLSearchParams: {
prototype: ["entries", "keys", "values"],
},
Without the values
entries in those, the problem is gone.
It looks like this is an issue from Debugger.Object.isSameNative
, or with the values
function itself that is "shared" between those different classes?
arai, I could use some help here, do you understand what's going on?
Assignee | ||
Comment 5•10 months ago
|
||
We don't differentiate multiple methods with same native, and apparently most of DOM methods are using mozilla::dom::binding_detail::GenericMethod template function, which means many unrelated JS functions share single native function, which means the function name isn't sufficient as condition, especially for methods with generic name such as "values".
I think we need to apply the same rule as getters, that also compares JitInfo as well.
Assignee | ||
Comment 6•10 months ago
|
||
err, I misremembered about the situation for getters. we just allow all getters.
anyway, I'll look into adding JitInfo as condition.
Assignee | ||
Comment 7•10 months ago
|
||
Actually, we had the method once added for this, for getters: https://phabricator.services.mozilla.com/D168336 and then removed https://phabricator.services.mozilla.com/D175091 when optimizing getters.
and this is a kind of regression from the latter change, given it had also been handling methods with GenericMethod
.
we should've kept using isSameNativeWithJitInfo
to handle methods in allowlist.
Assignee | ||
Comment 8•10 months ago
|
||
This just reverts bug 1831826 change.
Assignee | ||
Comment 9•10 months ago
|
||
This reverts a part of bug Bug 1815381 change.
Comment 10•9 months ago
|
||
Comment 11•9 months ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b02314c7599e
https://hg.mozilla.org/mozilla-central/rev/de9ecee6742f
Updated•9 months ago
|
Comment 12•8 months ago
|
||
Reproducible on a 2024-02-21 Nightly build on macOS 12.
Verified as fixed on Firefox Nightly 126.0a1 and Firefox 125.0 on macOS 12, Windows 10, Ubuntu 22.
Description
•