Bug 104830 - Document is never released when an Image is created inside unload event listener.
Summary: Document is never released when an Image is created inside unload event liste...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-12-12 12:06 PST by Yongjun Zhang
Modified: 2012-12-13 10:49 PST (History)
3 users (show)

See Also:

Attachments
Patch, don't trigger error event in ImageLoader if the page is being dismissed. (2.64 KB, patch)
2012-12-12 12:56 PST, Yongjun Zhang 		no flags Details | Formatted Diff | Diff
Don't trigger error event in ImageLoader if the page is being dismissed. (2.60 KB, patch)
2012-12-12 13:05 PST, Yongjun Zhang 		darin: review+
Details | Formatted Diff | Diff
Address review comments, add a helper function pageIsBeingDismissed and only call it when image is null. (2.88 KB, patch)
2012-12-13 10:30 PST, Yongjun Zhang 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yongjun Zhang 2012-12-12 12:06:48 PST 
If we create an Image object inside unload event listener and set src attribute to it, the Document will never be released.  As shown in the following HTML:

<html> <head> 
<script type="text/javascript">
 window.onunload = unloadPage;

p = [];
function unloadPage()
{
 p[0] = new Image;
 p[0].src = "https://www.my_random_testpage.com/image.png";
}

function reload()
{
    setTimeout(function() {location.reload();}, 500);
}
</script>
</head>
 <body onload="reload()">
</body> </html>

In mac, running "heap" periodically will show the number of HTMLDocument keeps growing.
Comment 1 Yongjun Zhang 2012-12-12 12:11:59 PST 
<rdar://problem/12797901>
Comment 2 Yongjun Zhang 2012-12-12 12:12:48 PST 
And this also happens on onbeforeunload and onpagehide listeners too.
Comment 3 Yongjun Zhang 2012-12-12 12:56:51 PST 
Created attachment 179107 [details]
Patch, don't trigger error event in ImageLoader if the page is being dismissed.
Comment 4 Yongjun Zhang 2012-12-12 13:05:01 PST 
Created attachment 179112 [details]
Don't trigger error event in ImageLoader if the page is being dismissed.
Comment 5 Darin Adler 2012-12-12 19:55:09 PST 
Comment on attachment 179112 [details]
Don't trigger error event in ImageLoader if the page is being dismissed.

View in context: https://bugs.webkit.org/attachment.cgi?id=179112&action=review

> Source/WebCore/loader/ImageLoader.cpp:203
> +        Frame* frame = document()->frame();
> +        bool pageIsBeingDismissed = frame && frame->loader()->pageDismissalEventBeingDispatched() != FrameLoader::NoDismissal;

It seems wasteful to do all this computation when newImage is non-zero. Could you put this into a helper function so it’s only computed when newImage is 0?
Comment 6 Yongjun Zhang 2012-12-12 22:52:25 PST 
(In reply to comment #5)
> (From update of attachment 179112 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=179112&action=review
> 
> > Source/WebCore/loader/ImageLoader.cpp:203
> > +        Frame* frame = document()->frame();
> > +        bool pageIsBeingDismissed = frame && frame->loader()->pageDismissalEventBeingDispatched() != FrameLoader::NoDismissal;
> 
> It seems wasteful to do all this computation when newImage is non-zero. Could you put this into a helper function so it’s only computed when newImage is 0?

yes that is a good point. I will make it into a help function which only get called when image is 0.
Comment 7 Yongjun Zhang 2012-12-13 10:30:10 PST 
Created attachment 179299 [details]
Address review comments, add a helper function pageIsBeingDismissed and only call it when image is null.
Comment 8 WebKit Review Bot 2012-12-13 10:49:44 PST 
Comment on attachment 179299 [details]
Address review comments, add a helper function pageIsBeingDismissed and only call it when image is null.

Clearing flags on attachment: 179299

Committed r137615: <http://trac.webkit.org/changeset/137615>
Comment 9 WebKit Review Bot 2012-12-13 10:49:48 PST 
All reviewed patches have been landed.  Closing bug.