1254893 - [jsdbg2] Synthesize Debugger.Scripts and Debugger.Sources for wasm

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Shu-yu Guo [:shu]]

The dream is to have as much of the existing devtools to work out of the box with wasm as we can. To that end we'll try to synthesize various Debugger wrappers to transparently support wasm so that devtools JS doesn't need to treat wasm differently.

This is the very first part: synthesize enough to display placeholder source text for wasm modules.

Comment 1

[Shu-yu Guo [:shu]]

Attachment: Prep Debugger.Script for a tagged union referent.

Comment 2

[Shu-yu Guo [:shu]]

Attachment: Prep Debugger.Source for a tagged union referent.

Comment 3

[Shu-yu Guo [:shu]]

Attachment: Keep a list of wasm::Modules per compartment.

Comment 4

[Shu-yu Guo [:shu]]

Attachment: Synthesize Debugger.Scripts for wasm modules and find them via findScripts.

Comment 5

[Shu-yu Guo [:shu]]

Attachment: Synthesize Debugger.Sources for wasm modules.

Comment 6

[Shu-yu Guo [:shu]]

Attachment: Display placeholder text for synthesized Debugger.Sources.

Comment 7

[Shu-yu Guo [:shu]]

Attachment: Add a .format property on Debugger.Script and Debugger.Source.

Comment 8

[Shu-yu Guo [:shu]]

I'll write tests tomorrow. I am tired now.

Comment 9

[Shu-yu Guo [:shu]]

Example output right now:

shu@cz ~/m/c/j/s/Debug (dbg-wasm)
% cat test.js
var g = newGlobal();
g.eval(`o = Wasm.instantiateModule(wasmTextToBinary('(module (func) (export "" 0))'));`);
var dbg = new Debugger(g);
var scripts = dbg.findScripts();
for (let script of scripts) {
    if (script.format === "wasm")
        print(script.source.text);
}
                                                                                                                                                                
shu@cz ~/m/c/j/s/Debug (dbg-wasm)
% dist/bin/js test.js
.--.      .--.   ____       .-'''-. ,---.    ,---.
|  |_     |  | .'  __ `.   / _     \|    \  /    |
| _( )_   |  |/   '  \  \ (`' )/`--'|  ,  \/  ,  |
|(_ o _)  |  ||___|  /  |(_ o _).   |  |\_   /|  |
| (_,_) \ |  |   _.-`   | (_,_). '. |  _( )_/ |  |
|  |/    \|  |.'   _    |.---.  \  :| (_ o _) |  |
|  '  /\  `  ||  _( )_  |\    `-'  ||  (_,_)  |  |
|    /  \    |\ (_ o _) / \       / |  |      |  |
`---'    `---` '.(_,_).'   `-...-'  '--'      '--'

Comment 10

[Jim Blandy :jimb]

Comment on attachment 8728295 [details] [diff] [review]
Prep Debugger.Script for a tagged union referent.

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

::: js/src/vm/Debugger.cpp
@@ +4905,5 @@
> +//   2. A wasm JSFunction, denoting a synthesized wasm function script.
> +//      NYI!
> +using DebuggerScriptReferent = Variant<JSScript*, WasmModuleObject*>;
> +
> +// Very very scary. For internal use only.

Comments like this are unhelpful. And this function is perfectly well-typed anyway.

@@ +5049,5 @@
> +    CallArgs args = CallArgsFromVp(argc, vp);                                       \
> +    RootedObject obj(cx, DebuggerScript_check(cx, args.thisv(), fnname));           \
> +    if (!obj)                                                                       \
> +        return false;                                                               \
> +    Rooted<DebuggerScriptReferent> referent(cx, GetScriptReferent(obj))

This is possible because we #included "js/GCVariant.h", from bug 1254453, right?

Comment 11

[Jim Blandy :jimb]

(In reply to Shu-yu Guo [:shu] from comment #9)
> Example output right now:

Oh, I love it.

Comment 12

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to Shu-yu Guo [:shu] from comment #9)
> .--.      .--.   ____       .-'''-. ,---.    ,---.
> |  |_     |  | .'  __ `.   / _     \|    \  /    |
> | _( )_   |  |/   '  \  \ (`' )/`--'|  ,  \/  ,  |
> |(_ o _)  |  ||___|  /  |(_ o _).   |  |\_   /|  |
> | (_,_) \ |  |   _.-`   | (_,_). '. |  _( )_/ |  |
> |  |/    \|  |.'   _    |.---.  \  :| (_ o _) |  |
> |  '  /\  `  ||  _( )_  |\    `-'  ||  (_,_)  |  |
> |    /  \    |\ (_ o _) / \       / |  |      |  |
> `---'    `---` '.(_,_).'   `-...-'  '--'      '--'

