878287 - nsScriptLoader needs to pass the script element as a compile option to JSAPI

Status: RESOLVED DUPLICATE of bug 941876

(Core :: DOM: Core & HTML, defect)

Description

[Eddy Bruel [:ejpbruel]]

To implement Debugger.Source.prototype.element, which returns a Debugger.Object instance to the element on which a script was defined, we need to pass that element as a compile option to JSAPI when the script is compiled.

Comment 1

[Eddy Bruel [:ejpbruel]]

Attachment: Patch to be reviewed

Comment 2

[Eddy Bruel [:ejpbruel]]

Oh I just realized the name of the test files still refer to the bug from which this bug was split off. That might be confusing, so Ill change that.

Comment 3

[Eddy Bruel [:ejpbruel]]

Attachment: Patch to be reviewed (v2)

New patch with updated names for the test file and a spurious line from the test removed.

Also flagging smaug for review this time, as he might be able to review this during the weekend, and hsivonen might not.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 756857 [details] [diff] [review]
Patch to be reviewed (v2)


>     options.setFileAndLine(url.get(), aRequest->mLineNo)
>            .setVersion(JSVersion(aRequest->mJSVersion));
>+    JS::Rooted<JSObject*> global(cx, globalObject->GetGlobalJSObject());
>+    if (aRequest->mElement) {
We crash hard already higher up in the method if mElement is null, no useless null check.

>+      JS::Rooted<JS::Value> value(cx, JSVAL_NULL);
>+      rv = nsContentUtils::WrapNative(cx, global, aRequest->mElement,
>+                                      value.address());
>+      if (NS_FAILED(rv))
>+        return rv;
if (expr) {
  stmt;
}
Or just NS_ENSURE_SUCCESS(rv, rv);


>+
>+      JS::Rooted<JSObject *> element(cx, &value.toObject());
JS::Rooted<JSObject*>


>+    iframe.onload = function () {
>+        var dbg = new Debugger;
>+        ok(dbg, "Should be able to create debugger");
>+
>+        var windowDO = dbg.addDebuggee(iframe.contentWindow);
>+        ok(windowDO, "Should be able to obtain Debugger.Object instance for contentWindow");
>+
>+        var script = iframe.contentWindow.document.createElement("script");
>+        script.src = "http://mochi.test:8888/tests/content/base/test/chrome/nochrome_bug878287.js";
>+
>+        var scriptDO = windowDO.makeDebuggeeValue(script);
>+        ok(scriptDO, "Should be able to obtain Debugger.Object instance for script");
>+
>+        dbg.onNewScript = function (script) {
Just wondering, why Degugger has onNewScript property which doesn't follow the naming convention of
onfoo properties, and isn't actually event handler like rest of the onfoos in the web platform.

Comment 5

[Henri Sivonen (:hsivonen)]

Comment on attachment 756857 [details] [diff] [review]
Patch to be reviewed (v2)

I don't know JSAPI well enough to review the API usage properly. smaug already commented on the stylistic matters. Anyway, smaug's r+ is sufficient here.

Comment 6

[Jim Blandy :jimb]

This was implemented as part of bug 941876, and landed in changeset fbb3ea68c86e.