1159469 - Fix/Implement JSAPI accessors for ES6 Map/Set for WebIDL maplike/setlike

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Splitting out the required JSAPI changes from bug 1123516 because things are getting messy over there. While there's already been a few updates r+'d, there's still more than needs to be done.

Comment 1

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

One of the problems that hasn't been addressed for MapObject/SetObject is handling crossing compartments, similar to what Date, Object, and others do. Right now, we just call obj->as<MapObject/SetObject>(), which fails for boxed Map/Set objects.

From talking to bz on IRC, one solution is to have the C++ API functions assert ObjectClassIs then call unbox as needed, while leaving the *_impl functions to just assume they have a valid object in 'this' already.

jorendorff, do you have any opinions on this?

Comment 2

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1 (v1, FINAL) - Add public jsapi ES6 Map Delete function; r=jorendorff

Moving patch from bug 1123516

Comment 3

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 2 (v2; FINAL) - Add public jsapi ES6 Set convenience functions; r=jorendorff

Moving patch from bug 1123516

Comment 4

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 3 (v1; FINAL) - Add forEach convenience function for maplike/setlike; r=jorendorff

Moving patch from Bug 1123516

Comment 5

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 4 (v1) - Add get function to JSJitCallArguments

Comment 6

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

From IRC:

<qDot> jorendorff: Got time to discuss bug 1159469 real quick? I've
got the rest of maplike/setlike WebIDL in for review, but it'll be
blocked on unboxing MapObject/SetObject if it passes still.
<jorendorff> qDot: hmmm. OK, suppose we have the API implementation
silently unwrap these things
<jorendorff> qDot: then we need to also rewrap the arguments (into the
map/set's compartment) and the results (into the caller's compartment)
as well...
<jorendorff> qDot: that's doable. is it sufficient in your use case?
or would it be bad?
<qDot> jorendorff: As far as I'm aware, I think that'll do it, but I'm
still pretty new to dealing with this stuff so I can't say for sure.
That sounds like a good path to start down at least though.
<jorendorff> qDot: well, it would be nice if we had a general way of
doing this, across all types, but that would be a big chunk of weird
template code
<jorendorff> qDot: so I guess go ahead.

Comment 7

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Ok, so, following up from IRC...

Trying to implement Comment 6, here's what I've got for the MapGet case right now: https://pastebin.mozilla.org/8834041

Here's an example of binding code that will use that: https://pastebin.mozilla.org/8834040

I think get/set/add are about the most complicated cases for this, as it requires making sure we unwrap the backing object, wrap what we're adding to the backing object into the backing object's compartment, then wrap the result into the caller's compartment.

I'd like to make sure that's what I'm actually doing in the end here, so while I'm somewhat confident in what I've got so far, I'm asking a grown up for help.

Comment 8

[Boris Zbarsky [:bzbarsky]]

OK.  So in the case of the binding consumer as it stands now, the following seem to be true:

1)  The obj and key passed to MapGet would be same-compartment with cx.
2)  The obj may be a cross-compartment wrapper for a Map.

For a set, I expect that the obj, key, and value would all be same-compartment with cx...

In any case, given that I'm not sure what the MapGet impl is trying to do.  It's unwrapping the obj (which makes some sense, since it may be a cross-compartment wrapper).  Then it's wrapping the key into the compartment of obj... but that compartment may not be the same as that of unwrappedObj, right?  Is this missing entering the compartment of unwrappedObj followed by wrapping the key into the _cx_ compartment?  Then, of course, it does the MapObject::get() and then wraps the value into the compartment of cx..  I expect that entering the compartment of unwrappedObj somewhere in here, and exiting it before that last JS_WrapValue call, would make more sense.

From a performance point of view, this whole setup is deeply unsatisfactory.  First, consider the case of Xrays (or other cross-compartment API calls).  We unwrap them in the binding, get the backing map, and wrap it into the Xray compartment.  Then we call MapGet, which unwraps the Xray around the Map, wraps the key into that compartment, does the actual get, then rewraps the result into the Xray compartment. 

Next, the non-Xray case.  Now the only real problem is the no-op JS_WrapValue call, which takes it sweet time to figure out it's a no-op.  Oh, and since bindings don't really trust JSAPI here they would go ahead and try wrapping again.  Though maybe we could remove this part.  The cost of the JS_WrapValue would still be there.

Anyway, apart from the performance concerns this seems plausible if we actually enter the compartment of unwrappedObj somewhere in there.

Comment 9

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Ok, I hadn't put in the same compartment checks yet, I suppose those will help some on the performance.

And would changing obj->compartment()->wrap(cx, &wrappedKey); to unwrappedObj->wrap() do what we expect, instead of entering unwrappedObj's compartment? Isn't that what we'd want anyways if we want the arguments in the compartment of the backing object?

Comment 10

[Boris Zbarsky [:bzbarsky]]

> unwrappedObj->wrap()

You mean unwrappedObj->compartment()->wrap()?

I don't know.  In particular I don't know what happens if you call wrap() on some compartment and pass a JSContext that's not in that compartment.  That's something for Jason to respond to.

Comment 11

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 3 (v2) - Add forEach convenience function for maplike/setlike; r=jorendorff

Updating Patch 3, as the changes (that were unneeded and shouldn't have been in the patch in the first place) to the hosted function name caused breakage pretty much everywhere on try.

Comment 12

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 5 (v1) - Make sure public jsapi Map/Set calls deal with compartments/proxies

Comment 13

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 5 (v2) - Make sure public jsapi Map/Set calls deal with compartments/proxies

Comment 14

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8613140 [details] [diff] [review]
Patch 5 (v2) - Make sure public jsapi Map/Set calls deal with compartments/proxies

Un-r?'ing for the moment while I work on things I talked about on IRC with bz.

Comment 15

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

IRC conversation with bz on fixing patch 5:

<bz> jorendorff, qdot: Right now we end up doing a bunch of silly
JS_Wrap* crud

<bz> jorendorff, qdot: which is not needed in the common case ... and
in practice not needed in the uncommon case

<bz> jorendorff, qdot: since our particular uncommon case then does
its own faster things anyway....

<jorendorff> bz: I don't understand any of that

<bz> which part?

<qDot> bz: Yeah, the wrap at the end of get is still a sore point. :|
* bz is making several claims:

<bz> 1)  JS_WrapObject/Value is ass-slow even when it's a no-op.

<bz> 2)  As a result the bindings code knows to avoid calling it
unless it's really needed.

<bz> That's the story for the same-compartment case.

<bz> The cross-compartment case here is a tail of woe (e.g. we unwrap
to get the reflector, then get its backing obj, then wrap it.  Then we
call one of our API methods, which unwrap the backing obj and do
whatever then wrap the result.

<bz> Er, tale of woe.

<bz> And in particular, for the cross-compartment case we're ensuring
that the JS engine API entry point is called with the cx and obj in
the same compartment.
* bz looks at the current state of the patches.

<bz> So looks like JS::MapGet is only conditionally wrapping, which is
good for perf....

<bz> But it also removed the same-compartment asserts, why?
* bz just doesn't understand the patch 5 in bug 1159469

<bz> What is it expecting in terms of incoming compartments?

<bz> e.g.....

<bz> let's look at JS::SetHas

<bz> How is that supposed to work?

<bz> qdot: ?

<qDot> Just a sec.
* bz feels like we need a few more tests here... ;)

<qDot> Ok, so, with SetHas, we assume the key is already in the
compartment of the context, so we just need to possibly unwrap the
backing object into the compartment also, right?

<bz> So let's say at the callsite to SetHas, obj is a
cross-compartment wrapper for a Set

<bz> and cx, obj, key are all in the same compartment.

<bz> Make sense?

<qDot> Yes.

<bz> OK, what will happen after that point?

<bz> we will unwrap the obj into an actual Set

