[Webkit-unassigned] [Bug 51736] Implement RemoteFontStream's skip behavior (in FontCustomPlatformData.cpp)

[bugzilla-daemon at webkit.org]

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


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #77666|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #7 from David Levin <levin at chromium.org>  2011-01-05 16:31:22 PST ---
(From update of attachment 77666)
View in context: https://bugs.webkit.org/attachment.cgi?id=77666&action=review

> WebCore/ChangeLog:8
> +        No new tests.

Should say why. We can see there are no new tests.

Is it not testable for some reason?

>>> WebCore/platform/graphics/skia/FontCustomPlatformData.cpp:153
>>>          if (!m_buffer->data() || !m_buffer->size())
>> 
>> I'm confused.  Your'e removing a null check of buffer, yet using buffer unconditionally?
> 
> The condition is now at line 157 of the new file. The 'read' and 'skip' features only differ at line 158.

The change removes the check on buffer (which is the skip bytes feature being implemented). The code is only using m_buffer (except on line 158 where it does the check for buffer).

> WebCore/platform/graphics/skia/FontCustomPlatformData.cpp:156
> +        size_t toReadOrSkip = (left > size) ? size : left;

Seems like this should use std::min.

Also the name "toReadOrSkip" is awkward. How about bytesToRead or bytesToConsume? (Even though the bytes may not be written to buffer, they are still "read" or else this function ("read") should be named something different.)

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