|Summary:||REGRESSION (r85375): Load event is sometimes lost when multiple image elements use the same URL|
|Product:||WebKit||Reporter:||Alexey Proskuryakov <ap>|
|Component:||Images||Assignee:||Alexey Proskuryakov <ap>|
|Version:||528+ (Nightly build)|
Comment 1 Alexey Proskuryakov 2011-05-28 14:41:15 PDT
Created attachment 95275 [details] proposed fix
Comment 2 Darin Adler 2011-05-28 15:33:27 PDT
Comment on attachment 95275 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=95275&action=review > Source/WebCore/html/HTMLImageElement.h:76 > + bool hasPendingActivity() const; I’m not sure it’s useful to have this separate function any more if it’s really the same as !haveFiredLoadEvent. Is this a virtual function? Is there a reason callers need to ask this more abstract question instead of asking haveFiredLoadEvent? Is there a reason this is not inlined?
Comment 3 Geoffrey Garen 2011-05-28 16:12:16 PDT
Comment on attachment 95275 [details] proposed fix This patch reintroduces a memory leak in the case of a canceled load. Alexey and I are working on a fix.
Comment 4 Alexey Proskuryakov 2011-05-28 16:31:36 PDT
> Is there a reason callers need to ask this more abstract question instead of asking haveFiredLoadEvent? Yes, I think that callers want the answer to the general question related to garbage collection, which matches the ActiveDOMObject one. > Is there a reason this is not inlined? Hmm, maybe it should be now.
Comment 5 Alexey Proskuryakov 2011-05-28 17:27:22 PDT
Created attachment 95277 [details] proposed fix
Comment 6 Geoffrey Garen 2011-05-28 17:31:52 PDT
Comment on attachment 95277 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=95277&action=review r=me > Source/WebCore/ChangeLog:44 > + (WebCore::ImageEventSender::dispatchPendingEvents): Clear the current loader fro dispatching Typo: "fro".
Comment 7 Alexey Proskuryakov 2011-05-28 18:38:44 PDT
Committed <http://trac.webkit.org/changeset/87628> with an additional simple fix for massive test failure.
Comment 8 Simon Fraser (smfr) 2011-05-28 21:30:20 PDT
Comment on attachment 95277 [details] proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=95277&action=review > Source/WebCore/loader/ImageLoader.cpp:228 > + ASSERT_UNUSED(m_image, resource == m_image.get()); Shouldn't this be ASSERT_UNUSED(resource, resource == m_image.get());
Comment 9 Alexey Proskuryakov 2011-05-28 21:43:20 PDT
Changed to plain ASSERT in <http://trac.webkit.org/changeset/87633> (no effect on release or debug builds).
Comment 10 Ademar Reis 2011-05-30 11:13:47 PDT
Revision r87628 cherry-picked into qtwebkit-2.2 with commit db21b6f <http://gitorious.org/webkit/qtwebkit/commit/db21b6f> Revision r87633 cherry-picked into qtwebkit-2.2 with commit 855622e <http://gitorious.org/webkit/qtwebkit/commit/855622e>