[webkit-reviews] review granted: [Bug 43704] Web font is printed as blank if it is not cached : [Attachment 78779] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Yuzo Fujishima <yuzo at google.com>'s
request for review:
Bug 43704: Web font is printed as blank if it is not cached
https://bugs.webkit.org/show_bug.cgi?id=43704

Attachment 78779: Patch
https://bugs.webkit.org/attachment.cgi?id=78779&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=78779&action=review

I'm sad about this patch not having tests, but it seems like the consensus is
that we should accept this patch.

> Source/WebCore/loader/cache/CachedResourceLoader.h:146
> +    ResourceCacheValidationSuppressor(CachedResourceLoader* loader) :
m_loader(loader), m_previousState(false)

The initializers should be on their own lines.

> Source/WebCore/loader/cache/CachedResourceLoader.h:148
> +	   if (m_loader) {

Shouldn't we ASSERT that m_loader is non-NULL?	What situation would we get
into where we wanted to use a null loader?

> Source/WebCore/page/Frame.cpp:506
> +    // In setting printing, we should not validate resources already cached
for the document.
> +    // See https://bugs.webkit.org/show_bug.cgi?id=43704

Having a test would be better than having this comment.