Bernd Reporter
	Description • 24 years ago

http://lxr.mozilla.org/seamonkey/source/docshell/base/nsWebShell.cpp#905

channel can be zero. I just crashed there by switching between pages back and 
forward. I think there should be zero pointer check of the do_QueryInterface 
result. Win98 CVS20001221.

	stephend@netscape.com (gone - use stephen.donner@gmail.com instead)
	Comment 1 • 24 years ago

TB23406579Q, I crashed here reading news.

	stephend@netscape.com (gone - use stephen.donner@gmail.com instead)
	Comment 2 • 24 years ago

*** Bug 63523 has been marked as a duplicate of this bug. ***

	Blake Ross
	Comment 4 • 24 years ago

Why is |channel| null, though?  Maybe |aRequest| is null? Rick, is this patch 
okay or is there more going on here?

	rpotts (gone) Assignee
	Comment 5 • 24 years ago

The channel should never be null in OnStateChange(...)

I'm looking into why this is sometimes happening.  It appears that the channel 
is sometimes null if the document has been cancelled...  This is why you mostly 
see the crash when using back/forward...

I believe that adding a null-pointer check in nsWebShell should be harmless (for 
now)...

	Blake Ross
	Comment 6 • 24 years ago

Hrm...so should I check this in in the interim (r=rpotts?)?

	rpotts (gone) Assignee
	Comment 7 • 24 years ago

I've checked in a null-pointer test at the top of nsWebShell::OnStateChange(...)

This should stop the crash, but I'm leaving the bug open until I can figure out 
why the channel is null in the first place.

-- rick

	(not reading, please use seth@sspitzer.org instead)
	Comment 8 • 24 years ago

*** Bug 63630 has been marked as a duplicate of this bug. ***

	(not reading, please use seth@sspitzer.org instead)
	Comment 9 • 24 years ago

*** Bug 63632 has been marked as a duplicate of this bug. ***

	Asa Dotzler [:asa]
	Comment 10 • 24 years ago

*** Bug 63623 has been marked as a duplicate of this bug. ***

	Asa Dotzler [:asa]
	Comment 11 • 24 years ago

*** Bug 63548 has been marked as a duplicate of this bug. ***

	Asa Dotzler [:asa]
	Comment 12 • 24 years ago

*** Bug 63637 has been marked as a duplicate of this bug. ***

	Asa Dotzler [:asa]
	Comment 13 • 24 years ago

adding mostfreq, this breaks the Bugzilla Helper.  Adding dogfood keyword.

	Bernd Reporter
	Comment 14 • 24 years ago

*** Bug 63638 has been marked as a duplicate of this bug. ***

	hirata masakazu
	Comment 15 • 24 years ago

All/All

	rpotts (gone) Assignee
	Comment 16 • 24 years ago

Are people still seeing this crash - after the patch was checked in?

If not, I'd like to remove the keywords and leave the bug open until I can 
investigate (and fix) why the channel is being passed in as null...

-- rick

	stephend@netscape.com (gone - use stephen.donner@gmail.com instead)
	Comment 17 • 24 years ago

No, I'm not seeing this crash anymore.  

	Bernd Reporter
	Comment 18 • 24 years ago

*** Bug 63812 has been marked as a duplicate of this bug. ***

	David Baron :dbaron:
	Comment 19 • 24 years ago

This is still the #1 topcrash in current builds.  Perhaps |aRequest| is garbage
rather than null?  See ftp://ftp.mozilla.org/pub/data/crash-data/ for more
details.

In talkback it looks like the problem started in the evening builds on
2000-12-21, although this bug was filed earlier in the day.  Any chance of
looking through checkins to see what might have caused it?

	Bernd Reporter
	Comment 20 • 24 years ago

the earliest crash is Build: 2000122021 CrashDate: 2000-12-21 
fix for bug 15345 by rpotts 12/20/2000 15:18

	Asa Dotzler [:asa]
	Comment 21 • 24 years ago

