Put Debugger object into chrome scratchpad

RESOLVED FIXED in Firefox 15

Status

()

Firefox
Developer Tools: Scratchpad
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jorendorff, Assigned: jorendorff)

Tracking

({dev-doc-needed})

unspecified
Firefox 15
dev-doc-needed
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Steps to reproduce:

1. Set pref devtools.chrome.enabled to true
2. Visit http://www.squarefree.com/shell/shell.html
3. Open scratchpad; select menu Environment -> Browser.
4. Paste this code into Scratchpad and run it (Ctrl+R or Cmd+R).

    var dbg = new Debugger(gBrowser.selectedBrowser.contentWindow);
    dbg.onDebuggerStatement = function (frame) { alert("hello world"); }

5. In the squarefree tab, type "debugger" and hit enter.

Expected: an alert that says "hello world"

Observed: step 4 fails with:
    /*
    Exception: Debugger is not defined
    @Scratchpad:1
    */
(Assignee)

Updated

5 years ago
Assignee: general → nobody
Component: JavaScript Engine → Developer Tools: Scratchpad
Product: Core → Firefox
QA Contact: general → developer.tools.scratchpad
Version: Other Branch → unspecified
(Assignee)

Comment 1

5 years ago
Created attachment 618771 [details] [diff] [review]
v1
Attachment #618771 - Flags: review?(dcamp)
(Assignee)

Comment 2

5 years ago
Comment on attachment 618771 [details] [diff] [review]
v1

r?bholley for the C++/IDL.
r?dcamp for everything else.
Attachment #618771 - Flags: review?(bobbyholley+bmo)

Updated

5 years ago
Attachment #618771 - Flags: review?(dcamp) → review+
Comment on attachment 618771 [details] [diff] [review]
v1


>diff --git a/js/ductwork/debugger/IJSDebugger.idl b/js/ductwork/debugger/IJSDebugger.idl
>--- a/js/ductwork/debugger/IJSDebugger.idl
>+++ b/js/ductwork/debugger/IJSDebugger.idl

> [scriptable, uuid(2fc14cc6-4ed0-4bbf-a7dd-e535bf088eb5)]
> interface IJSDebugger : nsISupports
> {
>   /**
>-   * Define the global Debugger constructor.
>+   * Define the global Debugger constructor on a given global.
>    */
>   [implicit_jscontext]
>-  void addClass();
>+  void addClass(in jsval global);
> };

Needs an iid rev.

>diff --git a/js/ductwork/debugger/JSDebugger.cpp b/js/ductwork/debugger/JSDebugger.cpp

>+  JSObject* obj = &global.toObject();
>+  obj = JS_UnwrapObject(obj);

Just pass , /* stopAtOuter = */ false here, skip the subsequent innerization, and throw if the result doesn't have the global flag. 

>+  if (!obj) {
>+    return NS_ERROR_FAILURE;
>   }
> 
>-  if (!JS_DefineDebuggerObject(cx, global)) {
>-    return NS_ERROR_FAILURE;
>+  {
>+    JSAutoEnterCompartment aec;
>+    if (!aec.enter(cx, obj)) {
>+      return NS_ERROR_FAILURE;
>+    }
>+
>+    obj = JS_ObjectToInnerObject(cx, obj);
>+    if (!obj) {
>+      return NS_ERROR_FAILURE;
>+    }
>+  }
>+
>+  {
>+    JSAutoEnterCompartment aec;

I'd prefer ac2. But the entire first AutoEnterCompartment is unnecessary given the above. So let's just unwrap, enter the compartment, check that it's the global, and define the object.

r=bholley with that.
Attachment #618771 - Flags: review?(bobbyholley+bmo) → review+

Updated

5 years ago
Blocks: 750364
(Assignee)

Comment 4

5 years ago
(In reply to Bobby Holley (:bholley) from comment #3)
> Needs an iid rev.

Oops. Thanks.

> >+  JSObject* obj = &global.toObject();
> >+  obj = JS_UnwrapObject(obj);
> 
> Just pass , /* stopAtOuter = */ false here, skip the subsequent
> innerization, and throw if the result doesn't have the global flag. 

JS_UnwrapObject doesn't have an optional second param. Actually I think it's extern "C" and used from C code (barf). So I added another function, JS_UnwrapObjectAndInnerize.

Ready to land when the tree reopens.
(In reply to Jason Orendorff [:jorendorff] from comment #4)
> JS_UnwrapObject doesn't have an optional second param. Actually I think it's
> extern "C" and used from C code (barf). So I added another function,
> JS_UnwrapObjectAndInnerize.

Erm, any reason not to just kill the extern C stuff and use js::UnwrapObject from jsfriendapi? Or maybe move it to jsapi if need be? Seems silly to have both.
https://hg.mozilla.org/mozilla-central/rev/421f51f36a75
Assignee: nobody → jorendorff
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 15
this was landed directly on central?

Bobby, do we need a follow-up to address your question in Comment 5 or can this stick as-landed?

This also looks like something that could use some documentation.
(In reply to Rob Campbell [:rc] (:robcee) from comment #7)
> this was landed directly on central?
> 
> Bobby, do we need a follow-up to address your question in Comment 5 or can
> this stick as-landed?

These shims are silly, but there's no duplicated code, so I don't really mind.
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.