[Webkit-unassigned] [Bug 106842] adding support for DiscardablePixelRef for caching lazily decoded images

[bugzilla-daemon at webkit.org]

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





--- Comment #69 from Min Qin <qinmin at chromium.org>  2013-01-25 16:43:39 PST ---
(From update of attachment 184824)
View in context: https://bugs.webkit.org/attachment.cgi?id=184824&action=review

>> Source/WebCore/platform/graphics/chromium/DiscardablePixelRef.cpp:89
>> +        m_lockedMemory = m_discardable->data();
> 
> this seems confused. what's the purpose of m_firstOnLock if we do the exact same thing (except for ASSERTS) in both branches?

we cannot lock the memory twice, since the memory is initially locked after allocation, the first onLockPixels() should not lock the memory again. So m_firstOnLock just skips it, while in other cases we call lock().

>> Source/WebCore/platform/graphics/chromium/DiscardablePixelRef.cpp:104
>> +    if (pixelRef && pixelRef->getURI() && !strncmp(pixelRef->getURI(), labelDiscardable, sizeof(labelDiscardable)))
> 
> SkString has equals() and operator== and several other utilities. use those, don't use strncmp

will fix

>> Source/WebCore/platform/image-decoders/ImageDecoder.h:422
>> +            m_frameBufferCache[0].setMemoryAllocator(allocator);
> 
> I don't think this scheme will work for any multi-frame format, since frames in the cache with index > 0 won't have our allocator set.  It looks busted for gif/ico at least.  I guess if the worst case is that these frames won't use our allocator it's OK for now, but it needs a FIXME here at least pointing out the problem.

multiframe won't work for lazy image decoding, so this should not be hit. I will add a FIXME here,

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