[Webkit-unassigned] [Bug 30879] New: Loading HTML with a JS alert() when the DocumentLoader has been set to not defer data load results in ASSERT

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=30879

           Summary: Loading HTML with a JS alert() when the DocumentLoader
                    has been set to not defer data load results in ASSERT
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: Macintosh
        OS/Version: Mac OS X 10.6
            Status: NEW
          Severity: Normal
          Priority: P3
         Component: Page Loading
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: vestbo at webkit.org


Qt's QWebFrame::setHtml() uses
DocumentLoader::setDeferMainResourceDataLoad(false) to force synchronuous
loading:

WTF::PassRefPtr<WebCore::DocumentLoader>
FrameLoaderClientQt::createDocumentLoader(const WebCore::ResourceRequest&
request, const SubstituteData& substituteData)
{
    RefPtr<DocumentLoader> loader = DocumentLoader::create(request,
substituteData);
    if (substituteData.isValid())
        loader->setDeferMainResourceDataLoad(false);
    return loader.release();
}

When the subsituteData that's being loaded has a piece of JS that does an
alert("foo"), we end up in Chrome::runJavaScriptAlert(), which then uses a
PageGroupLoadDeferrer to protect against paralell event loops:

// Defer loads in case the client method runs a new event loop that would
// otherwise cause the load to continue while we're in the middle of executing
JavaScript.
PageGroupLoadDeferrer deferrer(m_page, true);

After the alert() dialog has been closed by the user, the PageGroupLoadDeferrer
goes out of scope, resulting in a call to
MainResourceLoader::setDefersLoading(false).

The problem as I understand it is that MainResourceLoader::setDefersLoading()
does not take into account the case where the resources has already been
loaded, so it starts a new load, either though loadNow() or through the timer.

My fix so far is to use m_response as a sign that the request has already been
loaded:

http://gist.github.com/220820

My questions are:

   1. Is my assumption about the bug correct?
   2. If so, is that the right sign to use?
   3. Is it in the right place? I.e should it be moved to loadNow() et al?

I see ResourceLoader has a check for  if (!defers &&
!m_deferredRequest.isNull()), perhaps that's what's preventing this bug from
appearing there?

Thanks for any input :)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.