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)
Other Applications Graveyard
Venkman JS Debugger
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mnyromyr, Assigned: mnyromyr)
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
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]
************************************************************
Assignee | ||
Comment 1•16 years ago
|
||
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 2•16 years ago
|
||
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+
Assignee | ||
Comment 3•16 years ago
|
||
(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)
Comment 4•16 years ago
|
||
(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! :-) ).
Updated•16 years ago
|
Attachment #381766 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment 5•16 years ago
|
||
Comment on attachment 381766 [details] [diff] [review]
accept windowless scripts, v2
This looks good, also like the comments. Thanks! r=me
Assignee | ||
Comment 6•16 years ago
|
||
Landed as <http://hg.mozilla.org/venkman/rev/06ea5135b7f3>.
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.
Updated•6 years ago
|
Product: Other Applications → Other Applications Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•