1609426 - Some DOM methods don't work with eager evaluation

Status: VERIFIED FIXED

(DevTools :: Console, enhancement, P2)

Description

[:Harald Kirschner :digitarald]

What were you doing?

1. Type document.getElementsByName('div') in Console

What happened?

No result preview.

What should have happened?

Eager eval works.

Anything else we should know?

Exploring the DOM is one of the most common examples used when showing off.

Comment 1

[:Harald Kirschner :digitarald]

Logan, can you help me understand if we need help from the DOM team to get those enabled or is there a whitelist that we can extend?

Comment 2

[Logan Smyth [:loganfsmyth]]

I don't think we need help from the DOM team, but we'll have to come up with a list of API functions that we support. So far we have a whitelist of all of the standard ECMAScript language functions that I assume Brian could come up with: https://searchfox.org/mozilla-central/rev/2e355fa82aaa87e8424a9927c8136be184eeb6c7/devtools/server/actors/webconsole/eval-with-debugger.js#332-458 So we'll need to come up with what new entries we want to add to that list for functions from other specifications.

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Emilio was saying we could look into extracting the [Pure] keywords from .webidl files.
Not sure how we could that, but we should check what's our options there.

Comment 4

[Logan Smyth [:loganfsmyth]]

Ah I wasn't aware of that annotation. I could certainly see that being useful as a way to build up the list for us.

Comment 5

[:Harald Kirschner :digitarald]

If we decide to go the Pure route, we should spin out a separate bug for non-Pure but side-effect free methods like document.getSelection from bug 1612560.

Comment 6

[Logan Smyth [:loganfsmyth]]

Attachment: Bug 1609426 - Part 1: Allow CCW natives for isSameNative. r=arai!

Comment 7

[Logan Smyth [:loganfsmyth]]

Attachment: Bug 1609426 - Part 2: Expose more possible Sandbox bindings. r=bzbarsky!

Depends on D61963

Comment 8

[Logan Smyth [:loganfsmyth]]

Attachment: Bug 1609426 - Part 3: Automatically generate the list of pure functions. r=jlast!

Depends on D61964

Comment 9

[Logan Smyth [:loganfsmyth]]

Attachment: Bug 1609426 - Part 4: Allow native DOM bindings during eager eval. r=jlast!

Depends on D61965

Comment 10

[Boris Zbarsky [:bzbarsky]]

If we decide to go the Pure route, we should spin out a separate bug for non-Pure but side-effect free methods

If you find such methods, they should just get marked [Pure].

Comment 11

[:Harald Kirschner :digitarald]

If you find such methods, they should just get marked [Pure].

Some are mostly-pure-with-caveats, like String.match (see bug 1609429); but Logan also found DOM methods that should be pure but are not marked as such – we will file those.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/60b53a2f8f19
Part 1: Allow CCW natives for isSameNative. r=arai
https://hg.mozilla.org/integration/autoland/rev/05583692c421
Part 2: Expose more possible Sandbox bindings. r=bzbarsky

Comment 13

[Logan Smyth [:loganfsmyth]]

@Harald Are you fine with the pure-ewith-caveats approach? bz raised an interesting case in https://phabricator.services.mozilla.com/D61966#inline-376904 around triggering layout, and I'm not sure how far we want to go with allowing side-effects for things like that.

Comment 14

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/60b53a2f8f19
https://hg.mozilla.org/mozilla-central/rev/05583692c421

Comment 15

[:Harald Kirschner :digitarald]

@Harald Are you fine with the pure-ewith-caveats approach? bz raised an interesting case in https://phabricator.services.mozilla.com/D61966#inline-376904 around triggering layout, and I'm not sure how far we want to go with allowing side-effects for things like that.

On the benefits side, size measurements are useful for developers exploring a page. If I understand the side-effect correctly: triggering layout is mostly a problem in a code flow that flushes pending CSS changes. Side-effect free should not update styling, so forcing layout does not seem an obvious issue.

I am OK with having those in our side-effect-free-ish list (including String::match) and revisit if we get bug reports.

Comment 16

[Logan Smyth [:loganfsmyth]]

Woops, only some of these patches landed so this is not done yet.

Comment 17

[Logan Smyth [:loganfsmyth]]

Attachment: Bug 1609426 - WIP: Testing build-time WebIDL processing.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/48bead797c47
Part 3: Automatically generate the list of pure functions. r=jlast
https://hg.mozilla.org/integration/autoland/rev/1d24a1f7cf64
Part 4: Allow native DOM bindings during eager eval. r=jlast,bzbarsky

Comment 19

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/48bead797c47
https://hg.mozilla.org/mozilla-central/rev/1d24a1f7cf64

Comment 20

[Cristian Fogel, QA [:cfogel]]

Verified with 75.0b6 on Windows 10, macOS 10.15, Ubuntu 19.04.