304896 - Assertion on reload

Status: RESOLVED FIXED

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

Description

[Blake Kaplan (:mrbkap) (inactive)]

Occasionally, when I reload, I hit:
###!!! ASSERTION:
XPConnect is being called on a scope without a 'Components' property!

This is pretty much always bad. It usually means that native code is
making a callback to an interface implemented in JavaScript, but the
document where the JS object was created has already been cleared and the
global properties of that document's window are *gone*. Generally this
indicates a problem that should be addressed in the design and use of the
callback code.

I'll attach the stack in an attachment. jst and I spent some time yesterday
debugging this and weren't able to figure out why this was happening. It seems
like the code was doing the right thing. I don't know if this was introduced by
my checkin for bug 303267.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: stack

Comment 2

[Mike Schroepfer]

Can you reproduce and determine if this is critical enough to block?

Comment 3

[Johnny Stenback (:jst)]

This is splitwindow fallout that should IMO be fixed on the branch. We just
figured out what's causing this and the fix is very straight forward. Blake's
hacking up the patch.

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: reset mNavigator's docshell

The problem was that we weren't correctly reparenting the navigator object in
xpconnect, so that we ended up looking up its scope into the (cleared) old
inner window's global object. This was because the navigator was having its
docshell cleared in SetNewDocument and the docshell wasn't getting reset when
it was pulled out of the bfcache. Now we do that (and refuse to create the
navigator altogether if it's unable to get a good global object to attach to.

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 193000 [details] [diff] [review]
reset mNavigator's docshell

Yeah, r+sr=jst

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 193000 [details] [diff] [review]
reset mNavigator's docshell

NS_ERROR_UNEXPECTED seems better than NS_ERROR_INVALID_POINTER.  a=me either
way.

/be

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk.

Comment 8

[[e-mail]]

Comment on attachment 193000 [details] [diff] [review]
reset mNavigator's docshell

>         mNavigator = wsh->GetNavigator(); // This assignment addrefs.
>+
>+        // Update mNavigator's docshell pointer now.
>+        mNavigator->SetDocShell(mDocShell);
>+        mNavigator->LoadingNewDocument();
Is it reasonable for mNavigator to be null here, or was my crash caused by a
different bug?

Comment 9

[[e-mail]]

Yes, it's reasonable for mNavigator to be null, see the other GetNavigator.

Comment 10

[Mats Palmgren (inactive)]

This caused bug 305166?

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: null-check mNavigator

Easy workaround: only go back to pages that have scrollbars ;-).

Comment 12

[Johnny Stenback (:jst)]

Comment on attachment 193174 [details] [diff] [review]
null-check mNavigator

r+sr=jst

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 193174 [details] [diff] [review]
null-check mNavigator

Might as well get approval for this before I check the first patch into the
branch.

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk. Marking FIXED again. Sorry for missing this on the first
time around.

Comment 15

[Brendan Eich [:brendan]]

Comment on attachment 193174 [details] [diff] [review]
null-check mNavigator

a=me for 1.8b4.

/be

Comment 16

[Blake Kaplan (:mrbkap) (inactive)]

Both patches checked into MOZILLA_1_8_BRANCH (at the same time).