This seems to be the only solution to bug 757639 and bug 752764. But boy is it going to be some work. And it needs to happen this cycle. When an object is accessed via an Xray wrapper in non-transparent mode, expando properties get defined on the Xray wrapper itself. This becomes a noticeable problem with CPG, because same-origin scripts don't necessarily see the same expando objects like they expect to. We solved this with a crappy workaround for Location objects in bug 739796, but that strategy has become untenable. We need something more robust. We need a way to lookup properties, define properties, and trace a per-origin expando object. So we need either 1 runtime-wide map with two keys (viewer origin, target object) or 1 map per origin. The trickiest question is where the expando objects should live. Here are the options I can think of: 1 - They live in the first compartment created for an origin (or rather, whatever compartment they're first lazily created for). They hold that compartment alive as long as there are any other compartments alive for the given origin. 2 - Same as #1, but the expando objects migrate around to allow compartments to die. 3 - They live in a separate per-origin compartment. This is clean, but potentially memory-intensive. Maybe we could do something special to those compartments to make them cheaper? 4 - They live in a dedicated system-principal compartment. 5 - They live in a dedicated null-principal compartment. Options 4 and 5 eschew a lot of compartment gymnastics, but put lots of cross-origin information in the same compartment. If we're manually curating these objects, maybe the chance that this could leak information is pretty low. Option 5 definitely seems better than option 4, but I'm not sure if it would break something somehow. Thoughts?

Does it make any sense to put the expando in the compartment of the wrapee?

I don't know this stuff at all, but I had an idea for an alternate solution. Currently, each system compartment could have its own Xray wrapper for a given content object. When one system compartment creates an expando on its Xray wrapper, we want that property to be visible to the other system compartments. What if we just did the dumbest possible thing and created the property on all the Xray wrappers? That is, any operation that would normally change (create/update/delete) an expando property on one of these wrappers would have a special hook. The hook would iterate over all system compartments, check if the compartment has a wrapper for the given target object, and do the update to that wrapper. This wouldn't be fast, but I'm guessing we don't care so much about speed here. The thing I like about this is that it doesn't require any non-local reasoning. We aren't breaking any of our existing invariants or doing anything weird.

We don't care about squeezing out every bit of speed, but we do care if we get noticeeable UI lag from this stuff. How long will walking over several hundred compartments as you describe take?

