1805199 - Support getters on the global in more cases

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, task, P1)

Description

[Jan de Mooij [:jandem]]

We currently only support native getters that can take the Window instead of the WindowProxy as this. It's now pretty straight-forward to support passing the WindowProxy as receiver in CacheIR. This lets us support scripted getters, which should improve the demo in bug 1805045.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1805199 part 1 - Pass getter kind to EmitCallGetterResultNoGuards. r?iain!

This way we don't have to recompute the kind by calling IsCacheableGetPropCall
a second time. We still assert the values match in debug builds.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1805199 part 2 - Support more getters on the global in CacheIR. r?iain!

Before this patch, we supported only native getters that can take the global object
as this argument (DOM getters) for GetProp on WindowProxy and GetGName.

This patch extends this to also support getters (native and scripted) that need
the WindowProxy if we have one.

Depends on D164540

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1805199 part 3 - Support more setters on the global in CacheIR. r?iain!

Similar to the previous patch, but for setters.

The WindowProxy optimization for SetProp currently only supports data properties,
so there's only the set-name case we have to change.

Depends on D164541

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4718eaf6f421
part 1 - Pass getter kind to EmitCallGetterResultNoGuards. r=iain
https://hg.mozilla.org/integration/autoland/rev/91bd3ba3870f
part 2 - Support more getters on the global in CacheIR. r=iain
https://hg.mozilla.org/integration/autoland/rev/01e995a2bb73
part 3 - Support more setters on the global in CacheIR. r=iain

Comment 5

[Cristian Tuns]

Backed out for causing multiple failures on GlobalObject.h

• Backout link
• Push with failures
• Failure Log
• Failure line: Assertion failure: !data().windowProxy, at /builds/worker/checkouts/gecko/js/src/vm/GlobalObject.h:1003

Comment 6

[Cristian Tuns]

Also xpcshell failures: https://treeherder.mozilla.org/logviewer?job_id=399613944&repo=autoland

Comment 7

[Jan de Mooij [:jandem]]

(In reply to Cristian Tuns from comment #5)

Backed out for causing multiple failures on GlobalObject.h

The problem here was that the browser can sometimes call SetWindowProxy when the global already has a WindowProxy, failing the assertion I added. However in this case it's exactly the same window proxy JSObject* so we can just ignore it.

(In reply to Cristian Tuns from comment #6)

Also xpcshell failures: https://treeherder.mozilla.org/logviewer?job_id=399613944&repo=autoland

Hm this one doesn't seem obviously related to this bug, but I verified it's green on Try.

Comment 8

[Cristian Tuns]

Maybe this will help: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=RPpErzBSRWynWEr5vsXIxw.0&searchStr=android%2C7.0%2Cx86-64%2Cwebrender%2Cdebug%2Cxpcshell%2Ctests%2Cwithout%2Cfission%2Cenabled%2Ctest-android-em-7.0-x86_64-qr%2Fdebug-geckoview-xpcshell-nofis%2Cx1&fromchange=e702795cdb9481d33a6e437441d889d7b049be69&tochange=066e149dd107635c449c2002fa1a10db4e9f07f0

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c2b32c470a1e
part 1 - Pass getter kind to EmitCallGetterResultNoGuards. r=iain
https://hg.mozilla.org/integration/autoland/rev/10ffcc14a7da
part 2 - Support more getters on the global in CacheIR. r=iain
https://hg.mozilla.org/integration/autoland/rev/f0aaba028dcd
part 3 - Support more setters on the global in CacheIR. r=iain

Comment 10

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/c2b32c470a1e
https://hg.mozilla.org/mozilla-central/rev/10ffcc14a7da
https://hg.mozilla.org/mozilla-central/rev/f0aaba028dcd