Closed Bug 1593698 Opened 5 months ago Closed 3 months ago

Add a preference to enable weak references in the browser

Categories

(Core :: JavaScript Engine, task, P1)

task

Tracking

()

RESOLVED FIXED
mozilla74
Tracking Status
firefox74 --- fixed

People

(Reporter: jonco, Assigned: jonco)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Following on from bug 1587098, we will add a pref to enable weak references and finalization groups in the browser when we are ready. This will be initially disabled. This might also be restricted to nightly only at first.

The pref will be: javascript.options.experimental.weakrefs

Priority: -- → P1

Andrew, you reviewed these changes as part of a different bug but you had a question about workers. The patch sets the appropriate option in xpc::SetPrefableRealmOptions which is called by DedicatedWorkerGlobalScope::WrapGlobalObject but not by any of the other users of JS::RealmOptions in that file (e.g. the methods for wrapping shared worker global, service worker global etc). Is this sufficient? If not should the other method also be calling xpc::SetPrefableRealmOptions already?

Flags: needinfo?(continuation)

I don't know what the right way to set worker prefs is. Maybe Andrew can answer your question.

Flags: needinfo?(continuation) → needinfo?(bugmail)

So I think the only comprehensively applied options we have across all worker types are those in RuntimeService::Init() where sDefaultJSSettings gets initialized: https://searchfox.org/mozilla-central/rev/e7c61f4a68b974d5fecd216dc7407b631a24eb8f/dom/workers/RuntimeService.cpp#1523. Those get propagated in the WorkerPrivate constructor at https://searchfox.org/mozilla-central/rev/131338e5017bc0283d86fb73844407b9a2155c98/dom/workers/WorkerPrivate.cpp#2184.

The other WrapJSObject calls also call CopyJSRealmOptions, which is something: https://searchfox.org/mozilla-central/rev/e7c61f4a68b974d5fecd216dc7407b631a24eb8f/dom/workers/WorkerPrivate.h#538

It seems reasonable to me to add a call to xpc::SetPrefableRealmOptions. I'd also be fine with some other cleanup to unify things more.

One thing that I'm not 100% sure about is the .setSharedMemoryAndAtomicsEnabled(sSharedMemoryEnabled) line in SetPrefableRealmOptions at https://searchfox.org/mozilla-central/rev/131338e5017bc0283d86fb73844407b9a2155c98/js/xpconnect/src/XPCJSContext.cpp#773. ServiceWorkerGlobalScope is spec'ed to have a [[CanBlock]] value of false at https://html.spec.whatwg.org/#integration-with-the-javascript-agent-formalism so if that impacts that, then that should not be turned on. I'm not sure we have a clear canonical place to map such things now in the same way WebIDL helps us avoid exposing things in certain globals... :tt has been working in this area, adding him on CC.

Flags: needinfo?(bugmail)

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:jonco, could you have a look please?
For more information, please visit auto_nag documentation.

Flags: needinfo?(jcoppeard)

I'm waiting for browser support for finalization groups to be ready to land before landing this.

Flags: needinfo?(jcoppeard)
Assignee: nobody → jcoppeard
Attachment #9108710 - Attachment description: Bug 1593698 - Add a pref to enable support for weak references, off by default r=mccr8 → Bug 1593698 - Add a pref to enable support for weak references in nightly builds, off by default r?mccr8
Status: NEW → ASSIGNED
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2079d95da4a2
Add a pref to enable support for weak references in nightly builds, off by default r=mccr8

Backed out 4 changesets (Bug 1596756, Bug 1593698, Bug 1608069) for causing mochitest leakchecks.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=37d24b36ce318b86179bae59b11a487da555e7be

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=284440348&repo=autoland&lineNumber=3619

Backout link: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&selectedJob=284440348&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=166a6d535af3b4122cb9f8648d4a76e8e87b9608

