Last Comment Bug 750145 - "Assertion failure: failedURI (We don't have a URI for history APIs.)"
: "Assertion failure: failedURI (We don't have a URI for history APIs.)"
Status: RESOLVED FIXED
: assertion, regression, testcase
Product: Core
Classification: Components
Component: Document Navigation (show other bugs)
: Trunk
: x86_64 Mac OS X
: -- critical (vote)
: mozilla15
Assigned To: O. Atsushi (Torisugari)
:
Mentors:
Depends on:
Blocks: 594645 673752
  Show dependency treegraph
 
Reported: 2012-04-29 19:12 PDT by Jesse Ruderman
Modified: 2012-05-04 09:32 PDT (History)
6 users (show)
ryanvm: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (must be local) (asserts fatally when loaded) (327 bytes, text/html)
2012-04-29 19:12 PDT, Jesse Ruderman
no flags Details
stack trace (5.24 KB, text/plain)
2012-04-29 19:13 PDT, Jesse Ruderman
no flags Details
Fix v1 (2.42 KB, patch)
2012-05-01 02:03 PDT, O. Atsushi (Torisugari)
bugs: review+
Details | Diff | Review
testcase2 (NS_ERROR_UNKNOWN_PROTOCOL) (234 bytes, text/html)
2012-05-04 09:32 PDT, O. Atsushi (Torisugari)
no flags Details

Description Jesse Ruderman 2012-04-29 19:12:38 PDT
Created attachment 619468 [details]
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 2012-04-29 19:13:02 PDT
Created attachment 619469 [details]
stack trace
Comment 2 O. Atsushi (Torisugari) 2012-04-29 20:06:05 PDT
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) 2012-04-29 20:14:01 PDT
(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) 2012-05-01 02:03:51 PDT
Created attachment 619869 [details] [diff] [review]
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) 2012-05-01 02:20:53 PDT
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] 2012-05-01 02:55:25 PDT
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) 2012-05-01 03:26:17 PDT
(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) 2012-05-01 05:18:24 PDT
> And <about:neterror> may want to use iframes in the future.
e.g. bug 637619 (maybe bug 157531).
Comment 9 Olli Pettay [:smaug] 2012-05-02 04:39:28 PDT
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) 2012-05-02 06:18:55 PDT
Filed: bug 751137
Comment 11 Ryan VanderMeulen [:RyanVM] 2012-05-03 03:59:36 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/483acaa66ef1

Should this have a test?
Comment 12 Ed Morley [:emorley] 2012-05-04 03:58:44 PDT
https://hg.mozilla.org/mozilla-central/rev/483acaa66ef1
Comment 13 O. Atsushi (Torisugari) 2012-05-04 09:32:02 PDT
Created attachment 621072 [details]
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.

Note You need to log in before you can comment on or make changes to this bug.