<qDot> Yes.

<bz> which is now no longer same-compartment with cx or key

<bz> then we will do SetObject::has

<bz> which will look for key

<bz> right?

<qDot> Yeah ok my understanding of how unwrapping works is apparently
wrong.

<bz> Let me try to describe a test that will fail

<bz> Say you have an interface that is setlike<Node>

<bz> And you have this HTML document: <iframe></iframe>

<bz> Now you have script in the parent document that gets a setlike
from the child document and does:

<bz>   setlike.add(frames[0].documentElement)

<bz> This will add the actual documentElement reflector to the Set,
since we will enter the child frame compartment at the add() call

<bz> Now the parent document does:
MySetlikeInterface.prototype.has.call(setlike,
frames[0].documentElement)

<bz> Now we enter the has in the compartment of the parent, with the
this object and the first arg cross-compartment wrappers.

<bz> We extract the backing set and wrap it into the compartment of
the parent.

<bz> We call JS::SetHas with cx, obj, key all in the compartment of
the parent.

<bz> We unwrap the underlying Set.

<bz> And call SetObject::has

<bz> But we pass it a cross-compartment wrapper to
frames[0].documentElement

<bz> What's stored in the set is the reflector.

<bz> So I expect with the code as currently written this will return
false.

<bz> which is clearly not expected.

<bz> (should add a test, etc)

<qDot> I'll go add that case to
https://github.com/qdot/gecko-hg/blob/1123516-maplike/dom/bindings/test/test_bug1123516_maplikesetlikechrome.xul
and see.

<bz> ок

<bz> Er, ok

<qDot> As that's the test case this whole thing is trying to fix, but
I guess it doesn't go far enough.

<bz> You can get similar things with Xrays, if you do the add via Xray
and the get without Xray or vice versa

<qDot> Though before I go that far... So how would I fix this then?

<qDot> 'cause I'm not sure what I can/can't access via wrappers now.

<bz> So I think there are two principled solutions.

<bz> Both solutions require the public JS API to assert that all its
arguments (cx, obj, key) are same-compartment.

<bz> Solution #1 is that we allow that compartment to be whatever we
want.  Then we need to unwrap the obj and if that isn't the identity
operation wrap the other args (key, value) into the compartment of the
underlying object before calling on to the raw thing.

<bz> And on the way back out if the return value has a concept of
compartment wrap it back into the caller compartment.

<bz> Solution #2 is that we require the public JS API to be called in
the compartment of the underlying Map/Set and push all that work off
onto consumers.

<bz> Solution #1 is probably a bit better because it probably plays
nicer with Xray stuff whenever we add Map/Set xrays

<qDot> So multiple reflectors for the same object will always identify
as equal, right?

<bz> There is no such thing

<qDot> Oh, ok. Even better.

<bz> well, not in webidl land

<bz> An object has a single reflector

<bz> and may have cross-compartment wrappers for that reflector.

<bz> Given any compartment X, there is only one object that is in
compartment X and is either a reflector or a cross-compartment wrapper
for a given DOM object.

<bz> In other words, if you start with an object in compartment X,
wrap it into compartment Y, then wrap the result back into compartment
X

<bz> then you get the object you started with

Comment 16

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 5 (v3) - Make sure public jsapi Map/Set calls deal with compartments/proxies

Updated script security to address issues from Comment 15, tried to remove some of the code repetition also.

Comment 17

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Test for Patch 5 are in Bug 1123516 Patch 4. Sorry about the location, but at least a test does exist now. :/

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8615715 [details] [diff] [review]
Patch 5 (v3) - Make sure public jsapi Map/Set calls deal with compartments/proxies

> forEach(const char* funcName, JSContext *cx, HandleObject obj, HandleValue callbackFn, HandleValue thisArg)

The bits here look wrong to me.

If we come in with "obj" a cross-compartment wrapper, then we'll set up args with the callee/args[0]/args[1] in one compartment but "this" in another compartment.  Why do we need the unwrap in this method, exactly?

