Closed Bug 1358914 Opened 3 years ago Closed 3 years ago

Avoid calling evalInSandbox when loading bootstrap scopes

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: kmag, Assigned: kmag)

References

(Blocks 1 open bug)

Details

(Whiteboard: triaged)

Attachments

(1 file)

As far as I can tell, this doesn't actually slow anything down much, but it makes reading profiles confusing, and is generally unnecessary.
Component: WebExtensions: General → Add-ons Manager
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?
Attachment #8860756 - Flags: review?(rhelmer) → review+
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.
Priority: -- → P2
Whiteboard: triaged
Pushed by maglione.k@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e12dcb4be855
Avoid calling evalInSandbox when creating bootstrap scopes. r=rhelmer
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 fake@sdk.hybrid.addon: 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 < loadBootstrapScope@XPIProvider.jsm:5220:7 < callBootstrapMethod@XPIProvider.jsm:5292:9 < installAddonFromLocation@XPIProvider.jsm:4516:5 < async*installTemporaryAddon@XPIProvider.jsm:4407:12 < installTemporaryAddon@AddonManager.jsm:2280:12 < installTemporaryAddon@AddonManager.jsm:3630:12 < startup@ExtensionTestCommon.jsm:106:14 < _receiveMessageAPI/<@SpecialPowersObserverAPI.js:594:18 < promise callback*_receiveMessageAPI@SpecialPowersObserverAPI.js:580:9 < SpecialPowersObserver.prototype._receiveMessage@SpecialPowersObserver.jsm:105:10 < SpecialPowersObserver.prototype.receiveMessage@SpecialPowersObserver.jsm:311:14
[task 2017-05-15T21:23:41.783714Z] 21:23:41     INFO - GECKO(3795) | 1494883421779	addons.xpi	WARN	Add-on fake@sdk.hybrid.addon is missing bootstrap method install
[task 2017-05-15T21:23:41.800093Z] 21:23:41     INFO - GECKO(3795) | 1494883421797	addons.xpi	WARN	Add-on fake@sdk.hybrid.addon 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
Flags: needinfo?(kmaglione+bmo)
https://hg.mozilla.org/integration/mozilla-inbound/rev/40852f4be7493d4a41a4f3754c91fbc37c4855a6
Bug 1358914: Avoid calling evalInSandbox when creating bootstrap scopes. r=rhelmer
https://hg.mozilla.org/mozilla-central/rev/40852f4be749
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Flags: needinfo?(kmaglione+bmo)
You need to log in before you can comment on or make changes to this bug.