1358914 - Avoid calling evalInSandbox when loading bootstrap scopes

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect, P2)

Description

[Kris Maglione [:kmag]]

As far as I can tell, this doesn't actually slow anything down much, but it makes reading profiles confusing, and is generally unnecessary.

Comment 1

[Kris Maglione [:kmag]]

Attachment: Bug 1358914: Avoid calling evalInSandbox when creating bootstrap scopes.

Review commit: https://reviewboard.mozilla.org/r/132694/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/132694/

Comment 2

[Robert Helmer [:rhelmer]]

Comment on attachment 8860756 [details]
Bug 1358914: Avoid calling evalInSandbox when creating bootstrap scopes.

https://reviewboard.mozilla.org/r/132694/#review135882

::: toolkit/mozapps/extensions/internal/XPIProvider.jsm
(Diff revision 1)
> -      activeAddon.bootstrapScope.__SCRIPT_URI_SPEC__ = uri;
> -      Components.utils.evalInSandbox(
> -        "Components.classes['@mozilla.org/moz/jssubscript-loader;1'] \
> -                   .getService(Components.interfaces.mozIJSSubScriptLoader) \
> -                   .loadSubScript(__SCRIPT_URI_SPEC__);",
> -                   activeAddon.bootstrapScope, "ECMAv5");

Do we not need to set the JS version here anymore?

IIRC we do this elsewhere for content and bg scripts in WE?

Comment 3

[Kris Maglione [:kmag]]

Comment on attachment 8860756 [details]
Bug 1358914: Avoid calling evalInSandbox when creating bootstrap scopes.

https://reviewboard.mozilla.org/r/132694/#review135882

> Do we not need to set the JS version here anymore?
> 
> IIRC we do this elsewhere for content and bg scripts in WE?

It doesn't actually have any effect. The subscript loader just uses JSVERSION_LATEST.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e12dcb4be855
Avoid calling evalInSandbox when creating bootstrap scopes. r=rhelmer

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for timing out in toolkit/components/extensions/test/mochitest/test_chrome_ext_hybrid_addons.html:

https://hg.mozilla.org/integration/autoland/rev/7a2841383525164d5651218a06f3b5e74bd570af

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=e12dcb4be8551c14073c891a4c9f790e0d790215&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=99253234&repo=autoland