>+CallObjFunc(RetT(*ObjFunc)(JSContext*, HandleObject), JSContext* cx, HandleObject obj)

Why don't we need to enter the compartment of UnwrappedObj here before we call ObjFunc?

>+        if (obj->is<WrapperObject>()) {

How about just:

  if (obj != unwrappedObj)

?

Same in other places where that test is done.

>+        JS_WrapValue(cx, rval);

JS_WrapValue is fallible.  If it fails, then what?

For that matter, isn't JSCompartment::wrap fallible?

>+        // Caller is expected to deal with return value, so nothing to rewrap here.

I'm not sure what that means.  Why wouldn't we rewrap here?  I think we should, if unwrappedObj != obj.

Comment 19

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Ok, fixing up comments, though I'm going to hold off on re-r?'ing jorendorff 'til this stuff passes :bz's review, should've been treading :jorendorff as a module peer review in the first place, sorry about that.
 
Got a couple of questions (mostly for my own education) about Comment #18...

(In reply to Not doing reviews right now from comment #18)

> >+CallObjFunc(RetT(*ObjFunc)(JSContext*, HandleObject), JSContext* cx, HandleObject obj)
> 
> Why don't we need to enter the compartment of UnwrappedObj here before we
> call ObjFunc?

I'm not exactly sure how compartments come into play in the case of the clear() and size() functions? Once we've got access to the object via unwrapping it, how do those functions interact with the compartment of the object, since neither adds or references anything to the map/set?

> >+        // Caller is expected to deal with return value, so nothing to rewrap here.
> 
> I'm not sure what that means.  Why wouldn't we rewrap here?  I think we
> should, if unwrappedObj != obj.

This has to do with the context of how these are called by maplike/setlike, which may be a little too specific since we're hanging these functions off of the public jsapi. I put the comment in because maplike and setlike return their interface instead of the map/set object, so I was just expecting them to wrap that and ignore this. However, I suppose there may be other people who want to use this outside of that context, so I'll wrap this and just ignore the return value anyways in my own code.

Comment 20

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

> > >+        // Caller is expected to deal with return value, so nothing to rewrap here.
> > 
> > I'm not sure what that means.  Why wouldn't we rewrap here?  I think we
> > should, if unwrappedObj != obj.

Actually, ignore my reply in comment 19, the comment I made in the code is bad. There's nothing /to/ rewrap here since add/set don't return anything at the jsapi level, and this doesn't have to align with the es6 return expectations 'cause we're in c++, not js. I'm just getting my function specifications confused. Removing code comment.

Comment 21

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 5 (v4) - Make sure public jsapi Map/Set calls deal with compartments/proxies

Comment 22

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 5 (v5) - Make sure public jsapi Map/Set calls deal with compartments/proxies

Forgot to remove WrapperObject.h include that is no longer needed.

Comment 23

[Boris Zbarsky [:bzbarsky]]

> I'm not exactly sure how compartments come into play in the case of the clear() and
> size() functions?

They may not for now, but it's just good hygiene.  All the arguments are in the unwrappedObj compartment at that point, so might as well put the cx in it too.

Though looking at clear(), it can throw an exception, no?  And we'd want that exception to be created in the right compartment....  size() looks like it doesn't use cx at all, but I'd rather not depend on that going forward.

> There's nothing /to/ rewrap here

Ah, indeed.  Agreed.

Comment 24

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8616287 [details] [diff] [review]
Patch 5 (v5) - Make sure public jsapi Map/Set calls deal with compartments/proxies

Do we have tests for the forEach behavior over Xrays?  Looking at the impl of forEach, it's not obvious to me that it would actually work correctly when Xrays are involved... but maybe it would.

Past that, I personally would not create scopes for the JSAutoCompartment when there is nothing in the function after the scope, but that's something to check with Jason for this module.

The CallObjFunc overload that has a MutableHandleValue rval _does_ need to wrap that back into the caller compartment, no?

Apart from that and maybe documenting the preconditions/postconditions for these methods in terms of compartments in jsapi.h this looks good.

Comment 25

[Boris Zbarsky [:bzbarsky]]

Oh, and JS_WrapValue vs obj->compartment()->wrap(), but I again assume Jason will handle that.

Comment 26

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

(In reply to Not doing reviews right now from comment #24)

> 
> The CallObjFunc overload that has a MutableHandleValue rval _does_ need to
> wrap that back into the caller compartment, no?

Ok, actually, this breaks over Xrays anyways. Here's a pastebin of the CallObjFunc for iterators with added return value wrapping, and the addition to the chrome test to check it:

https://pastebin.mozilla.org/8836153

In the chrome test, I always get back an opaque object from the iterator CallObjFunc, but I'm not sure why. Map/Set iterator queries create a new iterator object and pass that back, and since we're in the context of the backing object, I'm not sure why we can't get it wrapped back into the caller's compartment?

Comment 27

[Boris Zbarsky [:bzbarsky]]

Probably because we don't have Xray support for iterators, so when we do wrap it back into the caller's compartment we get a non-function-value-property-only wrapper for the iterator object?

Comment 28

[Bobby Holley (:bholley)]

Yes, bug 1023984 and bug 1155700 are still open - I never had time to do them. Happy to mentor that project if somebody's keen on it.

Comment 29

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Ok, well, two questions then:

1) bz: Is it even going to be possible to land bug 1123516 without xray support for iterators now? Just wondering if this can be filed as followup.
2) bholley: How much work do you think those two bugs would be? I'm at least curious about how Xrays work now (since they're not, for me :) ), and if it's doable in a reasonable amount of time I could take a look. Mainly just worried about balancing it against other quarterly goals.

