230283 - crashes when aborting pageload of another site [@nsHTMLDocument::RegisterNamedItems]

Status: RESOLVED FIXED

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

Description

[Volkmar W. Pogatzki]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6b) Gecko/20040106 Firebird/0.7+
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6b) Gecko/20040106 Firebird/0.7+

Following the first link "schemas" on "http://schneegans.de/sv/" and stopping
loading of that new page by using the Esc button during loading crashes Firebird.

Reproducible: Always

Steps to Reproduce:
1. view http://schneegans.de/sv/
2. klick first link "schemas"
3. while new page loads, press Esc button
Actual Results:  
Firebird crashed

Expected Results:  
Continue viewing http://schneegans.de/sv/

Comment 1

[Oleg Sidletskiy]

also reproducable on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7a)
Gecko/20040101 Firebird/0.7+ (aebrahim), 
note that this can be best reproduces with a slow connection and when the status
bar displays "transfering data..."

Comment 2

[Jason Barnabe (np)]

Also occurs on Seamonkey 2004010122.

Confirming, reassigning, upping priority to Critical, adding crash keyword,
moving to Browser/Browser-General, removing Firebird blocking0.8?, adding
Seamonkey blocking1.6?. (*phew*)

Comment 3

[Frank Wein [:mcsmurf]]

Attachment: Stacktrace

stacktrace from a win2k trunk opt build with symbols

Comment 4

[Frank Wein [:mcsmurf]]

Getting 2 assertions here before crash:
###!!! ASSERTION: mDocElement not in doc?: 'mDocument->IndexOf(mDocElement) != -
1', file e:/mozilla/tree6/mozilla/content/xml/document/src/nsXMLContentSink.cpp,
 line 269

###!!! ASSERTION: Null content!: 'aContent', file e:/mozilla/tree6/mozilla/conte
nt/html/document/src/nsHTMLDocument.cpp, line 1369

Comment 5

[chris hofmann]

would need a patch quickly to get this into 1.6.   does anyone know if this is a
recent regression?

Comment 6

[chris hofmann]

can't get 1.4 to crash...

Comment 7

[chris hofmann]

1.5 also doesn't seem to crash

Comment 8

[chris hofmann]

caillon, jst, dbaron...  any ideas on this one?

Comment 9

[chris hofmann]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.6b) Gecko/20031201  did
crash  but it seemed a bit harder to make the timimg line up to make the crash
happen.

Comment 10

[chris hofmann]

> Getting 2 assertions here before crash:
> ###!!! ASSERTION: mDocElement not in doc?: 'mDocument->IndexOf(mDocElement) != -
> 1', file e:/mozilla/tree6/mozilla/content/xml/document/src/nsXMLContentSink.cpp,
> line 269

code around this changed in Oct.

1.293 <[e-mail]> 10 Oct 2003 17:27
Make sure to notify document observers even about content that gets added to
the document before StartLayout(). Block those notifications from getting
passed on to the frame constructor before StartLayout(), though. Bug 220930,
r=peterv, sr=jst

Comment 11

[chris hofmann]

looks like 1.5 branched on Aug 29. so it looks like maybe sometime between Aug.
29 and Dec 1.  Narrowing the window is going to be touger since nightly builds
only go back to Dec 1.

bunch of decomtamination work going on since then too..
http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/content/xml/document/src/nsXMLContentSink.cpp

Comment 12

[Frank Wein [:mcsmurf]]

Older nightlys exist at http://archive.mozilla.org/pub/mozilla/nightly/ :) (i
didn't even know we had such an archive, but it's quite useful)

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The crash is caused by mDocElement being null in
nsXMLContentSink::DidBuildModel().  This calls ContentInserted with aContainer
and aContent being null, which calls RegisterNamedItems, which dereferences the
null pointer.

Comment 14

[Johnny Stenback (:jst)]

Attachment: Don't notify about a null document element being inserted into the document.

Comment 15

[Johnny Stenback (:jst)]

Taking.

Comment 16

[Johnny Stenback (:jst)]

Fix checked in on trunk and 1.6 branch.

Comment 17

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 138720 [details] [diff] [review]
Don't notify about a null document element being inserted into the document.

>+      mDocument->BeginUpdate(UPDATE_CONTENT_MODEL);
>+      mDocument->ContentInserted(nsnull, mDocElement,
>+                                 // XXXbz is this last arg relevant if
>+                                 // the container is null?
>+                                 mDocument->IndexOf(mDocElement));
>+      mDocument->EndUpdate(UPDATE_CONTENT_MODEL);


How about using mozAutoDocUpdate?

Comment 18

[Serge Gautherie (:sgautherie)]

Side-note:

In the same code block, what about:
255      nsIScriptLoader *loader = mDocument->GetScriptLoader();
and
291      mDocument->EndLoad();
when mDocument is null ?

Comment 19

[Christopher Aillon (sabbatical, not receiving bugmail)]

mDocument can't be null at that point.  (we ensure that the content sink gets
created with a document).  Besides, we'd crash much sooner if there wasn't one.

Comment 20

[Serge Gautherie (:sgautherie)]

My mistake:
I looked for |mDocument| while the issue was on |mDocElement| :-<

Comment 21

[Johnny Stenback (:jst)]

This code now uses mozAutoDocUpdate.