Use new_sandbox=False in all ui and api modules until multiple sandbox support is available

RESOLVED INVALID

Status

Testing
Firefox UI Tests
RESOLVED INVALID
3 years ago
2 years ago

People

(Reporter: whimboo, Unassigned)

Tracking

(Blocks: 1 bug)

Version 2
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [blocked])

(Reporter)

Description

3 years ago
As written on bug 1122583 we will have problems with data kept in the global context of a execute_script call. Tests and other modules could reset the sandbox and the formerly set data is lost. Especially registered callbacks for events and observers can leak that way.

We should update all of our modules to explicitly always use new_sandbox=False for the time being. We should also ensure to only store those variables on `global` or `this`, which really have to retain state between multiple calls.
(Reporter)

Updated

3 years ago
Depends on: 1122609
(Reporter)

Updated

3 years ago
Whiteboard: [blocked]
So, I just realized that the implementation sandbox re-use in bug 755036 clobbers the sandbox when switching between windows, which is going to cause this to be incorrect. This was for consistency with the implementation on the content side (and it does that because the current window is used as the prototype for the execution sandbox, as is the case in chrome context).

I think we're best off for the moment hanging observer/event state off of testUtils as discussed in 1122583.
ni? just to make sure you see this before spending cycles going down this road. Sorry, I should have thought of this before.
Flags: needinfo?(hskupin)
(Reporter)

Comment 3

3 years ago
Yes, observer/event handling is currently on hold. I will implement something similar with the identical API on bug 1123401.
Assignee: hskupin → nobody
Flags: needinfo?(hskupin)
(Reporter)

Updated

3 years ago
Status: ASSIGNED → NEW
(Reporter)

Comment 4

2 years ago
Bug 1149618 actually implemented a way to use multiple sandboxes and retain values between invocations. Means there is no need for this workaround anymore and we can go ahead with the observer implementation.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Depends on: 1149618
No longer depends on: 1122583
QA Contact: hskupin
Resolution: --- → INVALID
(Assignee)

Updated

2 years ago
Product: Mozilla QA → Testing
You need to log in before you can comment on or make changes to this bug.