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

Status: RESOLVED INVALID

(Testing :: Firefox UI Tests, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

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.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

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.

Comment 2

[Chris Manchester (limited bugmail, email directly)]

ni? just to make sure you see this before spending cycles going down this road. Sorry, I should have thought of this before.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Yes, observer/event handling is currently on hold. I will implement something similar with the identical API on bug 1123401.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

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.