[task 2020-01-10T19:06:19.706Z] 19:06:19     INFO - TEST-PASS | leakcheck | tab no leaks detected!
[task 2020-01-10T19:06:19.706Z] 19:06:19     INFO - leakcheck | Processing leak log file /tmp/tmpqrzFWB.mozrunner/runtests_leaks_tab_pid1656.log
[task 2020-01-10T19:06:19.707Z] 19:06:19     INFO - 
[task 2020-01-10T19:06:19.707Z] 19:06:19     INFO - == BloatView: ALL (cumulative) LEAK AND BLOAT STATISTICS, tab process 1656
[task 2020-01-10T19:06:19.708Z] 19:06:19     INFO - 
[task 2020-01-10T19:06:19.708Z] 19:06:19     INFO -      |<----------------Class--------------->|<-----Bytes------>|<----Objects---->|
[task 2020-01-10T19:06:19.709Z] 19:06:19     INFO -      |                                      | Per-Inst   Leaked|   Total      Rem|
[task 2020-01-10T19:06:19.709Z] 19:06:19     INFO -    0 |TOTAL                                 |       34       16| 4438547        1|
[task 2020-01-10T19:06:19.710Z] 19:06:19     INFO -  305 |JS Object                             |       16       16|       1        1|
[task 2020-01-10T19:06:19.712Z] 19:06:19     INFO - 
[task 2020-01-10T19:06:19.713Z] 19:06:19     INFO - nsTraceRefcnt::DumpStatistics: 1264 entries
[task 2020-01-10T19:06:19.713Z] 19:06:19     INFO - TEST-INFO | leakcheck | tab leaked 1 JS Object
[task 2020-01-10T19:06:19.713Z] 19:06:19     INFO - TEST-UNEXPECTED-FAIL | leakcheck | tab 16 bytes leaked (JS Object)
[task 2020-01-10T19:06:19.713Z] 19:06:19     INFO - 
[task 2020-01-10T19:06:19.713Z] 19:06:19     INFO - runtests.py | Running tests: end.
[task 2020-01-10T19:06:19.735Z] 19:06:19     INFO - Buffered messages finished
[task 2020-01-10T19:06:19.735Z] 19:06:19     INFO - Running manifest: layout/base/tests/mochitest.ini
[task 2020-01-10T19:06:19.736Z] 19:06:19     INFO - The following extra prefs will be set:
[task 2020-01-10T19:06:19.736Z] 19:06:19     INFO -   plugin.load_flash_only=false
[task 2020-01-10T19:06:19.756Z] 19:06:19     INFO -  Setting pipeline to PAUSED ...
[task 2020-01-10T19:06:19.757Z] 19:06:19     INFO -  Pipeline is PREROLLING ...
[task 2020-01-10T19:06:19.757Z] 19:06:19     INFO -  Pipeline is PREROLLED ...
[task 2020-01-10T19:06:19.757Z] 19:06:19     INFO -  Setting pipeline to PLAYING ...
[task 2020-01-10T19:06:19.757Z] 19:06:19     INFO -  New clock: GstSystemClock
[task 2020-01-10T19:06:19.793Z] 19:06:19     INFO -  Got EOS from element "pipeline0".
[task 2020-01-10T19:06:19.793Z] 19:06:19     INFO -  Execution ended after 33393731 ns.
[task 2020-01-10T19:06:19.793Z] 19:06:19     INFO -  Setting pipeline to PAUSED ...
[task 2020-01-10T19:06:19.793Z] 19:06:19     INFO -  Setting pipeline to READY ...
[task 2020-01-10T19:06:19.793Z] 19:06:19     INFO -  Setting pipeline to NULL ...
[task 2020-01-10T19:06:19.793Z] 19:06:19     INFO -  Freeing pipeline ...
[task 2020-01-10T19:06:20.101Z] 19:06:20     INFO -  pk12util: PKCS12 IMPORT SUCCESSFUL
Flags: needinfo?(jcoppeard)
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f36ee6cabccd
Add a pref to enable support for weak references in nightly builds, off by default r=mccr8
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla74
Flags: needinfo?(jcoppeard)
You need to log in before you can comment on or make changes to this bug.