+++ This bug was initially created as a clone of Bug #751783 +++

We also need that feature for the upcoming Firefox Desktop Marionette tests. So its blocking our work on bug 1080766. Here some testcases: self.marionette.set_context('chrome') self.marionette.execute_script("global.foobar = 3;") self.assertEqual(self.marionette.execute_script("return global.foobar;", new_sandbox=False), 3) self.marionette.set_context('chrome') self.marionette.execute_script("this.foobar = 3;") self.assertEqual(self.marionette.execute_script("return this.foobar;", new_sandbox=False), 3) Particularly `global` is completely undefined, and assigning to `this` works in the first call, but the property is undefined in the second call.

David mentioned on IRC that an additional problem here could be that we also do not re-use the former sandbox even with new_sandbox=False. This seems to only work on content scope. Here some links he pasted: http://dxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-server.js#879 http://dxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-server.js#889 http://dxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-server.js#922-930 http://dxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-listener.js#493-500

I can look into it this week.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8fc99da1a4d7 https://treeherder.mozilla.org/#/jobs?repo=try&revision=0d9b0e58e3f8

Try from comment 4 is pretty green.

/r/2097 - Bug 755036 - Re-use the execution sandbox in marionette's executeScript and executeAsyncScript in chrome scope when requested. Pull down this commit: hg pull review -r 8f1abb3f6573d034b3e32c86fa79220adca983fa

Chris, did you miss to add new tests for chrome? I only see the removal of two blank tests in your patch, but none which actually test the globals in chrome scope.

(In reply to Henrik Skupin (:whimboo) from comment #8) > Chris, did you miss to add new tests for chrome? I only see the removal of > two blank tests in your patch, but none which actually test the globals in > chrome scope. The chrome test classes inherit from the content ones -- taking out those stubs makes them run in chrome.

Ah, cool then! Thanks a lot for doing that so quickly!

https://reviewboard.mozilla.org/r/2095/#review1323 This looks good! I think that this may fail if the user switches windows and then executes some script with new_sandbox=False, since that would cause her to be using a sandbox attached to the wrong window. To handle that possibility, we may want to compare the sandbox's window against the current window, and throw an error if they're not the same. In content, we handle similar situations by setting sandbox = null in switchToFrame, and we could do that here as well, but in switchToWindow.

https://reviewboard.mozilla.org/r/2095/#review1325 Aha, I'll clobber the sandbox when switching windows for consistency. Thanks!

Inbound is closed. Here's a patch with the fix suggested by jgriffin in comment 11.

Comment on attachment 8545402 [details] [diff] [review] Re-use the execution sandbox in marionette's executeScript and executeAsyncScript in chrome scope when requested. Review of attachment 8545402 [details] [diff] [review]: ----------------------------------------------------------------- r=jgriffin

I can verify this works great now with Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Firefox/38.0 ID:20150115030228 CSet: c1f6345f2803