Bill's solution is good in that it punishes the less common cases (expandos on location objects, expandos on cross-origin objects, and chrome expandos on content). But I'd like to avoid it if we can think of something more performant. (In reply to Boris Zbarsky (:bz) from comment #1) > Does it make any sense to put the expando in the compartment of the wrapee? Hm, that simplifies lifetime management. It's a bit scary, but maybe it's ok if we're only accessing these things from special handling code (i.e., they're never exposed to script). Offhand, I can't think of any security issues. So we'd probably want to hang an optional object off the wrappee (lazily allocated once an expando is placed). is this feasible somehow with JSAPI? Defining properties is dynamic, but exposes them to script. Reserved slots seems like what we want, but I don't see any way in JSAPI to make an object grow a reserved slot. Is there one? If so, GC stuff would be much-simplified. The object itself would be an index of expando objects for different origins. It would be nice if we could just make the object a dictionary keyed off of origin, but that would only work if there were a canonical representation of an origin, which I'm not sure there is (since the CheckSameOrigin code has lots of special cases). Alternatively, we could just assume that an object is unlikely to have expandos placed on it from lots of different origins, and just do a dumb linked list via reserved slots. When we want to find our expando object, we grab the underlying object, examine the appropriate slot, and traverse the linked list until we find something that compares same-origin or until we hit the end. This seems like a saner approach to me. So the only real roadblock (modulo security concerns I haven't thought of) is that I don't know how to make a JS object grow reserved slots. Is there such a thing as an unreserved slot?

There is no way to add a reserved slot, but you could just add a reserved slot to all our xpconnect and new-binding objects...

(In reply to Boris Zbarsky (:bz) from comment #5) > There is no way to add a reserved slot, but you could just add a reserved > slot to all our xpconnect and new-binding objects... Is that significant memory overhead? Or is it negligible?

It's 8 bytes per object. I _think_ that's ok, personally. For example, XPCWrappedNative is right now 11 words (44 and 88 bytes on 32-bit and 64-bit respectively), not even counting the JSObject parts.... Another option, of course, is some sort of hashtable on the compartment, indexed by wrappee or something.

(In reply to Boris Zbarsky (:bz) from comment #7) > It's 8 bytes per object. I _think_ that's ok, personally. For example, > XPCWrappedNative is right now 11 words (44 and 88 bytes on 32-bit and 64-bit > respectively), not even counting the JSObject parts.... Yeah, but the DOM objects are largely slim wrappers, right? Anyway, if you're ok with it, then I am. > Another option, of course, is some sort of hashtable on the compartment, > indexed by wrappee or something. Yeah. I really like the reserved slot solution though, because it means that tracing happens automatically.

> Yeah, but the DOM objects are largely slim wrappers, right? Could be. And in new bindings they certainly are, and then they're only about 40 bytes in size. I still think that the one extra slot is probably ok, but ccing some other people who might have opinions.

(In reply to Boris Zbarsky (:bz) from comment #3) > We don't care about squeezing out every bit of speed, but we do care if we > get noticeeable UI lag from this stuff. How long will walking over several > hundred compartments as you describe take? I guess it would depend on how many of the system compartments have wrappers for the object being updated. If very few of them do, then you're just iterating and doing a hashtable lookup for each one. So I would guess that the entire property operation would get at most a few times slower in this case. But if we actually have to update the property in 200 system compartments, then the slowdown would probably be ~200x. Even that wouldn't slow down the UI unless you were doing a lot of them, though. Sticking the expando object in the content compartment seems okay, too, as long as we wrap all the properties on it.

CCing Mano and Mak, so that they know we're working on this.

One issue I've been considering here is how we avoid leaking the consumer compartment (that is to say, the one placing the expandos). When the expandos are placed, we'll allocate the hidden object in the compartment of the target / wrappee, and attach the relevant expandos. Modulo primitives, these will generally be cross-compartment wrappers back to the consumer's compartment. This is mostly good, since when the target goes away the expando objects naturally get collected as well. But what happens if the target should outlive the consumer? This is kind of an inherent problem, because the expandos are supposed to be shared per-origin. So taking the example of system compartments (the primary consumer here), we could have a compartment that wants to go away, but whose expandos also need to be available to the ~100 other system compartments. Here are a couple of things we could do easily: 1 - When an xray wrapper for a compartment goes away, remove all properties on the corresponding expando object that point back to that compartment. 2 - Reference-count the expando object. When all Xray wrappers to it go away, throw it away and let it be GCed. Unfortunately, both of these apply pretty weird and confusing lifetime semantics that I don't think are acceptable. If a consumer pulls an ephemeral Xray wrapper off a DOM tree, places an expando on it, and allows that Xray wrapper to go away, the expando would be lost. This would be a regression from the current state of affairs. Currently, we store expando objects in a map in the consumer compartment private that traces (and cycle-collects) them. What we probably want is to somehow nuke all the cross-compartment wrappers back into the consumer compartments when those compartments want to die. Knowing when a compartment "wants" to die can be tricky though. We could probably use khuey's Nuker in some cases, but it has limitations. It can't be used for content->content, and it currently only knows when windows want to die. Things like sandboxes are another story. Hm.

So, I've been looking into the current situation with lifetime management, and now think that maybe the new world is more or less equivalent to the old one. Currently, the expando object lives in a map in the consumer's compartment private. It is traced during the call to TraceXPConnectRoots, and only goes away once the target WN is explicitly expired by the CC. The target WN is set as a preserved wrapper, so it only goes away when it's CCed, which only happens when the underlying node is removed from the DOM. So, in effect, the expandos (and thus the consumer compartment) have a lifetime bounded below by the lifetime of the target object. This is more or less what things will look like in the new state of affairs, except that the tracing situation will be simplified. Huzzah.

I was working on this in the paris office today. Alex pointed out that this, under the current plan, will break jetpack, because jetpack _wants_ different sets for same-origin code. This used to be handled by using separate compartments, but now that we have compartment-per-global, that's the default. So we probably need some kind of special identifier that we use in addition to the same-origin check. It's not yet clear to me whether this should go on the principal or on the compartment private. Principal makes more sense to me, but it might be more work. What do you think, gabor?

Can I haz access to bug 752764 ? This one is depending on it and I don't know what info I'm missing. Anyway first I really preferred putting one map with two keys and some sane API on the runtime. I'm still not against that one, because that is what really connects all these compartments. About Bill's idea, unfortunately for jetpack addons, even with a few addons around and let's say 15 open tabs, reaching several hundreds of compartments can be very life like... By the way I don't think this should be system compartment specific thing, sandbox can use xray wrappers with any other origins as it's principal, so we can face the same problem there. First I didn't really like what Boris suggested, because it kind of breaks a principal for me. Those expando objects really belong to the xray wrapper that lives in the other compartment. And to me conceptually this is really weird because an object (the wrapper) and its properties (expandos) should be stored in the same compartment or if not at least not in another compartment that has less privileges and should not be able to see them. So now the original object is the only one (and its compartment) who cann't see those properties, yet it would store them (or they will kind of live in it's compartment). I clearly see the advantages of this solution, because in a way this is really practical, it just feels weird conceptually (to me). But on the other hand I see that this can work and I don't adore the one big map on the runtime idea either... (btw wouldn't it be easier this way to find a raw pointers to some object from the system conpartment by offsetting the wrappe?) Jetpack. Personally I'm not happy that jetpack is depending on something like this. I will look into it if it is avoidable. This is a totally unintuitive behavior, that you want to fix here and we should not depend on it. On the other hand I don't want to break everything in jetpack ofc. So jetpack uses sandboxes, we can just add a new flag for this (when my patches are landed this should be even simpler than it is right now) and then just use a specific key in that map (like a pair of <origin, some_pointer_that_is_null_by_default>)

FWIW, I'm most of the way through hacking up Boris' solution. (In reply to Gabor Krizsanits [:krizsa :gabor] from comment #15) > Jetpack. Personally I'm not happy that jetpack is depending on something > like this. I will look into it if it is avoidable. This is a totally > unintuitive behavior, that you want to fix here and we should not depend on > it. Well, it actually seems pretty reasonable to me. Suppose we've got 2 addons that run content scripts. Each one runs with the principal of the page it's running against, but they're different addons, so they really shouldn't see each others' expandos. > On the other hand I don't want to break everything in jetpack ofc. So > jetpack uses sandboxes, we can just add a new flag for this (when my patches > are landed this should be even simpler than it is right now) and then just > use a specific key in that map (like a pair of <origin, > some_pointer_that_is_null_by_default>) What about somehow adding that extra bit of identity information into nsExpandedPrincipal itself? Is that stuff going to land before the branch, assuming I get to your most recent review tomorrow morning?

(In reply to Bobby Holley (:bholley) from comment #16) > Well, it actually seems pretty reasonable to me. Suppose we've got 2 addons > that run content scripts. Each one runs with the principal of the page it's > running against, but they're different addons, so they really shouldn't see > each others' expandos. Ok this makes sense, I forgot about this case... > What about somehow adding that extra bit of identity information into > nsExpandedPrincipal itself? Is that stuff going to land before the branch, > assuming I get to your most recent review tomorrow morning? If you want to store that identity information in principals, it should go into the nsBasePrincipal, since the nsExpandedPrincipal will not be used in every case. Only in some cases like when let's say the content-script wants to XHR to a specific server and modify the page. In simple cases it will just use the principal of the content. I think the patches can be landed quickly from here on, and since it's adding a feature that can be used only from internally it should be relatively safe to land anyway, but let's discuss that after your review.

For the sake of clarity, here is a sample snippet that reproduce jetpack content scripts. We are basically expecting to get different expandos on two sandboxes using the same *content* origin: let Cu = Components.utils; let s1 = Cu.Sandbox(content); s1.window = content; Cu.evalInSandbox("window.foo = 'bar';", s1); let s2 = Cu.Sandbox(content); s2.window = content; // We are expecting to get `undefined` here: Cu.reportError(Cu.evalInSandbox("window.foo", s2)); If we decide that Sandbox contructor with default value will share expandos, we are going to break addons using old SDK versions that would not contains the opt-out option given to Sandbox. Otherwise we are using sandboxes for CommonJS modules too. Here, we are using system principal, so expando are currently shared. It might be cool to keep the same behavior, but I don't think jetpack addons modules are relying on this. Actually, it may even be a good idea to stop sharing them between modules!

I'd also be fine with making expando sharing opt-in for sandboxes. Pre-CPG each sandbox had its own compartment, right? So that would maintain the old behavior.

(In reply to Bobby Holley (:bholley) from comment #19) > I'd also be fine with making expando sharing opt-in for sandboxes. Pre-CPG > each sandbox had its own compartment, right? So that would maintain the old > behavior. You are right. Expandos aren't shared: let Cu = Components.utils; let p = Components.classes["@mozilla.org/systemprincipal;1"]. createInstance(Components.interfaces.nsIPrincipal); let s1 = Cu.Sandbox(p); s1.window = content; Cu.evalInSandbox("window.foo = 'bar';", s1); let s2 = Cu.Sandbox(p); s2.window = content; Cu.reportError(Cu.evalInSandbox("window.foo", s2));

So, I've got this working for WNs, I think. I still need to write some thorough test coverage and special-case sandboxes (which we decided get their own unique expando object in all cases). But hopefully that shouldn't take long. It's possible that this could land before the branch if I finish it tonight (european time), it gets r+ed before the weekend, and I land it monday morning (european time) before people wake up and the merge happens. But that might be pushing it. I think it's also fine just to land after the merge, and backport it immediately. I know peter's been changing some stuff in XrayWrapper, so we should probably coordinate. It probably makes the most sense to land this stuff first, since presumably peter's changes won't be backported (and landing this first keeps the delta between the trunk landing and the aurora landing small). Or, more likely, maybe we don't conflict much if at all. The bigger concern is what to do about nodelist proxies and new dom bindings. These require a refactoring, because currently we don't have separate expando objects for them (they're combined into a single holder object). This refactoring was planned (since, among other things, the current architecture makes it impossible for us to correctly implement getOwnPropertyDescriptor), but might turn out to be big (not sure yet), in which case it might not be desirable on aurora. So we should think about whether we're ok shipping without expando sharing for Nodelists and XHR. The impact is probably pretty limited. But we could also just backport the refactor and be done with it.

I've got patches for this that seem to work (for WNs only - nodelists newbindings not included). They pass all the xpconnect tests, including the ones I wrote specifically for this subject. Pushing to try: https://tbpl.mozilla.org/?tree=Try&rev=6c52b9afe0bb

Added a patch for bug 760118 and pushed to try again: https://tbpl.mozilla.org/?tree=Try&rev=9253c698dff7

(In reply to Bobby Holley (:bholley) from comment #23) > Added a patch for bug 760118 Err, bug 761121.

ray wrappers require that their wrappee be non-slim, so this works out perfectly.

Note: This overloads the naming of some of the existing infrastructure, but the signatures etc are sufficient to disambiguate. The other infrastructure goes away in a subsequent patch. Note: We tag sandbox expandos with their global to make sure that the expandos are never shared between sandboxes. A consequence of this scheme is that an expando from a sandbox to an object will _always_ result in a GC edge back to the sandbox, meaning that the sandbox is always kept alive for the lifetime of the expando target. This could happen before, but only if a non-primitive expando was placed (since the value of the expando would live in the consumer's compartment). We could avoid this edge by using a reference-counted Identity() object instead, but I suspect it's not worth worrying about.

This is more or less just a backout of bug 739796, that caused so much pain. Huzzah!

This is green, so patches attached. Blake, this is a pretty high-priority item, FWIW.

Comment on attachment 629800 [details] [diff] [review] Part 1 - Refactor slim wrapper reserved slots so that we can use the same slot for expando objects in the non-slim case. v1 Review of attachment 629800 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/src/xpcprivate.h @@ +312,5 @@ > + > +inline XPCWrappedNativeProto* GetSlimWrapperProto(JSObject *obj) > +{ > + MOZ_ASSERT(IS_SLIM_WRAPPER(obj)); > + const js::Value &v = js::GetReservedSlot(obj, WRAPPER_MULTISLOT); JS::Value @@ +324,5 @@ > +{ > + MOZ_ASSERT(IS_SLIM_WRAPPER(obj)); > + JS_SetReservedSlot(obj, WRAPPER_MULTISLOT, JSVAL_NULL); > + MOZ_ASSERT(!IS_SLIM_WRAPPER(obj)); > +} Assert that arguments are non-null in all those?

Comment on attachment 629801 [details] [diff] [review] Part 2 - Implement expando object infrastructure for WN Xrays. v1 Review of attachment 629801 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/src/xpcprivate.h @@ +330,5 @@ > +inline void SetExpandoChain(JSObject *obj, JSObject *chain) > +{ > + MOZ_ASSERT(IS_WN_WRAPPER(obj)); > + JS_SetReservedSlot(obj, WRAPPER_MULTISLOT, chain ? JS::ObjectValue(*chain) > + : JSVAL_NULL); JS::ObjectOrNullValue(chain) ::: js/xpconnect/wrappers/XrayWrapper.cpp @@ +177,5 @@ > + if (!chain) { > + XPCWrappedNative *wn = > + static_cast<XPCWrappedNative *>(xpc_GetJSPrivate(target)); > + nsRefPtr<nsXPCClassInfo> ci; > + CallQueryInterface(wn->Native(), getter_AddRefs(ci)); Can this use do_QI? @@ +199,5 @@ > + // anyone else, so we tag it with the sandbox global. > + JSObject *consumerGlobal = js::GetGlobalForObjectCrossCompartment(wrapper); > + bool isSandbox = !strcmp(js::GetObjectJSClass(consumerGlobal)->name, "Sandbox"); > + expandoObject = AttachExpandoObject(cx, target, ObjectPrincipal(wrapper), > + isSandbox ? consumerGlobal : nsnull); A helper for JSObject *consumerGlobal = js::GetGlobalForObjectCrossCompartment(obj); bool isSandbox = !strcmp(js::GetObjectJSClass(consumerGlobal)->name, "Sandbox"); return isSandbox ? consumerGlobal : NULL; ?

Comment on attachment 629802 [details] [diff] [review] Part 3 - Copy expando objects during object transplanting. v1 Review of attachment 629802 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/wrappers/XrayWrapper.cpp @@ +207,5 @@ > > +bool > +CloneExpandoChain(JSContext *cx, JSObject *dst, JSObject *src) > +{ > + MOZ_ASSERT(js::GetContextCompartment(cx) == js::GetObjectCompartment(dst)); js::IsObjectInContextCompartment?

Comment on attachment 629803 [details] [diff] [review] Part 4 - Expose AutoIdVector wrapping. v1 Review of attachment 629803 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsfriendapi.cpp @@ +167,5 @@ > return cx->compartment->wrap(cx, desc); > } > > +JS_FRIEND_API(JSBool) > +JS_WrapAutoIdVector(JSContext *cx, js::AutoIdVector &props) JS::AutoIdVector

Comment on attachment 629800 [details] [diff] [review] Part 1 - Refactor slim wrapper reserved slots so that we can use the same slot for expando objects in the non-slim case. v1 Review of attachment 629800 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/src/xpcpublic.h @@ +80,5 @@ > + > +// NB: This slot isn't actually reserved for us on globals, because SpiderMonkey > +// uses the first N slots on globals internally. The fact that we use it for > +// wrapped global objects is totally broken. But due to a happy coincidence, the > +// JS engine never uses that slot. This still needs fixing though. See bug 760095. Is this true? I thought GLOBAL_FLAGS_WITH_SLOTS saved us here (note that we use it in XPCONNECT_GLOBAL_FLAGS).

Comment on attachment 629802 [details] [diff] [review] Part 3 - Copy expando objects during object transplanting. v1 Review of attachment 629802 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/src/XPCWrappedNative.cpp @@ +1622,5 @@ > return NS_ERROR_OUT_OF_MEMORY; > > + // Expandos from other compartments are attached to the target JS object. > + // Copy them over, and let the old ones die a natural death. > + SetExpandoChain(newobj, nsnull); This mirrors the SetReservedSlot in XPCWrappedNative::FinishInit, right?

Comment on attachment 629804 [details] [diff] [review] Part 5 - Switch WN Xrays to use the new expando infrastructure. v1 Review of attachment 629804 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/wrappers/XrayWrapper.cpp @@ +844,5 @@ > + // in the target compartment. > + if (expando) { > + JSAutoEnterCompartment ac; > + if (!ac.enter(cx, expando) || > + !JS_GetPropertyDescriptorById(cx, expando, id, flags, desc)) Braces around the if body, please. I think the engine's style guide says that the brace actually goes under the 'if'. @@ +963,5 @@ > + if (expando) { > + JSAutoEnterCompartment ac; > + if (!ac.enter(cx, expando) || > + !js::GetPropertyNames(cx, expando, flags, &props)) > + return false; Braces and indentation here.

Comment on attachment 629806 [details] [diff] [review] Part 7 - Remove double-wrapping infrastructure for Location objects. v1 Review of attachment 629806 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/xpconnect/wrappers/WrapperFactory.cpp @@ +383,1 @@ > // * The object is a location object. One of these lines is redundant.

Comment on attachment 629808 [details] [diff] [review] Part 8 - Tests. v1 Review of attachment 629808 [details] [diff] [review]: ----------------------------------------------------------------- rs=mrbkap

(In reply to Blake Kaplan (:mrbkap) from comment #38) > Is this true? I thought GLOBAL_FLAGS_WITH_SLOTS saved us here (note that we > use it in XPCONNECT_GLOBAL_FLAGS). Under the current system, the global slots of the JS engine are supposed to come first, then the user slots. This will ideally change in bug 760095.

(In reply to Blake Kaplan (:mrbkap) from comment #39) > > + SetExpandoChain(newobj, nsnull); > > This mirrors the SetReservedSlot in XPCWrappedNative::FinishInit, right? Not sure I follow. |newobj| comes from JS_CloneObject, which only clones reserved slots for proxies (I wasn't sure why this was the case). This means that its reserved slots end up void (I think), and I wanted to maintain the invariant that the expando slot was never undefined (either null/JSObject for WNs, or double for slim wrappers). FinishInit is never called on transplanted objects AFAICT.

All feedback addressed. I'm just waiting for the stuff in bug 761121 to be resolved and then this can land.

Alright, all ready to go. One last try push: https://tbpl.mozilla.org/?tree=Try&rev=59ca5bf73d4c

Alright, looks good. pushed to m-i: http://hg.mozilla.org/integration/mozilla-inbound/rev/fd7e26627150 http://hg.mozilla.org/integration/mozilla-inbound/rev/a14914f6351e http://hg.mozilla.org/integration/mozilla-inbound/rev/54c4bef6fa3b http://hg.mozilla.org/integration/mozilla-inbound/rev/c1a8d1df547e http://hg.mozilla.org/integration/mozilla-inbound/rev/7b94a8ce02d5 http://hg.mozilla.org/integration/mozilla-inbound/rev/27b07b872924 http://hg.mozilla.org/integration/mozilla-inbound/rev/cfafb4e9cc34 http://hg.mozilla.org/integration/mozilla-inbound/rev/20f5351a2b46

We should set the tracking flag here, since it was set on bug 757639 which I just duped to this bug.

Comment on attachment 629808 [details] [diff] [review] Part 8 - Tests. v1 [Approval Request Comment] Flagging this entire patch stack for m-a approval, per bug 757639 comment 14. It's a big change, but we just branched.

(In reply to Bobby Holley (:bholley) from comment #44) > for WNs, or double for slim wrappers). FinishInit is never called on > transplanted objects AFAICT. Right, all I was saying was that this was maintaining the invariant for WNs that was taken care of by the JS_SetReservedSlot in FinishInit.

(In reply to Blake Kaplan (:mrbkap) from comment #51) > Right, all I was saying was that this was maintaining the invariant for WNs > that was taken care of by the JS_SetReservedSlot in FinishInit. Ah, I see! Yes, that makes sense. :-)

https://hg.mozilla.org/mozilla-central/rev/20f5351a2b46 https://hg.mozilla.org/mozilla-central/rev/cfafb4e9cc34 https://hg.mozilla.org/mozilla-central/rev/27b07b872924 https://hg.mozilla.org/mozilla-central/rev/7b94a8ce02d5 https://hg.mozilla.org/mozilla-central/rev/c1a8d1df547e https://hg.mozilla.org/mozilla-central/rev/54c4bef6fa3b https://hg.mozilla.org/mozilla-central/rev/a14914f6351e https://hg.mozilla.org/mozilla-central/rev/fd7e26627150

Comment on attachment 629808 [details] [diff] [review] Part 8 - Tests. v1 [Triage Comment] Given where we are in the cycle, the possibility of add-on regressions in bug 757639, and our desire to leave bug 650353 in FF15, approving for Aurora 15.

We found a GC hazard in this patch - patch is waiting on review over in bug 763381. I'm going to wait until that lands before landing this on aurora.

Please nominate bug 763381 for aurora approval if that blocks landing this on mozilla-aurora.

Landed this and dependent patches to m-a: http://hg.mozilla.org/releases/mozilla-aurora/rev/e5f9e8f2816f http://hg.mozilla.org/releases/mozilla-aurora/rev/c5fb78e2f25d http://hg.mozilla.org/releases/mozilla-aurora/rev/8b9657fd6e9a http://hg.mozilla.org/releases/mozilla-aurora/rev/bbc9b0b8ce43 http://hg.mozilla.org/releases/mozilla-aurora/rev/135875b9d635 http://hg.mozilla.org/releases/mozilla-aurora/rev/0fcc2bb0d9ab http://hg.mozilla.org/releases/mozilla-aurora/rev/f0bd81f02782 http://hg.mozilla.org/releases/mozilla-aurora/rev/c11a05b374e2 http://hg.mozilla.org/releases/mozilla-aurora/rev/d52872ca5dc5 http://hg.mozilla.org/releases/mozilla-aurora/rev/e1053b2eb16e http://hg.mozilla.org/releases/mozilla-aurora/rev/6f63ee67ea69