75623 - OnStartDocumentLoad() can be called >1 time

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Chris Waterson]

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

[Chris Waterson]

Attachment: proposed fix (plus cleanup for darin's recent changes)

Comment 2

[Chris Waterson]

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

[Chris Waterson]

Attachment: stack trace illustrating how we get 2nd OnStartDocumentLoad()

Comment 4

[Scott MacGregor]

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

Comment 5

[Peter "jag" Annema]

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

[Chris Waterson]

Attachment: incorporate jag's suggestion

Comment 7

[Chris Waterson]

Fix checked in.

Comment 8

[Chris Waterson]

Sorry for the bustage. I did a half-assed job of incorporating jag's 
recommendation.