[webkit-reviews] review granted: [Bug 237703] Native image Lazyloading sometimes does not load : [Attachment 462202] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Rob Buis <rbuis at igalia.com>'s request
for review:
Bug 237703: Native image Lazyloading sometimes does not load
https://bugs.webkit.org/show_bug.cgi?id=237703

Attachment 462202: Patch

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




--- Comment #34 from Darin Adler <darin at apple.com> ---
Comment on attachment 462202
  --> https://bugs.webkit.org/attachment.cgi?id=462202
Patch

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

> COMMIT_MESSAGE:8
> +Fix the problem that data url placeholders are not replaced with the target
image by
> +keeping track of pending url loads ; if the new request is done with the
same url we keep
> +the original url since it contains the original base url, otherwise we use
the new url.

Lets call these "URL" in comments, not "url".

> Source/WebCore/loader/ImageLoader.cpp:166
> +    m_currentURL = nullAtom();

Could use { } instead of nullAtom(), which is actually even more efficient.

> Source/WebCore/loader/ImageLoader.cpp:229
> +	   // Use url from original request for same url loads in order to
preserve the original base url.

Lets call these "URL" in comments, not "url".

> Source/WebCore/loader/ImageLoader.cpp:274
> +	       m_currentURL = nullAtom();

Ditto.

> Source/WebCore/loader/ImageLoader.cpp:282
> +	   m_currentURL = nullAtom();

Ditto.

> Source/WebCore/loader/ImageLoader.cpp:392
> +    m_currentURL = nullAtom();

Ditto.

> Source/WebCore/loader/ImageLoader.h:127
> +    AtomString m_currentURL;

I’m not sure that "current" is the right name for this URL; nothing about
"current" makes it clear that we’d have to clear "current URL" in all those
places.