[task 2017-05-15T21:23:41.575156Z] 21:23:41     INFO - TEST-START | toolkit/components/extensions/test/mochitest/test_chrome_ext_hybrid_addons.html
[task 2017-05-15T21:23:41.782312Z] 21:23:41     INFO - GECKO(3795) | 1494883421774	addons.xpi	WARN	Error loading bootstrap.js for [e-mail]: ReferenceError: __SCRIPT_URI_SPEC__ is not defined (resource://gre/modules/addons/XPIProvider.jsm -> jar:file:///tmp/generated-extension.xpi!/bootstrap.js:3:11) JS Stack trace: @bootstrap.js:3:11 < [e-mail]:5220:7 < [e-mail]:5292:9 < [e-mail]:4516:5 < async*[e-mail]:4407:12 < [e-mail]:2280:12 < [e-mail]:3630:12 < [e-mail]:106:14 < _receiveMessageAPI/<@SpecialPowersObserverAPI.js:594:18 < promise callback*[e-mail]:580:9 < [e-mail]:105:10 < [e-mail]:311:14
[task 2017-05-15T21:23:41.783714Z] 21:23:41     INFO - GECKO(3795) | 1494883421779	addons.xpi	WARN	Add-on [e-mail] is missing bootstrap method install
[task 2017-05-15T21:23:41.800093Z] 21:23:41     INFO - GECKO(3795) | 1494883421797	addons.xpi	WARN	Add-on [e-mail] is missing bootstrap method startup
[task 2017-05-15T21:28:59.557439Z] 21:28:59     INFO - TEST-INFO | started process screentopng
[task 2017-05-15T21:28:59.915344Z] 21:28:59     INFO - TEST-INFO | screentopng: exit 0
[task 2017-05-15T21:28:59.915413Z] 21:28:59     INFO - Buffered messages logged at 21:23:41
[task 2017-05-15T21:28:59.915470Z] 21:28:59     INFO - SpawnTask.js | Entering test test_sdk_hybrid_addon_with_jpm_module_loader
[task 2017-05-15T21:28:59.915509Z] 21:28:59     INFO - Extension loaded
[task 2017-05-15T21:28:59.915571Z] 21:28:59     INFO - Buffered messages finished
[task 2017-05-15T21:28:59.915638Z] 21:28:59     INFO - TEST-UNEXPECTED-FAIL | toolkit/components/extensions/test/mochitest/test_chrome_ext_hybrid_addons.html | Test timed out. 
[task 2017-05-15T21:28:59.915752Z] 21:28:59     INFO - reportError@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:121:7
[task 2017-05-15T21:28:59.915813Z] 21:28:59     INFO - TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:142:7
[task 2017-05-15T21:28:59.915879Z] 21:28:59     INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5
[task 2017-05-15T21:28:59.915941Z] 21:28:59     INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5
[task 2017-05-15T21:28:59.916007Z] 21:28:59     INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5
[task 2017-05-15T21:28:59.916075Z] 21:28:59     INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5
[task 2017-05-15T21:28:59.916149Z] 21:28:59     INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5
[task 2017-05-15T21:28:59.916217Z] 21:28:59     INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5
[task 2017-05-15T21:28:59.916296Z] 21:28:59     INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5
[task 2017-05-15T21:28:59.916375Z] 21:28:59     INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5
[task 2017-05-15T21:28:59.916774Z] 21:28:59     INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5
[task 2017-05-15T21:28:59.917279Z] 21:28:59     INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5
[task 2017-05-15T21:28:59.917697Z] 21:28:59     INFO - setTimeout handler*TestRunner._checkForHangs@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:163:5
[task 2017-05-15T21:28:59.918139Z] 21:28:59     INFO - TestRunner.runTests@chrome://mochikit/content/tests/SimpleTest/TestRunner.js:380:5
[task 2017-05-15T21:28:59.918627Z] 21:28:59     INFO - RunSet.runtests@chrome://mochikit/content/tests/SimpleTest/setup.js:194:3
[task 2017-05-15T21:28:59.918918Z] 21:28:59     INFO - RunSet.runall@chrome://mochikit/content/tests/SimpleTest/setup.js:173:5
[task 2017-05-15T21:28:59.919252Z] 21:28:59     INFO - hookupTests@chrome://mochikit/content/tests/SimpleTest/setup.js:266:5
[task 2017-05-15T21:28:59.919629Z] 21:28:59     INFO - parseTestManifest@chrome://mochikit/content/manifestLibrary.js:38:5
[task 2017-05-15T21:28:59.919955Z] 21:28:59     INFO - getTestManifest/req.onload@chrome://mochikit/content/manifestLibrary.js:49:11
[task 2017-05-15T21:28:59.920295Z] 21:28:59     INFO - EventHandlerNonNull*getTestManifest@chrome://mochikit/content/manifestLibrary.js:45:3
[task 2017-05-15T21:28:59.920620Z] 21:28:59     INFO - hookup@chrome://mochikit/content/tests/SimpleTest/setup.js:246:5
[task 2017-05-15T21:28:59.920997Z] 21:28:59     INFO - linkAndHookup@chrome://mochikit/content/harness.xul:59:3
[task 2017-05-15T21:28:59.921323Z] 21:28:59     INFO - parseTestManifest@chrome://mochikit/content/manifestLibrary.js:38:5
[task 2017-05-15T21:28:59.921632Z] 21:28:59     INFO - getTestManifest/req.onload@chrome://mochikit/content/manifestLibrary.js:49:11
[task 2017-05-15T21:28:59.921973Z] 21:28:59     INFO - EventHandlerNonNull*getTestManifest@chrome://mochikit/content/manifestLibrary.js:45:3
[task 2017-05-15T21:28:59.922290Z] 21:28:59     INFO - getTestList@chrome://mochikit/content/chrome-harness.js:260:3
[task 2017-05-15T21:28:59.922576Z] 21:28:59     INFO - loadTests@chrome://mochikit/content/harness.xul:38:3
[task 2017-05-15T21:28:59.922907Z] 21:28:59     INFO - EventListener.handleEvent*@chrome://mochikit/content/harness.xul:62:5
[task 2017-05-15T21:29:00.559427Z] 21:29:00     INFO - Not taking screenshot here: see the one that was previously logged

Comment 6

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/40852f4be7493d4a41a4f3754c91fbc37c4855a6
Bug 1358914: Avoid calling evalInSandbox when creating bootstrap scopes. r=rhelmer

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/40852f4be749