Comment 30

[Boris Zbarsky [:bzbarsky]]

> Is it even going to be possible to land bug 1123516 without xray support 

Yes.  I mean, if people try to do the iteration via Xrays it will fail, but that's OK.  Esp. if it fails by throwing an exception instead of failing by iterating an empty sequence.  If it does the latter, we should make it throw for now.  And yes, a followup to fix.

Comment 31

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 5 (v6) - Make sure public jsapi Map/Set calls deal with compartments/proxies; r=bz

Updated Patch 5 to address comment 24, comment 25, and comment 27. Waiting on review of test coverage in bug 1123516 before asking jorendorff for review.

Comment 32

[Bobby Holley (:bholley)]

(In reply to Kyle Machulis [:kmachulis] [:qdot] (USE NEEDINFO?) from comment #29)
> 2) bholley: How much work do you think those two bugs would be? I'm at least
> curious about how Xrays work now (since they're not, for me :) ), and if
> it's doable in a reasonable amount of time I could take a look. Mainly just
> worried about balancing it against other quarterly goals.

Fixing those two bugs would be an awesome accomplishment, and would complete the last big shortcomings of our Xray layer. People have certainly been asking for it (see bug 1155700), and it would make our frontend code more secure.

As for the time estimate, it's pretty hard to say - each JSXray is a bit of a custom job, so it depends how many wrinkles there end up being. Guessing somewhat wildly, I'd imagine this taking me 2-4 days, and taking 1-3 weeks for a skilled platform hacker that isn't me. Fitzgen recently did some fairly straightforward XrayToJS-ing (for SavedStack), and Tooru did RegExp in bug 1079919.

I'd suggest starting with map/set first, since those are a bit more of a standard case, and then moving on to iterator. The nice thing about doing this is that there are a bunch of examples of how to add xray support for other objects (bug 975042, bug 987669, bug 1014991, and most other deps of bug 914970).

Let me know if you're interested in taking it on and we can talk in more detail.

Comment 33

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Comment on attachment 8617538 [details] [diff] [review]
Patch 5 (v6) - Make sure public jsapi Map/Set calls deal with compartments/proxies; r=bz

Ok. I think this is finally ready for a module reviewer.

Comment 34

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 5 (v7) - Make sure public jsapi Map/Set calls deal with compartments/proxies; r=bz

Removed Xray check from MapObject, since spidermonkey builds can't see xpconnect.

Comment 35

[Jason Orendorff [:jorendorff]]

Comment on attachment 8620808 [details] [diff] [review]
Patch 5 (v7) - Make sure public jsapi Map/Set calls deal with compartments/proxies; r=bz

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

Sorry for the slow review here!

::: js/src/builtin/MapObject.cpp
@@ +2241,5 @@
> +CallObjFunc(RetT(*ObjFunc)(JSContext*, HandleObject), JSContext* cx, HandleObject obj)
> +{
> +    CHECK_REQUEST(cx);
> +    assertSameCompartment(cx, obj);
> +    // Always unwrap, in case this is an xray or cross-compartment wrapper.

Style nit: Always put a blank line before a comment in SM code, unless it's right at the beginning of a block. (Same thing in many other places in this patch, including one spot in a header file.)

@@ +2266,5 @@
> +    // compartment of the unwrapped map/set.
> +    RootedValue wrappedKey(cx, key);
> +    if (obj != unwrappedObj &&
> +        !JS_WrapValue(cx, &wrappedKey))
> +        return false;

Another style nit: When mixing "should I do this?" and "try this; did it fail?" conditions, SM style is like this:

    if (obj != unwrappedObj) {
        if (!JS_WrapValue(cx, &wrappedKey))
            return false;
    }

@@ +2328,5 @@
> +        // If we passed in a wrapper, wrap our key into its compartment now.
> +        if (obj != unwrappedObj &&
> +            !JS_WrapValue(cx, &wrappedKey))
> +            return false;
> +        ret = MapObject::get(cx, unwrappedObj, wrappedKey, rval);

I think the right thing here is to remove the variable `ret` and say:

        if (!MapObject::get(...))
            return false;

If MapObject::get() fails, it's better not to call JS_WrapValue(cx, rval), as the caller must ignore rval afterwards anyway.

@@ +2338,5 @@
>  }
>  
>  JS_PUBLIC_API(bool)
> +JS::MapSet(JSContext *cx, HandleObject obj,
> +           HandleValue key, HandleValue val)

Style nit: Please put all this on a single line (assuming it does fit in 99 columns).

@@ +2355,5 @@
> +        RootedValue wrappedValue(cx, val);
> +        if (obj != unwrappedObj &&
> +            (!JS_WrapValue(cx, &wrappedKey) ||
> +             !JS_WrapValue(cx, &wrappedValue)))
> +            return false;

Style nit: In this case, one option is:

    if (obj != unwrappedObj) {
        if (!JS_WrapValue(cx, foo))
            return false;
        if (!JS_WrapValue(cx, bar))
            return false;
    }

If a condition runs to multiple lines, the consequent must be wrapped in curly braces, so the alternative is:

    if (obj != unwrappedObj) {
        if (!JS_WrapValue(cx, foo) ||
            !JS_WrapValue(cx, bar))
        {
            return false;
        }
    }

Comment 36

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 5 (v8) - Make sure public jsapi Map/Set calls deal with compartments/proxies; r=bz r=jorendorff

Review comments addressed

Comment 37

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/00d135e921a6
https://hg.mozilla.org/integration/mozilla-inbound/rev/ffb604b4d4d8
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc7c27ff1da6
https://hg.mozilla.org/integration/mozilla-inbound/rev/b709a5567a9a
https://hg.mozilla.org/integration/mozilla-inbound/rev/ecd1133c2179

Comment 38

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/00d135e921a6
https://hg.mozilla.org/mozilla-central/rev/ffb604b4d4d8
https://hg.mozilla.org/mozilla-central/rev/bc7c27ff1da6
https://hg.mozilla.org/mozilla-central/rev/b709a5567a9a
https://hg.mozilla.org/mozilla-central/rev/ecd1133c2179