Bug 17667 – REGRESSION (r30587): Document::implicitClose() not called when done loading page (Image does not get scaled to fit)

• Component: Page Loading
• Status: RESOLVED FIXED

[Oliver Hunt]

The image at the top of the post is not correctly scaled

[Oliver Hunt]

Looking in inspector one difference i noticed was that there was no listing for the img elements inline style...

The script library the page uses does not alter the inline style when running in Safari.

I hit this assertion failure when navigating away from this page:

void FrameView::scheduleRelayout()
{
    ASSERT(!m_frame->document() || !m_frame->document()->inPageCache());

I cannot reproduce either problem with a low-bandwidth connection.

Setting a breakpoint at Document::implicitClose() confirms that the load event is never dispatched when the page finishes loading, thus the script that's supposed to resize the images doesn't run. The load event *is* dispatched when you navigate out. By then the page is in the page cache and scripts really should not be executing. The event handler runs, resizes the image, and triggers a layout of a now-cached page, which causes the assertion failure.

Indeed, you can force the images to resize by entering
javascript: document.close()
in the address bar after the page has finished loading.

The page links to a 404 style sheet, <http://factoryjoe.com/blog/wp-content/plugins/diso-profile/profile.css>, so I highly suspect <http://trac.webkit.org/projects/webkit/changeset/30587>.

<rdar://problem/5779658>

Created attachment 19526 [details]
Call setLoadInProgress(false) in the 4xx case

[Oliver Hunt]

Comment on attachment 19526 [details]
Call setLoadInProgress(false) in the 4xx case

r=me, though what's with "\ No newline at end of file" in the middle of the testcase?

[Geoffrey Garen]

r=me too, but I noticed another potential problem: do we need to surround the call to object->error() inside Loader::didReceiveData with calls to setLoadInProgress(true) and then setLoadInProgress(false)?

(In reply to comment #10)
> r=me too, but I noticed another potential problem: do we need to surround the
> call to object->error() inside Loader::didReceiveData with calls to
> setLoadInProgress(true) and then setLoadInProgress(false)?

I think so but I do not know how to make a test case to prove it.

(In reply to comment #9)
> (From update of attachment 19526 [details] [edit])
> r=me, though what's with "\ No newline at end of file" in the middle of the
> testcase?

There used not to be a newline, now there should be.

Landed in <http://trac.webkit.org/projects/webkit/changeset/30762>.