Closed Bug 494060 Opened 16 years ago Closed 16 years ago

JavaScript Error: "console.targetWindow is null" in venkman-debugger.js, line 311

Categories

(Other Applications Graveyard :: Venkman JS Debugger, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mnyromyr, Assigned: mnyromyr)

Details

Attachments

(1 file, 1 obsolete file)

Seen with current SeaMonkey trunk: - Activate exception tracing inside Venkman. - Restart SeaMonkey with -browser -venkman. During startup, the JS module file XPCOMUtils.jsm is loaded, but - of course - has no target window, causing: vnk: execution hook: function XPCOMUtils_QueryInterface(iid=XPComponent:{7}) in <file:/home/kd/projekte/mozilla/mozilla.org/obj/sr/mozilla/dist/bin/modules/XPCOMUtils.jsm> line 265 ************************************************************ * Call to xpconnect wrapped JSObject produced this error: * [Exception... "'[JavaScript Error: "console.targetWindow is null" {file: "chrome://venkman/content/venkman-debugger.js" line: 311}]' when calling method: [jsdIExecutionHook::onExecute]" nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)" location: "JS frame :: file:///home/kd/projekte/mozilla/mozilla.org/obj/sr/mozilla/dist/bin/modules/XPCOMUtils.jsm :: XPCOMUtils_QueryInterface :: line 265" data: yes] ************************************************************
Attached patch accept that JS modules don't need a window (obsolete) (deleted) — Splinter Review
This patch takes care of the fact that JS modules usually neither belong to windows nor do they need a wrapper...
Assignee: nobody → mnyromyr
Attachment #378727 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 378727 [details] [diff] [review] accept that JS modules don't need a window >diff --git a/resources/content/venkman-debugger.js b/resources/content/venkman-debugger.js >--- a/resources/content/venkman-debugger.js >+++ b/resources/content/venkman-debugger.js >@@ -301,38 +301,39 @@ function jsdExecutionHook (frame, type, > console.baseWindow.enabled = true; > > if (!ASSERT(cx, "no cx in execution hook")) > return hookReturn; > > var glob = cx.globalObject; > if (!ASSERT(glob, "no glob in execution hook")) > return hookReturn; >- >+ > console.targetWindow = getBaseWindowFromWindow(glob.getWrappedValue()); >- targetWasEnabled = console.targetWindow.enabled; >- if (console.targetWindow != console.baseWindow) >+ if (console.targetWindow) // JS modules usually don't have a target window > { >- cx.scriptsEnabled = false; >- console.targetWindow.enabled = false; >+ targetWasEnabled = console.targetWindow.enabled; >+ if (console.targetWindow != console.baseWindow) >+ { >+ cx.scriptsEnabled = false; >+ console.targetWindow.enabled = false; >+ } Are you sure leaving cx.scriptsEnabled alone if there is no targetWindow is the Right Thing to do? I am not sure if it is... >diff --git a/resources/content/venkman-records.js b/resources/content/venkman-records.js >--- a/resources/content/venkman-records.js >+++ b/resources/content/venkman-records.js >@@ -161,17 +161,19 @@ function FrameRecord (jsdFrame) > > this.functionName = jsdFrame.functionName; > if (!jsdFrame.isNative) > { > this.scriptWrapper = getScriptWrapper(jsdFrame.script); > this.location = getMsg(MSN_FMT_FRAME_LOCATION, > [getFileFromPath(jsdFrame.script.fileName), > jsdFrame.line, jsdFrame.pc]); >- this.functionName = this.scriptWrapper.functionName; >+ // JS modules usually don't have a scriptWrapper >+ if (this.scriptWrapper) >+ this.functionName = this.scriptWrapper.functionName; We should probably set a different name if there is no scriptWrapper. We already use a custom name for eval scripts, see http://hg.mozilla.org/venkman/file/f27ea2ff7bf3/resources/content/venkman-debugger.js#l477 . Iff this.scriptWrapper is only null for jsmodules here (something I wouldn't like to assume, but there we go...) then we should reflect that this is a jsmodule context in this special name. Also, as far as I know it's a bug that JS modules don't have a scriptwrapper, though I might be wrong there... r=me with those fixed / explained to me :-).
Attachment #378727 - Flags: review?(gijskruitbosch+bugs) → review+
Attached patch accept windowless scripts, v2 (deleted) — Splinter Review
(In reply to comment #2) > Are you sure leaving cx.scriptsEnabled alone if there is no targetWindow > is the Right Thing to do? Actually, no. I also took the liberty to comment on what is happening in jsdExecutionHook and did some minor whitespace cleanup in this function. > >+++ b/resources/content/venkman-records.js > >@@ -161,17 +161,19 @@ function FrameRecord (jsdFrame) Btw: I needed to patch the function, because otherwise I'll die here when setting exception handling to "stop". > Also, as far as I know it's a bug that JS modules don't have a > scriptwrapper, though I might be wrong there... The basic scriptwrapper problem doesn't seem to be the module as such (there are in fact .jsm with a scriptwrapper!), but the special code construct in XPCOMUtils.jsm: function makeQI(interfaceNames) { return function XPCOMUtils_QueryInterface(iid) { if (iid.equals(Ci.nsISupports)) return this; for each(let interfaceName in interfaceNames) { if (Ci[interfaceName].equals(iid)) return this; } throw Cr.NS_ERROR_NO_INTERFACE; }; } I don't get a call to jsdIScriptHook.onScriptCreate for the stack-created function XPCOMUtils_QueryInterface, thus we don't create a scriptwrapper. (Since you seem to know this bug, which one is it? I couldn't find it.) As a workaround, we maybe could create a scriptwrapper for yet-unknown scripts inside getScriptWrapper, but this doesn't feel quite right. So I just decided to wallpaper the error here; the old comment was wrong, of course.
Attachment #378727 - Attachment is obsolete: true
Attachment #381766 - Flags: review?(gijskruitbosch+bugs)
(In reply to comment #3) > > Also, as far as I know it's a bug that JS modules don't have a > > scriptwrapper, though I might be wrong there... > > The basic scriptwrapper problem doesn't seem to be the module as such (there > are in fact .jsm with a scriptwrapper!), but the special code construct in > XPCOMUtils.jsm: > > function makeQI(interfaceNames) { > return function XPCOMUtils_QueryInterface(iid) { > if (iid.equals(Ci.nsISupports)) > return this; > for each(let interfaceName in interfaceNames) { > if (Ci[interfaceName].equals(iid)) > return this; > } > throw Cr.NS_ERROR_NO_INTERFACE; > }; > } > > I don't get a call to jsdIScriptHook.onScriptCreate for the stack-created > function XPCOMUtils_QueryInterface, thus we don't create a scriptwrapper. > (Since you seem to know this bug, which one is it? I couldn't find it.) There is bug 459996, but it doesn't really have that much detail. I put two and two together and presumed from the fix and that bug that (part of) the problem was a lack of scriptwrappers for JSMs. I will have to have a look at the patch at a later time, sorry (will try to make it soon though! :-) ).
Attachment #381766 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 381766 [details] [diff] [review] accept windowless scripts, v2 This looks good, also like the comments. Thanks! r=me
Status: NEW → RESOLVED
Closed: 16 years ago
OS: Linux → All
Hardware: x86 → All
Resolution: --- → FIXED
Bug 418426 is still reproducible with this patch. The call to cx.scriptsEnabled = false throws an exception: [Exception... "Component returned failure code: 0x80004002 (NS_NOINTERF ACE) [jsdIContext.scriptsEnabled]" nsresult: "0x80004002 (NS_NOINTERFACE)" loc ation: "JS frame :: chrome://venkman/content/venkman-debugger.js :: jsdExecution Hook :: line 323" data: no]
is there a released version with this fix? it's not on AMO nor can i find a dev build anywhere in the (seemingly unmaintained) venkman pages.
Product: Other Applications → Other Applications Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: