[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





--- Comment #7 from Carlos Garcia Campos <cgarcia at igalia.com>  2013-05-08 11:02:12 PST ---
(In reply to comment #5)
> (From update of attachment 201079 [details])
> 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.

Sure.

> > 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.

Yes, I also thought about the names, my initial idea was to rename the current one to buffer or resourceBuffer and this new as data, but I wanted to keep the patch as simple as possible and buffer doesn't sound like a great name anyway. For now, I left both as data so that they can be renamed when refactoring. 

Do apple EWS bots run the tests with asserts enabled?

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