474358 - ###!!! ASSERTION: Inner window detected in Equality hook!: 'other->IsOuterWindow()'

Status: RESOLVED FIXED

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

Description

[Carsten Book [:Tomcat]]

Attachment: assertion stack

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090119 Shiretoko/3.1b3pre

Steps to reproduce:
-> Install the latest Firebug 1.4 alpha from http://getfirebug.com/releases/firebug/1.4X/
-> Start Firefox with Firebug installed 
--> Assertion

###!!! ASSERTION: Inner window detected in Equality hook!: 'other->IsOuterWindow()', file c:/work/mozilla/builds/1.9.1/mozilla/dom/src/base/nsDOMClassInfo.cpp,line 6603

Comment 3

[Johnny Stenback (:jst)]

Meant to say, that this is not a blocker as this is a harmless assertion in this particular case.

Comment 4

[Jesse Ruderman]

Bug 479924 also involves this assertion.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #3)
> Meant to say, that this is not a blocker as this is a harmless assertion in
> this particular case.

Johnny, in my case it hangs the complete browser. I have to kill the process. Seen with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090312 Minefield/3.2a1pre

Comment 6

[Boris Zbarsky [:bzbarsky]]

fwiw, in a debug build I also end up getting:

Assertion failure: 0, at /Users/bzbarsky/mozilla/1.9-branch/mozilla/js/jsd/jsd_scpt.c:806

which of course kills my build..

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Ok, so its not only me. Asking one more time for blocking1.9.1. If you still think it shouldn't block I'm fine, but just putting it onto your radar again.

Comment 8

[Johnny Stenback (:jst)]

I still don't think we'll be blocking on this any longer for 1.9.1.

Comment 9

[Rob Campbell [:rc] (:robcee)]

bz mentioned that he's seeing this a ton in his debug builds and I have as well the few times I've run in that environment.

talking with him in irc, he mentioned "this assert means that you asked he jsd api for something, and it handed you an object that you shouldn't be messing with".

He suggested outerizing the window object for the window we're debugging and pointed to http://mxr.mozilla.org/mozilla/source/js/src/jsobj.h#113 as what we could do.

Comment 10

[Rob Campbell [:rc] (:robcee)]

Attachment: 474358.diff

Comment 11

[Rob Campbell [:rc] (:robcee)]

timeless asked for additional comment on why this won't affect existing consumers of this API.

1) type is unaffected, we're still returning a jsval
2) in the case where jsval is wrapping an object, we ensure that we're returning an outer object. At the JS level, this should be what the consumer was interested in anyway.
3) verified working in Firebug

Comment 12

[timeless]

Comment on attachment 390070 [details] [diff] [review]
474358.diff

please include a space after the comma in the macro definition

also, include a comment saying the origin of your stolen macro

and probably a comment above the place from whence you stole it saying "hi, this macro is duplicated in jsd/jsd_val.c, if you change things, please please please please update that too!"

jsval val is canonical instead of myjsval

Comment 13

[Johnny Stenback (:jst)]

Not holding 1.9.2 for this, but we should obviously try to get the patch in ASAP.

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 390070 [details] [diff] [review]
474358.diff

>diff -r e1ae3719e72a js/jsd/jsd_val.c
>+    if (JSVAL_IS_OBJECT(jsdval->val)) {

Make this

!JSVAL_IS_PRIMITIVE(jsdval->val)

to avoid null pointer crashes.

>+        jsc = JSD_GetDefaultJSContext(jsdc);
>+        obj = JSVAL_TO_OBJECT(myjsval);
>+        OBJ_TO_OUTER_OBJECT(jsc, obj);

Need

if (!obj) {
  JS_ClearPendingException(jsc);
}

or similar here.

Comment 15

[timeless]

Comment on attachment 390070 [details] [diff] [review]
474358.diff


oh speaking of canonicals, please use JSContext *cx;

Comment 16

[Rob Campbell [:rc] (:robcee)]

awesome. Thanks for the feedback everybody. I'll update and attach a new version tomorrow morning.

Comment 17

[Rob Campbell [:rc] (:robcee)]

Attachment: 474358-2.diff

updated patch with issues addressed.

mrbkap: check the if (!obj) else clause for correctness. This seemed to me to be what you were suggesting, but want to make sure.

thanks for the reviews,

Comment 18

[Rob Campbell [:rc] (:robcee)]

Attachment: 474358-3.diff

replaced pronouns with specifics in jsobj.h comment for clarity.

Comment 19

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 390231 [details] [diff] [review]
474358-3.diff

>diff -r e1ae3719e72a js/jsd/jsd_val.c
>+        OBJ_TO_OUTER_OBJECT(cx, obj);
>+        if (!obj)
>+            JS_ClearPendingException(cx);

Please set val to JSVAL_NULL here. Otherwise, we'll still hit the assertion we're trying to avoid. r=mrbkap with that.

Comment 20

[Rob Campbell [:rc] (:robcee)]

Attachment: 474358-final.diff

all nits are in, ready to checkin

Comment 21

[Rob Campbell [:rc] (:robcee)]

pushed to m-c:
changeset:   30637:488f1d1a9108
tag:         tip
user:        Rob Campbell <[e-mail]>
date:        Fri Jul 24 10:20:34 2009 -0300
summary:     bug 474358 - ASSERTION: Inner window detected in Equality hook, isOuterWindow, p=me, r=timeless, sr=mrbkap