https://youtu.be/Mh5LY4Mz15o?t=3s

Comment 13

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8728301 [details] [diff] [review]
Add a .format property on Debugger.Script and Debugger.Source.

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

Drive-by nit (the best kind of nit): needs docs.

Comment 14

[Shu-yu Guo [:shu]]

(In reply to Jim Blandy :jimb from comment #10)
> Comment on attachment 8728295 [details] [diff] [review]
> Prep Debugger.Script for a tagged union referent.
> 
> Review of attachment 8728295 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> @@ +5049,5 @@
> > +    CallArgs args = CallArgsFromVp(argc, vp);                                       \
> > +    RootedObject obj(cx, DebuggerScript_check(cx, args.thisv(), fnname));           \
> > +    if (!obj)                                                                       \
> > +        return false;                                                               \
> > +    Rooted<DebuggerScriptReferent> referent(cx, GetScriptReferent(obj))
> 
> This is possible because we #included "js/GCVariant.h", from bug 1254453,
> right?

That's right. I'll land the other bug with this one.

Comment 15

[Terrence Cole [:terrence]]

Comment on attachment 8728297 [details] [diff] [review]
Keep a list of wasm::Modules per compartment.

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

Looks great!

Comment 16

[Luke Wagner [:luke]]

Comment on attachment 8728297 [details] [diff] [review]
Keep a list of wasm::Modules per compartment.

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

Perfect, thanks!

Comment 17

[Shu-yu Guo [:shu]]

Attachment: Synthesize Debugger.Scripts for wasm modules and find them via findScripts.

Comment 18

[Shu-yu Guo [:shu]]

Attachment: Synthesize Debugger.Sources for wasm modules.

Comment 19

[Shu-yu Guo [:shu]]

Attachment: Display placeholder text for synthesized Debugger.Sources.

Comment 20

[Shu-yu Guo [:shu]]

Attachment: Add a .format property on Debugger.Script.

Debugger.Source can be distinguished via .introductionType == "wasm", so
.format isn't needed.

Comment 21

[Shu-yu Guo [:shu]]

Attachment: Support wasm for most of the Debugger.Source properties.

Comment 22

[Shu-yu Guo [:shu]]

Attachment: Fire onNewScript for new wasm modules.

Comment 23

[Shu-yu Guo [:shu]]

Attachment: Perfunctory tests.

These really aren't very good, but I don't want to put too much effort into
testing something that's moving so quickly.

Comment 24

[Shu-yu Guo [:shu]]

Attachment: Append "> wasm" to URLs of wasm Debugger.Sources to hack around blacklisting in devtools.

The problem here is that the wasm modules have the caller's filename (i.e. the
file that called Wasm.instantiateModule, which is the current nonstandard way
to make wasm modules). If that filename happens to be blacklisted for whatever
reason, no wasm sources will get displayed.

This is a quick and dirty hack to work around this until wasm modules become
real modules and have real URLs.

Comment 25

[Jim Blandy :jimb]

Comment on attachment 8728296 [details] [diff] [review]
Prep Debugger.Source for a tagged union referent.

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

