760109 - Make __exposedProps__ more aware of the prototype

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

When returning an array from chrome to content, using methods like .forEach fire a warning about __exposedProps__.

Should the array be created in the context of the webpage of should the array prototype have the correct __exposedProps__ defined?

Comment 1

[Bobby Holley (:bholley)]

CCing sicking and blake. Any idea what we want to do here? Should we require consumers to explicitly add any Array.prototype and Object.prototype methods they want to use to __exposedProps__, or should we whitelist some stuff?

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

if we whitelist stuff, is there any risk that the actions that .forEach takes, like executing getters etc, could be used to gain chrome-level access?

I'd really like to whitelist things and not require them to be enumerated in __exposedProps__, but only if it's safe.

Comment 3

[Bobby Holley (:bholley)]

We've been discussing this over email,  but it's probably worth moving the discussion to the bug. I'll paste what's been said for posterity.

Comment 4

[Bobby Holley (:bholley)]

Blake said:

> Hey Everyone,
>
> We should probably file a bug to hash this out for real, but here are my
> thoughts about COWs...
>
> So, __exposedProps__ on COWs is probably here to stay for better or
> worse. Our goal, in that case, should be to make these objects appear as
> "normal" as possible. The problem is that things get pretty complicated
> pretty fast, especially if someone wants to expose an object with a
> custom prototype to content. One idea I had that was impossible before
> CPG was, when *getting* a function off of an object, do the following:
>  - If the property is in exposed props: return the property.
>  - Else, follow the prototype chain until the first builtin prototype
> (Object.prototype, Array.prototype, etc.). Once we hit it, do the rest
> of the lookup on the content window's builtin prototype. That way,
> toString, toSource and all the rest of the functions exist (and come
> from content, negating a bunch of security concerns) and they're usable.
>
> For setting, if the object is settable in __exposedProps__ it gets set,
> otherwise we throw.
>
> I don't know if that behavior is too odd, though. No matter what, I
> think we need an easy way to create objects in chrome that behave, by
> default, like true content objects, the no expandos rule might need to
> be changed, too.
>
> I got around the COW being too odd problem before this by introducing
> Components.utils.{createObjectIn(window),makeObjectPropsNormal(obj)}.
> See
> http://mxr.mozilla.org/mozilla-central/source/dom/base/ConsoleAPI.js#98
> and
> http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/content/extensions-content.js#162
> for examples. I think that API is basically garbage, but it was the
> fastest way to victory back in the day. My crazy COW idea was impossible
> because there wasn't any single prototype that we could follow (since
> the same COW could be exposed to multiple windows in the same
> compartment). Now it's possible, but still odd.
>
> What do you think?

Comment 5

[Bobby Holley (:bholley)]

I said:

> What happens when things on Object.prototype and Array.prototype get invoked on a
> proxy? Do they deal appropriately? If so, this sounds like a reasonable plan,
> though I'm not the expert here.

Comment 6

[Bobby Holley (:bholley)]

Blake said:

> I believe that everything should Just Work (TM) for arrays. I'm less
> sure what happens for functions, we'd need to test.
>
> Waldo and Luke were discussing alternative ideas, though. The core
> question that I have is: is there a clean way of making chrome objects
> that are exposed to content look like content objects from content by
> default?
>
> -Blake

Comment 7

[Bobby Holley (:bholley)]

Luke said:

> Instead of viewing this situation as "exposing chrome objects to content"
> which seems to produce all sorts of ontological problems, could we instead
> take the tack: there is a content object that is being implemented by the
> browser (host object in ES parlance?).  Maybe the implementation is C++
> (via JSNative, JSClass, etc) or maybe it is implemented in JS.  That basic
> philosophy leads to a clear separation of "the content object being
> implemented" and "the chrome JS implementation".  It also suggests that the
> tool we use is scripted proxy handlers.  If these somehow fall short, perhaps
> we could extend scripted proxy handlers -- analogous to how we extended
> js::ProxyHandler -- with chrome-only features.
>
> Did I wave my hands suggestively enough?
>
> Cheers,
> Luke

Comment 8

[Bobby Holley (:bholley)]

> Luke said:
> 
> That basic
> > philosophy leads to a clear separation of "the content object being
> > implemented" and "the chrome JS implementation".  It also suggests that the
> > tool we use is scripted proxy handlers.

