[webkit-reviews] review requested: [Bug 215998] Fix crash in image-loading-lazy-slow.html WPT test : [Attachment 407968] Patch

[bugzilla-daemon at webkit.org]

Rob Buis <rbuis at igalia.com> has asked  for review:
Bug 215998: Fix crash in image-loading-lazy-slow.html WPT test
https://bugs.webkit.org/show_bug.cgi?id=215998

Attachment 407968: Patch

https://bugs.webkit.org/attachment.cgi?id=407968&action=review




--- Comment #9 from Rob Buis <rbuis at igalia.com> ---
Comment on attachment 407968
  --> https://bugs.webkit.org/attachment.cgi?id=407968
Patch

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

>> Source/WebCore/loader/ImageLoader.cpp:280
>> +		if (document.lazyLoadImageObserver().isObserved(element()))
> 
> Do we need this if check?

Right, that one was a bit ugly, I changed isDeferred to include testing for
LoadImmediate, and instead of above check check for isDeferred.

>> Source/WebCore/loader/ImageLoader.cpp:281
>> +		    LazyLoadImageObserver::unobserve(element(),
element().document());
> 
> This is ok as is, but it would be a bit better if it was reverse so that we
unobserve closer to setting m_image.
> Something like:
> if (!m_image)
>    unobserve....
> 
> There are other code paths that sets back m_image to nullptr.
> Should we unobserve in those cases as well?
> 
> Maybe we should introduce a clearImage() method that would set back m_image
to nullptr and unobserve.

I think in the other places we do not need to unobserve. But I added a method
resetLazyImageLoading which we can re-use if needed.