::: js/src/vm/Debugger.cpp
@@ +102,5 @@
> +
> +// Either a real ScriptSourceObject or a synthesized.
> +//
> +// If synthesized, the referent is a WasmModuleObject, denoting the
> +// synthesized source of a wasm module.

Simplify:

// Either a ScriptSourceObject, for ordinary JS, or a WasmModuleObject, denoting
// the synthesized source of a wasm module.

@@ +3914,5 @@
> +            DebuggerSourceReferent referent = GetSourceReferent(&debuggerSource.toObject());
> +            if (!referent.is<ScriptSourceObject*>()) {
> +                JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_UNEXPECTED_TYPE,
> +                                     "query object's 'source' property",
> +                                     "not undefined nor a Debugger.Source object for a JS script");

"neither undefined"

@@ +6144,2 @@
>          TraceManuallyBarrieredCrossCompartmentEdge(trc, obj, &referent,
> +                                                   "Debugger.Source source referent");

Why this change to the string?

@@ +6210,5 @@
>      return false;
>  }
>  
>  static NativeObject*
> +DebuggerSource_check(JSContext* cx, const Value& thisv, const char* fnname)

Wouldn't it be better to use HandleValue here?

Comment 26

[Shu-yu Guo [:shu]]

Attachment: Synthesize Debugger.Scripts for wasm modules and find them via findScripts.

The previous version's filtering of sources was plain wrong. Keep a variant and
match on that; it's simpler and correcter.

Comment 27

[Shu-yu Guo [:shu]]

Attachment: Tests for perfunctory functionality of wasm Debugger.Scripts and Debugger.Sources.

Comment 28

[Shu-yu Guo [:shu]]

Attachment: Keep a list of wasm::Modules per compartment.

I ended up having to add a sweep method to update the weak backpointer on
wasm::Module due to WasmModuleObjects moving due to compaction.

Could use a second look. Is adding a sweep phase the only way to fix up these
weak backpointers?

Comment 29

[Terrence Cole [:terrence]]

Comment on attachment 8729359 [details] [diff] [review]
Keep a list of wasm::Modules per compartment.

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

As discussed on IRC, the Module -> ModuleObject edge should just be strong so that the normal heap traversal will update the edge when compacting.

Comment 30

[Shu-yu Guo [:shu]]

Attachment: Keep a list of wasm::Modules per compartment.

Strong reference version. Much simpler.

Comment 31

[Jim Blandy :jimb]

Comment on attachment 8728903 [details] [diff] [review]
Synthesize Debugger.Scripts for wasm modules and find them via findScripts.

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

Posting comments I had so far on the old version of patch. I will look at the new one.

::: js/src/vm/Debugger.h
@@ +678,5 @@
> +
> +    template <typename ReferentVariant, typename Referent, typename Map>
> +    JSObject* wrapVariantReferent(JSContext* cx, Map& map, CrossCompartmentKey::Kind keyKind,
> +                                  Handle<ReferentVariant> referent);
> +    JSObject* wrapVariantReferent(JSContext* cx, Handle<DebuggerScriptReferent> referent);

These function names (newVariantWrapper, wrapVariantReferent) are terrible. I have no idea what a "variant wrapper" is.

@@ +984,5 @@
> +     * script), synthesizing a new one if needed. The context |cx| must be in
> +     * the debugger compartment; |wasmModule| must be a WasmModuleObject in
> +     * the debuggee compartment.
> +     */
> +    JSObject* synthesizeWasmScript(JSContext* cx, Handle<WasmModuleObject*> wasmModule);

This should be named wrapWasmModule.

Comment 32

[Jim Blandy :jimb]

Comment on attachment 8729333 [details] [diff] [review]
Synthesize Debugger.Scripts for wasm modules and find them via findScripts.

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

::: js/src/doc/Debugger/Debugger.Script.md
@@ +42,5 @@
>  equivalent functions or evaluated code—that is, scripts representing the
>  same source code, at the same position in the same source file,
>  evaluated in the same lexical environment.
>  
> +# Debugger.Script for WebAssembly

The number of '#' marks determines the header level, which affects the nesting of headers. By making this a top-level header, you've made all the subsequent sections children of "Debugger.Script for WebAssembly", which is wrong.

It's not adequate to simply say, at the top, "A `Debugger.Script` instance may refer to a sequence of bytecode", and then mention "these instances" six paragraphs later. You need to start at the top and say something like:

A `Debugger.Script` refers to either a sequence of JavaScript bytecode, or a block of WebAssembly code. The two cases are distinguished by the `format` property, which is either `"js"` or `"wasm"`.

For JavaScript, ...

For WebAssembly, ...

The format property must also be documented in its place along with all the other accessors.

This is not the first time I've sent back doc patches to you for being obviously inadequate, in ways that you shouldn't need a patch review to see. You have experience reading documentation; I feel like these problems should be apparent to you. I suspect that you didn't put much care into writing the documentation. If I allow patches like this to land, the documentation will become worse and worse. 

If you feel that the documentation has no audience and isn't worth maintaining, then that's a issue we can discuss with the team. Perhaps I'm overestimating the value of internal documentation of this sort. But until such a decision is reached, we should maintain a certain level of quality.

As things stand, this makes me pretty upset, because I put a lot of time into writing these docs originally, and tried to make them good.

@@ +47,5 @@
> +
> +A `Debugger.Script` instance may also refer to WebAssembly code. These
> +instances are distinguished by their `format` property being `"wasm"`.
> +
> +Currently, only the `format` and `text` properties works for WebAssembly

There is no `text` property on a Debugger.Script.

::: js/src/vm/Debugger.cpp
@@ +3927,5 @@
>                  return false;
>              }
>  
> +            hasSource = true;
> +            source = GetSourceReferent(&debuggerSource.toObject());

Yes, this is much nicer.

