[Webkit-unassigned] [Bug 115804] Avoid unnecessary data copies when loading subresources with DoNotBufferData option

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=115804


Brady Eidson <beidson at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #201079|review?                     |review-
               Flag|                            |




--- Comment #5 from Brady Eidson <beidson at apple.com>  2013-05-08 10:45:36 PST ---
(From update of attachment 201079)
View in context: https://bugs.webkit.org/attachment.cgi?id=201079&action=review

Good direction.

We plan to do a pretty significant overhaul of Cache/Loader land somewhat soon, so it'll be important to make very subtle things like this more difficult to screw up while refactoring.

That's the motivation for my review comments below.

> Source/WebCore/loader/cache/CachedImage.cpp:402
> +void CachedImage::data(const char* data, int length)
> +{
> +    CachedImage::data(ResourceBuffer::create(data, length), false);
> +}

Let's throw the ASSERT(m_options.dataBufferingPolicy == DoNotBufferData); here, too.

> Source/WebCore/loader/cache/CachedResource.h:105
> +    virtual void data(const char* /*data*/, int /*length*/) { }

Lets give this an implementation in CachedResource.cpp and have it ASSERT(m_options.dataBufferingPolicy == DoNotBufferData)

Assuming all tests continue passing without that ASSERT firing, I'd like us to take it a step further and name this method something different instead of just overloading ::data().  "unbufferedData()" is my first instinct, though I'm aware that's not perfect.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.