1402066 - too much recursion in ExtensionUtils.jsm and Schemas.jsm

Status: VERIFIED FIXED

(WebExtensions :: General, defect)

Description

[EntranceJew]

Attachment: console_output.log

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.91 Safari/537.36

Steps to reproduce:

1) download https://addons.mozilla.org/en-US/firefox/addon/better-tweetdeck-17/
2) open the options page



Actual results:

the options screen does not load, ctrl+shift+i reveals that an internal error has happened within firefox's files and enabling extension debugging reveals similar information


Expected results:

the version in the top left should have updated from 3.0.0 and the information should have updated to show the options screen

Comment 1

[EntranceJew]

Latest known version where it works:
https://i.imgur.com/ytvz0ww.png
Earliest known version where it stops working:
https://i.imgur.com/DJQhPnH.png
https://i.imgur.com/hdLd9tu.png

This happens on Windows 10 -- I am currently on version 1703 (15063.608) according to:
https://technet.microsoft.com/en-us/windows/release-info.aspx

This does not seem to happen on mac builds:
https://twitter.com/BetterTDeck/status/910914564811841536

Other reports of the issue:
https://twitter.com/SeriousHoax/status/910919430418264064
https://twitter.com/voxelsprite/status/903372510564982786

Extension source for the current hosted version:
https://github.com/eramdam/BetterTweetDeck/tree/3.3.10

Comment 2

[chylex]

This build of Nightly also works, I had it installed in an old VM. Broke after update, reverting to it makes it work again, so the error happened sometime after it but I haven't tried downloading a bunch of versions to see which build broke exactly.

https://download-origin.cdn.mozilla.net/pub/firefox/nightly/2017/08/2017-08-07-11-34-52-mozilla-central/firefox-57.0a1.en-US.win64.installer.exe

Comment 3

[SeriousHoax]

I can assure that I've the same problem too. It worked when I first installed Firefox 57 Nightly(Not sure which version). But later after an update this problem began. I'm on Windows 10 too. 

https://imgur.com/a/nFMwm

Comment 4

[Alice0775 White]

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c0c52209c8f277f0465725d53b2e61a5f51f6132&tochange=aad791871dfbd7e61d9cf1bfb1f027b1fae41e14

@:kmag,
Your bunch of patch seems to be causing the problem. Can you look at this?

Comment 5

[Kris Maglione [:kmag]]

Attachment: Bug 1402066: Fix ordering of base schema data in extension child processes.

This is necessary so that the Permission type is available for permission API
validation.

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

Comment 6

[Kris Maglione [:kmag]]

Comment on attachment 8911264 [details]
Bug 1402066: Fix ordering of base schema data in extension child processes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182756/diff/1-2/

Comment 7

[Kris Maglione [:kmag]]

Attachment: Bug 1402066: Part 2 - Enable permissions tests in OOP mode.

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

Comment 8

[Kris Maglione [:kmag]]

Comment on attachment 8911264 [details]
Bug 1402066: Fix ordering of base schema data in extension child processes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/182756/diff/2-3/

Comment 9

[Andrew Swan [:aswan]]

Comment on attachment 8911264 [details]
Bug 1402066: Fix ordering of base schema data in extension child processes.

https://reviewboard.mozilla.org/r/182756/#review188526