@@ +5066,5 @@
> +
> +        CrossCompartmentKey key(keyKind, object, untaggedReferent);
> +        if (!object->compartment()->putWrapper(cx, key, ObjectValue(*wrapper))) {
> +            map.remove(untaggedReferent);
> +            ReportOutOfMemory(cx);

Wouldn't the call to putWrapper have reported the OOM already itself?

@@ +5099,3 @@
>  
> +JSObject*
> +Debugger::synthesizeWasmScript(JSContext* cx, Handle<WasmModuleObject*> wasmModule)

This should be named wrapWasmScript.

Comment 33

[Jim Blandy :jimb]

Comment on attachment 8728907 [details] [diff] [review]
Add a .format property on Debugger.Script.

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

::: js/src/vm/Debugger.cpp
@@ +5304,5 @@
> +struct DebuggerScriptGetFormatMatcher
> +{
> +    using ReturnType = const char*;
> +    ReturnType match(HandleScript script) { return "js"; }
> +    ReturnType match(Handle<WasmModuleObject*> wasmModule) { return "wasm"; }

You could just put these in CommonPropertyNames.h; but it's fine like this too.

Comment 34

[Shu-yu Guo [:shu]]

Attachment: Update Debugger.Script docs.

Comment 35

[Jim Blandy :jimb]

Comment on attachment 8728908 [details] [diff] [review]
Support wasm for most of the Debugger.Source properties.

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

::: js/src/vm/Debugger.cpp
@@ +6713,5 @@
>      args.rval().setUndefined();
>      return true;
>  }
>  
> +class DebuggerSourceGetSourceMapUrlMatcher

Is there a reason we can't have ReturnType = const char16_t *, and then leave all the string allocation logic in DebuggerSource_getSourceMapUrl?

Comment 36

[Shu-yu Guo [:shu]]

(In reply to Jim Blandy :jimb from comment #35)
> Comment on attachment 8728908 [details] [diff] [review]
> Support wasm for most of the Debugger.Source properties.
> 
> Review of attachment 8728908 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/vm/Debugger.cpp
> @@ +6713,5 @@
> >      args.rval().setUndefined();
> >      return true;
> >  }
> >  
> > +class DebuggerSourceGetSourceMapUrlMatcher
> 
> Is there a reason we can't have ReturnType = const char16_t *, and then
> leave all the string allocation logic in DebuggerSource_getSourceMapUrl?

Because then we can't distinguish a nullptr between being OOM and simply not having a source map URL, for which we return null.

Comment 37

[Shu-yu Guo [:shu]]

(In reply to Shu-yu Guo [:shu] from comment #36)
> (In reply to Jim Blandy :jimb from comment #35)
> > Comment on attachment 8728908 [details] [diff] [review]
> > Support wasm for most of the Debugger.Source properties.
> > 
> > Review of attachment 8728908 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: js/src/vm/Debugger.cpp
> > @@ +6713,5 @@
> > >      args.rval().setUndefined();
> > >      return true;
> > >  }
> > >  
> > > +class DebuggerSourceGetSourceMapUrlMatcher
> > 
> > Is there a reason we can't have ReturnType = const char16_t *, and then
> > leave all the string allocation logic in DebuggerSource_getSourceMapUrl?
> 
> Because then we can't distinguish a nullptr between being OOM and simply not
> having a source map URL, for which we return null.

Wait, that doesn't make any sense. I'll change ReturnType = const char16_t*.

Comment 38

[Jim Blandy :jimb]

Comment on attachment 8728912 [details] [diff] [review]
Append "> wasm" to URLs of wasm Debugger.Sources to hack around blacklisting in devtools.

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

::: js/src/vm/Debugger.cpp
@@ +6511,5 @@
>  struct DebuggerSourceGetURLMatcher
>  {
> +    JSContext* cx_;
> +    explicit DebuggerSourceGetURLMatcher(JSContext* cx) : cx_(cx) { }
> +  public:

Why the 'public:'? It's a struct.

@@ +6527,5 @@
> +        // TODOshu: Until wasm modules have real URLs, append "> wasm" to the
> +        // end to prevent them from being blacklisted by devtools by having
> +        // the same value as a source mapped URL.
> +        char buf[4096];
> +        JS_snprintf(buf, sizeof(buf), "%s > wasm", wasmModule->module().filename());

Could we use JS_smprintf and JS_smprintf_free here instead?

Comment 39

[Shu-yu Guo [:shu]]

Attachment: Update Debugger.Source docs.

Comment 40

[Jim Blandy :jimb]

Comment on attachment 8729334 [details] [diff] [review]
Tests for perfunctory functionality of wasm Debugger.Scripts and Debugger.Sources.

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

Need to check that the .format of an ordinary Debugger.Script is js.

Need to check cases of more than one Wasm module in the map; I didn't see anything that creates more than one.

Need to check that wrapping the same Wasm module twice gets the identical Debugger.Script; I guess doing two findScripts calls is the only way to make this happen.

Need to check that findScript; compacting GC; findScript gives you same Wasm module wrapper (i.e. that your weakmaps are getting correctly updated for object relocations)

::: js/src/jit-test/tests/debug/wasm-01.js
@@ +14,5 @@
> +    break;
> +  }
> +}
> +
> +print(dbg.findScripts().length);

This can probably come out, right?

Comment 41

[Jim Blandy :jimb]

Comment on attachment 8729334 [details] [diff] [review]
Tests for perfunctory functionality of wasm Debugger.Scripts and Debugger.Sources.

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

Test that onNewScript reports a distinct wrapper for each new Wasm module.

Comment 42

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5ae46c02868d
https://hg.mozilla.org/integration/mozilla-inbound/rev/ab08650fe473
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9a5f0786a8b
https://hg.mozilla.org/integration/mozilla-inbound/rev/67d769eeda1f
https://hg.mozilla.org/integration/mozilla-inbound/rev/937999c2a3f8
https://hg.mozilla.org/integration/mozilla-inbound/rev/134ae6e2f92e
https://hg.mozilla.org/integration/mozilla-inbound/rev/f257362ae80d
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1ecce85ac43
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb0831a7ef13
https://hg.mozilla.org/integration/mozilla-inbound/rev/3300390349c6
https://hg.mozilla.org/integration/mozilla-inbound/rev/9ac34b44f276
https://hg.mozilla.org/integration/mozilla-inbound/rev/293d196105f1
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf4a26491ea5

Comment 43

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ec2bed5822e5

Comment 44

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a3a44b86faaa

Comment 45

[Luke Wagner [:luke]]

Attachment: cross-the-streams

Comment 46

[Shu-yu Guo [:shu]]

Comment on attachment 8729812 [details] [diff] [review]
cross-the-streams

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

The usability issue with having to reload is a bummer, but fine by me since this is all experimental.

::: js/src/asmjs/Wasm.cpp
@@ +1600,5 @@
>  
>      if (!CreateInstance(cx, exportObj, instance))
>          return false;
>  
> +    if (cx->compartment()->debuggerObservesAsmJS()) {

This use is fine by me for the demo, but I'm not sure how this flag should interact with wasm yet. This flag being on *prevents* asm.js compilation at all, so firstly, logically I'm not sure we should reach this point if the flag is meant to behave analogously for wasm.

Secondly, I'm all about the debugger working seamlessly whether it was opened after the fact or before. I hope we eventually can generate text regardless of when the debugger was opened (by refetching the code, maybe).

Comment 47

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ba5dd6053cc4

Comment 48

[Luke Wagner [:luke]]

(In reply to Shu-yu Guo [:shu] from comment #46)
> This flag being on *prevents* asm.js compilation at
> all, so firstly, logically I'm not sure we should reach this point if the
> flag is meant to behave analogously for wasm.

With asm.js we can fall back to JS.  Not so for wasm.  Actually we have an improvement here over asm.js: with debugger open, wasm games run at full speed.

> Secondly, I'm all about the debugger working seamlessly whether it was
> opened after the fact or before. I hope we eventually can generate text
> regardless of when the debugger was opened (by refetching the code, maybe).

Yes, agreed we should improve here.  I like the idea of (1) always save bits when "names" (i.e. symbols) section is present and (2) if not, attempt to fetch bits from the network (though that requires es6 module integration).

Comment 49

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/940ebacc15d2

Comment 50

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5ae46c02868d
https://hg.mozilla.org/mozilla-central/rev/ab08650fe473
https://hg.mozilla.org/mozilla-central/rev/f9a5f0786a8b
https://hg.mozilla.org/mozilla-central/rev/67d769eeda1f
https://hg.mozilla.org/mozilla-central/rev/937999c2a3f8
https://hg.mozilla.org/mozilla-central/rev/134ae6e2f92e
https://hg.mozilla.org/mozilla-central/rev/f257362ae80d
https://hg.mozilla.org/mozilla-central/rev/c1ecce85ac43
https://hg.mozilla.org/mozilla-central/rev/eb0831a7ef13
https://hg.mozilla.org/mozilla-central/rev/3300390349c6
https://hg.mozilla.org/mozilla-central/rev/9ac34b44f276
https://hg.mozilla.org/mozilla-central/rev/293d196105f1
https://hg.mozilla.org/mozilla-central/rev/bf4a26491ea5
https://hg.mozilla.org/mozilla-central/rev/ec2bed5822e5
https://hg.mozilla.org/mozilla-central/rev/a3a44b86faaa
https://hg.mozilla.org/mozilla-central/rev/ba5dd6053cc4
https://hg.mozilla.org/mozilla-central/rev/940ebacc15d2

Comment 51

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/584c27117cd6
Forgot to commit test.

Comment 52

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/584c27117cd6