[webkit-reviews] review granted: [Bug 106733] CachedImage's container size may not survive cached reloads : [Attachment 183869] Preserve container size requests across image loads

[bugzilla-daemon at webkit.org]

Tim Horton <timothy_horton at apple.com> has granted Philip Rogers
<pdr at google.com>'s request for review:
Bug 106733: CachedImage's container size may not survive cached reloads
https://bugs.webkit.org/show_bug.cgi?id=106733

Attachment 183869: Preserve container size requests across image loads
https://bugs.webkit.org/attachment.cgi?id=183869&action=review

------- Additional Comments from Tim Horton <timothy_horton at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=183869&action=review


Seems pretty reasonable. A few comments:

> Source/WebCore/loader/cache/CachedImage.cpp:121
> +    if (!m_pendingContainerSizeRequests.isEmpty() &&
resourceToRevalidate()->isImage()) {

Is it possible for resourceToRevalidate()->isImage() to be false?

> Source/WebCore/loader/cache/CachedImage.cpp:123
> +	   ContainerSizeRequests m_switchContainerSizeRequests;

Why does this local have an m_?

> Source/WebCore/loader/cache/CachedImage.h:112
> +    virtual void switchClientsToRevalidatedResource();

OVERRIDE, please.

> LayoutTests/http/tests/svg/resources/delayCachedLoad.php:4
> +  // Delay load by 0.07s. This was found to be the lowest value
> +  // required for webkit.org/b/106733
> +  usleep(70000);

:\

Is this machine dependent or anything (or, how is it not)? I'm always sad when
I see things like this.