OnStartDocumentLoad() can be called >1 time




18 years ago
18 years ago


(Reporter: waterson, Assigned: adamlock)



Firefox Tracking Flags

(Not tracked)



(3 attachments)



18 years ago
nsDocLoader::OnStartRequest() has logic that seems to be incorrect.
Specifically, I'm seeing OnStartDocumentLoad() called on a document more than
one time. What appears to be happening is that...

1. The main document's load group has no more URLs, but for some reason still
   considers itself to be busy. (My test case has three IFRAMEs; maybe it
   has something to do with that.)

2. The scrollbars on one of the IFRAMEs gets painted. This kicks off an image
   load to get the scrollbar's background image.

3. The load group's active count goes from zero to one.

This screws up the layout regression tests, and maybe other stuff. I think the
fix is simple; specifically, be sure to check that the load flags are set such
that this is a nsIRequest::LOAD_DOCUMENT_URI.

Comment 1

18 years ago
Created attachment 30487 [details] [diff] [review]
proposed fix (plus cleanup for darin's recent changes)

Comment 2

18 years ago
Sorry the patch is messy. It also includes what I believe to be valid cleanup:
the docloader shouldn't assume that requests are channels. (See bug 75576, e.g.)
The stuff relevant to this bug is moving the code that retrieves the |loadFlags|
in OnStartRequest(), and the additional check to make sure we're doing an
nsIRequest::LOAD_DOCUMENT_URI before calling OnStartDocumentLoad().

Comment 3

18 years ago
Created attachment 30491 [details] [diff] [review]
stack trace illustrating how we get 2nd OnStartDocumentLoad()

Comment 4

18 years ago
these changes look good and safe to me chris. sr=mscott

Comment 5

18 years ago
Instead of:

+        *getter_Copies(aStr) = nsCRT::strdup(NS_ConvertUCS2toUTF8(name).get());

you should use:

*getter_Copies(aStr) = ToNewUTF8String(nsLocalString(name));

That saves you a copy (#include "nsReadableUtils.h").

Comment 6

18 years ago
Created attachment 30694 [details] [diff] [review]
incorporate jag's suggestion

Comment 7

18 years ago
Fix checked in.
Last Resolved: 18 years ago
Resolution: --- → FIXED

Comment 8

18 years ago
Sorry for the bustage. I did a half-assed job of incorporating jag's 
You need to log in before you can comment on or make changes to this bug.