750145 - "Assertion failure: failedURI (We don't have a URI for history APIs.)"

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Jesse Ruderman]

Attachment: testcase (must be local) (asserts fatally when loaded)

1. Save a copy of the testcase locally.
2. Using a debug build of Firefox, load the testcase from a file: URL.

Assertion failure: failedURI (We don't have a URI for history APIs.), at /Users/jruderman/trees/mozilla-central/docshell/base/nsDocShell.cpp:7531

Probably a regression from bug 673752. Also CCing jlebar, who fixed a few other bugs with "location = '404';" in the testcase recently.

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[O. Atsushi (Torisugari)]

I'm about to delete the MOZ_ASSERT, and bug 478927 is just waiting for Testcase's review. I'd like to know what hit the MOZ_ASSERT though.

If I understand correctly, the dynamic child iframe prevents the parent docshell from firing onLocationChange. In my opinion, this should not happen, no matter whtat the parent docshell is loading...

Comment 3

[O. Atsushi (Torisugari)]

(In reply to O. Atsushi (Torisugari) from comment #2)
> I'm about to delete the MOZ_ASSERT, and bug 478927 is just waiting for
> Testcase's review. I'd like to know what hit the MOZ_ASSERT though.
> 
> If I understand correctly, the dynamic child iframe prevents the parent
> docshell from firing onLocationChange. In my opinion, this should not
> happen, no matter whtat the parent docshell is loading...

Well, no, please forget this comment.

Comment 4

[O. Atsushi (Torisugari)]

Attachment: Fix v1

It seems to me that 

> document.createElement("iframe");

will set the iframe docshell's initial load type |LOAD_ERROR_PAGE|. This kind of "Like father, like son" strategy is dangerous. I'm not too sure setting LOAD_CMD_HISTORY/LOAD_CMD_RELOAD is safe, but at least LOAD_ERROR_PAGE is destructive.

Comment 5

[O. Atsushi (Torisugari)]

Umm, I have no idea how to write the unit test for this bug. attachment 619468 [details] depends on synchronous fallback of NS_ERROR_NOT_FOUND, but bug 282432's fix will make the testcase invalid.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 619869 [details] [diff] [review]
Fix v1

Hmm, is this really the problem. Or do we end up having
parent docshell being loaded while child docshell hasn't started the load yet.
Could we cancel the load in the child in that case?

Comment 7

[O. Atsushi (Torisugari)]

(In reply to Olli Pettay [:smaug] from comment #6)
> Hmm, is this really the problem. Or do we end up having
> parent docshell being loaded while child docshell hasn't started the load
> yet.

Probably both are problems. I think we should cancel such a load, if possible. And <about:neterror> may want to use iframes in the future.

Comment 8

[O. Atsushi (Torisugari)]

> And <about:neterror> may want to use iframes in the future.
e.g. bug 637619 (maybe bug 157531).

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 619869 [details] [diff] [review]
Fix v1

Please file a followup to actually cancel child shell loads when parent is in error

Comment 10

[O. Atsushi (Torisugari)]

Filed: bug 751137

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/483acaa66ef1

Should this have a test?

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/483acaa66ef1

Comment 13

[O. Atsushi (Torisugari)]

Attachment: testcase2 (NS_ERROR_UNKNOWN_PROTOCOL)

NS_ERROR_UNKNOWN_PROTOCOL is also synchronous, and I guess nobody will insist that asyncOpen should success, since we can't create nsIChannel object. Besides, this should work (or fail) both on local and on the web.