Bug 14568 - load event can fire prematurely for frameless documents (affects Acid3?)
Summary: load event can fire prematurely for frameless documents (affects Acid3?)
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 523.x (Safari 3)
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-07-09 22:44 PDT by Geoffrey Garen
Modified: 2014-08-26 22:24 PDT (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Geoffrey Garen 2007-07-09 22:44:35 PDT
FrameLoader tracks loading progress. Without a FrameLoader, a Document doesn't know whether loading is in progress. Therefore, a Document might end up dispatching the load event too early.

This is the responsible code, in Document.cpp:

void Document::close()
{
    Frame* frame = this->frame();
    if (frame) {
        // This code calls implicitClose() if all loading has completed.
        FrameLoader* frameLoader = frame->loader();
        frameLoader->endIfNotLoadingMainResource();
        frameLoader->checkCompleted();
    } else {
        // Because we have no frame, we don't know if all loading has completed,
        // so we just call implicitClose() immediately. FIXME: This might fire
        // the load event prematurely.
        implicitClose();
    }
}

I don't know if this is a real-world problem or not.
Comment 1 David Kilzer (:ddkilzer) 2007-11-29 20:48:00 PST
See Bug 15170, Bug 15805 and Bug 16097.

Comment 2 Eric Seidel (no email) 2008-03-28 14:25:56 PDT
I think this might be related to our @font-face load-event problems with Acid3.  I'm not certain of that though.
Comment 3 Alexey Proskuryakov 2009-06-04 03:51:46 PDT
Is this still an issue? Could you please attach a test case?
Comment 4 Stephen Chenney 2012-10-16 12:25:45 PDT
This code still appears in a crash stack. Load http://www.speckproducts.com/ in a debug build of Chrome (and probably Safari too) and you will hit an assert due to an onload event firing during an SVG font load. I have yet to reduce it to a Layout test, and as I am unfamiliar with how all this works, I find it rather challenging. 

On the other hand, just because the code goes through this path does not mean it's a problem. There should not be any onload event until after the font has loaded _and_ the calling code has finished layout, which seems impossible to track. We also believe that an SVG font externally loaded should not be able to invoke script in the first place.
Comment 5 Myles C. Maxfield 2014-08-26 14:29:52 PDT
This causes a crash in CSSSegmentedFontFace::getFontData() due to the function getting called inside itself. (The load events run Javascript which causes a layout which ends up calling the function again)
Comment 6 Alexey Proskuryakov 2014-08-26 15:03:21 PDT
Frameless documents can't load subresources, so I think that the comment is incorrect, and the bug is invalid.

What Myles is referring to is that implicitClose() dispatches pending load events on ALL documents, as it calls these static functions:

    ImageLoader::dispatchPendingBeforeLoadEvents();
    ImageLoader::dispatchPendingLoadEvents();
    ImageLoader::dispatchPendingErrorEvents();

    HTMLLinkElement::dispatchPendingLoadEvents();
    HTMLStyleElement::dispatchPendingLoadEvents();

This is outright crazy, and does cause problems due to running JS at inopportune time. I'm not sure whether the frameLoader->checkCompleted() code path has the same problem (i.e. whether it ends up synchronously calling implicitClose() too).
Comment 7 Andreas Kling 2014-08-26 21:49:21 PDT
(In reply to comment #6)
> Frameless documents can't load subresources, so I think that the comment is incorrect, and the bug is invalid.
> 
> What Myles is referring to is that implicitClose() dispatches pending load events on ALL documents, as it calls these static functions:
> 
>     ImageLoader::dispatchPendingBeforeLoadEvents();
>     ImageLoader::dispatchPendingLoadEvents();
>     ImageLoader::dispatchPendingErrorEvents();
> 
>     HTMLLinkElement::dispatchPendingLoadEvents();
>     HTMLStyleElement::dispatchPendingLoadEvents();

Could we pass the Document* to these things and make them dispatch document-local events only? Would that solve the issue? I don't understand why they work globally.
Comment 8 Alexey Proskuryakov 2014-08-26 22:24:23 PDT
The SVG issue is now tracked as bug 136269.