[Webkit-unassigned] [Bug 108545] Document is never released if an image's src attribute is changed to a url blocked by content-security-policy.

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Yongjun Zhang <yongjun_zhang at apple.com>  2013-02-01 11:48:02 PST ---
(In reply to comment #3)
> (From update of attachment 185903 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=185903&action=review
> 
> Great catch.
> 
> Another potential way to fix this bug would be do dispatch the error event immediately in this case, not through errorEventSender().dispatchEventSoon(). That would be observable through JavaScript, and I don't know which behavior is right. Can you find it out from the spec and from other browsers' behavior?

Looks like the spec does expect to fire the error event asynchronously, from http://www.whatwg.org/specs/web-apps/current-work/multipage/embedded-content-1.html#the-img-element (section 12):

"If selected source is null, then set the element to the broken state, queue a task to fire a simple event named error at the img element"

> 
> Note that we currently dispatch the event synchronously for an empty URL.
> 

This sounds like behaves differently from the spec.

> I do not fully understand why canceling the error event causes a world leak. Is there something else going wrong in lower level code?

In ImageLoader::updateFromElement, when the image url is blocked by cross-site-voilation or content-security-policy, newImage is null and we schedule error event in ImageLoader.cpp:208:

            m_hasPendingErrorEvent = true;
            errorEventSender().dispatchEventSoon(this);

Later, we cancel the error event in the same call stack since m_hasPendingErrorEvent is true, in ImageLoader.cpp:226:

        if (m_hasPendingErrorEvent)
            errorEventSender().cancelEvent(this);

At the end of ImageLoader::updateFromElement, we ref the source Element in updateHasPendingEvent() since m_hasPendingErrorEvent is still true, at ImageLoader.cpp:256:

    updatedHasPendingEvent();

At this point, the error event is cancelled so errorEventSender won't call dispatchPendingErrorEvents for this ImageLoader any more.  However, the source Element is still ref-ed and we will leak.

> 
> > Source/WebCore/ChangeLog:12
> > +        No new tests, manually verified by using heap tool in OS X Safari.
> 
> It is difficult to test for abandoned memory, but can we test for the event being fired?

Good idea!  I will make a test for that.

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