Closed
Bug 1398601
Opened 7 years ago
Closed 7 years ago
Fix subscript loader behavior inside a shared JSM
Categories
(Core :: XPConnect, enhancement, P1)
Core
XPConnect
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: tcampbell, Assigned: tcampbell)
References
Details
Attachments
(7 files, 1 obsolete file)
59 bytes,
text/x-review-board-request
|
jorendorff
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
kmag
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
jorendorff
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
kmag
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
kmag
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
tromey
:
review+
|
Details |
1.68 KB,
patch
|
Details | Diff | Splinter Review |
See: https://bugzilla.mozilla.org/show_bug.cgi?id=1186409#c161
With the new JSM global sharing (Bug 1186409), we load JSMs into a NonSyntacticVariablesObject instead of a private global. To facilitate this, we added a new API in Bug 1395360.
Using the subscript loader from within a JSM NSVO will currently try to put the NSVO inside a WithEnvironment. This is inconsistent with the non-shared case where a global would be at root of chain, not within a With.
The expected chain should be:
> [JSM Shared Global/BackstagePass]
> [JSM Shared Global LexicalEnvironmentObject]
> [NSVO]
> [Extensible LexicalEnvironmentObject] **
> [WithEnvironment(target object)]
> [Extensible LexicalEnvironmentObject]
>
> ** This environment holds the |this| pointer of things like |(function() { return this; })()|
This should be achieved by updating the ExecuteInJSMEnvironment API.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → tcampbell
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•7 years ago
|
||
TODO:
- Update ExecuteInJSMEnvironment API
- Assert in CreateObjectsForEnvironmentChain we don't wrap NSVO
- Update mozJSSubscriptLoader to call these
- Add xpcshell test cases similar to the other environment tests
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 7•7 years ago
|
||
I put together a crude prototype.
Running a try run with global sharing enabled here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4782b4c35771c667420e8728ea9f26416e66dc37
This still depends on IndirectEval being fixed, so a couple xpcshell tests should fail.
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8906399 [details]
Bug 1398601 - Fix subscript loader when using JSM global sharing
https://reviewboard.mozilla.org/r/178104/#review183042
::: js/xpconnect/loader/mozJSSubScriptLoader.cpp:186
(Diff revision 1)
> + } else if (js::IsJSMEnvironment(targetObj)) {
> + MOZ_DIAGNOSTIC_ASSERT(loadScope == targetObj);
> + if (!ExecuteInJSMEnvironment(cx, script, targetObj)) {
> + return false;
> + }
> + // FIXME - Support return values
Nah, let's not. Code compiled to return a value runs much slower than it would otherwise. Let's just throw if someone tries to execute a return value script in a JSM environment.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 12•7 years ago
|
||
Oops, I didn't handle the cross-compartment subscript loading correctly. I also added a diagnostic the check if we are about to subscript load into the bare shared JSM global.
One behavior difference once we turn on global sharing occurs when a JSM loads a subscript into a target of another JSM. Without sharing, this was a cross-compartment subscript load that would pollute the target. With sharing, it is no long a cross-compartment subscript load and will pollute the caller JSM instead.
Another try attempt: https://treeherder.mozilla.org/#/jobs?repo=try&revision=faebdf60947c75a568ae5326b354a25bd28898db
Comment 13•7 years ago
|
||
(In reply to Ted Campbell [:tcampbell] from comment #12)
> One behavior difference once we turn on global sharing occurs when a JSM
> loads a subscript into a target of another JSM. Without sharing, this was a
> cross-compartment subscript load that would pollute the target. With
> sharing, it is no long a cross-compartment subscript load and will pollute
> the caller JSM instead.
Yup, that's expected (see bug 1186409 comment 78). There's not really anything we can do about it, so I don't think it's worth worrying about.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 18•7 years ago
|
||
GetTargetObject shouldn't match the NSVO of a framescript or the subscript loader will be unhappy.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=81cb50bc9287f755b42ec4d961c92c1c1869c384
Assignee | ||
Comment 19•7 years ago
|
||
Ah, I see that :kmag pointed out this issue in https://bugzilla.mozilla.org/show_bug.cgi?id=1186409#c163 . I proposed FindTargetObject only do the NSVO thing if it is from the loader global.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8906401 -
Flags: review?(kmaglione+bmo)
Assignee | ||
Updated•7 years ago
|
Attachment #8906399 -
Flags: review?(kmaglione+bmo)
Assignee | ||
Comment 25•7 years ago
|
||
The patches are working. I still need to remove the copy-paste code in JS side, but the xpconnect side is ready for review.
Assignee | ||
Updated•7 years ago
|
Priority: -- → P1
Comment 26•7 years ago
|
||
mozreview-review |
Comment on attachment 8906399 [details]
Bug 1398601 - Fix subscript loader when using JSM global sharing
https://reviewboard.mozilla.org/r/178104/#review183420
::: js/xpconnect/loader/mozJSComponentLoader.cpp:456
(Diff revision 4)
> - if (!aTargetObject) {
> + //
> + // If the target object was not in the JSM shared global, return the global
> + // instead. This is needed when calling the subscript loader within a frame
> + // script, since it the FrameScript NSVO will have been found.
> + if (!aTargetObject ||
> + !IsLoaderGlobal(js::GetGlobalForObjectCrossCompartment(aTargetObject))) {
Hm. I'm on the fence about this. I think we really probably do want to use the NSVO as the default import target in frame scripts, especially since there's no way for the scripts to access it as an explicit target (`this` points to the global in frame scripts, unlike in JSMs).
But I can also see an argument for fixing that as a follow-up.
::: js/xpconnect/loader/mozJSSubScriptLoader.cpp:191
(Diff revision 4)
> - if (loadScope != targetObj &&
> - loadScope &&
> - !JS_IsGlobalObject(loadScope))
> - {
> - MOZ_DIAGNOSTIC_ASSERT(js::GetObjectCompartment(loadScope) ==
> - js::GetObjectCompartment(targetObj));
> -
> - if (!envChain.append(loadScope)) {
> + if (js::GetObjectCompartment(loadScope) != js::GetObjectCompartment(targetObj)) {
> + // If target is cross-compartment, check the target isn't in JSM
> + // shared global compartment or we will contaiminate it.
> + // NOTE: If loadScope is already a shared-global JSM, we can't
> + // determine which JSM the target belongs too.
> + JSObject* targetGlobal = js::GetGlobalForObjectCrossCompartment(targetObj);
> + MOZ_DIAGNOSTIC_ASSERT(!mozJSComponentLoader::Get()->IsLoaderGlobal(targetGlobal),
> + "Don't load subscript into target in a shared-global JSM");
I'd rather keep the compartment check in DoLoadSubScriptWithOptions, and stick to the null check here. And if we do the target global check there, we can just throw rather than asserting.
Attachment #8906399 -
Flags: review?(kmaglione+bmo) → review+
Comment 27•7 years ago
|
||
mozreview-review |
Comment on attachment 8906401 [details]
Bug 1398601 - Add testcase for loading subscripts within a JSM
https://reviewboard.mozilla.org/r/178108/#review183428
Attachment #8906401 -
Flags: review?(kmaglione+bmo) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8906397 -
Flags: review?(jorendorff)
Attachment #8906398 -
Flags: review?(jorendorff)
Attachment #8906400 -
Flags: review?(jorendorff)
Assignee | ||
Comment 35•7 years ago
|
||
I moved the compartment check back to DoLoadSubScriptWithOptions. Deferring changing the FrameScript behaviour as a follow-up.
Comment 36•7 years ago
|
||
mozreview-review |
Comment on attachment 8906397 [details]
Bug 1398601 - Add js::IsJSMEnvironment to jsfriendapi
https://reviewboard.mozilla.org/r/178100/#review183446
I am the worst reviewer
::: js/src/builtin/Eval.cpp:543
(Diff revision 2)
> }
> +
> +JS_FRIEND_API(bool)
> +js::IsJSMEnvironment(JSObject* obj)
> +{
> + return obj->is<NonSyntacticVariablesObject>();
This could use a comment explaining why the implementation is correct (or pointing to the comment that explains the general situation).
::: js/src/jsfriendapi.h:2893
(Diff revision 2)
> ExecuteInGlobalAndReturnScope(JSContext* cx, JS::HandleObject obj, JS::HandleScript script,
> JS::MutableHandleObject scope);
>
> // These functions are only for JSM component loader, please don't use this for anything else!
> extern JS_FRIEND_API(JSObject*)
> NewJSMEnvironment(JSContext* cx);
Bad luck: I just realized "nobody should ever use this" is not an acceptable excuse for not documenting a public function. Please comment all these functions briefly.
Attachment #8906397 -
Flags: review?(jorendorff) → review+
Comment 37•7 years ago
|
||
mozreview-review |
Comment on attachment 8906398 [details]
Bug 1398601 - Support target objects in js::ExecuteInJSMEnvironment
https://reviewboard.mozilla.org/r/178102/#review183460
::: js/src/builtin/Eval.cpp:448
(Diff revision 4)
> {
> return fun->maybeNative() == IndirectEval;
> }
>
> static bool
> -ExecuteInNonSyntacticGlobalInternal(JSContext* cx, HandleObject global, HandleScript scriptArg,
> +ExecuteInNonSyntacticGlobalInternal(JSContext* cx, HandleScript scriptArg, HandleObject env)
NonSyntacticGlobal isn't a good name, even for internal use only. If anything, the idea that these objects behave kind of like global objects will seem *less* sensible for people working inside the engine.
ExecuteInExtensibleLexicalEnvironment perhaps?
::: js/src/builtin/Eval.cpp:520
(Diff revision 4)
> +{
> assertSameCompartment(cx, varEnv);
> MOZ_ASSERT(cx->compartment()->getNonSyntacticLexicalEnvironment(varEnv));
> + MOZ_DIAGNOSTIC_ASSERT(scriptArg->noScriptRval());
>
> RootedObject global(cx, &varEnv->global());
`global` is now unused, I think.
::: js/src/jsfriendapi.h:2898
(Diff revision 4)
> NewJSMEnvironment(JSContext* cx);
>
> extern JS_FRIEND_API(bool)
> ExecuteInJSMEnvironment(JSContext* cx, JS::HandleScript script, JS::HandleObject nsvo);
>
> +extern JS_FRIEND_API(bool)
Short comment, please, at least explaining the preconditions on nsvo and script.
Attachment #8906398 -
Flags: review?(jorendorff) → review+
Comment 38•7 years ago
|
||
mozreview-review |
Comment on attachment 8906400 [details]
Bug 1398601 - Don't allow NSVO in js::CreateObjectsForEnvironmentChain
https://reviewboard.mozilla.org/r/178106/#review183482
Attachment #8906400 -
Flags: review?(jorendorff) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8906397 -
Attachment is obsolete: true
Assignee | ||
Comment 43•7 years ago
|
||
- Added comments to the APIs
- Flattened the first two patches
- s/ExecuteInNonSyntacticGlobalInternal/ExecuteInExtensibleLexicalEnvironment/
- Removed the dead 'global' local variable
Comment 44•7 years ago
|
||
mozreview-review |
Comment on attachment 8906398 [details]
Bug 1398601 - Support target objects in js::ExecuteInJSMEnvironment
https://reviewboard.mozilla.org/r/178102/#review183532
::: js/src/builtin/Eval.cpp:528
(Diff revision 5)
> + // them to the environment. These are added after the NSVO environment.
> + if (!targetObj.empty()) {
> + // The environment chain will be as follows:
> + // GlobalObject / BackstagePass
> + // LexicalEnvironmentObject[this=global]
> + // NonSyntacticVariablesObject
// NonSyntacticVariablesObject (the JSMEnvironment)
::: js/src/builtin/Eval.cpp:576
(Diff revision 5)
> +
> +JS_FRIEND_API(bool)
> +js::IsJSMEnvironment(JSObject* obj)
> +{
> + // NOTE: This also returns true if the NonSyntacticVariablesObject was
> + // created for reasons other than the JSM loader.
Heh.
::: js/src/jsfriendapi.h:2897
(Diff revision 5)
> +//
> +// A 'JSMEnvironment' refers to an environment chain constructed for JSM loading
> +// in a shared global. Internally it is a NonSyntacticVariablesObject with a
> +// corresponding extensible LexicalEnvironmentObject that is accessible by
> +// JS_ExtensibleLexicalEnvironment. The |this| value of that lexical environment
> +// is the NSVO itself.
Feel free to use or discard this. (Of course, if you use it, you have to fix my mistakes. Nothing's free...)
```
// Normal global environment (ES6): JSM "global" environment:
//
// * - extensible lexical environment
// | (code runs in this environment;
// | `let/const` bindings go here)
// |
// * - JSMEnvironment (=== `this`)
// | (`var` bindings go here)
// |
// * - extensible lexical environment * - extensible lexical environment
// | (code runs in this environment; | (empty)
// | `let/const` bindings go here) |
// | |
// * - actual global (=== `this`) * - shared JSM global
// (var bindings go here; and (Object, Math, etc. live here)
// Object, Math, etc. live here)
```
::: js/src/jsfriendapi.h:2930
(Diff revision 5)
> +// NOTE: This may find NonSyntacticVariablesObject generated by other embedding
> +// such as a Gecko FrameScript. Caller can check the compartment if needed.
> extern JS_FRIEND_API(JSObject*)
> GetJSMEnvironmentOfScriptedCaller(JSContext* cx);
>
> +// Determine if current object is a JSMEnvironment
"Determine if obj is a JSMEnvironment"
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 49•7 years ago
|
||
Pushed by tcampbell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e9816be61b49
Support target objects in js::ExecuteInJSMEnvironment r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/dd4af7998505
Fix subscript loader when using JSM global sharing r=kmag
https://hg.mozilla.org/integration/autoland/rev/7a4bb5a1848a
Add testcase for loading subscripts within a JSM r=kmag
https://hg.mozilla.org/integration/autoland/rev/7aac2595bc17
Don't allow NSVO in js::CreateObjectsForEnvironmentChain r=jorendorff
Comment 50•7 years ago
|
||
Backed out in https://hg.mozilla.org/integration/autoland/rev/fe7465d53091 for Windows (7/8/10, opt/pgo/debug) Marionette (Mn and MnH) crashes/assertion failures like https://treeherder.mozilla.org/logviewer.html#?job_id=130218034&repo=autoland
Assignee | ||
Comment 51•7 years ago
|
||
The failing test case is of course the pref-toggle test added in Bug 1398499. I'm going through the log now and see a number of |TypeError: ... is undefined| that need to be addressed. I'm not sure if these do to Bug 1396050, or if the marionette scripts are being sloppy.
The assertion that fires is that we are trying to load a subscript into a JSM |this| without it being script-compiled for non-syntactic environment support.
Assignee | ||
Comment 52•7 years ago
|
||
Oops.. those tests are supposed to throw the TypeErrors.
Assignee | ||
Comment 53•7 years ago
|
||
When we start using JSMEnvironments from subscript loader, we trip assertions about non-syntactic environment scripts. In CloneAndExecuteScript, we can clone script to make it compatible. ExecuteInJSMEnvironment was based off ExecuteInGlobalAndReturnScope which does not implicitly clone. In the FrameScript case, the script cache will only allow one version to be stored and doesn't use cache if it does not match.
To fix this patch, I'll have the startup-cache use the same prefixing we did for JSM loader.
This test only failed on Windows due to divergent implementations of Subprocess.jsm.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8907311 -
Flags: review?(kmaglione+bmo)
Comment 58•7 years ago
|
||
mozreview-review |
Comment on attachment 8907311 [details]
Bug 1398601 - Add global/non-syntactic prefix to subscript loader cache
https://reviewboard.mozilla.org/r/178978/#review184098
Attachment #8907311 -
Flags: review?(kmaglione+bmo) → review+
Comment 59•7 years ago
|
||
Pushed by tcampbell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/776a65d43a5e
Support target objects in js::ExecuteInJSMEnvironment r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/eddc38b0afd0
Add global/non-syntactic prefix to subscript loader cache r=kmag
https://hg.mozilla.org/integration/autoland/rev/f5c26c3407c0
Fix subscript loader when using JSM global sharing r=kmag
https://hg.mozilla.org/integration/autoland/rev/05957a92b1a5
Add testcase for loading subscripts within a JSM r=kmag
https://hg.mozilla.org/integration/autoland/rev/b728872f4d9a
Don't allow NSVO in js::CreateObjectsForEnvironmentChain r=jorendorff
Comment 60•7 years ago
|
||
Backed out in https://hg.mozilla.org/integration/autoland/rev/a45742d015d3 for unexpected crashtest assertions, https://treeherder.mozilla.org/logviewer.html#?job_id=130503890&repo=autoland
Comment 61•7 years ago
|
||
Urgh. I probably should have looked more closely. The cache path logic is duplicated in DoLoadSubScriptWithOptions :(
Assignee | ||
Comment 62•7 years ago
|
||
Yeah, just found that. Putting up a patch..
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 67•7 years ago
|
||
Comment on attachment 8907311 [details]
Bug 1398601 - Add global/non-syntactic prefix to subscript loader cache
Rewrote this patch to factor out the cache key generation to one place.
Attachment #8907311 -
Flags: review+ → review?(kmaglione+bmo)
Comment 68•7 years ago
|
||
This let browser-chrome's browser_ext_devtools_network.js frequently failing. There are also assertions before, but only this test fails:
See the Linux bc16 one in https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=82f171552905ea7bdbc60688db5b4aad3a3f4b95&filter-searchStr=4e1b40c7bb84b37a00fd934a28d025a1dae2e2d0
Comment 69•7 years ago
|
||
Before the last push, these Talos regressions were observed:
== Change summary for alert #9406 (as of September 13 2017 01:12 UTC) ==
Regressions:
16% tpaint summary windows10-64 pgo e10s 222.00 -> 258.22
10% sessionrestore windows10-64 pgo e10s 426.00 -> 467.42
9% sessionrestore_no_auto_restore windows10-64 pgo e10s515.33 -> 564.00
8% ts_paint windows10-64 pgo e10s 574.71 -> 620.50
4% sessionrestore_many_windows windows10-64 pgo e10s3,617.38 -> 3,769.83
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=9406
Comment 70•7 years ago
|
||
After the backout, the perf baselines returned to normal:
== Change summary for alert #9416 (as of September 13 2017 03:11 UTC) ==
Improvements:
15% tpaint summary windows10-64 pgo e10s 258.12 -> 220.31
9% sessionrestore windows10-64 pgo e10s 470.04 -> 427.17
9% sessionrestore_no_auto_restore windows10-64 pgo e10s564.42 -> 515.00
7% ts_paint windows10-64 pgo e10s 622.00 -> 577.08
4% sessionrestore_many_windows windows10-64 pgo e10s3,771.71 -> 3,626.58
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=9416
Assignee | ||
Comment 71•7 years ago
|
||
That push broke the startup script cache, so talos regression makes sense.
Assignee | ||
Comment 72•7 years ago
|
||
(In reply to Sebastian Hengst [:aryx][:archaeopteryx] (needinfo on intermittent or backout) from comment #68)
> This let browser-chrome's browser_ext_devtools_network.js frequently
> failing. There are also assertions before, but only this test fails:
>
> See the Linux bc16 one in
> https://treeherder.mozilla.org/#/
> jobs?repo=autoland&revision=82f171552905ea7bdbc60688db5b4aad3a3f4b95&filter-
> searchStr=4e1b40c7bb84b37a00fd934a28d025a1dae2e2d0
The startup cache errors are fixed in my staged copy now. The |panel is undefined| message needs further investigation.
Assignee | ||
Comment 73•7 years ago
|
||
As discussed on #developers. There were also failures in tc-M(dt7) of devtools/client/commandline/test/browser_gcli_async.js. This is a bug in the test where they load a subscript with wrong char encoding. Previously, a specific ordering of script cache behavior would ignore the (incorrect) default char encoding and the test would work.
Comment 74•7 years ago
|
||
mozreview-review |
Comment on attachment 8907311 [details]
Bug 1398601 - Add global/non-syntactic prefix to subscript loader cache
https://reviewboard.mozilla.org/r/178978/#review184548
::: js/xpconnect/loader/mozJSSubScriptLoader.cpp:95
(Diff revision 2)
> NS_IMPL_ISUPPORTS(mozJSSubScriptLoader, mozIJSSubScriptLoader)
>
> +#define JSSUB_CACHE_PREFIX(aType) "jssubloader/" aType
> +
> +static void
> +SubscriptCachePath(JSContext* cx, nsIURI* uri, JS::HandleObject targetObj, nsACString& cachePath)
Thank you!
Attachment #8907311 -
Flags: review?(kmaglione+bmo) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 80•7 years ago
|
||
mozreview-review |
Comment on attachment 8907733 [details]
Bug 1398601 - Fix devtools browser_gcli_async test
https://reviewboard.mozilla.org/r/179410/#review184582
Thank you for the patch. This looks good. I just wanted to note that we discussed this on irc and concluded that this was the best route forward, because there's no good spot to force the test encoding to be UTF-8 without impacting other things; and that we don't believe the encoding is relevant to the substance of the test.
Attachment #8907733 -
Flags: review?(ttromey) → review+
Comment 81•7 years ago
|
||
Pushed by tcampbell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1ef5858d537c
Support target objects in js::ExecuteInJSMEnvironment r=jorendorff
https://hg.mozilla.org/integration/autoland/rev/62d3e87c505c
Fix devtools browser_gcli_async test r=tromey
https://hg.mozilla.org/integration/autoland/rev/354b9d0b2101
Add global/non-syntactic prefix to subscript loader cache r=kmag
https://hg.mozilla.org/integration/autoland/rev/675444347ccd
Fix subscript loader when using JSM global sharing r=kmag
https://hg.mozilla.org/integration/autoland/rev/51d0521b16aa
Add testcase for loading subscripts within a JSM r=kmag
https://hg.mozilla.org/integration/autoland/rev/f70469dfd87a
Don't allow NSVO in js::CreateObjectsForEnvironmentChain r=jorendorff
Comment 82•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1ef5858d537c
https://hg.mozilla.org/mozilla-central/rev/62d3e87c505c
https://hg.mozilla.org/mozilla-central/rev/354b9d0b2101
https://hg.mozilla.org/mozilla-central/rev/675444347ccd
https://hg.mozilla.org/mozilla-central/rev/51d0521b16aa
https://hg.mozilla.org/mozilla-central/rev/f70469dfd87a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Assignee | ||
Comment 83•7 years ago
|
||
This fixes failures on non-DIAGNOSTIC_ASSERT builds.
Flags: needinfo?(ryanvm)
Comment 84•7 years ago
|
||
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/ef0f1085d54e
Fix wunused-variable errors when MOZ_DIAGNOSTIC_ASSERT isn't available. a=RyanVM
Comment 85•7 years ago
|
||
Flags: needinfo?(ryanvm)
You need to log in
before you can comment on or make changes to this bug.
Description
•