::: toolkit/components/extensions/Schemas.jsm:2724
(Diff revision 3)
> -        for (let [url, schema] of msg.data) {
> +        // If we're given a Map, the ordering of the initial items
> +        // matters, so swap with our current data to make sure its
> +        // entries appear first.

Could you clarify this a bit with "make sure the new entries appear first".  Also note that data comes from a different compartment so it needs to be copied with the `new Map()` bit.

Comment 10

[Andrew Swan [:aswan]]

Comment on attachment 8911407 [details]
Bug 1402066: Part 2 - Enable permissions tests in OOP mode.

https://reviewboard.mozilla.org/r/182874/#review188528

::: toolkit/components/extensions/ExtensionXPCShellUtils.jsm:698
(Diff revision 1)
>  
>    set remoteContentScripts(val) {
>      REMOTE_CONTENT_SCRIPTS = !!val;
>    },
>  
> -  loadContentPage(url, remote = undefined, redirectUrl = undefined) {
> +  loadContentPage(url, {extension = undefined, remote = undefined, redirectUrl = undefined} = {}) {

Can you document the arguments here?  The meaning of `extension` wasn't clear to me without reading the code.

Comment 11

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d828e2c01a60febf598dcc544da266042c74731b
Bug 1402066: Part 1 - Fix ordering of base schema data in extension child processes. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/641430a496824e34170786f6f3bc5e587f2056eb
Bug 1402066: Part 2 - Enable permissions tests in OOP mode. r=aswan

Comment 12

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

Backed out for frequently failing modified xpcshell test toolkit/components/extensions/test/xpcshell/test_ext_redirects.js on Android 4.3 debug:

https://hg.mozilla.org/integration/mozilla-inbound/rev/6a9d008002efc9044416d5fff5713f899c954ebb
https://hg.mozilla.org/integration/mozilla-inbound/rev/998dd0100a61ba5ceafe8fe27b13753b3537a8bd

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=641430a496824e34170786f6f3bc5e587f2056eb&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=133288472&repo=mozilla-inbound

[task 2017-09-26T08:56:09.057Z] 08:56:09     INFO -  TEST-START | xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_redirects.js
[task 2017-09-26T09:01:09.057Z] 09:01:09  WARNING -  TEST-UNEXPECTED-TIMEOUT | xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_redirects.js | Test timed out
[task 2017-09-26T09:01:09.057Z] 09:01:09     INFO -  TEST-INFO took 300000ms
[task 2017-09-26T09:01:09.225Z] 09:01:09     INFO -  xpcshell return code: -1
[task 2017-09-26T09:01:19.095Z] 09:01:19     INFO -  mozdevice Timeout exceeded for shell call 'adb shell /data/local/xpcb/xpcw /storage/sdcard/tests/xpc/toolkit/components/extensions/test/xpcshell -r /storage/sdcard/tests/xpc/c/httpd.manifest --greomni /data/local/xpcb/target.apk -m -s -e 'const _HEAD_JS_PATH = "/storage/sdcard/tests/xpc/head.js";' -e 'const _MOZINFO_JS_PATH = "/storage/sdcard/tests/xpc/p/mozinfo.json";' -e 'const _TESTING_MODULES_DIR = "/storage/sdcard/tests/xpc/m";' -f /storage/sdcard/tests/xpc/head.js -e 'const _SERVER_ADDR = "localhost"' -e 'const _HEAD_FILES = ["/storage/sdcard/tests/xpc/toolkit/components/extensions/test/xpcshell/head.js", "/storage/sdcard/tests/xpc/toolkit/components/extensions/test/xpcshell/head_telemetry.js"];' -e 'const _JSDEBUGGER_PORT = 0;' -e 'const _TEST_FILE = ["test_ext_redirects.js"];' -e 'const _TEST_NAME = "xpcshell.ini:toolkit/components/extensions/test/xpcshell/test_ext_redirects.js"' -e '_execute_test(); quit(0);'; echo $?'
[task 2017-09-26T09:01:20.859Z] 09:01:20  WARNING -  TEST-UNEXPECTED-FAIL | Received SIGINT (control-C), so stopped run. (Use --keep-going to keep running tests after killing one with SIGINT)

Comment 13

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8049a55e5f8cd2cef6167689fa9aabc55f400baf
Bug 1402066: Part 1 - Fix ordering of base schema data in extension child processes. r=aswan

https://hg.mozilla.org/integration/mozilla-inbound/rev/95a7edc3d438289870d60085917532eb50196d52
Bug 1402066: Part 2 - Enable permissions tests in OOP mode. r=aswan

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8049a55e5f8c
https://hg.mozilla.org/mozilla-central/rev/95a7edc3d438

Comment 15

[Kris Maglione [:kmag]]

Comment on attachment 8911264 [details]
Bug 1402066: Fix ordering of base schema data in extension child processes.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1390010
[User impact if declined]: This causes breakage whenever an extension tries to use the optional permissions API.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: No.
[Needs manual test from QE? If yes, steps to reproduce]: Listed in comment 0.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: Low-risk.
[Why is the change risky/not risky?]: The change is fairly minimal, and merely corrects the ordering of schema data in extension child processes. The affected code is covered by comprehensive tests, and the tests which should have caught the original issue have been enabled for out-of-process extensions.
[String changes made/needed]: None.

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8911264 [details]
Bug 1402066: Fix ordering of base schema data in extension child processes.

Fix a recent regression, taking it.
Should be in 57b4

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c4554c786af5
https://hg.mozilla.org/releases/mozilla-beta/rev/e73836222b57

Comment 18

[marius.santa]

Attachment: options working.PNG

This bug is verified on Firefox 57.0b4 (20170928180207) and Firefox 58.0a1 (20171002100134) under Windows 10 64/32-bit.

Please see the attached screenshot.