1822662 - window.innerWidth/innerHeight is no longer evaluated eagerly

Status: RESOLVED DUPLICATE of bug 1815381

(DevTools :: Console, defect, P3)

Description

[Masatoshi Kimura [:emk]]

Steps to reproduce:

1. Open Web Console.
2. Type [innerWidth,innerHeight]

Actual result:
innerWidth and innerHeight are not evaluated eagerly.

Expected result:
innerWidth and innerHeight should be evaluated eagerly. Chrome DevTools works as expected and Firefox 110 worked.

Regression range: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=b6a3b24b90c420472fe71460e387a8dfd31e2d3d&tochange=5c2b32e98bede1e87f5243d8a929de67179d9b83

Comment 1

[Masatoshi Kimura [:emk]]

Bug 1806598 seems to treat getters on Window interface as safe by default. So I believe this is unexpected. Or do innerWidth and innerHeight getters have an observable side effect?

Comment 2

[BugBot [:suhaib / :marco]]

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

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

For more information, please visit auto_nag documentation.

Comment 3

[Tooru Fujisawa [:arai]]

yes, some properties will become not-eagerly-evaluateable after the change.
this is due to many webidl lacks the annotation about side-effect.
we need to add more annotation

Comment 4

[Tooru Fujisawa [:arai]]

actually, getting innerWidth and innerHeight can flush the pending style calculation, and the operation seems to be visible to JS via transition start time.
so, they're not actually side-effect-free.

If that kind of side-effect is acceptable in the context of eager-evaluation, we would need to add yet-another WebIDL annotation specific to eager-evaluation, and use it everywhere that has no critical side-effect.

Comment 5

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

:arai, any idea what the severity might be here?

Comment 6

[Masatoshi Kimura [:emk]]

(In reply to Tooru Fujisawa [:arai] from comment #4)

If that kind of side-effect is acceptable in the context of eager-evaluation, we would need to add yet-another WebIDL annotation specific to eager-evaluation, and use it everywhere that has no critical side-effect.

How about enabling the new annotation for all DOM getters by default? I don't think allowlisting scales here.

Comment 7

[Tooru Fujisawa [:arai]]

:nchevobbe, can I have your input here, for severity and how to annotate eagerly-evaluateable getters?

Comment 8

[Tooru Fujisawa [:arai]]

if we mark all DOM getters "safe for eager evaluation", we'll need a map that contains all DOM getters,
currently there are ~3000 native DOM getters in tree:
https://searchfox.org/mozilla-central/search?q=getterinfo%20%3D%20%7B&path=

then, creating a JS Map with those entries each time doesn't sound efficient, and using the ChromeUtils method way explained in bug 1815381 comment might be better.

(In reply to Tooru Fujisawa [:arai] from bug 1815381 comment #0)

Possible solution I can think of is to create ChromeUtils method that takes a function and checks if the function is side-effect-free.
That means, move the allowlist creation and check from devtools JS code to native ChromeUtils method.

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

(Priority/Severity is very low, eager evaluation is a comfort feature, the user can still evaluate the expression, even if it might feel slighly longer)

(In reply to Tooru Fujisawa [:arai] from comment #8)

if we mark all DOM getters "safe for eager evaluation", we'll need a map that contains all DOM getters,
currently there are ~3000 native DOM getters in tree:
https://searchfox.org/mozilla-central/search?q=getterinfo%20%3D%20%7B&path=

then, creating a JS Map with those entries each time doesn't sound efficient, and using the ChromeUtils method way explained in bug 1815381 comment might be better.

I like the idea of having a method in ChromeUtils instead of manually crafting the getter Map in devtools code

Comment 10

[Tooru Fujisawa [:arai]]

Fixed bug 1815381 patch.