*** Bug 63849 has been marked as a duplicate of this bug. ***

	hirata masakazu
	Comment 22 • 24 years ago

Is bug 62643 a dupe of this bug?
I am wondering because the stacktrace for bug 62738 which was duped to 62643,
and that of bug 63630 duped to this one are nearly identical. Though crash in
bug 63630 disappeared after the patch here, 62643 still crashes.  Moreover, I
have started to get crash as in bug 65181 that gives me the same stacktrace.

	Håkan Waara
	Comment 23 • 24 years ago

Can't we get this null-check patch in, at least temporary? This is a topcrash,
mostfreq and dogfood bug. So we can't just stay idle about it. 

r=hwaara on the patch if that matters...

	David Baron :dbaron:
	Comment 24 • 24 years ago

A slightly different null pointer check was already checked in (nsWebShell.cpp
rev 1.508).

	Judson Valeski
	Comment 25 • 24 years ago

*** Bug 63634 has been marked as a duplicate of this bug. ***

	Judson Valeski
	Comment 26 • 24 years ago

anyone have a test case for this?

	Asa Dotzler [:asa]
	Comment 27 • 24 years ago

<Hixie> Is this still a crasher? Should it really be targetted at mozilla 0.9?

	rpotts (gone) Assignee
	Comment 28 • 24 years ago

Removing keywords and moving out to a later milestone...

I'm working on a "correct patch" which prevents the nsIRequest from ever being
NULL :-)

	rpotts (gone) Assignee
	Comment 30 • 24 years ago

I've just attached a patch that fixes this bug...  It also fixes bug #55867 bug
#26295 bug #63247...

All of these are related bugs :-)

	rpotts (gone) Assignee
	Comment 31 • 24 years ago

These patches fix the following bugs:
-------------------------------------
* bug #55867 which was opened to provide a real fix of the problem described in
  bug #27687.

* bug #63529 which resulted from mDocumentRequest being null.  A while ago I
  added a null pointer check in the DocShell/WebShell...  But these patches
  make sure that mDocumentRequest is nenver null if the first place..

* bug #26295 which was the assertion that either the LoadGroup or DocLoader
  was still busy after being canceled.

* bug #63247 which was tracking the need to change the arguments being passed
  on nsIHTTPEventSink::OnRedirect(...)

These patches also affect the following bugs:
----------------------------------------------
* bug #76286 - I reworked the ASSERTION that waterson added to verify that
  mDocumentRequest was never null.  I also reworked the way the mDocumentRequest
 is being set - to guarentee that it is NEVER null :-)

* bug #59827 - I reworked the way that the STATE_TRANSFERRING notification is
  being fired

* bug #27687 - I removed the band-aid patch in nsHTTPChannel since the real fix
 (in bug #55867) is provided.

Here's a quick overview of the changes that these patches introduce...
----------------------------------------------------------------------
* nsLoadGroup has been modified to prevent new requests from being added while
  it is in the process of canceling its requests...

* OnRedirect(...) has been reworked in nsHTTPChannel.
  The nsIHTTPEventSink::OnRedirect notification is now fired *before* the
  redirect occurs - and it is now cancelable (via a FAILURE return code)

  When a channel is redirected the status code passed out through OnStopRequest
  is now NS_BINDING_REDIRECTED

* The signature of nsIHTTPEventSink::OnRedirect(...) has changed to be two
  channels - the old one and the new one...

* nsDocLoader::OnStartRequest has been changed to only rely on the
  LOAD_DOCUMENT_URI flag to determine if a request represents the "document
  channel" or not...  This is a more reliable check that seeing if the
  count == 1.  It ensures that mDocumentRequest is never null during a page
  load...

* nsDocLoader::DocLoaderIsEmpty() was changed to not pass a status code.  In
  the current code, the argument was ignored - because it is useless :-)

thanks,
-- rick

	rpotts (gone) Assignee
	Comment 32 • 24 years ago

patch checked in.

-- rick