I don't follow this part.

Comment 9

[Luke Wagner [:luke]]

(In reply to Bobby Holley (:bholley) from comment #8)
Scripted proxies would allow a clear separation of "the object" and "the implementation of the object".

Comment 10

[Bobby Holley (:bholley)]

And why is it helpful that they be scripted, exactly? Are you proposing that we implement COWs with scripted proxies, or that somebody who wants to exposed an object to content should write a scripted proxy?

Comment 11

[Luke Wagner [:luke]]

I'm talking about the case (which Blake was talking about on Friday and in comment 4, but I'm missing the general picture here) where we want to expose a new thing to content written in (chrome) JS.

Comment 12

[Bobby Holley (:bholley)]

(In reply to Luke Wagner [:luke] from comment #11)
> I'm talking about the case (which Blake was talking about on Friday and in
> comment 4, but I'm missing the general picture here) where we want to expose
> a new thing to content written in (chrome) JS.

So the suggestion is that chrome expose a scripted proxy instead of a cross-compartment wrapper to the actual object?

Comment 13

[Luke Wagner [:luke]]

Yes.  As in the original email, I'm not saying I fully understand the problem, just that it seems to make sense and I wanted to know if it could work.

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

I think part of the problem here is that there are two problems that I'm helpfully (or not) conflating into one:
  * Implementing interfaces (e.g. DOM things) in JS and exposing that to content.
  * Random objects that happen to "fall" from chrome to content (or, more generally, the case where we want to expose some functionality that isn't an IDL interface/DOM thing. This is the case for console and the InstallTrigger object).

For the first case, in order to get things like webapi behaviors correct, we're going to have to have some sort of layer that does the Right Thing (TM). So this is more of a question about the second case. This is also a use-case that extension authors are likely to run into. The trick is that this has to be "safe" by default and as correct as possible by default.

Comment 15

[Luke Wagner [:luke]]

Even for the second case, scripted proxies seem attractive since the make a strong separation between content and chrome.  So, instead of trying to create a chrome function and somehow make it sorta look like it's in content except that when you run it, it's in chrome, you would just write your chrome function and then make it the handler of a function proxy that lives in content.  Perhaps this will stress script proxy handlers' ability to look and feel like real functions/objects, but that seems good since it will force us to work out the kinks (like transparent ccws already forced us to do to with non-scripted proxies).

Comment 16

[[:fabrice] Fabrice Desré]

Attachment: patch

This patch adds a createArrayIn() that behave like createObjectIn(). I understand this is not the clean, long term solution we want, but it allows us to remove all warnings that we currently have.

Comment 17

[Blake Kaplan (:mrbkap) (inactive)]

Fabrice, would you mind filing a new bug for your patch? I'll give it r+ there and we can get it checked in.

Comment 18

[[:fabrice] Fabrice Desré]

(In reply to Blake Kaplan (:mrbkap) from comment #17)
> Fabrice, would you mind filing a new bug for your patch? I'll give it r+
> there and we can get it checked in.

I moved it to bug 766378

Comment 19

[Bobby Holley (:bholley)]

I've got patches that pass XPConnect tests. Pushing to try:
https://tbpl.mozilla.org/?tree=Try&rev=00dfcc3d3734

Comment 20

[Bobby Holley (:bholley)]

Orange. Made some fixes and pushed again:
https://tbpl.mozilla.org/?tree=Try&rev=74fd0356d673

Comment 21

[Bobby Holley (:bholley)]

This looks green. Uploading patches and flagging for review.

Comment 22

[Bobby Holley (:bholley)]

Attachment: Part 1 - Don't force wrappers to use the wrapped proto if they decide to use something different. v1

Comment 23

[Bobby Holley (:bholley)]

Attachment: Part 2 - Add an API to lookup proto key via standard prototype objects and vice-versa. v1

Comment 24

[Bobby Holley (:bholley)]

Attachment: Part 3 - When COWing objects with standard prototypes, use the prototype in the home compartment instead. v2

Comment 25

[Bobby Holley (:bholley)]

Attachment: Part 4 - Introduce an explicit ChromeObjectWrapper. v1

For now it's identical to ChromeObjectWrapperBase. Custom behavior comes in the next patch.

Comment 26

[Bobby Holley (:bholley)]

Attachment: Part 5 - Override traps in ChromeObjectWrapper to bounce lookups to the local prototype chain. v2

Comment 27

[Bobby Holley (:bholley)]

Attachment: Part 6 - Tests. v2

Comment 28

[Al Billings [:abillings - ex-MoCo]]

Blake, can you get these reviewed or find someone else to review them? These have been waiting two weeks now.

Comment 29

[Lukas Blakk [:lsblakk] use ?needinfo]

[Triage comment]
This came up in ESR triage, tracking for beta/aurora/esr

Comment 30

[Blake Kaplan (:mrbkap) (inactive)]

I'm going to get to this tomorrow.

Comment 31

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 639015 [details] [diff] [review]
Part 1 - Don't force wrappers to use the wrapped proto if they decide to use something different. v1

Review of attachment 639015 [details] [diff] [review]:
-----------------------------------------------------------------

r=me assuming you've audited the places where we create wrapper to ensure that we use the right prototypes there.

Comment 32

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 639016 [details] [diff] [review]
Part 2 - Add an API to lookup proto key via standard prototype objects and vice-versa. v1

Review of attachment 639016 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsobj.cpp
@@ +4303,5 @@
> +JSProtoKey
> +js_IdentifyClassPrototype(JSObject *obj)
> +{
> +    JSObject &global = obj->global();
> +    for (uint32_t key = JSProto_Null + 1; key < JSProto_LIMIT; ++key) {

Instead of searching like this, why can't you use obj's class's proto key?

Comment 33

[Bobby Holley (:bholley)]

(In reply to Blake Kaplan (:mrbkap) from comment #31)
> r=me assuming you've audited the places where we create wrapper to ensure
> that we use the right prototypes there.

I can't speak for other SM consumers, but |wrapper| will always come directly from wrapObjectCallback, so in Gecko that means WrapperFactory::Rewrap. And there we unconditionally used |wrappedProto| as the proto until the changes made in this bug.

Comment 34

[Bobby Holley (:bholley)]

(In reply to Blake Kaplan (:mrbkap) from comment #32)
> Instead of searching like this, why can't you use obj's class's proto key?

Right, duh.

I think I did it the dumb way because the proto shares the jsclass with instances, so having the key doesn't necessarily make a proto. But we can of course use the key from the jsclass as a hint for where to look. Attaching an updated patch.

Comment 35

[Bobby Holley (:bholley)]

Attachment: Part 2 - Add an API to lookup proto key via standard prototype objects and vice-versa. v2

This patch also does some minor rebasing over the constantly changing rooting/handle API.

Comment 36

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 639017 [details] [diff] [review]
Part 3 - When COWing objects with standard prototypes, use the prototype in the home compartment instead. v2

Review of attachment 639017 [details] [diff] [review]:
-----------------------------------------------------------------

I'd like a comment somewhere explaining explicitly how the prototype chain works here... Just to make sure I have it right we have for chrome object co:

COW(co) -> COW(co.__proto__) -> COW(co.__proto__) -> standard prototype

::: js/xpconnect/wrappers/WrapperFactory.cpp
@@ +301,5 @@
>      bool usingXray = false;
>  
> +    // By default we use the wrapped proto of the underlying object as the
> +    // prototype for our wrapper, but we may select something different below.
> +    JSObject *wrapperProto = wrappedProto;

Please choose a different name for one of these variables. Having one character of difference is a little too confusing for me.

@@ +370,5 @@
>                                          ComponentsObjectPolicy>::singleton;
>          } else {
>              wrapper = &FilteringWrapper<CrossCompartmentSecurityWrapper,
>                                          ExposedPropertiesOnly>::singleton;
> +            // If the prototype of the chrome object being wrapped is a prototype

Uber-nit: blank line before the comment.

@@ +376,5 @@
> +            // that we can safely take advantage of things like .forEach().
> +            JSProtoKey key = JSProto_Null;
> +            JSObject *unwrappedProto = NULL;
> +            if (wrappedProto && IsCrossCompartmentWrapper(wrappedProto) &&
> +                (unwrappedProto = Wrapper::wrappedObject(wrappedProto))) {

Why make a distinction between the various Object and Function.prototypes? i.e. why so many &&s?

Comment 37

[Bobby Holley (:bholley)]

(In reply to Blake Kaplan (:mrbkap) from comment #36)
> > +            // that we can safely take advantage of things like .forEach().
> > +            JSProtoKey key = JSProto_Null;
> > +            JSObject *unwrappedProto = NULL;
> > +            if (wrappedProto && IsCrossCompartmentWrapper(wrappedProto) &&
> > +                (unwrappedProto = Wrapper::wrappedObject(wrappedProto))) {
> 
> Why make a distinction between the various Object and Function.prototypes?
> i.e. why so many &&s?

The first handles the fact that wrappedProto may be null (if we're wrapping an object with NULL as the given prototype).

The second check _might_ be unnecessary given the current setup (i.e. wrappedProto will probably always be wrapped), but I'd sure like to make sure given that we subsequently call Wrapper::wrappedObject (which asserts that we're a wrapper).

Or maybe I'm misunderstanding your concern? I didn't grok the first sentence.

Comment 38

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to Bobby Holley (:bholley) from comment #37)
> Or maybe I'm misunderstanding your concern? I didn't grok the first sentence.

It isn't terribly important. Basically, I don't see why we don't do "if (wrapped) unwrap()" and operate on that. If we somehow end up with a content object with a chrome prototype (which should never happen, but still), I think we should still redirect the proto chain.

Comment 39

[Bobby Holley (:bholley)]

(In reply to Blake Kaplan (:mrbkap) from comment #38)
> (In reply to Bobby Holley (:bholley) from comment #37)
> > Or maybe I'm misunderstanding your concern? I didn't grok the first sentence.
> 
> It isn't terribly important. Basically, I don't see why we don't do "if
> (wrapped) unwrap()" and operate on that. If we somehow end up with a content
> object with a chrome prototype (which should never happen, but still), I
> think we should still redirect the proto chain.

But that's exactly what we're doing. We're not checking if the leaf object is a wrapper, we're checking if the proto is a wrapper. And I think we very much should do that if we're going to do this prototype munging stuff.

Are you able to get to the rest of the reviews soon? We're really close and it's important to get this landed everywhere. :-)

Comment 40

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 639019 [details] [diff] [review]
Part 5 - Override traps in ChromeObjectWrapper to bounce lookups to the local prototype chain. v2

Review of attachment 639019 [details] [diff] [review]:
-----------------------------------------------------------------

So, this works, but I'd like to see a followup on not doing the standard prototype lookup twice.

Also, while we're here, I think (either in this bug or a followup) that we need to allow content-side expandos on COWs so that COWs act exactly like content objects from the content side. I was kind of hoping that this bug could be used to kill off Components.utils.createObjectIn/makeObjectPropsNormal. Basically, we need COWs to behave exactly like content objects from content's point of view, except for behaviors explicitly let through by __exposedProps__.

Comment 41

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 639020 [details] [diff] [review]
Part 6 - Tests. v2

rs=me

Comment 42

[Bobby Holley (:bholley)]

(In reply to Blake Kaplan (:mrbkap) from comment #40)
> So, this works, but I'd like to see a followup on not doing the standard
> prototype lookup twice.

Filed bug 778086 (and bug 778085).

> Also, while we're here, I think (either in this bug or a followup) that we
> need to allow content-side expandos on COWs so that COWs act exactly like
> content objects from the content side. I was kind of hoping that this bug
> could be used to kill off
> Components.utils.createObjectIn/makeObjectPropsNormal. Basically, we need
> COWs to behave exactly like content objects from content's point of view,
> except for behaviors explicitly let through by __exposedProps__.

Filed bug 778089.

Comment 43

[Bobby Holley (:bholley)]

Pushed to m-i:
http://hg.mozilla.org/integration/mozilla-inbound/rev/cfd51eea1b4c
http://hg.mozilla.org/integration/mozilla-inbound/rev/fff143d8b00f
http://hg.mozilla.org/integration/mozilla-inbound/rev/3f534f828e6e
http://hg.mozilla.org/integration/mozilla-inbound/rev/e63099f7911c
http://hg.mozilla.org/integration/mozilla-inbound/rev/09ec57dbdfec
http://hg.mozilla.org/integration/mozilla-inbound/rev/0394338f67b9

Comment 44

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I'm getting occasional compartment mismatches on startup from this patch. It happens at this line:

http://hg.mozilla.org/integration/mozilla-inbound/file/a5b21cffeebf/js/xpconnect/wrappers/WrapperFactory.cpp#l407

The problem seems to be that unwrappedProto is not in the same compartment as obj.

I don't have any patches applied, but it may have something to do with my profile. I'll try to keep things in a state where I can reproduce. Let me know if you need more info.

Comment 45

[Bobby Holley (:bholley)]

Thanks bill. I filed and posted a patch for bug 778409. I'm curious about what chrome code is running at startup that manages to trigger this case, but we certainly shouldn't crash.

Comment 46

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0394338f67b9
https://hg.mozilla.org/mozilla-central/rev/09ec57dbdfec
https://hg.mozilla.org/mozilla-central/rev/e63099f7911c
https://hg.mozilla.org/mozilla-central/rev/3f534f828e6e
https://hg.mozilla.org/mozilla-central/rev/fff143d8b00f
https://hg.mozilla.org/mozilla-central/rev/cfd51eea1b4c

Comment 47

[Lukas Blakk [:lsblakk] use ?needinfo]

As this is being tracked for 15/16 we'd be looking to see this uplifted to branches if someone can do the nomination and provide the risk assessment to landing.

Comment 48

[Lukas Blakk [:lsblakk] use ?needinfo]

Tracking this for the ESR10 release alongside 15, please confirm that this can land cleanly on that branch and nominate for uplift to that branch as well.  See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more details if needed.

Comment 49

[Bobby Holley (:bholley)]

Comment on attachment 639019 [details] [diff] [review]
Part 5 - Override traps in ChromeObjectWrapper to bounce lookups to the local prototype chain. v2

Flagging this patch stack for aurora and beta approval. It applies with some minor rebasing to both. There was only one non-trivial change in the rebase, and I'll flag for review on that interdiff.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): it's complicated.
User impact if declined: Twofold:

1 - bug 768101.

2 - In FF15, extension developers will start seeing a warning about missing __exposedProps__ on chrome objects exposed to content, via bug 760070. Without the patches in this bug, they'll see this warning everything they do things like .forEach on chrome arrays. This will cause unnecessary developer pain and overly-broad __exposedProps__ declarations in extension, because by the time __exposedProps__ actually becomes mandatory, those uses will actually become ok. 

Testing completed (on m-c, etc.): Baked on m-c.
Risk to taking this patch (and alternatives if risky): Medium risk.  
String or UUID changes made by this patch: none

Comment 50

[Bobby Holley (:bholley)]

Attachment: beta interdiff

Flagging luke for review on the interdiff I had to make when backporting this stuff to beta.

Comment 51

[Bobby Holley (:bholley)]

(In reply to Lukas Blakk [:lsblakk] from comment #48)
> Tracking this for the ESR10 release alongside 15, please confirm that this
> can land cleanly on that branch and nominate for uplift to that branch as
> well.  See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process
> for more details if needed.

Given that ESR is pre-cpg, landing this bug there kind of gives me the creeps. I think it should still be ok, but there are lots of cases (multiple standard prototypes in a single compartment) where whatever behavior we choose will be completely untested.

Comment 52

[Luke Wagner [:luke]]

Comment on attachment 648734 [details] [diff] [review]
beta interdiff

This patch looks good.  Btw, on trunk, there are several sources where we know compartment->maybeGlobal() is non-null:
 - obj->global() (since objects mark their parents)
 - cx->global() (since the context's current scope will be marked which will mark the global)

Comment 55

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 639019 [details] [diff] [review]
Part 5 - Override traps in ChromeObjectWrapper to bounce lookups to the local prototype chain. v2

Please go ahead and land on Aurora so we can get some bake time there before landing to Beta, I'd like to see Luke's feedback on the beta interdiff before approving.

Comment 56

[Bobby Holley (:bholley)]

(In reply to Lukas Blakk [:lsblakk] from comment #55)
> Please go ahead and land on Aurora so we can get some bake time there before
> landing to Beta, I'd like to see Luke's feedback on the beta interdiff
> before approving.

Luke already r+ed the interdiff.

Comment 57

[Bobby Holley (:bholley)]

Pushed to m-a along with bug 778409:
http://hg.mozilla.org/releases/mozilla-aurora/rev/2bfad756dbc0
http://hg.mozilla.org/releases/mozilla-aurora/rev/5929ae1d45ce
http://hg.mozilla.org/releases/mozilla-aurora/rev/f3675f4d2c37
http://hg.mozilla.org/releases/mozilla-aurora/rev/3ca7effdcc23
http://hg.mozilla.org/releases/mozilla-aurora/rev/39b5e3d7f323
http://hg.mozilla.org/releases/mozilla-aurora/rev/0a48148c39e6
http://hg.mozilla.org/releases/mozilla-aurora/rev/064eb8de0ad7

Comment 58

[Lukas Blakk [:lsblakk] use ?needinfo]

(In reply to Bobby Holley (:bholley) from comment #56)
> (In reply to Lukas Blakk [:lsblakk] from comment #55)
> > Please go ahead and land on Aurora so we can get some bake time there before
> > landing to Beta, I'd like to see Luke's feedback on the beta interdiff
> > before approving.
> 
> Luke already r+ed the interdiff.

Oops, my bad. Also regarding ESR, I think we should go ahead and land for complete coverage and we'll monitor the Enterprise mailing list for any potential fallout from our users.

Comment 59

[Bobby Holley (:bholley)]

I'm out until thursday, but I've already got the patches ready, so with my few minutes of wifi I'm pushing them so as to increase the bake time (since they're somewhat risky). I apologize in advance if there's any fallout I'm not around the handle - please back out if that's the case:

http://hg.mozilla.org/releases/mozilla-beta/rev/f1ab32e4dcf2
http://hg.mozilla.org/releases/mozilla-beta/rev/b26ca48fdd78
http://hg.mozilla.org/releases/mozilla-beta/rev/02ea1e45600d
http://hg.mozilla.org/releases/mozilla-beta/rev/96257b728a3b
http://hg.mozilla.org/releases/mozilla-beta/rev/56a81ccc6e3e
http://hg.mozilla.org/releases/mozilla-beta/rev/e8af884321f6
http://hg.mozilla.org/releases/mozilla-beta/rev/5514b1c32253

Comment 60

[Ioana (away)]

The automated test that verified this fix is passing (mochitest/chrome/js/xpconnect/tests/chrome/test_bug760109.xul):
https://tbpl.mozilla.org/php/getParsedLog.php?id=14258029&full=1&branch=mozilla-central.

Please let me know if you need me to verify this issue any other way.

Comment 61

[Lukas Blakk [:lsblakk] use ?needinfo]

Bholley, can you do the ESR landing this week? We'll be shopping out next week's build as a quasi-beta and we need this in.

Comment 62

[Lukas Blakk [:lsblakk] use ?needinfo]

Still looking for an ESR nomination & landing here.

Comment 63

[Bobby Holley (:bholley)]

Pushed to esr10:
remote:   https://hg.mozilla.org/releases/mozilla-esr10/rev/5b9415fd0acd
remote:   https://hg.mozilla.org/releases/mozilla-esr10/rev/0a669b640cc0
remote:   https://hg.mozilla.org/releases/mozilla-esr10/rev/19067f5e319f
remote:   https://hg.mozilla.org/releases/mozilla-esr10/rev/5fbc397dca3d
remote:   https://hg.mozilla.org/releases/mozilla-esr10/rev/e2d2ecb490ec
remote:   https://hg.mozilla.org/releases/mozilla-esr10/rev/8b852b1b5b3d

Comment 64

[Bobby Holley (:bholley)]

Doh! The source location where js::GetGeneric ends up in in esr10 is #ifdef DEBUG (which it isn't on the other branches). And IsCrossCompartmentWrapper wasn't yet JS_FRIEND_API (which breaks windows).

Pushing a bustage fix here. I've got to head out for PTO now, unfortunately, so if this doesn't work feel free to back out if the fix isn't obvious.

https://hg.mozilla.org/releases/mozilla-esr10/rev/e4abf9153668

Comment 65

[Lukas Blakk [:lsblakk] use ?needinfo]

Looks like the bustage fix worked, thanks Bobby.

Comment 66

[Ioana (away)]

Marking with [qa-] since this bug fix comes with an automated test that verifies it - js/xpconnect/tests/chrome/test_bug760109.xul.

Comment 67

[Ryan VanderMeulen [:RyanVM]]

Test pushed:
https://hg.mozilla.org/mozilla-central/rev/3ba2cd25663b